Skip to content

Commit eb3a7da

Browse files
maxlambrechtmaxlambrecht
committed
fix: enforce strict X.509-SVID leaf URI SAN validation
Signed-off-by: Max Lambrecht <maxlambrecht@gmail.com>
1 parent ff80784 commit eb3a7da

5 files changed

Lines changed: 146 additions & 11 deletions

File tree

‎java-spiffe-core/src/main/java/io/spiffe/internal/CertificateUtils.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ public static SpiffeId getSpiffeId(final X509Certificate certificate) throws Cer
118118
throw new CertificateException("Certificate contains multiple SPIFFE IDs");
119119
}
120120

121-
if (spiffeIds.size() < 1) {
121+
if (spiffeIds.isEmpty()) {
122122
throw new CertificateException("Certificate does not contain SPIFFE ID in the URI SAN");
123123
}
124124

@@ -143,16 +143,25 @@ public static boolean isCA(final X509Certificate cert) {
143143

144144
public static boolean hasKeyUsageCertSign(final X509Certificate cert) {
145145
boolean[] keyUsage = cert.getKeyUsage();
146+
if (keyUsage == null) {
147+
return false;
148+
}
146149
return keyUsage[KEY_CERT_SIGN.index()];
147150
}
148151

149152
public static boolean hasKeyUsageDigitalSignature(final X509Certificate cert) {
150153
boolean[] keyUsage = cert.getKeyUsage();
154+
if (keyUsage == null) {
155+
return false;
156+
}
151157
return keyUsage[DIGITAL_SIGNATURE.index()];
152158
}
153159

154160
public static boolean hasKeyUsageCRLSign(final X509Certificate cert) {
155161
boolean[] keyUsage = cert.getKeyUsage();
162+
if (keyUsage == null) {
163+
return false;
164+
}
156165
return keyUsage[CRL_SIGN.index()];
157166
}
158167

‎java-spiffe-core/src/main/java/io/spiffe/svid/x509svid/X509Svid.java‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import java.security.cert.CertificateParsingException;
1717
import java.security.cert.X509Certificate;
1818
import java.security.spec.InvalidKeySpecException;
19+
import java.util.Collection;
1920
import java.util.Collections;
2021
import java.util.List;
2122
import java.util.Objects;
@@ -27,6 +28,8 @@
2728
*/
2829
public class X509Svid {
2930

31+
private static final int URI_SAN_TYPE = 6;
32+
3033
SpiffeId spiffeId;
3134

3235
/**
@@ -238,13 +241,38 @@ private static void validateLeafSpiffeId(final SpiffeId spiffeId) throws X509Svi
238241
private static SpiffeId getSpiffeId(final List<X509Certificate> x509Certificates) throws X509SvidException {
239242
final SpiffeId spiffeId;
240243
try {
241-
spiffeId = CertificateUtils.getSpiffeId(x509Certificates.get(0));
244+
X509Certificate leaf = x509Certificates.get(0);
245+
validateLeafHasSingleUriSan(leaf);
246+
spiffeId = CertificateUtils.getSpiffeId(leaf);
242247
} catch (CertificateException e) {
243248
throw new X509SvidException(e.getMessage(), e);
244249
}
245250
return spiffeId;
246251
}
247252

253+
private static void validateLeafHasSingleUriSan(final X509Certificate leaf)
254+
throws CertificateException, X509SvidException {
255+
final Collection<List<?>> subjectAlternativeNames = leaf.getSubjectAlternativeNames();
256+
257+
int uriSanCount = 0;
258+
if (subjectAlternativeNames != null) {
259+
for (List<?> sanEntry : subjectAlternativeNames) {
260+
if (sanEntry == null || sanEntry.isEmpty()) {
261+
continue;
262+
}
263+
264+
Object sanType = sanEntry.get(0);
265+
if (sanType instanceof Integer && (Integer) sanType == URI_SAN_TYPE) {
266+
uriSanCount++;
267+
}
268+
}
269+
}
270+
271+
if (uriSanCount != 1) {
272+
throw new X509SvidException("Leaf certificate must contain exactly one URI SAN");
273+
}
274+
}
275+
248276
private static PrivateKey generatePrivateKey(final byte[] privateKeyBytes,
249277
final KeyFileFormat keyFileFormat,
250278
final List<X509Certificate> x509Certificates)

‎java-spiffe-core/src/test/java/io/spiffe/internal/CertificateUtilsTest.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@
2828
import static io.spiffe.internal.AsymmetricKeyAlgorithm.RSA;
2929
import static io.spiffe.utils.TestUtils.toUri;
3030
import static io.spiffe.utils.X509CertificateTestUtils.createCertificate;
31+
import static io.spiffe.utils.X509CertificateTestUtils.createCertificateWithoutKeyUsage;
3132
import static io.spiffe.utils.X509CertificateTestUtils.createRootCA;
33+
import static org.junit.jupiter.api.Assertions.assertFalse;
3234
import static org.junit.jupiter.api.Assertions.assertEquals;
3335
import static org.junit.jupiter.api.Assertions.assertNotNull;
3436
import static org.junit.jupiter.api.Assertions.fail;
@@ -196,4 +198,21 @@ void testGetTrustDomain() throws Exception {
196198
fail(e);
197199
}
198200
}
201+
202+
@Test
203+
void keyUsageChecks_noKeyUsageExtension() throws Exception {
204+
final CertAndKeyPair rootCa = createRootCA("C = US, O = SPIFFE", "spiffe://domain.test");
205+
final CertAndKeyPair leaf = createCertificateWithoutKeyUsage(
206+
"C = US, O = SPIRE",
207+
"C = US, O = SPIRE",
208+
"spiffe://domain.test/workload",
209+
rootCa,
210+
false
211+
);
212+
213+
X509Certificate certificate = leaf.getCertificate();
214+
assertFalse(CertificateUtils.hasKeyUsageDigitalSignature(certificate));
215+
assertFalse(CertificateUtils.hasKeyUsageCertSign(certificate));
216+
assertFalse(CertificateUtils.hasKeyUsageCRLSign(certificate));
217+
}
199218
}

‎java-spiffe-core/src/test/java/io/spiffe/svid/x509svid/X509SvidTest.java‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717
import java.nio.file.Path;
1818
import java.nio.file.Paths;
1919
import java.security.cert.X509Certificate;
20+
import java.util.Arrays;
2021
import java.util.stream.Stream;
2122

2223
import static io.spiffe.utils.TestUtils.toUri;
2324
import static io.spiffe.utils.X509CertificateTestUtils.createCertificate;
25+
import static io.spiffe.utils.X509CertificateTestUtils.createCertificateWithUriSans;
2426
import static io.spiffe.utils.X509CertificateTestUtils.createRootCA;
2527
import static org.junit.jupiter.api.Assertions.assertEquals;
2628
import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -113,7 +115,7 @@ static Stream<Arguments> provideX509SvidScenarios() {
113115
.certsPath(leafEmptyID)
114116
.keyPath(keyRSA)
115117
.hint("")
116-
.expectedError("Certificate does not contain SPIFFE ID in the URI SAN")
118+
.expectedError("Leaf certificate must contain exactly one URI SAN")
117119
.build()
118120
),
119121
Arguments.of(TestCase
@@ -352,6 +354,28 @@ void parseRaw_leafSpiffeIdWithRootOnlyPath_isRejected() throws Exception {
352354
assertEquals("Path cannot have a trailing slash", exception.getMessage());
353355
}
354356

357+
@Test
358+
void parseRaw_leafWithMultipleUriSans_isRejected() throws Exception {
359+
CertAndKeyPair rootCa = createRootCA("C = US, O = SPIFFE", "spiffe://example.org");
360+
CertAndKeyPair leaf = createCertificateWithUriSans(
361+
"C = US, O = SPIRE",
362+
"C = US, O = SPIFFE",
363+
Arrays.asList("spiffe://example.org/workload", "https://example.org/workload"),
364+
rootCa,
365+
false
366+
);
367+
368+
byte[] certBytes = leaf.getCertificate().getEncoded();
369+
byte[] keyBytes = leaf.getKeyPair().getPrivate().getEncoded();
370+
371+
X509SvidException exception = assertThrows(
372+
X509SvidException.class,
373+
() -> X509Svid.parseRaw(certBytes, keyBytes)
374+
);
375+
376+
assertEquals("Leaf certificate must contain exactly one URI SAN", exception.getMessage());
377+
}
378+
355379
@ParameterizedTest
356380
@MethodSource("provideX509SvidScenarios")
357381
void parseX509Svid(TestCase testCase) {

‎java-spiffe-core/src/testFixtures/java/io/spiffe/utils/X509CertificateTestUtils.java‎

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@
2828
import java.security.cert.X509Certificate;
2929
import java.time.Instant;
3030
import java.time.temporal.ChronoUnit;
31+
import java.util.ArrayList;
32+
import java.util.Collections;
3133
import java.util.Date;
34+
import java.util.List;
3235

3336
public class X509CertificateTestUtils {
3437

@@ -53,7 +56,43 @@ public static CertAndKeyPair createCertificate(String subject, String issuerSubj
5356
KeyPair certKeyPair = generateKeyPair();
5457
PrivateKey issuerKey = issuer.keyPair.getPrivate();
5558
JcaX509v3CertificateBuilder builder = getCertificateBuilder(certKeyPair, subject, issuerSubject);
56-
addCertExtensions(builder, spiffeId, isCa);
59+
addCertExtensions(builder, Collections.singletonList(spiffeId), isCa, true);
60+
X509Certificate cert = getSignedX509Certificate(issuerKey, builder);
61+
return new CertAndKeyPair(cert, certKeyPair);
62+
}
63+
64+
/**
65+
* Creates a certificate with a custom list of URI SAN values.
66+
*/
67+
public static CertAndKeyPair createCertificateWithUriSans(
68+
String subject,
69+
String issuerSubject,
70+
List<String> uriSans,
71+
CertAndKeyPair issuer,
72+
boolean isCa
73+
) throws Exception {
74+
KeyPair certKeyPair = generateKeyPair();
75+
PrivateKey issuerKey = issuer.keyPair.getPrivate();
76+
JcaX509v3CertificateBuilder builder = getCertificateBuilder(certKeyPair, subject, issuerSubject);
77+
addCertExtensions(builder, uriSans, isCa, true);
78+
X509Certificate cert = getSignedX509Certificate(issuerKey, builder);
79+
return new CertAndKeyPair(cert, certKeyPair);
80+
}
81+
82+
/**
83+
* Creates a certificate without a KeyUsage extension.
84+
*/
85+
public static CertAndKeyPair createCertificateWithoutKeyUsage(
86+
String subject,
87+
String issuerSubject,
88+
String spiffeId,
89+
CertAndKeyPair issuer,
90+
boolean isCa
91+
) throws Exception {
92+
KeyPair certKeyPair = generateKeyPair();
93+
PrivateKey issuerKey = issuer.keyPair.getPrivate();
94+
JcaX509v3CertificateBuilder builder = getCertificateBuilder(certKeyPair, subject, issuerSubject);
95+
addCertExtensions(builder, Collections.singletonList(spiffeId), isCa, false);
5796
X509Certificate cert = getSignedX509Certificate(issuerKey, builder);
5897
return new CertAndKeyPair(cert, certKeyPair);
5998
}
@@ -63,25 +102,41 @@ private static KeyPair generateKeyPair() throws NoSuchAlgorithmException {
63102
return keyGen.generateKeyPair();
64103
}
65104

66-
private static void addCertExtensions(JcaX509v3CertificateBuilder builder, String spiffeId, boolean isCa) throws CertIOException {
105+
private static void addCertExtensions(
106+
JcaX509v3CertificateBuilder builder,
107+
List<String> uriSans,
108+
boolean isCa,
109+
boolean includeKeyUsage
110+
) throws CertIOException {
67111
builder.addExtension(Extension.basicConstraints, true, new BasicConstraints(isCa));
68112

69113
if (isCa) {
70-
KeyUsage usage = new KeyUsage(KeyUsage.keyCertSign | KeyUsage.digitalSignature | KeyUsage.cRLSign);
71-
builder.addExtension(Extension.keyUsage, true, usage);
114+
if (includeKeyUsage) {
115+
KeyUsage usage = new KeyUsage(KeyUsage.keyCertSign | KeyUsage.digitalSignature | KeyUsage.cRLSign);
116+
builder.addExtension(Extension.keyUsage, true, usage);
117+
}
72118
} else {
73-
KeyUsage usage = new KeyUsage(KeyUsage.digitalSignature | KeyUsage.keyEncipherment | KeyUsage.keyAgreement);
74-
builder.addExtension(Extension.keyUsage, true, usage);
119+
if (includeKeyUsage) {
120+
KeyUsage usage = new KeyUsage(KeyUsage.digitalSignature | KeyUsage.keyEncipherment | KeyUsage.keyAgreement);
121+
builder.addExtension(Extension.keyUsage, true, usage);
122+
}
75123

76124
ASN1EncodableVector purposes = new ASN1EncodableVector();
77125
purposes.add(KeyPurposeId.id_kp_serverAuth);
78126
purposes.add(KeyPurposeId.id_kp_clientAuth);
79127
builder.addExtension(Extension.extendedKeyUsage, false, new DERSequence(purposes));
80128
}
81129

82-
if (StringUtils.isNotBlank(spiffeId)) {
130+
List<GeneralName> uriGeneralNames = new ArrayList<>();
131+
for (String uriSan : uriSans) {
132+
if (StringUtils.isNotBlank(uriSan)) {
133+
uriGeneralNames.add(new GeneralName(GeneralName.uniformResourceIdentifier, uriSan));
134+
}
135+
}
136+
137+
if (!uriGeneralNames.isEmpty()) {
83138
builder.addExtension(Extension.subjectAlternativeName, false,
84-
new GeneralNames(new GeneralName(GeneralName.uniformResourceIdentifier, spiffeId )));
139+
new GeneralNames(uriGeneralNames.toArray(new GeneralName[0])));
85140
}
86141
}
87142

0 commit comments

Comments
 (0)