-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rfc: add dns names rfc #103
Conversation
The environment value is read by the initializer init container and used as | ||
argument for the `NewMeshCert()` call. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not solve the lateral movement problem: with a foothold in the mesh, I can obtain my own certificate with SAN of my choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory, the coordinator could still validate requests using the EDG_DNS_NAMES variable since it is part of the policy and only return certs that were specified there. Not sure if I like this design.
Note that `NewMeshCert()` does not create the cert on the application layer but | ||
only retrieves the cert created on layer 4. | ||
In order to pass additional arguments from the initializer side, we'd need to | ||
shift the certificate creation and therefore the attestation to the application | ||
layer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: if we need an OSI layer here, shouldn't it be 6?
Why is this a bad thing? Wouldn't it be better anyway to attest on the aTLS layer and then use the attestation result in the RPC handler to generate a fresh certificate?
Let the manifest set by the user inside the coordinator contain a mapping from | ||
workload policy hash to DNS names. When creating the manifest we map each | ||
workload to `*`. This can then be changed later by the user. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imho this is the way to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@3u13r does this have obvious downsides that we are not seeing? I also believe that this is a usable solution.
As an improvement we can add a annotation similar as described above containing | ||
a comma separated list of DNS names. This is then used as an override for the | ||
DNS names when generating the manifest. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to make life as easy as possible for users, we can also think about doing a best-effort analysis of the selectors used by services and try to match them to policies.
Confidential pods should contain an environment value that contains a list | ||
of additional DNS names the pod should receive. | ||
|
||
## The Problem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section is fundamentally wrong. The coordinator does bind the certificate to an identity, in the form of the attestation report (currently, we embed the full report as bytes, in the future these should be parsed claims). And IMO this is the only valid identity in a confidential service mesh.
DNS names in the service mesh shouldn't, and IMO cannot, be used as workload identity, nor as a security feature/aspect im general. This would require a trusted DNS resolver at least. DNS names in our service mesh are rather a convenience feature that enables interaction with tools that are picky and don't like wildcard certs.
## The Problem | ||
|
||
We currently make use of certificates to distinguish between attested pods that | ||
are part of one mesh and unattested pods. The coordinator who singes the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are part of one mesh and unattested pods. The coordinator who singes the | |
are part of one mesh and unattested pods. The coordinator who signs the |
The environment value is read by the initializer init container and used as | ||
argument for the `NewMeshCert()` call. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory, the coordinator could still validate requests using the EDG_DNS_NAMES variable since it is part of the policy and only return certs that were specified there. Not sure if I like this design.
Let the manifest set by the user inside the coordinator contain a mapping from | ||
workload policy hash to DNS names. When creating the manifest we map each | ||
workload to `*`. This can then be changed later by the user. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@3u13r does this have obvious downsides that we are not seeing? I also believe that this is a usable solution.
Summary from in person discussion:
This enables users to implement custom authentication on top of what we provide. |
After yesterdays discussion I found a flaw in my proposed architecture. Namely, we'd need to move the attestation and certificate creation to the application layer. For a more detailed explanation see below.
I also described an alternative approach (that is essentially the status quo) with another suggestion on how to improve things.
Let me know what you think. I hope I described the problem detailed enough that everyone can have an informed opinion about this.