-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
tls: implement SPIFFE Certificate Validator for independent multiple trust domain support #14884
Conversation
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
api/envoy/extensions/transport_sockets/tls/v3/tls_spiffe_validator_config.proto
Outdated
Show resolved
Hide resolved
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
@@ -374,4 +375,11 @@ message CertificateValidationContext { | |||
// Certificate trust chain verification mode. | |||
TrustChainVerification trust_chain_verification = 10 | |||
[(validate.rules).enum = {defined_only: true}]; | |||
|
|||
// The configuration of an extension specific certificate validator. | |||
// If specified, all the values above are *ignored*. |
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.
Sorry to wander in here, I have a quick question. Does this comment imply that I can't use match_subject_alt_names
at the same time as custom_validator_config
? Will the SPIFFE validator extension support name matching?
Wondering the same about require_signed_certificate_timestamp
and allow_expired_certificate
, both of which also seem like they'd still be desirable when using SPIFFE validation
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.
Related question: will a combined validation context work as expected if a custom validator config is set in one of them? i.e. can I set the name matching in a default context and then additionally configure the SPIFFE validator?
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.
Hi @evan2645, thanks for dropping by!
In the current implementation, Envoy does not use any of existing fields in CertificateValidationContext.*
when CertificateValidationContext.custom_validator_config
is set. So setting match_subject_alt_names
, require_signed_certificate_timestamp
and allow_expired_certificate
would have no effect at all.
As for require_signed_certificate_timestamp
and allow_expired_certificate
, it's easy to implement, but I would like to limit the scope of this initial PR. So I would love to work on adding support for these fields in other PRs. (So I would change this description here to, like: If specified, the usage of all the values above depends on the custom validator
.)
For I got it. Choose the trust bundle based on the given trust domain first, and then verify the SAN name matching, and so maybe you want to have SAN matching conditions per trust bundle, right?match_subject_alt_names
, I would like to ask you about what the expected behavior looks like? When the name match, then how should we choose the trust domain?
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.
Hi @evan2645, thanks for dropping by!
Thanks for the warm welcome :)
it's easy to implement, but I would like to limit the scope of this initial PR.
Oh sure! Sorry, I didn't mean to imply that these things must be addressed right now. I just wanted to call attention to it, and make sure that it is at least being considered in the grand scheme of things. I'd probably call support for require_signed_certificate_timestamp
and allow_expired_certificate
as nice to have. The feature provided by match_subject_alt_names
configurable is much more important.
So I would change this description here to, like: If specified, the usage of all the values above depends on the custom validator.
That sounds pretty good, especially if it means we'll be able to use the existing fields/values rather than having to redefine them on the custom validator portion of the proto
For match_subject_alt_names, I would like to ask you about what the expected behavior looks like? When the name match, then how should we choose the trust domain?I got it. Choose the trust bundle based on the given trust domain first, and then verify the SAN name matching, and so maybe you want to have SAN matching conditions per trust bundle, right?
It could be that I want to allow multiple SPIFFE IDs that are in different trust domains. To support this, what we'll probably want to do is:
- Extract peer SPIFFE ID
- Choose correct bundle based on trust domain
- Validate peer SVID using the selected bundle
- Compare the validated SPIFFE ID to the allowed names from
match_subject_alt_names
I don't think that we want to tie the matching to a particular bundle.
Related question: will a combined validation context work as expected if a custom validator config is set in one of them? i.e. can I set the name matching in a default context and then additionally configure the SPIFFE validator?
I think this would work as expected if the custom validator is able to consume the existing fields/values from the validation context proto?
The important part here is that the SPIFFE identity and bundles may be supplied by one system, and the authorization policy (e.g. match_subject_alt_names
) from a different system. Since the config for authentication and authorization are mixed into the same proto, the only way we can (currently) solve for this is through the use of a combined validation context in which match_subject_alt_names
is provided by one and custom_validator_config
is provided by the other. If there is a better way to accomplish this, I'd love to hear about it
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.
I think this would work as expected if the custom validator is able to consume the existing fields/values from the validation context proto?
The important part here is that the SPIFFE identity and bundles may be supplied by one system, and the authorization policy (e.g. match_subject_alt_names) from a different system. Since the config for authentication and authorization are mixed into the same proto, the only way we can (currently) solve for this is through the use of a combined validation context in which match_subject_alt_names is provided by one and custom_validator_config is provided by the other. If there is a better way to accomplish this, I'd love to hear about it
OK, that makes sense. Thanks for the detailed description 👍
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.
Thanks for your work on this @mathetake please let me know if there's anything else I can do to help ❤️
…trust domain support (envoyproxy#14884) Adds the extension point for certificate validations, and its first implementation for SPIFFE multi trust domain support in a single listener or cluster. Resolves envoyproxy#14614 and envoyproxy#9284. Risk Level: low (only adding the new extension point and one implementation for it) Testing: unit tests and integration tests. Docs Changes: Release Notes: tls: implement SPIFFE Certificate Validator for independent multiple trust domain support. Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
…ndependent multiple trust domain support (envoyproxy#14884)
Signed-off-by: Takeshi Yoneda takeshi@tetrate.io
This is the sequel to the previous refactoring PR: #14757
Commit Message: tls: implement SPIFFE Certificate Validator for independent multiple trust domain support.
Additional Description: Adds the extension point for certificate validations, and its first implementation for SPIFFE multi trust domain support in a single listener or cluster. Resolves #14614 and #9284.
Risk Level: low (only adding the new extension point and one implementation for it)
Testing: unit tests and integration tests.
Docs Changes:
Release Notes: tls: implement SPIFFE Certificate Validator for independent multiple trust domain support.
cc @lizan