Skip to content

Commit bcf118b

Browse files
core: reject a JWK entry missing x5c when loading a SPIFFE trust bundle (#12911)
`SpiffeUtil.extractCert` silently truncates a SPIFFE trust bundle when a JWK `keys[]` entry is missing `x5c`: ```java 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.
1 parent 0585d48 commit bcf118b

3 files changed

Lines changed: 46 additions & 1 deletion

File tree

core/src/main/java/io/grpc/internal/SpiffeUtil.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,8 @@ private static List<X509Certificate> extractCert(List<Map<String, ?>> keysNode,
232232
checkJwkEntry(keyNode, trustDomainName);
233233
List<String> rawCerts = JsonUtil.getListOfStrings(keyNode, "x5c");
234234
if (rawCerts == null) {
235-
break;
235+
throw new IllegalArgumentException(String.format("'x5c' parameter is required. Certificate "
236+
+ "loading for trust domain '%s' failed.", trustDomainName));
236237
}
237238
if (rawCerts.size() != 1) {
238239
throw new IllegalArgumentException(String.format("Exactly 1 certificate is expected, but "

core/src/test/java/io/grpc/internal/SpiffeUtilTest.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@ public static class CertificateApiTest {
230230
private static final String SPIFFE_TRUST_BUNDLE_DUPLICATES = "spiffebundle_duplicates.json";
231231
private static final String SPIFFE_TRUST_BUNDLE_WRONG_ROOT = "spiffebundle_wrong_root.json";
232232
private static final String SPIFFE_TRUST_BUNDLE_WRONG_SEQ = "spiffebundle_wrong_seq_type.json";
233+
private static final String SPIFFE_TRUST_BUNDLE_MISSING_X5C = "spiffebundle_missing_x5c.json";
233234
private static final String DOMAIN_ERROR_MESSAGE =
234235
" Certificate loading for trust domain 'google.com' failed.";
235236

@@ -351,6 +352,10 @@ public void loadTrustBundleFromFileFailureTest() {
351352
iae = assertThrows(IllegalArgumentException.class, () -> SpiffeUtil
352353
.loadTrustBundleFromFile(copyFileToTmp(SPIFFE_TRUST_BUNDLE_CORRUPTED_CERT)));
353354
assertEquals("Certificate can't be parsed." + DOMAIN_ERROR_MESSAGE, iae.getMessage());
355+
// Check the exception if a key entry is missing the 'x5c' parameter
356+
iae = assertThrows(IllegalArgumentException.class, () -> SpiffeUtil
357+
.loadTrustBundleFromFile(copyFileToTmp(SPIFFE_TRUST_BUNDLE_MISSING_X5C)));
358+
assertEquals("'x5c' parameter is required." + DOMAIN_ERROR_MESSAGE, iae.getMessage());
354359
// Check the exception if 'kty' value differs from 'RSA'
355360
iae = assertThrows(IllegalArgumentException.class, () -> SpiffeUtil
356361
.loadTrustBundleFromFile(copyFileToTmp(SPIFFE_TRUST_BUNDLE_WRONG_KTY)));
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
{
2+
"trust_domains": {
3+
"google.com": {
4+
"keys": [
5+
{
6+
"kty": "RSA",
7+
"use": "x509-svid",
8+
"x5c": ["MIIELTCCAxWgAwIBAgIUVXGlXjNENtOZbI12epjgIhMaShEwDQYJKoZIhvcNAQEL
9+
BQAwVjELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
10+
GEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDEPMA0GA1UEAwwGdGVzdGNhMB4XDTI0
11+
MDkxNzE2MTk0NFoXDTM0MDkxNTE2MTk0NFowTjELMAkGA1UEBhMCVVMxCzAJBgNV
12+
BAgMAkNBMQwwCgYDVQQHDANTVkwxDTALBgNVBAoMBGdSUEMxFTATBgNVBAMMDHRl
13+
c3QtY2xpZW50MTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAOcTjjcS
14+
SfG/EGrr6G+f+3T2GXyHHfroQFi9mZUz80L7uKBdECOImID+YhoK8vcxLQjPmEEv
15+
FIYgJT5amugDcYIgUhMjBx/8RPJaP/nGmBngAqsuuNCaZfyaHBRqN8XdS/AwmsI5
16+
Wo+nru0+0/7aQFdqqtd2+e9dHjUWwgHxXvMgC4hkHpsdCGIZWVzWyBliwTYQYb1Y
17+
yYe1LzqqQA5OMbZfKOY9MYDCEYOliRiunOn30iIOHj9V5qLzWGfSyxCRuvLRdEP8
18+
iDeNweHbdaKuI80nQmxuBdRIspE9k5sD1WA4vLZpeg3zggxp4rfLL5zBJgb/33D3
19+
d9Rkm14xfDPihhkCAwEAAaOB+jCB9zBZBgNVHREEUjBQhiZzcGlmZmU6Ly9mb28u
20+
YmFyLmNvbS9jbGllbnQvd29ya2xvYWQvMYYmc3BpZmZlOi8vZm9vLmJhci5jb20v
21+
Y2xpZW50L3dvcmtsb2FkLzIwHQYDVR0OBBYEFG9GkBgdBg/p0U9/lXv8zIJ+2c2N
22+
MHsGA1UdIwR0MHKhWqRYMFYxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0
23+
YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQxDzANBgNVBAMM
24+
BnRlc3RjYYIUWrP0VvHcy+LP6UuYNtiL9gBhD5owDQYJKoZIhvcNAQELBQADggEB
25+
AJ4Cbxv+02SpUgkEu4hP/1+8DtSBXUxNxI0VG4e3Ap2+Rhjm3YiFeS/UeaZhNrrw
26+
UEjkSTPFODyXR7wI7UO9OO1StyD6CMkp3SEvevU5JsZtGL6mTiTLTi3Qkywa91Bt
27+
GlyZdVMghA1bBJLBMwiD5VT5noqoJBD7hDy6v9yNmt1Sw2iYBJPqI3Gnf5bMjR3s
28+
UICaxmFyqaMCZsPkfJh0DmZpInGJys3m4QqGz6ZE2DWgcSr1r/ML7/5bSPjjr8j4
29+
WFFSqFR3dMu8CbGnfZTCTXa4GTX/rARXbAO67Z/oJbJBK7VKayskL+PzKuohb9ox
30+
jGL772hQMbwtFCOFXu5VP0s="]
31+
},
32+
{
33+
"kty": "RSA",
34+
"use": "x509-svid"
35+
}
36+
]
37+
}
38+
}
39+
}

0 commit comments

Comments
 (0)