Skip to content

Commit

Permalink
Merge pull request square#2438 from square/jwilson_0325_ccc
Browse files Browse the repository at this point in the history 
Make CertificateChainCleaner an abstract class.
  • Loading branch information
@JakeWharton
JakeWharton committed Mar 25, 2016
2 parents bb0d8f2 + a2466ac commit 338ad7f
Show file tree
Hide file tree
Showing 8 changed files with 215 additions and 249 deletions.
62 changes: 27 additions & 35 deletions okhttp-tests/src/test/java/okhttp3/CertificateChainCleanerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import javax.net.ssl.SSLPeerUnverifiedException;
import okhttp3.internal.HeldCertificate;
import okhttp3.internal.tls.CertificateChainCleaner;
import okhttp3.internal.tls.RealTrustRootIndex;
import org.junit.Test;

import static org.junit.Assert.assertEquals;
Expand All @@ -34,19 +33,18 @@ public final class CertificateChainCleanerTest {
HeldCertificate root = new HeldCertificate.Builder()
.serialNumber("1")
.build();
CertificateChainCleaner council = new CertificateChainCleaner(
new RealTrustRootIndex(root.certificate));
assertEquals(list(root), council.clean(list(root)));
CertificateChainCleaner cleaner = CertificateChainCleaner.get(root.certificate);
assertEquals(list(root), cleaner.clean(list(root)));
}

