Skip to content

Commit afb9b09

Browse files
committed
core: update SPIFFE certificate extraction to comply with X509-SVID spec
- Modify `SpiffeUtil.extractCert` to ignore all but the first certificate if the `x5c` JWK parameter contains multiple values. - Modify `SpiffeUtil.extractCert` to skip the JWK entry (`continue`) instead of stopping execution (`break`) or throwing when `x5c` is missing or contains an empty list, complying with the requirement that entries without `x5c` must be ignored. - Update `SpiffeUtilTest.java` to treat multi-cert, missing `x5c`, and empty `x5c` list cases as success cases, asserting that the valid certificates are successfully extracted. - Add `spiffebundle_ignored_keys.json` to verify the combination of missing `x5c`, empty `x5c` list, and multi-cert parameters.
1 parent 7fdcde1 commit afb9b09

2 files changed

Lines changed: 52 additions & 17 deletions

File tree

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -231,14 +231,8 @@ private static List<X509Certificate> extractCert(List<Map<String, ?>> keysNode,
231231
for (Map<String, ?> keyNode : keysNode) {
232232
checkJwkEntry(keyNode, trustDomainName);
233233
List<String> rawCerts = JsonUtil.getListOfStrings(keyNode, "x5c");
234-
if (rawCerts == null) {
235-
throw new IllegalArgumentException(String.format("'x5c' parameter is required. Certificate "
236-
+ "loading for trust domain '%s' failed.", trustDomainName));
237-
}
238-
if (rawCerts.size() != 1) {
239-
throw new IllegalArgumentException(String.format("Exactly 1 certificate is expected, but "
240-
+ "%s found. Certificate loading for trust domain '%s' failed.", rawCerts.size(),
241-
trustDomainName));
234+
if (rawCerts == null || rawCerts.isEmpty()) {
235+
continue;
242236
}
243237
InputStream stream = new ByteArrayInputStream((CERTIFICATE_PREFIX + rawCerts.get(0) + "\n"
244238
+ CERTIFICATE_SUFFIX)

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

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,8 @@ public static class CertificateApiTest {
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";
233233
private static final String SPIFFE_TRUST_BUNDLE_MISSING_X5C = "spiffebundle_missing_x5c.json";
234+
private static final String SPIFFE_TRUST_BUNDLE_EMPTY_X5C = "spiffebundle_empty_x5c.json";
235+
private static final String SPIFFE_TRUST_BUNDLE_IGNORED_KEYS = "spiffebundle_ignored_keys.json";
234236
private static final String DOMAIN_ERROR_MESSAGE =
235237
" Certificate loading for trust domain 'google.com' failed.";
236238

@@ -330,6 +332,54 @@ public void loadTrustBundleFromFileSuccessTest() throws Exception {
330332
assertEquals("foo.bar.com", spiffeId_ec.get().getTrustDomain());
331333
}
332334

335+
@Test
336+
public void loadTrustBundleFromFileWithMultiCertsSuccessTest() throws Exception {
337+
SpiffeBundle tb = SpiffeUtil.loadTrustBundleFromFile(
338+
copyFileToTmp(SPIFFE_TRUST_BUNDLE_WRONG_MULTI_CERTS));
339+
assertEquals(1, tb.getSequenceNumbers().size());
340+
assertEquals(123L, (long) tb.getSequenceNumbers().get("google.com"));
341+
assertEquals(1, tb.getBundleMap().size());
342+
assertEquals(1, tb.getBundleMap().get("google.com").size());
343+
Optional<SpiffeId> spiffeId = SpiffeUtil.extractSpiffeId(
344+
tb.getBundleMap().get("google.com").toArray(new X509Certificate[0]));
345+
assertTrue(spiffeId.isPresent());
346+
assertEquals("foo.bar.com", spiffeId.get().getTrustDomain());
347+
}
348+
349+
@Test
350+
public void loadTrustBundleFromFileWithMissingX5cSuccessTest() throws Exception {
351+
SpiffeBundle tb = SpiffeUtil.loadTrustBundleFromFile(
352+
copyFileToTmp(SPIFFE_TRUST_BUNDLE_MISSING_X5C));
353+
assertEquals(1, tb.getBundleMap().size());
354+
assertEquals(1, tb.getBundleMap().get("google.com").size());
355+
}
356+
357+
@Test
358+
public void loadTrustBundleFromFileWithEmptyX5cSuccessTest() throws Exception {
359+
SpiffeBundle tb = SpiffeUtil.loadTrustBundleFromFile(
360+
copyFileToTmp(SPIFFE_TRUST_BUNDLE_EMPTY_X5C));
361+
assertEquals(1, tb.getBundleMap().size());
362+
assertEquals(1, tb.getBundleMap().get("google.com").size());
363+
Optional<SpiffeId> spiffeId = SpiffeUtil.extractSpiffeId(
364+
tb.getBundleMap().get("google.com").toArray(new X509Certificate[0]));
365+
assertTrue(spiffeId.isPresent());
366+
assertEquals("foo.bar.com", spiffeId.get().getTrustDomain());
367+
}
368+
369+
@Test
370+
public void loadTrustBundleFromFileWithIgnoredKeysSuccessTest() throws Exception {
371+
SpiffeBundle tb = SpiffeUtil.loadTrustBundleFromFile(
372+
copyFileToTmp(SPIFFE_TRUST_BUNDLE_IGNORED_KEYS));
373+
assertEquals(1, tb.getSequenceNumbers().size());
374+
assertEquals(123L, (long) tb.getSequenceNumbers().get("google.com"));
375+
assertEquals(1, tb.getBundleMap().size());
376+
assertEquals(1, tb.getBundleMap().get("google.com").size());
377+
Optional<SpiffeId> spiffeId = SpiffeUtil.extractSpiffeId(
378+
tb.getBundleMap().get("google.com").toArray(new X509Certificate[0]));
379+
assertTrue(spiffeId.isPresent());
380+
assertEquals("foo.bar.com", spiffeId.get().getTrustDomain());
381+
}
382+
333383
@Test
334384
public void loadTrustBundleFromFileFailureTest() {
335385
// Check the exception if JSON root element is different from 'trust_domains'
@@ -352,10 +402,6 @@ public void loadTrustBundleFromFileFailureTest() {
352402
iae = assertThrows(IllegalArgumentException.class, () -> SpiffeUtil
353403
.loadTrustBundleFromFile(copyFileToTmp(SPIFFE_TRUST_BUNDLE_CORRUPTED_CERT)));
354404
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());
359405
// Check the exception if 'kty' value differs from 'RSA'
360406
iae = assertThrows(IllegalArgumentException.class, () -> SpiffeUtil
361407
.loadTrustBundleFromFile(copyFileToTmp(SPIFFE_TRUST_BUNDLE_WRONG_KTY)));
@@ -371,11 +417,6 @@ public void loadTrustBundleFromFileFailureTest() {
371417
.loadTrustBundleFromFile(copyFileToTmp(SPIFFE_TRUST_BUNDLE_WRONG_USE)));
372418
assertEquals("'use' parameter must be 'x509-svid' but 'i_am_not_x509-svid' found."
373419
+ DOMAIN_ERROR_MESSAGE, iae.getMessage());
374-
// Check the exception if multiple certs are provided for 'x5c'
375-
iae = assertThrows(IllegalArgumentException.class, () -> SpiffeUtil
376-
.loadTrustBundleFromFile(copyFileToTmp(SPIFFE_TRUST_BUNDLE_WRONG_MULTI_CERTS)));
377-
assertEquals("Exactly 1 certificate is expected, but 2 found." + DOMAIN_ERROR_MESSAGE,
378-
iae.getMessage());
379420
}
380421

381422
@Test

0 commit comments

Comments
 (0)