diff --git a/java-spiffe-core/src/main/java/io/spiffe/internal/CertificateUtils.java b/java-spiffe-core/src/main/java/io/spiffe/internal/CertificateUtils.java index 5f48ceae..54c8e1f6 100644 --- a/java-spiffe-core/src/main/java/io/spiffe/internal/CertificateUtils.java +++ b/java-spiffe-core/src/main/java/io/spiffe/internal/CertificateUtils.java @@ -1,5 +1,6 @@ package io.spiffe.internal; +import io.spiffe.exception.InvalidSpiffeIdException; import io.spiffe.spiffeid.SpiffeId; import io.spiffe.spiffeid.TrustDomain; @@ -24,7 +25,6 @@ import static io.spiffe.internal.KeyUsage.CRL_SIGN; import static io.spiffe.internal.KeyUsage.DIGITAL_SIGNATURE; import static io.spiffe.internal.KeyUsage.KEY_CERT_SIGN; -import static org.apache.commons.lang3.StringUtils.startsWith; /** * Common certificate utility methods. @@ -32,7 +32,9 @@ public class CertificateUtils { private static final String SPIFFE_PREFIX = "spiffe://"; + private static final int SAN_TYPE_INDEX = 0; private static final int SAN_VALUE_INDEX = 1; + private static final int URI_SAN_TYPE = 6; private static final String PUBLIC_KEY_INFRASTRUCTURE_ALGORITHM = "PKIX"; private static final String X509_CERTIFICATE_TYPE = "X.509"; @@ -122,7 +124,11 @@ public static SpiffeId getSpiffeId(final X509Certificate certificate) throws Cer throw new CertificateException("Certificate does not contain SPIFFE ID in the URI SAN"); } - return SpiffeId.parse(spiffeIds.get(0)); + try { + return SpiffeId.parse(spiffeIds.get(0)); + } catch (InvalidSpiffeIdException | IllegalArgumentException e) { + throw new CertificateException("Certificate contains invalid SPIFFE ID in the URI SAN", e); + } } /** @@ -182,8 +188,9 @@ private static List getSpiffeIds(final X509Certificate certificate) thro } return certificate.getSubjectAlternativeNames() .stream() + .filter(san -> URI_SAN_TYPE == (Integer) san.get(SAN_TYPE_INDEX)) .map(san -> (String) san.get(SAN_VALUE_INDEX)) - .filter(uri -> startsWith(uri, SPIFFE_PREFIX)) + .filter(uri -> uri != null && uri.startsWith(SPIFFE_PREFIX)) .collect(Collectors.toList()); } diff --git a/java-spiffe-core/src/test/java/io/spiffe/internal/CertificateUtilsTest.java b/java-spiffe-core/src/test/java/io/spiffe/internal/CertificateUtilsTest.java index e9506abd..bf3cbe9e 100644 --- a/java-spiffe-core/src/test/java/io/spiffe/internal/CertificateUtilsTest.java +++ b/java-spiffe-core/src/test/java/io/spiffe/internal/CertificateUtilsTest.java @@ -22,6 +22,7 @@ import java.security.cert.X509Certificate; import java.security.spec.InvalidKeySpecException; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.List; @@ -33,6 +34,8 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; public class CertificateUtilsTest { @@ -170,6 +173,43 @@ void testGetSpiffeId() throws Exception { assertEquals(SpiffeId.parse("spiffe://domain.test/workload"), spiffeId); } + @Test + void testGetSpiffeId_certWithDnsAndIpSans_doesNotThrowClassCastException() throws Exception { + X509Certificate certificate = new CertificateWithSubjectAlternativeNames(Arrays.asList( + Arrays.asList(6, "spiffe://domain.test/workload"), + Arrays.asList(2, "workload.domain.test"), + Arrays.asList(7, "127.0.0.1") + )); + + SpiffeId spiffeId = CertificateUtils.getSpiffeId(certificate); + + assertEquals(SpiffeId.parse("spiffe://domain.test/workload"), spiffeId); + } + + @Test + void testGetSpiffeId_certWithNonStringSanType_doesNotThrowClassCastException() throws Exception { + X509Certificate certificate = new CertificateWithSubjectAlternativeNames(Arrays.asList( + Arrays.asList(6, "spiffe://domain.test/workload"), + Arrays.asList(0, new byte[]{1, 2, 3}) + )); + + SpiffeId spiffeId = CertificateUtils.getSpiffeId(certificate); + + assertEquals(SpiffeId.parse("spiffe://domain.test/workload"), spiffeId); + } + + @Test + void testGetSpiffeId_certWithMalformedSpiffeUriSan_throwsCertificateException() throws Exception { + X509Certificate certificate = new CertificateWithSubjectAlternativeNames(Collections.singletonList( + Arrays.asList(6, "spiffe://domain.test/workload/invalid path") + )); + + CertificateException exception = assertThrows(CertificateException.class, () -> CertificateUtils.getSpiffeId(certificate)); + + assertEquals("Certificate contains invalid SPIFFE ID in the URI SAN", exception.getMessage()); + assertInstanceOf(RuntimeException.class, exception.getCause()); + } + @Test void testGetSpiffeId_certNotContainSpiffeId_throwsCertificateException() throws Exception { final CertAndKeyPair rootCa = createRootCA("C = US, O = SPIFFE", "spiffe://domain.test"); @@ -215,4 +255,17 @@ void keyUsageChecks_noKeyUsageExtension() throws Exception { assertFalse(CertificateUtils.hasKeyUsageCertSign(certificate)); assertFalse(CertificateUtils.hasKeyUsageCRLSign(certificate)); } + + private static class CertificateWithSubjectAlternativeNames extends DummyX509Certificate { + private final Collection> subjectAlternativeNames; + + private CertificateWithSubjectAlternativeNames(Collection> subjectAlternativeNames) { + this.subjectAlternativeNames = subjectAlternativeNames; + } + + @Override + public Collection> getSubjectAlternativeNames() throws CertificateParsingException { + return subjectAlternativeNames; + } + } } diff --git a/java-spiffe-core/src/test/java/io/spiffe/svid/x509svid/X509SvidTest.java b/java-spiffe-core/src/test/java/io/spiffe/svid/x509svid/X509SvidTest.java index d1b1187c..2dbf01ef 100644 --- a/java-spiffe-core/src/test/java/io/spiffe/svid/x509svid/X509SvidTest.java +++ b/java-spiffe-core/src/test/java/io/spiffe/svid/x509svid/X509SvidTest.java @@ -16,6 +16,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.util.Arrays; import java.util.stream.Stream; @@ -25,6 +26,7 @@ import static io.spiffe.utils.X509CertificateTestUtils.createCertificateWithUriSans; import static io.spiffe.utils.X509CertificateTestUtils.createRootCA; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; @@ -346,12 +348,15 @@ void parseRaw_leafSpiffeIdWithRootOnlyPath_isRejected() throws Exception { byte[] certBytes = leaf.getCertificate().getEncoded(); byte[] keyBytes = leaf.getKeyPair().getPrivate().getEncoded(); - InvalidSpiffeIdException exception = assertThrows( - InvalidSpiffeIdException.class, + X509SvidException exception = assertThrows( + X509SvidException.class, () -> X509Svid.parseRaw(certBytes, keyBytes) ); - assertEquals("Path cannot have a trailing slash", exception.getMessage()); + assertEquals("Certificate contains invalid SPIFFE ID in the URI SAN", exception.getMessage()); + assertInstanceOf(CertificateException.class, exception.getCause()); + assertInstanceOf(InvalidSpiffeIdException.class, exception.getCause().getCause()); + assertEquals("Path cannot have a trailing slash", exception.getCause().getCause().getMessage()); } @Test