tls: enable match_subject_alt_names option in SPIFFE validator#15509
tls: enable match_subject_alt_names option in SPIFFE validator#15509mattklein123 merged 9 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
|
/retest |
|
Retrying Azure Pipelines: |
azdagron
left a comment
There was a problem hiding this comment.
This is exciting! Very happy to see this work move forward! I can't speak to the rest of the mechanics, but the actual SAN validation seems fine.
source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc
Outdated
Show resolved
Hide resolved
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
|
@azdagron Thanks for the review from the perspective of SPIFFE expert 👍 I really appreciate it 🙂 |
asraa
left a comment
There was a problem hiding this comment.
Thanks! LGTM besides the nit
source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc
Outdated
Show resolved
Hide resolved
eba45d9 to
2b791e2
Compare
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
2b791e2 to
8c4eaab
Compare
|
@PiotrSikora sorry my stale merge commit has requested your review. Never mind. |
|
/retest |
|
Retrying Azure Pipelines: |
asraa
left a comment
There was a problem hiding this comment.
Thanks! LGTM
@envoyproxy/senior-maintainers
|
Needs a main merge. /wait |
|
merged main and resolved conflicts. Thanks! |
| case GEN_EMAIL: { | ||
| ASN1_STRING* str = general_name->d.rfc822Name; | ||
| san.assign(reinterpret_cast<const char*>(ASN1_STRING_data(str)), ASN1_STRING_length(str)); | ||
| break; | ||
| } |
There was a problem hiding this comment.
Does this have explicit testing somewhere? I don't see any cert changes in this PR?
/wait-any
There was a problem hiding this comment.
I used this here for testing SAN matcher https://github.com/envoyproxy/envoy/pull/15509/files#diff-ee57e8aee637e17f1f512cef5e2bff807c263ab67922c85659721654913242f4R460, so that's why you don't see certs changes.
In any way, Added a dedicated test case of getSubjectAltNames for email SAN.generalNameAsString doesn't have any explicit unit test so I think we should.
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
|
/retest |
|
Retrying Azure Pipelines: |
mattklein123
left a comment
There was a problem hiding this comment.
Super, thanks for beefing up the test coverage!
Signed-off-by: Takeshi Yoneda takeshi@tetrate.io
Commit Message: tls: enable match_subject_alt_names option in SPIFFE validator
Additional Description: This is a follow up on #14884 and resolves #15392.