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 56a653a5..5f48ceae 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 @@ -118,7 +118,7 @@ public static SpiffeId getSpiffeId(final X509Certificate certificate) throws Cer throw new CertificateException("Certificate contains multiple SPIFFE IDs"); } - if (spiffeIds.size() < 1) { + if (spiffeIds.isEmpty()) { throw new CertificateException("Certificate does not contain SPIFFE ID in the URI SAN"); } @@ -143,16 +143,25 @@ public static boolean isCA(final X509Certificate cert) { public static boolean hasKeyUsageCertSign(final X509Certificate cert) { boolean[] keyUsage = cert.getKeyUsage(); + if (keyUsage == null) { + return false; + } return keyUsage[KEY_CERT_SIGN.index()]; } public static boolean hasKeyUsageDigitalSignature(final X509Certificate cert) { boolean[] keyUsage = cert.getKeyUsage(); + if (keyUsage == null) { + return false; + } return keyUsage[DIGITAL_SIGNATURE.index()]; } public static boolean hasKeyUsageCRLSign(final X509Certificate cert) { boolean[] keyUsage = cert.getKeyUsage(); + if (keyUsage == null) { + return false; + } return keyUsage[CRL_SIGN.index()]; } diff --git a/java-spiffe-core/src/main/java/io/spiffe/svid/x509svid/X509Svid.java b/java-spiffe-core/src/main/java/io/spiffe/svid/x509svid/X509Svid.java index 47d5fd2d..f79c1750 100644 --- a/java-spiffe-core/src/main/java/io/spiffe/svid/x509svid/X509Svid.java +++ b/java-spiffe-core/src/main/java/io/spiffe/svid/x509svid/X509Svid.java @@ -16,6 +16,7 @@ import java.security.cert.CertificateParsingException; import java.security.cert.X509Certificate; import java.security.spec.InvalidKeySpecException; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Objects; @@ -27,6 +28,8 @@ */ public class X509Svid { + private static final int URI_SAN_TYPE = 6; + SpiffeId spiffeId; /** @@ -238,13 +241,38 @@ private static void validateLeafSpiffeId(final SpiffeId spiffeId) throws X509Svi private static SpiffeId getSpiffeId(final List x509Certificates) throws X509SvidException { final SpiffeId spiffeId; try { - spiffeId = CertificateUtils.getSpiffeId(x509Certificates.get(0)); + X509Certificate leaf = x509Certificates.get(0); + validateLeafHasSingleUriSan(leaf); + spiffeId = CertificateUtils.getSpiffeId(leaf); } catch (CertificateException e) { throw new X509SvidException(e.getMessage(), e); } return spiffeId; } + private static void validateLeafHasSingleUriSan(final X509Certificate leaf) + throws CertificateException, X509SvidException { + final Collection> subjectAlternativeNames = leaf.getSubjectAlternativeNames(); + + int uriSanCount = 0; + if (subjectAlternativeNames != null) { + for (List sanEntry : subjectAlternativeNames) { + if (sanEntry == null || sanEntry.isEmpty()) { + continue; + } + + Object sanType = sanEntry.get(0); + if (sanType instanceof Integer && (Integer) sanType == URI_SAN_TYPE) { + uriSanCount++; + } + } + } + + if (uriSanCount != 1) { + throw new X509SvidException("Leaf certificate must contain exactly one URI SAN"); + } + } + private static PrivateKey generatePrivateKey(final byte[] privateKeyBytes, final KeyFileFormat keyFileFormat, final List x509Certificates) 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 7c2d8ffd..e9506abd 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 @@ -28,7 +28,9 @@ import static io.spiffe.internal.AsymmetricKeyAlgorithm.RSA; import static io.spiffe.utils.TestUtils.toUri; import static io.spiffe.utils.X509CertificateTestUtils.createCertificate; +import static io.spiffe.utils.X509CertificateTestUtils.createCertificateWithoutKeyUsage; import static io.spiffe.utils.X509CertificateTestUtils.createRootCA; +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.fail; @@ -196,4 +198,21 @@ void testGetTrustDomain() throws Exception { fail(e); } } + + @Test + void keyUsageChecks_noKeyUsageExtension() throws Exception { + final CertAndKeyPair rootCa = createRootCA("C = US, O = SPIFFE", "spiffe://domain.test"); + final CertAndKeyPair leaf = createCertificateWithoutKeyUsage( + "C = US, O = SPIRE", + "C = US, O = SPIRE", + "spiffe://domain.test/workload", + rootCa, + false + ); + + X509Certificate certificate = leaf.getCertificate(); + assertFalse(CertificateUtils.hasKeyUsageDigitalSignature(certificate)); + assertFalse(CertificateUtils.hasKeyUsageCertSign(certificate)); + assertFalse(CertificateUtils.hasKeyUsageCRLSign(certificate)); + } } 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 5689c25d..d1b1187c 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 @@ -17,10 +17,12 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.security.cert.X509Certificate; +import java.util.Arrays; import java.util.stream.Stream; import static io.spiffe.utils.TestUtils.toUri; import static io.spiffe.utils.X509CertificateTestUtils.createCertificate; +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.assertNotNull; @@ -113,7 +115,7 @@ static Stream provideX509SvidScenarios() { .certsPath(leafEmptyID) .keyPath(keyRSA) .hint("") - .expectedError("Certificate does not contain SPIFFE ID in the URI SAN") + .expectedError("Leaf certificate must contain exactly one URI SAN") .build() ), Arguments.of(TestCase @@ -352,6 +354,28 @@ void parseRaw_leafSpiffeIdWithRootOnlyPath_isRejected() throws Exception { assertEquals("Path cannot have a trailing slash", exception.getMessage()); } + @Test + void parseRaw_leafWithMultipleUriSans_isRejected() throws Exception { + CertAndKeyPair rootCa = createRootCA("C = US, O = SPIFFE", "spiffe://example.org"); + CertAndKeyPair leaf = createCertificateWithUriSans( + "C = US, O = SPIRE", + "C = US, O = SPIFFE", + Arrays.asList("spiffe://example.org/workload", "https://example.org/workload"), + rootCa, + false + ); + + byte[] certBytes = leaf.getCertificate().getEncoded(); + byte[] keyBytes = leaf.getKeyPair().getPrivate().getEncoded(); + + X509SvidException exception = assertThrows( + X509SvidException.class, + () -> X509Svid.parseRaw(certBytes, keyBytes) + ); + + assertEquals("Leaf certificate must contain exactly one URI SAN", exception.getMessage()); + } + @ParameterizedTest @MethodSource("provideX509SvidScenarios") void parseX509Svid(TestCase testCase) { diff --git a/java-spiffe-core/src/testFixtures/java/io/spiffe/utils/X509CertificateTestUtils.java b/java-spiffe-core/src/testFixtures/java/io/spiffe/utils/X509CertificateTestUtils.java index 6158a3c4..39206609 100644 --- a/java-spiffe-core/src/testFixtures/java/io/spiffe/utils/X509CertificateTestUtils.java +++ b/java-spiffe-core/src/testFixtures/java/io/spiffe/utils/X509CertificateTestUtils.java @@ -28,7 +28,10 @@ import java.security.cert.X509Certificate; import java.time.Instant; import java.time.temporal.ChronoUnit; +import java.util.ArrayList; +import java.util.Collections; import java.util.Date; +import java.util.List; public class X509CertificateTestUtils { @@ -53,7 +56,43 @@ public static CertAndKeyPair createCertificate(String subject, String issuerSubj KeyPair certKeyPair = generateKeyPair(); PrivateKey issuerKey = issuer.keyPair.getPrivate(); JcaX509v3CertificateBuilder builder = getCertificateBuilder(certKeyPair, subject, issuerSubject); - addCertExtensions(builder, spiffeId, isCa); + addCertExtensions(builder, Collections.singletonList(spiffeId), isCa, true); + X509Certificate cert = getSignedX509Certificate(issuerKey, builder); + return new CertAndKeyPair(cert, certKeyPair); + } + + /** + * Creates a certificate with a custom list of URI SAN values. + */ + public static CertAndKeyPair createCertificateWithUriSans( + String subject, + String issuerSubject, + List uriSans, + CertAndKeyPair issuer, + boolean isCa + ) throws Exception { + KeyPair certKeyPair = generateKeyPair(); + PrivateKey issuerKey = issuer.keyPair.getPrivate(); + JcaX509v3CertificateBuilder builder = getCertificateBuilder(certKeyPair, subject, issuerSubject); + addCertExtensions(builder, uriSans, isCa, true); + X509Certificate cert = getSignedX509Certificate(issuerKey, builder); + return new CertAndKeyPair(cert, certKeyPair); + } + + /** + * Creates a certificate without a KeyUsage extension. + */ + public static CertAndKeyPair createCertificateWithoutKeyUsage( + String subject, + String issuerSubject, + String spiffeId, + CertAndKeyPair issuer, + boolean isCa + ) throws Exception { + KeyPair certKeyPair = generateKeyPair(); + PrivateKey issuerKey = issuer.keyPair.getPrivate(); + JcaX509v3CertificateBuilder builder = getCertificateBuilder(certKeyPair, subject, issuerSubject); + addCertExtensions(builder, Collections.singletonList(spiffeId), isCa, false); X509Certificate cert = getSignedX509Certificate(issuerKey, builder); return new CertAndKeyPair(cert, certKeyPair); } @@ -63,15 +102,24 @@ private static KeyPair generateKeyPair() throws NoSuchAlgorithmException { return keyGen.generateKeyPair(); } - private static void addCertExtensions(JcaX509v3CertificateBuilder builder, String spiffeId, boolean isCa) throws CertIOException { + private static void addCertExtensions( + JcaX509v3CertificateBuilder builder, + List uriSans, + boolean isCa, + boolean includeKeyUsage + ) throws CertIOException { builder.addExtension(Extension.basicConstraints, true, new BasicConstraints(isCa)); if (isCa) { - KeyUsage usage = new KeyUsage(KeyUsage.keyCertSign | KeyUsage.digitalSignature | KeyUsage.cRLSign); - builder.addExtension(Extension.keyUsage, true, usage); + if (includeKeyUsage) { + KeyUsage usage = new KeyUsage(KeyUsage.keyCertSign | KeyUsage.digitalSignature | KeyUsage.cRLSign); + builder.addExtension(Extension.keyUsage, true, usage); + } } else { - KeyUsage usage = new KeyUsage(KeyUsage.digitalSignature | KeyUsage.keyEncipherment | KeyUsage.keyAgreement); - builder.addExtension(Extension.keyUsage, true, usage); + if (includeKeyUsage) { + KeyUsage usage = new KeyUsage(KeyUsage.digitalSignature | KeyUsage.keyEncipherment | KeyUsage.keyAgreement); + builder.addExtension(Extension.keyUsage, true, usage); + } ASN1EncodableVector purposes = new ASN1EncodableVector(); purposes.add(KeyPurposeId.id_kp_serverAuth); @@ -79,9 +127,16 @@ private static void addCertExtensions(JcaX509v3CertificateBuilder builder, Strin builder.addExtension(Extension.extendedKeyUsage, false, new DERSequence(purposes)); } - if (StringUtils.isNotBlank(spiffeId)) { + List uriGeneralNames = new ArrayList<>(); + for (String uriSan : uriSans) { + if (StringUtils.isNotBlank(uriSan)) { + uriGeneralNames.add(new GeneralName(GeneralName.uniformResourceIdentifier, uriSan)); + } + } + + if (!uriGeneralNames.isEmpty()) { builder.addExtension(Extension.subjectAlternativeName, false, - new GeneralNames(new GeneralName(GeneralName.uniformResourceIdentifier, spiffeId ))); + new GeneralNames(uriGeneralNames.toArray(new GeneralName[0]))); } }