Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
* Add `X509Bundle.marshal()` and `JwtBundle.marshal()` to serialize bundles as PEM certificates and as a JWKS document, the formats read by `parse`
* Publish a `java-spiffe-bom` Bill of Materials to align the versions of the java-spiffe modules.

### Fixed

* Reject X.509-SVID leaf certificates with a root SPIFFE ID path in `X509SvidValidator.verifyChain`, which `SpiffeTrustManager` uses for peer certificates (#510)

### Build

* Check core and provider binary compatibility against the latest stable Maven Central release.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ private static X509Svid createX509Svid(final byte[] certsBytes,
final PrivateKey privateKey = generatePrivateKey(privateKeyBytes, keyFileFormat, x509Certificates);
final SpiffeId spiffeId = getSpiffeId(x509Certificates);

validateLeafSpiffeId(spiffeId);
X509SvidProfile.validateLeafSpiffeId(spiffeId);
validateLeafCertificate(x509Certificates.get(0));

// there are intermediate CA certificates
Expand All @@ -228,13 +228,6 @@ private static X509Svid createX509Svid(final byte[] certsBytes,
return new X509Svid(spiffeId, x509Certificates, privateKey, hint);
}

private static void validateLeafSpiffeId(final SpiffeId spiffeId) throws X509SvidException {
final String path = spiffeId.getPath();
if (path == null || path.isEmpty()) {
throw new X509SvidException("Leaf certificate SPIFFE ID must have a non-root path");
}
}

private static SpiffeId getSpiffeId(final List<X509Certificate> x509Certificates) throws X509SvidException {
final SpiffeId spiffeId;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import io.spiffe.exception.X509SvidException;
import io.spiffe.internal.CertificateUtils;
import io.spiffe.spiffeid.SpiffeId;

import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
Expand All @@ -17,9 +18,17 @@ private X509SvidProfile() {

static void validateLeaf(final X509Certificate leaf) throws CertificateException, X509SvidException {
validateLeafHasSingleUriSan(leaf);
validateLeafSpiffeId(CertificateUtils.getSpiffeId(leaf));
validateLeafCertificate(leaf);
}

static void validateLeafSpiffeId(final SpiffeId spiffeId) throws X509SvidException {
final String path = spiffeId.getPath();
if (path == null || path.isEmpty()) {
throw new X509SvidException("Leaf certificate SPIFFE ID must have a non-root path");
}
}

static void validateLeafHasSingleUriSan(final X509Certificate leaf)
throws CertificateException, X509SvidException {
final Collection<List<?>> subjectAlternativeNames = leaf.getSubjectAlternativeNames();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,20 @@ void testVerifyChain_chainCannotBeVerifiedWithAuthorityInBundle_throwsCertificat
}
}

@Test
void verifyChain_leafWithRootPath_throwsCertificateException() throws Exception {
CertAndKeyPair rootPathLeaf = createCertificate("C = US, O = SPIRE", "C = US, O = SPIFFE", "spiffe://example.org", rootCa, false);
List<X509Certificate> rootPathChain = Arrays.asList(rootPathLeaf.getCertificate(), rootCa.getCertificate());

X509Bundle x509Bundle = new X509Bundle(TrustDomain.parse("example.org"), Collections.singleton(rootCa.getCertificate()));
try {
X509SvidValidator.verifyChain(rootPathChain, x509Bundle);
fail("exception is expected");
} catch (CertificateException e) {
assertEquals("Leaf certificate SPIFFE ID must have a non-root path", e.getMessage());
}
}

@Test
void verifyChain_noBundleForTrustDomain_throwsBundleNotFoundException() throws Exception {
HashSet<X509Certificate> x509Authorities = new HashSet<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,27 @@ void checkTrusted_leafCertificateWithAdditionalNonSpiffeUriSan_throwsCertificate
);
}

@Test
void checkTrusted_leafCertificateWithRootPathSpiffeId_acceptAnySpiffeId_throwsCertificateException() throws Exception {
PeerChain peerChain = createPeerChain((subject, issuerSubject, spiffeId, issuer) ->
createCertificate(subject, issuerSubject, "spiffe://example.org", issuer, false));
when(bundleSource.getBundleForTrustDomain(peerChain.trustDomain)).thenReturn(peerChain.bundle);

spiffeTrustManager = new SpiffeTrustManager(bundleSource);

CertificateException clientException = assertThrows(
CertificateException.class,
() -> spiffeTrustManager.checkClientTrusted(peerChain.chain, "")
);
assertEquals("Leaf certificate SPIFFE ID must have a non-root path", clientException.getMessage());

CertificateException serverException = assertThrows(
CertificateException.class,
() -> spiffeTrustManager.checkServerTrusted(peerChain.chain, "")
);
assertEquals("Leaf certificate SPIFFE ID must have a non-root path", serverException.getMessage());
}

@Test
void checkServerTrusted_verifierResult_ThrowCertificateException() throws BundleNotFoundException {
when(bundleSource.getBundleForTrustDomain(TrustDomain.parse("example.org"))).thenReturn(bundleKnown);
Expand Down