core: reject a JWK entry missing x5c when loading a SPIFFE trust bundle#12911
Open
TimurRakhmatullin86 wants to merge 1 commit into
Open
core: reject a JWK entry missing x5c when loading a SPIFFE trust bundle#12911TimurRakhmatullin86 wants to merge 1 commit into
TimurRakhmatullin86 wants to merge 1 commit into
Conversation
SpiffeUtil.extractCert did `break` when a keys[] entry had no x5c, silently returning only the certificates gathered before it and truncating the trust bundle. Every other error path in the method throws IllegalArgumentException, and the method contract states that an invalid element makes the whole bundle invalid. Throw instead of silently dropping the remaining certificates. Signed-off-by: Timur Rakhmatullin <174210871+TimurRakhmatullin86@users.noreply.github.com>
|
|
Member
|
Please sign the EasyCLA if you want us to review. |
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.
SpiffeUtil.extractCertsilently truncates a SPIFFE trust bundle when a JWKkeys[]entry ismissing
x5c:checkJwkEntryhas already guaranteed the entry is a declaredx509-svidkey (use == "x509-svid",ktyin{RSA, EC}), so an entry with nox5cis malformed. But instead of failing, thebreakabandons the loop and returns only the certificates collected before the bad entry — so a
trust domain whose
keysarray has a missing-x5centry ahead of valid ones loads a silentlytruncated trust store, and peers whose chain roots in a dropped CA fail verification (or a
partially loaded store is accepted with no error).
This also contradicts the method's own contract (
loadTrustBundleFromFilejavadoc: "If anyelement of the JSON content is invalid or unsupported, an
IllegalArgumentExceptionis thrown andthe entire Bundle is considered invalid"). Every other malformed condition in
extractCert(
use,kty,kid,x5c.size() != 1, unparseable cert) throws.Fix: throw
IllegalArgumentExceptionfor a missingx5c, consistent with the sibling errorpaths and the documented contract, instead of silently dropping certificates.
Tests: added
spiffebundle_missing_x5c.jsonand an assertion inSpiffeUtilTest.loadTrustBundleFromFileFailureTest. It fails against the current code (the oldbreakreturns a truncated bundle without throwing) and passes with the fix.:grpc-corecheckstyle/animalsniffer clean.