Skip to content

Commit b44ebcd

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 b44ebcd

4 files changed

Lines changed: 94 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
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"trust_domains": {
3+
"google.com": {
4+
"spiffe_sequence": 123,
5+
"keys": [
6+
{
7+
"kty": "RSA",
8+
"use": "x509-svid",
9+
"x5c": []
10+
},
11+
{
12+
"kty": "RSA",
13+
"use": "x509-svid",
14+
"x5c": ["MIIFsjCCA5qgAwIBAgIURygVMMzdr+Q7rsUaz189JozyHMwwDQYJKoZIhvcNAQELBQAwTjELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMQwwCgYDVQQHDANTVkwxDTALBgNVBAoMBGdSUEMxFTATBgNVBAMMDHRlc3QtY2xpZW50MTAeFw0yMTEyMjMxODQyNTJaFw0zMTEyMjExODQyNTJaME4xCzAJBgNVBAYTAlVTMQswCQYDVQQIDAJDQTEMMAoGA1UEBwwDU1ZMMQ0wCwYDVQQKDARnUlBDMRUwEwYDVQQDDAx0ZXN0LWNsaWVudDEwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQDJ4AqpGetyVSqGUuBJLVFla+7bEfca7UYzfVSSZLZ/X+JDmWIVN8UIPuFib5jhMEc3XaUnFXUmM7zEtz/ZG5hapwLwOb2C3ZxOP6PQjYCJxbkLie+b43UQrFu1xxd3vMhVJgcj/AIxEpmszuqOa6kUrkYifjJADQ+64kZgl66bsTdXMCzpxyFl9xUfff59L8OX+HUfAcoZz3emjg3ZJPYURQEmjdZTOau1EjFilwHgd989Jt7NKgx30NXoHmw7nusVBIY94fL2VKN3f1XVm0dHu5NI279Q6zr0ZBU7k5T3IeHnzsUesQS4NGlklDWoVTKk73Uv9Pna8yQsSW757PEbHOGp9Knu4bnoGPOlsG81yIPipO6hTgGFK24pF97M9kpGbWqYX4+2vLlrCAfcmsHqaUPmQlYeRVTT6vw7ctYo2kyUYGtnODXk76LqewRBVvkzx75QUhfjAyb740YcDmIenc56Tq6gebJHjhEmVSehR6xIpXP7SVeurTyhPsEQnpJHtgs4dcwWOZp7BvPNzHXmJqfr7vsshie3vS5kQ0u1e1yqAqXgyDjqKXOkx+dpgUTehSJHhPNHvTc5LXRsvvXKYz6FrwR/DZ8t7BNEvPeLjFgxpH7QVJFLCvCbXs5K6yYbsnLfxFIBPRnrbJkIsK+sQwnRdnsiUdPsTkG5B2lQfQIDAQABo4GHMIGEMB0GA1UdDgQWBBQ2lBp0PiRHHvQ5IRURm8aHsj4RETAfBgNVHSMEGDAWgBQ2lBp0PiRHHvQ5IRURm8aHsj4RETAPBgNVHRMBAf8EBTADAQH/MDEGA1UdEQQqMCiGJnNwaWZmZTovL2Zvby5iYXIuY29tL2NsaWVudC93b3JrbG9hZC8xMA0GCSqGSIb3DQEBCwUAA4ICAQA1mSkgRclAl+E/aS9zJ7t8+Y4n3T24nOKKveSIjxXm/zjhWqVsLYBI6kglWtih2+PELvU8JdPqNZK34Kl0Q6FWpVSGDdWN1i6NyORt2ocggL3ke3iXxRk3UpUKJmqwz81VhA2KUHnMlyE0IufFfZNwNWWHBv13uJfRbjeQpKPhU+yf4DeXrsWcvrZlGvAET+mcplafUzCp7Iv+PcISJtUerbxbVtuHVeZCLlgDXWkLAWJN8rf0dIG4x060LJ+j6j9uRVhb9sZn1HJV+j4XdIYm1VKilluhOtNwP2d3Ox/JuTBxf7hFHXZPfMagQE5k5PzmxRaCAEMJ1l2DvUbZw+shJfSNoWcBo2qadnUaWT3BmmJRBDh7ZReib/RQ1Rd4ygOyzP3E0vkV4/gqyjLdApXh5PZP8KLQZ+1JN/sdWt7VfIt9wYOpkIqujdll51ESHzwQeAK9WVCB4UvVz6zdhItB9CRbXPreWC+wCB1xDovIzFKOVsLs5+Gqs1m7VinG2LxbDqaKyo/FB0Hxx0acBNzezLWoDwXYQrN0T0S4pnqhKD1CYPpdArBkNezUYAjS725FkApuK+mnBX3U0msBffEaUEOkcyar1EW2m/33vpetD/k3eQQkmvQf4Hbiu9AF+9cNDm/hMuXEw5EXGA91fn0891b5eEW8BJHXX0jri0aN8g=="]
15+
}
16+
]
17+
}
18+
}
19+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"trust_domains": {
3+
"google.com": {
4+
"spiffe_sequence": 123,
5+
"keys": [
6+
{
7+
"kty": "RSA",
8+
"use": "x509-svid"
9+
},
10+
{
11+
"kty": "RSA",
12+
"use": "x509-svid",
13+
"x5c": []
14+
},
15+
{
16+
"kty": "RSA",
17+
"use": "x509-svid",
18+
"x5c": ["MIIFsjCCA5qgAwIBAgIURygVMMzdr+Q7rsUaz189JozyHMwwDQYJKoZIhvcNAQELBQAwTjELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMQwwCgYDVQQHDANTVkwxDTALBgNVBAoMBGdSUEMxFTATBgNVBAMMDHRlc3QtY2xpZW50MTAeFw0yMTEyMjMxODQyNTJaFw0zMTEyMjExODQyNTJaME4xCzAJBgNVBAYTAlVTMQswCQYDVQQIDAJDQTEMMAoGA1UEBwwDU1ZMMQ0wCwYDVQQKDARnUlBDMRUwEwYDVQQDDAx0ZXN0LWNsaWVudDEwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQDJ4AqpGetyVSqGUuBJLVFla+7bEfca7UYzfVSSZLZ/X+JDmWIVN8UIPuFib5jhMEc3XaUnFXUmM7zEtz/ZG5hapwLwOb2C3ZxOP6PQjYCJxbkLie+b43UQrFu1xxd3vMhVJgcj/AIxEpmszuqOa6kUrkYifjJADQ+64kZgl66bsTdXMCzpxyFl9xUfff59L8OX+HUfAcoZz3emjg3ZJPYURQEmjdZTOau1EjFilwHgd989Jt7NKgx30NXoHmw7nusVBIY94fL2VKN3f1XVm0dHu5NI279Q6zr0ZBU7k5T3IeHnzsUesQS4NGlklDWoVTKk73Uv9Pna8yQsSW757PEbHOGp9Knu4bnoGPOlsG81yIPipO6hTgGFK24pF97M9kpGbWqYX4+2vLlrCAfcmsHqaUPmQlYeRVTT6vw7ctYo2kyUYGtnODXk76LqewRBVvkzx75QUhfjAyb740YcDmIenc56Tq6gebJHjhEmVSehR6xIpXP7SVeurTyhPsEQnpJHtgs4dcwWOZp7BvPNzHXmJqfr7vsshie3vS5kQ0u1e1yqAqXgyDjqKXOkx+dpgUTehSJHhPNHvTc5LXRsvvXKYz6FrwR/DZ8t7BNEvPeLjFgxpH7QVJFLCvCbXs5K6yYbsnLfxFIBPRnrbJkIsK+sQwnRdnsiUdPsTkG5B2lQfQIDAQABo4GHMIGEMB0GA1UdDgQWBBQ2lBp0PiRHHvQ5IRURm8aHsj4RETAfBgNVHSMEGDAWgBQ2lBp0PiRHHvQ5IRURm8aHsj4RETAPBgNVHRMBAf8EBTADAQH/MDEGA1UdEQQqMCiGJnNwaWZmZTovL2Zvby5iYXIuY29tL2NsaWVudC93b3JrbG9hZC8xMA0GCSqGSIb3DQEBCwUAA4ICAQA1mSkgRclAl+E/aS9zJ7t8+Y4n3T24nOKKveSIjxXm/zjhWqVsLYBI6kglWtih2+PELvU8JdPqNZK34Kl0Q6FWpVSGDdWN1i6NyORt2ocggL3ke3iXxRk3UpUKJmqwz81VhA2KUHnMlyE0IufFfZNwNWWHBv13uJfRbjeQpKPhU+yf4DeXrsWcvrZlGvAET+mcplafUzCp7Iv+PcISJtUerbxbVtuHVeZCLlgDXWkLAWJN8rf0dIG4x060LJ+j6j9uRVhb9sZn1HJV+j4XdIYm1VKilluhOtNwP2d3Ox/JuTBxf7hFHXZPfMagQE5k5PzmxRaCAEMJ1l2DvUbZw+shJfSNoWcBo2qadnUaWT3BmmJRBDh7ZReib/RQ1Rd4ygOyzP3E0vkV4/gqyjLdApXh5PZP8KLQZ+1JN/sdWt7VfIt9wYOpkIqujdll51ESHzwQeAK9WVCB4UvVz6zdhItB9CRbXPreWC+wCB1xDovIzFKOVsLs5+Gqs1m7VinG2LxbDqaKyo/FB0Hxx0acBNzezLWoDwXYQrN0T0S4pnqhKD1CYPpdArBkNezUYAjS725FkApuK+mnBX3U0msBffEaUEOkcyar1EW2m/33vpetD/k3eQQkmvQf4Hbiu9AF+9cNDm/hMuXEw5EXGA91fn0891b5eEW8BJHXX0jri0aN8g=="]
19+
}
20+
]
21+
}
22+
}
23+
}

0 commit comments

Comments
 (0)