Repository navigation
core: Validate trust domain names in SPIFFE bundle map files - #13167
Open
hidingmyname wants to merge 1 commit into
Open
hidingmyname wants to merge 1 commit into
hidingmyname wants to merge 1 commit into
Conversation
loadTrustBundleFromFile used the trust_domains keys as they were. A key that is not a valid SPIFFE trust domain name, such as UPPER.example.com or invalid#character, was loaded but could never match a peer, because the peer's trust domain always comes from SpiffeUtil.parse() and is valid and lower case. The javadoc and gRFC A87 say that an invalid entry makes the whole bundle map invalid. Validate each key with the existing validateTrustDomain(), as grpc-go does. Fixes grpc#13154
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #13154
SpiffeUtil.loadTrustBundleFromFileused thetrust_domainskeys as they were. A key that isn't a valid SPIFFE trust domain name, likeUPPER.example.comorinvalid#character, loaded without an error but could never match a peer: the peer's trust domain always comes fromSpiffeUtil.parse(), so it is valid and lower case. Handshakes from that domain later failed with "Spiffe Trust Map doesn't contain trust domain ...", which doesn't point at the file. The javadoc and gRFC A87 both say that an invalid entry makes the whole bundle map invalid.This validates each key with the existing
validateTrustDomain(), the same checkparse()uses, so an invalid name now throwsIllegalArgumentExceptionlike other invalid content in the file. grpc-go also rejects the whole file in this case.FileWatcherCertificateProvideralready reports load errors throughonError()and keeps the previous map on a reload, so it needs no change.One behavior change: a file that contains an invalid key next to valid ones used to load, with the invalid entry silently unused. Now the whole file is rejected.
Added
loadTrustBundleFromFileInvalidTrustDomainTesttoSpiffeUtilTest, coveringinvalid#characterandUPPER.example.com. It fails without the change and passes with it, on JDK 21 and on JDK 8.:grpc-core:test(1583 tests) and checkstyle pass, as do theio.grpc.xds.internal.securitytests in:grpc-xds(745 tests), which use the bundle map.