diff --git a/okhttp-tests/src/test/java/okhttp3/CipherSuiteTest.java b/okhttp-tests/src/test/java/okhttp3/CipherSuiteTest.java index 683ef9be2576..63b302325390 100644 --- a/okhttp-tests/src/test/java/okhttp3/CipherSuiteTest.java +++ b/okhttp-tests/src/test/java/okhttp3/CipherSuiteTest.java @@ -21,6 +21,7 @@ import static okhttp3.CipherSuite.TLS_RSA_EXPORT_WITH_RC4_40_MD5; import static okhttp3.CipherSuite.TLS_RSA_WITH_AES_128_CBC_SHA256; import static okhttp3.CipherSuite.forJavaName; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertSame; @@ -89,24 +90,131 @@ public class CipherSuiteTest { } /** - * Legacy ciphers (whose javaName starts with "SSL_") are now considered different from the - * corresponding "TLS_" ciphers. In OkHttp 3.3.1, only 19 of those would have been valid; those 19 - * would have been considered equal to the corresponding "TLS_" ciphers. + * On the Oracle JVM some older cipher suites have the "SSL_" prefix and others have the "TLS_" + * prefix. On the IBM JVM all cipher suites have the "SSL_" prefix. + * + *

Prior to OkHttp 3.3.1 we accepted either form and consider them equivalent. And since OkHttp + * 3.7.0 this is also true. But OkHttp 3.3.1 through 3.6.0 treated these as different. */ @Test public void forJavaName_fromLegacyEnumName() { // These would have been considered equal in OkHttp 3.3.1, but now aren't. - assertNotEquals( + assertEquals( forJavaName("TLS_RSA_EXPORT_WITH_RC4_40_MD5"), forJavaName("SSL_RSA_EXPORT_WITH_RC4_40_MD5")); - - // The SSL_ one of these would have been invalid in OkHttp 3.3.1; it now is valid and not equal. - assertNotEquals( + assertEquals( forJavaName("TLS_DH_RSA_EXPORT_WITH_DES40_CBC_SHA"), forJavaName("SSL_DH_RSA_EXPORT_WITH_DES40_CBC_SHA")); - - // These would have not been valid in OkHttp 3.3.1, and now aren't equal. - assertNotEquals( + assertEquals( forJavaName("TLS_FAKE_NEW_CIPHER"), forJavaName("SSL_FAKE_NEW_CIPHER")); } + + @Test public void applyIntersectionRetainsSslPrefixes() throws Exception { + FakeSslSocket socket = new FakeSslSocket(); + socket.setEnabledProtocols(new String[] { "TLSv1" }); + socket.setSupportedCipherSuites(new String[] { "SSL_A", "SSL_B", "SSL_C", "SSL_D", "SSL_E" }); + socket.setEnabledCipherSuites(new String[] { "SSL_A", "SSL_B", "SSL_C" }); + + ConnectionSpec connectionSpec = new ConnectionSpec.Builder(true) + .tlsVersions(TlsVersion.TLS_1_0) + .cipherSuites("TLS_A", "TLS_C", "TLS_E") + .build(); + connectionSpec.apply(socket, false); + + assertArrayEquals(new String[] { "SSL_A", "SSL_C" }, socket.enabledCipherSuites); + } + + @Test public void applyIntersectionRetainsTlsPrefixes() throws Exception { + FakeSslSocket socket = new FakeSslSocket(); + socket.setEnabledProtocols(new String[] { "TLSv1" }); + socket.setSupportedCipherSuites(new String[] { "TLS_A", "TLS_B", "TLS_C", "TLS_D", "TLS_E" }); + socket.setEnabledCipherSuites(new String[] { "TLS_A", "TLS_B", "TLS_C" }); + + ConnectionSpec connectionSpec = new ConnectionSpec.Builder(true) + .tlsVersions(TlsVersion.TLS_1_0) + .cipherSuites("SSL_A", "SSL_C", "SSL_E") + .build(); + connectionSpec.apply(socket, false); + + assertArrayEquals(new String[] { "TLS_A", "TLS_C" }, socket.enabledCipherSuites); + } + + @Test public void applyIntersectionAddsSslScsvForFallback() throws Exception { + FakeSslSocket socket = new FakeSslSocket(); + socket.setEnabledProtocols(new String[] { "TLSv1" }); + socket.setSupportedCipherSuites(new String[] { "SSL_A", "SSL_FALLBACK_SCSV" }); + socket.setEnabledCipherSuites(new String[] { "SSL_A" }); + + ConnectionSpec connectionSpec = new ConnectionSpec.Builder(true) + .tlsVersions(TlsVersion.TLS_1_0) + .cipherSuites("SSL_A") + .build(); + connectionSpec.apply(socket, true); + + assertArrayEquals(new String[] { "SSL_A", "SSL_FALLBACK_SCSV" }, socket.enabledCipherSuites); + } + + @Test public void applyIntersectionAddsTlsScsvForFallback() throws Exception { + FakeSslSocket socket = new FakeSslSocket(); + socket.setEnabledProtocols(new String[] { "TLSv1" }); + socket.setSupportedCipherSuites(new String[] { "TLS_A", "TLS_FALLBACK_SCSV" }); + socket.setEnabledCipherSuites(new String[] { "TLS_A" }); + + ConnectionSpec connectionSpec = new ConnectionSpec.Builder(true) + .tlsVersions(TlsVersion.TLS_1_0) + .cipherSuites("TLS_A") + .build(); + connectionSpec.apply(socket, true); + + assertArrayEquals(new String[] { "TLS_A", "TLS_FALLBACK_SCSV" }, socket.enabledCipherSuites); + } + + @Test public void applyIntersectionToProtocolVersion() throws Exception { + FakeSslSocket socket = new FakeSslSocket(); + socket.setEnabledProtocols(new String[] { "TLSv1", "TLSv1.1", "TLSv1.2" }); + socket.setSupportedCipherSuites(new String[] { "TLS_A" }); + socket.setEnabledCipherSuites(new String[] { "TLS_A" }); + + ConnectionSpec connectionSpec = new ConnectionSpec.Builder(true) + .tlsVersions(TlsVersion.TLS_1_1, TlsVersion.TLS_1_2, TlsVersion.TLS_1_3) + .cipherSuites("TLS_A") + .build(); + connectionSpec.apply(socket, false); + + assertArrayEquals(new String[] { "TLSv1.1", "TLSv1.2" }, socket.enabledProtocols); + } + + static final class FakeSslSocket extends DelegatingSSLSocket { + private String[] enabledProtocols; + private String[] supportedCipherSuites; + private String[] enabledCipherSuites; + + FakeSslSocket() { + super(null); + } + + @Override public String[] getEnabledProtocols() { + return enabledProtocols; + } + + @Override public void setEnabledProtocols(String[] enabledProtocols) { + this.enabledProtocols = enabledProtocols; + } + + @Override public String[] getSupportedCipherSuites() { + return supportedCipherSuites; + } + + public void setSupportedCipherSuites(String[] supportedCipherSuites) { + this.supportedCipherSuites = supportedCipherSuites; + } + + @Override public String[] getEnabledCipherSuites() { + return enabledCipherSuites; + } + + @Override public void setEnabledCipherSuites(String[] enabledCipherSuites) { + this.enabledCipherSuites = enabledCipherSuites; + } + } } diff --git a/okhttp/src/main/java/okhttp3/CipherSuite.java b/okhttp/src/main/java/okhttp3/CipherSuite.java index a3acd52e2fca..a71ddc5ed9fa 100644 --- a/okhttp/src/main/java/okhttp3/CipherSuite.java +++ b/okhttp/src/main/java/okhttp3/CipherSuite.java @@ -15,8 +15,12 @@ */ package okhttp3; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; /** * TLS cipher @@ -31,11 +35,30 @@ * from conscrypt, which lists the cipher suites supported by Android. */ public final class CipherSuite { + /** + * Compares cipher suites names like "TLS_RSA_WITH_NULL_MD5" and "SSL_RSA_WITH_NULL_MD5", ignoring + * the "TLS_" or "SSL_" prefix which is not consistent across platforms. In particular some IBM + * JVMs use the "SSL_" prefix everywhere whereas Oracle JVMs mix "TLS_" and "SSL_". + */ + static final Comparator ORDER_BY_NAME = new Comparator() { + @Override public int compare(String a, String b) { + for (int i = 4, limit = Math.min(a.length(), b.length()); i < limit; i++) { + char charA = a.charAt(i); + char charB = b.charAt(i); + if (charA != charB) return charA < charB ? -1 : 1; + } + int lengthA = a.length(); + int lengthB = b.length(); + if (lengthA != lengthB) return lengthA < lengthB ? -1 : 1; + return 0; + } + }; + /** * Holds interned instances. This needs to be above the of() calls below so that it's - * initialized by the time those parts of {@code ()} run. + * initialized by the time those parts of {@code ()} run. Guarded by CipherSuite.class. */ - private static final ConcurrentMap INSTANCES = new ConcurrentHashMap<>(); + private static final Map INSTANCES = new TreeMap<>(ORDER_BY_NAME); // Last updated 2016-07-03 using cipher suites from Android 24 and Java 9. @@ -370,18 +393,25 @@ public final class CipherSuite { /** * @param javaName the name used by Java APIs for this cipher suite. Different than the IANA name - * for older cipher suites because the prefix is {@code SSL_} instead of {@code TLS_}. + * for older cipher suites because the prefix is {@code SSL_} instead of {@code TLS_}. */ - public static CipherSuite forJavaName(String javaName) { + public static synchronized CipherSuite forJavaName(String javaName) { CipherSuite result = INSTANCES.get(javaName); if (result == null) { - CipherSuite sample = new CipherSuite(javaName); - CipherSuite canonical = INSTANCES.putIfAbsent(javaName, sample); - result = (canonical == null) ? sample : canonical; + result = new CipherSuite(javaName); + INSTANCES.put(javaName, result); } return result; } + static List forJavaNames(String... cipherSuites) { + List result = new ArrayList<>(cipherSuites.length); + for (String cipherSuite : cipherSuites) { + result.add(forJavaName(cipherSuite)); + } + return Collections.unmodifiableList(result); + } + private CipherSuite(String javaName) { if (javaName == null) { throw new NullPointerException(); @@ -391,7 +421,7 @@ private CipherSuite(String javaName) { /** * @param javaName the name used by Java APIs for this cipher suite. Different than the IANA name - * for older cipher suites because the prefix is {@code SSL_} instead of {@code TLS_}. + * for older cipher suites because the prefix is {@code SSL_} instead of {@code TLS_}. * @param value the integer identifier for this cipher suite. (Documentation only.) */ private static CipherSuite of(String javaName, int value) { diff --git a/okhttp/src/main/java/okhttp3/ConnectionSpec.java b/okhttp/src/main/java/okhttp3/ConnectionSpec.java index 11fd12c3ea04..71f986749d6c 100644 --- a/okhttp/src/main/java/okhttp3/ConnectionSpec.java +++ b/okhttp/src/main/java/okhttp3/ConnectionSpec.java @@ -15,15 +15,15 @@ */ package okhttp3; -import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.List; import javax.net.ssl.SSLSocket; +import okhttp3.internal.Util; import static okhttp3.internal.Util.concat; import static okhttp3.internal.Util.indexOf; import static okhttp3.internal.Util.intersect; +import static okhttp3.internal.Util.nonEmptyIntersection; /** * Specifies configuration for the socket connection that HTTP traffic travels through. For {@code @@ -101,13 +101,7 @@ public boolean isTls() { * socket's enabled cipher suites should be used. */ public List cipherSuites() { - if (cipherSuites == null) return null; - - List result = new ArrayList<>(cipherSuites.length); - for (String cipherSuite : cipherSuites) { - result.add(CipherSuite.forJavaName(cipherSuite)); - } - return Collections.unmodifiableList(result); + return cipherSuites != null ? CipherSuite.forJavaNames(cipherSuites) : null; } /** @@ -115,13 +109,7 @@ public List cipherSuites() { * the SSL socket's enabled TLS versions should be used. */ public List tlsVersions() { - if (tlsVersions == null) return null; - - List result = new ArrayList<>(tlsVersions.length); - for (String tlsVersion : tlsVersions) { - result.add(TlsVersion.forJavaName(tlsVersion)); - } - return Collections.unmodifiableList(result); + return tlsVersions != null ? TlsVersion.forJavaNames(tlsVersions) : null; } public boolean supportsTlsExtensions() { @@ -146,16 +134,20 @@ void apply(SSLSocket sslSocket, boolean isFallback) { */ private ConnectionSpec supportedSpec(SSLSocket sslSocket, boolean isFallback) { String[] cipherSuitesIntersection = cipherSuites != null - ? intersect(String.class, cipherSuites, sslSocket.getEnabledCipherSuites()) + ? intersect(CipherSuite.ORDER_BY_NAME, sslSocket.getEnabledCipherSuites(), cipherSuites) : sslSocket.getEnabledCipherSuites(); String[] tlsVersionsIntersection = tlsVersions != null - ? intersect(String.class, tlsVersions, sslSocket.getEnabledProtocols()) + ? intersect(Util.NATURAL_ORDER, sslSocket.getEnabledProtocols(), tlsVersions) : sslSocket.getEnabledProtocols(); // In accordance with https://tools.ietf.org/html/draft-ietf-tls-downgrade-scsv-00 // the SCSV cipher is added to signal that a protocol fallback has taken place. - if (isFallback && indexOf(sslSocket.getSupportedCipherSuites(), "TLS_FALLBACK_SCSV") != -1) { - cipherSuitesIntersection = concat(cipherSuitesIntersection, "TLS_FALLBACK_SCSV"); + String[] supportedCipherSuites = sslSocket.getSupportedCipherSuites(); + int indexOfFallbackScsv = indexOf( + CipherSuite.ORDER_BY_NAME, supportedCipherSuites, "TLS_FALLBACK_SCSV"); + if (isFallback && indexOfFallbackScsv != -1) { + cipherSuitesIntersection = concat( + cipherSuitesIntersection, supportedCipherSuites[indexOfFallbackScsv]); } return new Builder(this) @@ -180,36 +172,19 @@ public boolean isCompatible(SSLSocket socket) { return false; } - if (tlsVersions != null - && !nonEmptyIntersection(tlsVersions, socket.getEnabledProtocols())) { + if (tlsVersions != null && !nonEmptyIntersection( + Util.NATURAL_ORDER, tlsVersions, socket.getEnabledProtocols())) { return false; } - if (cipherSuites != null - && !nonEmptyIntersection(cipherSuites, socket.getEnabledCipherSuites())) { + if (cipherSuites != null && !nonEmptyIntersection( + CipherSuite.ORDER_BY_NAME, cipherSuites, socket.getEnabledCipherSuites())) { return false; } return true; } - /** - * An N*M intersection that terminates if any intersection is found. The sizes of both arguments - * are assumed to be so small, and the likelihood of an intersection so great, that it is not - * worth the CPU cost of sorting or the memory cost of hashing. - */ - private static boolean nonEmptyIntersection(String[] a, String[] b) { - if (a == null || b == null || a.length == 0 || b.length == 0) { - return false; - } - for (String toFind : a) { - if (indexOf(b, toFind) != -1) { - return true; - } - } - return false; - } - @Override public boolean equals(Object other) { if (!(other instanceof ConnectionSpec)) return false; if (other == this) return true; diff --git a/okhttp/src/main/java/okhttp3/TlsVersion.java b/okhttp/src/main/java/okhttp3/TlsVersion.java index 391af88e3a4d..02cbeca3d18f 100644 --- a/okhttp/src/main/java/okhttp3/TlsVersion.java +++ b/okhttp/src/main/java/okhttp3/TlsVersion.java @@ -15,6 +15,10 @@ */ package okhttp3; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + /** * Versions of TLS that can be offered when negotiating a secure socket. See {@link * javax.net.ssl.SSLSocket#setEnabledProtocols}. @@ -49,6 +53,14 @@ public static TlsVersion forJavaName(String javaName) { throw new IllegalArgumentException("Unexpected TLS version: " + javaName); } + static List forJavaNames(String... tlsVersions) { + List result = new ArrayList<>(tlsVersions.length); + for (String tlsVersion : tlsVersions) { + result.add(forJavaName(tlsVersion)); + } + return Collections.unmodifiableList(result); + } + public String javaName() { return javaName; } diff --git a/okhttp/src/main/java/okhttp3/internal/Util.java b/okhttp/src/main/java/okhttp3/internal/Util.java index 5a88c8899804..043228f62d92 100644 --- a/okhttp/src/main/java/okhttp3/internal/Util.java +++ b/okhttp/src/main/java/okhttp3/internal/Util.java @@ -18,7 +18,6 @@ import java.io.Closeable; import java.io.IOException; import java.io.InterruptedIOException; -import java.lang.reflect.Array; import java.net.IDN; import java.net.ServerSocket; import java.net.Socket; @@ -26,6 +25,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; import java.util.List; import java.util.Locale; import java.util.TimeZone; @@ -63,6 +63,12 @@ public final class Util { /** GMT and UTC are equivalent for our purposes. */ public static final TimeZone UTC = TimeZone.getTimeZone("GMT"); + public static final Comparator NATURAL_ORDER = new Comparator() { + @Override public int compare(String a, String b) { + return a.compareTo(b); + } + }; + /** * Quick and dirty pattern to differentiate IP addresses from hostnames. This is an approximation * of Android's private InetAddress#isNumeric API. @@ -198,30 +204,43 @@ public static ThreadFactory threadFactory(final String name, final boolean daemo } /** - * Returns an array containing containing only elements found in {@code first} and also in {@code + * Returns an array containing containing only elements found in {@code first} and also in {@code * second}. The returned elements are in the same order as in {@code first}. */ @SuppressWarnings("unchecked") - public static T[] intersect(Class arrayType, T[] first, T[] second) { - List result = intersect(first, second); - return result.toArray((T[]) Array.newInstance(arrayType, result.size())); + public static String[] intersect( + Comparator comparator, String[] first, String[] second) { + List result = new ArrayList<>(); + for (String a : first) { + for (String b : second) { + if (comparator.compare(a, b) == 0) { + result.add(a); + break; + } + } + } + return result.toArray(new String[result.size()]); } /** - * Returns a list containing containing only elements found in {@code first} and also in {@code - * second}. The returned elements are in the same order as in {@code first}. + * Returns true if there is an element in {@code first} that is also in {@code second}. This + * method terminates if any intersection is found. The sizes of both arguments are assumed to be + * so small, and the likelihood of an intersection so great, that it is not worth the CPU cost of + * sorting or the memory cost of hashing. */ - private static List intersect(T[] first, T[] second) { - List result = new ArrayList<>(); - for (T a : first) { - for (T b : second) { - if (a.equals(b)) { - result.add(b); - break; + public static boolean nonEmptyIntersection( + Comparator comparator, String[] first, String[] second) { + if (first == null || second == null || first.length == 0 || second.length == 0) { + return false; + } + for (String a : first) { + for (String b : second) { + if (comparator.compare(a, b) == 0) { + return true; } } } - return result; + return false; } public static String hostHeader(HttpUrl url, boolean includeDefaultPort) { @@ -259,9 +278,9 @@ public static boolean isAndroidGetsocknameError(AssertionError e) { && e.getMessage().contains("getsockname failed"); } - public static int indexOf(T[] array, T value) { + public static int indexOf(Comparator comparator, String[] array, String value) { for (int i = 0, size = array.length; i < size; i++) { - if (equal(array[i], value)) return i; + if (comparator.compare(array[i], value) == 0) return i; } return -1; }