@Test public void normalizeUnknownSelfSignedCertificate() throws Exception {
HeldCertificate root = new HeldCertificate.Builder()
.serialNumber("1")
.build();
CertificateChainCleaner council = new CertificateChainCleaner(new RealTrustRootIndex());
CertificateChainCleaner cleaner = CertificateChainCleaner.get();

try {
council.clean(list(root));
cleaner.clean(list(root));
fail();
} catch (SSLPeerUnverifiedException expected) {
}
Expand All @@ -65,9 +63,8 @@ public final class CertificateChainCleanerTest {
.issuedBy(certA)
.build();

CertificateChainCleaner council = new CertificateChainCleaner(
new RealTrustRootIndex(root.certificate));
assertEquals(list(certB, certA, root), council.clean(list(certB, certA, root)));
CertificateChainCleaner cleaner = CertificateChainCleaner.get(root.certificate);
assertEquals(list(certB, certA, root), cleaner.clean(list(certB, certA, root)));
}

@Test public void orderedChainOfCertificatesWithoutRoot() throws Exception {
Expand All @@ -83,9 +80,8 @@ public final class CertificateChainCleanerTest {
.issuedBy(certA)
.build();

CertificateChainCleaner council = new CertificateChainCleaner(
new RealTrustRootIndex(root.certificate));
assertEquals(list(certB, certA, root), council.clean(list(certB, certA))); // Root is added!
CertificateChainCleaner cleaner = CertificateChainCleaner.get(root.certificate);
assertEquals(list(certB, certA, root), cleaner.clean(list(certB, certA))); // Root is added!
}

@Test public void unorderedChainOfCertificatesWithRoot() throws Exception {
Expand All @@ -105,9 +101,8 @@ public final class CertificateChainCleanerTest {
.issuedBy(certB)
.build();

CertificateChainCleaner council = new CertificateChainCleaner(
new RealTrustRootIndex(root.certificate));
assertEquals(list(certC, certB, certA, root), council.clean(list(certC, certA, root, certB)));
CertificateChainCleaner cleaner = CertificateChainCleaner.get(root.certificate);
assertEquals(list(certC, certB, certA, root), cleaner.clean(list(certC, certA, root, certB)));
}

@Test public void unorderedChainOfCertificatesWithoutRoot() throws Exception {
Expand All @@ -127,9 +122,8 @@ public final class CertificateChainCleanerTest {
.issuedBy(certB)
.build();

CertificateChainCleaner council = new CertificateChainCleaner(
new RealTrustRootIndex(root.certificate));
assertEquals(list(certC, certB, certA, root), council.clean(list(certC, certA, certB)));
CertificateChainCleaner cleaner = CertificateChainCleaner.get(root.certificate);
assertEquals(list(certC, certB, certA, root), cleaner.clean(list(certC, certA, certB)));
}

@Test public void unrelatedCertificatesAreOmitted() throws Exception {
Expand All @@ -148,10 +142,9 @@ public final class CertificateChainCleanerTest {
.serialNumber("4")
.build();

CertificateChainCleaner council = new CertificateChainCleaner(
new RealTrustRootIndex(root.certificate));
CertificateChainCleaner cleaner = CertificateChainCleaner.get(root.certificate);
assertEquals(list(certB, certA, root),
council.clean(list(certB, certUnnecessary, certA, root)));
cleaner.clean(list(certB, certUnnecessary, certA, root)));
}

@Test public void chainGoesAllTheWayToSelfSignedRoot() throws Exception {
Expand All @@ -171,14 +164,14 @@ public final class CertificateChainCleanerTest {
.issuedBy(certA)
.build();

CertificateChainCleaner council = new CertificateChainCleaner(
new RealTrustRootIndex(selfSigned.certificate, trusted.certificate));
CertificateChainCleaner cleaner = CertificateChainCleaner.get(
selfSigned.certificate, trusted.certificate);
assertEquals(list(certB, certA, trusted, selfSigned),
council.clean(list(certB, certA)));
cleaner.clean(list(certB, certA)));
assertEquals(list(certB, certA, trusted, selfSigned),
council.clean(list(certB, certA, trusted)));
cleaner.clean(list(certB, certA, trusted)));
assertEquals(list(certB, certA, trusted, selfSigned),
council.clean(list(certB, certA, trusted, selfSigned)));
cleaner.clean(list(certB, certA, trusted, selfSigned)));
}

@Test public void trustedRootNotSelfSigned() throws Exception {
Expand All @@ -198,12 +191,11 @@ public final class CertificateChainCleanerTest {
.serialNumber("4")
.build();

CertificateChainCleaner council = new CertificateChainCleaner(
new RealTrustRootIndex(trusted.certificate));
CertificateChainCleaner cleaner = CertificateChainCleaner.get(trusted.certificate);
assertEquals(list(certificate, intermediateCa, trusted),
council.clean(list(certificate, intermediateCa)));
cleaner.clean(list(certificate, intermediateCa)));
assertEquals(list(certificate, intermediateCa, trusted),
council.clean(list(certificate, intermediateCa, trusted)));
cleaner.clean(list(certificate, intermediateCa, trusted)));
}

@Test public void chainMaxLength() throws Exception {
Expand All @@ -214,9 +206,9 @@ public final class CertificateChainCleanerTest {
}

X509Certificate root = heldCertificates.get(heldCertificates.size() - 1).certificate;
CertificateChainCleaner council = new CertificateChainCleaner(new RealTrustRootIndex(root));
assertEquals(certificates, council.clean(certificates));
assertEquals(certificates, council.clean(certificates.subList(0, 9)));
CertificateChainCleaner cleaner = CertificateChainCleaner.get(root);
assertEquals(certificates, cleaner.clean(certificates));
assertEquals(certificates, cleaner.clean(certificates.subList(0, 9)));
}

@Test public void chainTooLong() throws Exception {
Expand All @@ -227,9 +219,9 @@ public final class CertificateChainCleanerTest {
}

X509Certificate root = heldCertificates.get(heldCertificates.size() - 1).certificate;
CertificateChainCleaner council = new CertificateChainCleaner(new RealTrustRootIndex(root));
CertificateChainCleaner cleaner = CertificateChainCleaner.get(root);
try {
council.clean(certificates);
cleaner.clean(certificates);
fail();
} catch (SSLPeerUnverifiedException expected) {
}
Expand Down
17 changes: 8 additions & 9 deletions okhttp/src/main/java/okhttp3/CertificatePinner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import javax.net.ssl.SSLPeerUnverifiedException;
import okhttp3.internal.Util;
import okhttp3.internal.tls.CertificateChainCleaner;
import okhttp3.internal.tls.TrustRootIndex;
import okio.ByteString;

/**
Expand Down Expand Up @@ -125,11 +124,11 @@ public final class CertificatePinner {
public static final CertificatePinner DEFAULT = new Builder().build();

private final List<Pin> pins;
private final TrustRootIndex trustRootIndex;
private final CertificateChainCleaner certificateChainCleaner;

private CertificatePinner(Builder builder) {
this.pins = Util.immutableList(builder.pins);
this.trustRootIndex = builder.trustRootIndex;
this.certificateChainCleaner = builder.certificateChainCleaner;
}

/**
Expand All @@ -145,8 +144,8 @@ public void check(String hostname, List<Certificate> peerCertificates)
List<Pin> pins = findMatchingPins(hostname);
if (pins.isEmpty()) return;

if (trustRootIndex != null) {
peerCertificates = new CertificateChainCleaner(trustRootIndex).clean(peerCertificates);
if (certificateChainCleaner != null) {
peerCertificates = certificateChainCleaner.clean(peerCertificates);
}

for (int c = 0, certsSize = peerCertificates.size(); c < certsSize; c++) {
Expand Down Expand Up @@ -289,18 +288,18 @@ boolean matches(String hostname) {
/** Builds a configured certificate pinner. */
public static final class Builder {
private final List<Pin> pins = new ArrayList<>();
private TrustRootIndex trustRootIndex;
private CertificateChainCleaner certificateChainCleaner;

public Builder() {
}

Builder(CertificatePinner certificatePinner) {
this.pins.addAll(certificatePinner.pins);
this.trustRootIndex = certificatePinner.trustRootIndex;
this.certificateChainCleaner = certificatePinner.certificateChainCleaner;
}

public Builder trustRootIndex(TrustRootIndex trustRootIndex) {
this.trustRootIndex = trustRootIndex;
public Builder certificateChainCleaner(CertificateChainCleaner certificateChainCleaner) {
this.certificateChainCleaner = certificateChainCleaner;
return this;
}

Expand Down
18 changes: 9 additions & 9 deletions okhttp/src/main/java/okhttp3/OkHttpClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@
import okhttp3.internal.Util;
import okhttp3.internal.http.StreamAllocation;
import okhttp3.internal.io.RealConnection;
import okhttp3.internal.tls.CertificateChainCleaner;
import okhttp3.internal.tls.OkHostnameVerifier;
import okhttp3.internal.tls.TrustRootIndex;

/**
* Factory for {@linkplain Call calls}, which can be used to send HTTP requests and read their
Expand Down Expand Up @@ -133,7 +133,7 @@ public void apply(ConnectionSpec tlsConfiguration, SSLSocket sslSocket, boolean
final InternalCache internalCache;
final SocketFactory socketFactory;
final SSLSocketFactory sslSocketFactory;
final TrustRootIndex trustRootIndex;
final CertificateChainCleaner certificateChainCleaner;
final HostnameVerifier hostnameVerifier;
final CertificatePinner certificatePinner;
final Authenticator proxyAuthenticator;
Expand Down Expand Up @@ -180,18 +180,18 @@ private OkHttpClient(Builder builder) {
throw new AssertionError(); // The system has no TLS. Just give up.
}
}
if (sslSocketFactory != null && builder.trustRootIndex == null) {
if (sslSocketFactory != null && builder.certificateChainCleaner == null) {
X509TrustManager trustManager = Platform.get().trustManager(sslSocketFactory);
if (trustManager == null) {
throw new IllegalStateException("Unable to extract the trust manager on " + Platform.get()
+ ", sslSocketFactory is " + sslSocketFactory.getClass());
}
this.trustRootIndex = Platform.get().trustRootIndex(trustManager);
this.certificateChainCleaner = CertificateChainCleaner.get(trustManager);
this.certificatePinner = builder.certificatePinner.newBuilder()
.trustRootIndex(trustRootIndex)
.certificateChainCleaner(certificateChainCleaner)
.build();
} else {
this.trustRootIndex = builder.trustRootIndex;
this.certificateChainCleaner = builder.certificateChainCleaner;
this.certificatePinner = builder.certificatePinner;
}
this.hostnameVerifier = builder.hostnameVerifier;
Expand Down Expand Up @@ -340,7 +340,7 @@ public static final class Builder {
InternalCache internalCache;
SocketFactory socketFactory;
SSLSocketFactory sslSocketFactory;
TrustRootIndex trustRootIndex;
CertificateChainCleaner certificateChainCleaner;
HostnameVerifier hostnameVerifier;
CertificatePinner certificatePinner;
Authenticator proxyAuthenticator;
Expand Down Expand Up @@ -388,7 +388,7 @@ public Builder() {
this.cache = okHttpClient.cache;
this.socketFactory = okHttpClient.socketFactory;
this.sslSocketFactory = okHttpClient.sslSocketFactory;
this.trustRootIndex = okHttpClient.trustRootIndex;
this.certificateChainCleaner = okHttpClient.certificateChainCleaner;
this.hostnameVerifier = okHttpClient.hostnameVerifier;
this.certificatePinner = okHttpClient.certificatePinner;
this.proxyAuthenticator = okHttpClient.proxyAuthenticator;
Expand Down Expand Up @@ -526,7 +526,7 @@ public Builder socketFactory(SocketFactory socketFactory) {
public Builder sslSocketFactory(SSLSocketFactory sslSocketFactory) {
if (sslSocketFactory == null) throw new NullPointerException("sslSocketFactory == null");
this.sslSocketFactory = sslSocketFactory;
this.trustRootIndex = null;
this.certificateChainCleaner = null;
return this;
}

Expand Down
13 changes: 0 additions & 13 deletions okhttp/src/main/java/okhttp3/internal/Platform.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@
import javax.net.ssl.SSLSocketFactory;
import javax.net.ssl.X509TrustManager;
import okhttp3.Protocol;
import okhttp3.internal.tls.AndroidTrustRootIndex;
import okhttp3.internal.tls.RealTrustRootIndex;
import okhttp3.internal.tls.TrustRootIndex;
import okio.Buffer;

import static okhttp3.internal.Internal.logger;
Expand Down Expand Up @@ -96,10 +93,6 @@ public X509TrustManager trustManager(SSLSocketFactory sslSocketFactory) {
}
}

public TrustRootIndex trustRootIndex(X509TrustManager trustManager) {
return new RealTrustRootIndex(trustManager.getAcceptedIssuers());
}

/**
* Configure TLS extensions on {@code sslSocket} for {@code route}.
*
Expand Down Expand Up @@ -243,12 +236,6 @@ public Android(Class<?> sslParametersClass, OptionalMethod<Socket> setUseSession
return readFieldOrNull(context, X509TrustManager.class, "trustManager");
}

@Override public TrustRootIndex trustRootIndex(X509TrustManager trustManager) {
TrustRootIndex result = AndroidTrustRootIndex.get(trustManager);
if (result != null) return result;
return super.trustRootIndex(trustManager);
}

@Override public void configureTlsExtensions(
SSLSocket sslSocket, String hostname, List<Protocol> protocols) {
// Enable SNI and session tickets.
Expand Down
65 changes: 0 additions & 65 deletions okhttp/src/main/java/okhttp3/internal/tls/AndroidTrustRootIndex.java
View file

This file was deleted.

Loading

0 comments on commit 338ad7f

Please sign in to comment.