Handle either TLS_ or SSL_ prefixes for cipher suite names.

Closes: square#3173

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.
+   *
+   * <p>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;
+    }
+  }
 }

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;
 
 /**
  * <a href="https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml">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<String> ORDER_BY_NAME = new Comparator<String>() {
+    @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 <clinit>()} run.
+   * initialized by the time those parts of {@code <clinit>()} run. Guarded by CipherSuite.class.
    */
-  private static final ConcurrentMap<String, CipherSuite> INSTANCES = new ConcurrentHashMap<>();
+  private static final Map<String, CipherSuite> 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<CipherSuite> forJavaNames(String... cipherSuites) {
+    List<CipherSuite> 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) {

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,27 +101,15 @@ public boolean isTls() {
    * socket's enabled cipher suites should be used.
    */
   public List<CipherSuite> cipherSuites() {
-    if (cipherSuites == null) return null;
-
-    List<CipherSuite> 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;
   }
 
   /**
    * Returns the TLS versions to use when negotiating a connection. Returns {@code null} if all of
    * the SSL socket's enabled TLS versions should be used.
    */
   public List<TlsVersion> tlsVersions() {
-    if (tlsVersions == null) return null;
-
-    List<TlsVersion> 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;

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<TlsVersion> forJavaNames(String... tlsVersions) {
+    List<TlsVersion> result = new ArrayList<>(tlsVersions.length);
+    for (String tlsVersion : tlsVersions) {
+      result.add(forJavaName(tlsVersion));
+    }
+    return Collections.unmodifiableList(result);
+  }
+
   public String javaName() {
     return javaName;
   }