Skip to content

core: reject a JWK entry missing x5c when loading a SPIFFE trust bundle#12911

Open
TimurRakhmatullin86 wants to merge 1 commit into
grpc:masterfrom
TimurRakhmatullin86:fix/spiffe-missing-x5c-throw
Open

core: reject a JWK entry missing x5c when loading a SPIFFE trust bundle#12911
TimurRakhmatullin86 wants to merge 1 commit into
grpc:masterfrom
TimurRakhmatullin86:fix/spiffe-missing-x5c-throw

Conversation

@TimurRakhmatullin86

Copy link
Copy Markdown

SpiffeUtil.extractCert silently truncates a SPIFFE trust bundle when a JWK keys[] entry is
missing x5c:

for (Map<String, ?> keyNode : keysNode) {
  checkJwkEntry(keyNode, trustDomainName);
  List<String> rawCerts = JsonUtil.getListOfStrings(keyNode, "x5c");
  if (rawCerts == null) {
    break;          // abandons the loop, dropping every remaining cert
  }
  if (rawCerts.size() != 1) {
    throw new IllegalArgumentException(...);   // sibling paths throw
  }
  ...
}

checkJwkEntry has already guaranteed the entry is a declared x509-svid key (use == "x509-svid",
kty in {RSA, EC}), so an entry with no x5c is malformed. But instead of failing, the break
abandons the loop and returns only the certificates collected before the bad entry — so a
trust domain whose keys array has a missing-x5c entry ahead of valid ones loads a silently
truncated
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 (loadTrustBundleFromFile javadoc: "If any
element of the JSON content is invalid or unsupported, an IllegalArgumentException is thrown and
the entire Bundle is considered invalid"
). Every other malformed condition in extractCert
(use, kty, kid, x5c.size() != 1, unparseable cert) throws.

Fix: throw IllegalArgumentException for a missing x5c, consistent with the sibling error
paths and the documented contract, instead of silently dropping certificates.

Tests: added spiffebundle_missing_x5c.json and an assertion in
SpiffeUtilTest.loadTrustBundleFromFileFailureTest. It fails against the current code (the old
break returns a truncated bundle without throwing) and passes with the fix. :grpc-core
checkstyle/animalsniffer clean.

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>
@linux-foundation-easycla

linux-foundation-easycla Bot commented Jul 20, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: TimurRakhmatullin86 / name: Timur Rakhmatullin (66d4fbf)

@shivaspeaks

Copy link
Copy Markdown
Member

Please sign the EasyCLA if you want us to review.

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.

2 participants