Skip to content

Commit

Permalink
[eclipse-leshan#371] Fix truncating of certificate chain.
Browse files Browse the repository at this point in the history
The certificate chain sent in a CERTIFICATE message should not contain
certificates representing authorities trusted by the peer (as indicated
in its CERTIFICATE_REQUEST message).

The filtering of such certificates from the chain has been erroneously
done on the certificates' issuer DN instead of using the subject DN.
This has been fixed.
  • Loading branch information
Kai Hudalla committed Aug 1, 2017
1 parent fc41b35 commit 7530b2c
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ public X509Certificate[] removeTrustedCertificates(X509Certificate[] chain) {
} else if (chain.length > 1) {
int i = 1;
for ( ; i < chain.length; i++) {
if (certificateAuthorities.contains(chain[i].getIssuerX500Principal())) {
if (certificateAuthorities.contains(chain[i].getSubjectX500Principal())) {
break;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,26 @@
******************************************************************************/
package org.eclipse.californium.scandium.dtls;

import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.security.PublicKey;
import java.security.cert.CertPath;
import java.security.cert.CertPathValidator;
import java.security.cert.CertificateFactory;
import java.security.cert.PKIXParameters;
import java.security.cert.TrustAnchor;
import java.security.cert.X509Certificate;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import javax.security.auth.x500.X500Principal;

Expand Down Expand Up @@ -172,27 +185,35 @@ public void testAddCertificateAuthorityAssertsMaxLength() {
@Test
public void testTruncateCertificateChainReturnsNonTrustedCertsOnly() throws Exception {

// GIVEN a certificate request with a root CA trust anchor
CertificateFactory factory = CertificateFactory.getInstance("X.509");
X509Certificate[] clientChain = DtlsTestTools.getClientCertificateChain();
X509Certificate[] trustAnchor = DtlsTestTools.getTrustedCertificates();
X509Certificate trustedCertToRemove = null;
X509Certificate[] trustAnchor = new X509Certificate[1];
trustAnchor[0] = DtlsTestTools.getTrustedRootCA();
Set<TrustAnchor> trustAnchors = getTrustAnchors(trustAnchor);
CertificateRequest req = new CertificateRequest(peerAddress);
req.addCertificateAuthorities(trustAnchor);

// WHEN removing trusted certificates from a certificate chain rooting in the trust anchor
List<X509Certificate> truncatedChain = Arrays.asList(req.removeTrustedCertificates(clientChain));
CertPath clientPath = factory.generateCertPath(truncatedChain);

// THEN none of the trust anchors is part of the truncated chain
for (X509Certificate trustedCert : trustAnchor) {
if (isCertificatePartOfChain(trustedCert, clientChain)) {
trustedCertToRemove = trustedCert;
break;
if (isCertificatePartOfChain(trustedCert, truncatedChain)) {
fail("truncated certificate list should not contain any trust anchors");
}
}
assertThat(trustedCertToRemove, is(notNullValue()));
// and the truncated chain can still be validated successfully based on the trust anchors
PKIXParameters params = new PKIXParameters(trustAnchors);
params.setRevocationEnabled(false);

CertificateRequest req = new CertificateRequest(peerAddress);
req.addCertificateAuthorities(trustAnchor);
X509Certificate[] truncatedChain = req.removeTrustedCertificates(clientChain);
assertTrue(truncatedChain.length < clientChain.length);
assertFalse(isCertificatePartOfChain(trustedCertToRemove, truncatedChain));
CertPathValidator validator = CertPathValidator.getInstance("PKIX");
validator.validate(clientPath, params);
}

/**
* Verifies that a certificate chain is not truncated when it includes not trusted certificates only.
* Verifies that a certificate chain is not truncated when it consists of untrusted certificates only.
*
* @throws Exception if the key cannot be loaded.
*/
Expand All @@ -205,12 +226,22 @@ public void testTruncateCertificateChainReturnsAllNonTrustedCerts() throws Excep
assertTrue(truncatedChain.length == certChain.length);
}

private static boolean isCertificatePartOfChain(X509Certificate cert, X509Certificate[] chain) {
private static boolean isCertificatePartOfChain(X509Certificate cert, List<X509Certificate> chain) {
for (X509Certificate certOfChain : chain) {
if (cert.getSubjectX500Principal().equals(certOfChain.getSubjectX500Principal())) {
return true;
}
}
return false;
}

private static Set<TrustAnchor> getTrustAnchors(X509Certificate[] trustedCertificates) {
Set<TrustAnchor> result = new HashSet<>();
if (trustedCertificates != null) {
for (X509Certificate cert : trustedCertificates) {
result.add(new TrustAnchor((X509Certificate) cert, null));
}
}
return result;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,14 @@ public final class DtlsTestTools {
public static final String TRUST_STORE_LOCATION = "certs/trustStore.jks";
public static final String SERVER_NAME = "server";
public static final String CLIENT_NAME = "client";
public static final String ROOT_CA_ALIAS = "root";
public static final long MAX_SEQUENCE_NO = 281474976710655L; // 2^48 - 1
private static KeyStore keyStore;
private static KeyStore trustStore;
private static X509Certificate[] trustedCertificates = new X509Certificate[1];
private static X509Certificate[] serverCertificateChain;
private static X509Certificate[] clientCertificateChain;
private static X509Certificate rootCaCertificate;

static {
try {
Expand All @@ -61,8 +63,12 @@ public final class DtlsTestTools {
trustedCertificates = new X509Certificate[trustStore.size()];
int j = 0;
for (Enumeration<String> e = trustStore.aliases(); e.hasMoreElements(); ) {
Certificate trustedCert = trustStore.getCertificate(e.nextElement());
String alias = e.nextElement();
Certificate trustedCert = trustStore.getCertificate(alias);
if (X509Certificate.class.isInstance(trustedCert)) {
if (alias.equals(ROOT_CA_ALIAS)) {
rootCaCertificate = (X509Certificate) trustedCert;
}
trustedCertificates[j++] = (X509Certificate) trustedCert;
}
}
Expand Down Expand Up @@ -257,4 +263,13 @@ public static PublicKey getClientPublicKey() throws IOException, GeneralSecurity
public static X509Certificate[] getTrustedCertificates() {
return trustedCertificates;
}

/**
* Gets the trusted root CA certificate.
*
* @return The certificate.
*/
public static X509Certificate getTrustedRootCA() {
return rootCaCertificate;
}
}

0 comments on commit 7530b2c

Please sign in to comment.