Skip to content

core: Validate trust domain names in SPIFFE bundle map files - #13167

Open
hidingmyname wants to merge 1 commit into
grpc:masterfrom
hidingmyname:core-spiffe-validate-trust-domain
Open

hidingmyname wants to merge 1 commit into
grpc:masterfrom
hidingmyname:core-spiffe-validate-trust-domain

Conversation

@hidingmyname

Copy link
Copy Markdown

Fixes #13154

SpiffeUtil.loadTrustBundleFromFile used the trust_domains keys as they were. A key that isn't a valid SPIFFE trust domain name, like UPPER.example.com or invalid#character, loaded without an error but could never match a peer: the peer's trust domain always comes from SpiffeUtil.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 check parse() uses, so an invalid name now throws IllegalArgumentException like other invalid content in the file. grpc-go also rejects the whole file in this case. FileWatcherCertificateProvider already reports load errors through onError() 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 loadTrustBundleFromFileInvalidTrustDomainTest to SpiffeUtilTest, covering invalid#character and UPPER.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 the io.grpc.xds.internal.security tests in :grpc-xds (745 tests), which use the bundle map.

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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SpiffeUtil.loadTrustBundleFromFile accepts invalid trust domain names (e.g. UPPER.example.com, invalid#character) as bundle map keys

1 participant