Merge "Switch over to a new TLS fallback strategy"

4 files changed, 98 insertions, 65 deletions

diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/TlsConfigurationTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/TlsConfigurationTest.java
index 9ca6bbc..28c983e 100644
--- a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/TlsConfigurationTest.java
+++ b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/TlsConfigurationTest.java
@@ -54,19 +54,40 @@ public class TlsConfigurationTest {
   }
 
   @Test
-  public void useDefault() throws Exception {
-    String[] defaultProtocols = { "SSLv3", "TLSv1" };
-    SSLSocket socket = createSocketWithEnabledProtocols(defaultProtocols);
+  public void tlsV1AndBelow() throws Exception {
+    SSLSocket compatibleSocket = createSocketWithEnabledProtocols("SSLv3", "TLSv1", "TLSv1.1");
+    try {
+      assertTrue(TlsConfiguration.TLS_V1_0_AND_BELOW.isCompatible(compatibleSocket));
+      TlsConfiguration.TLS_V1_0_AND_BELOW.configureProtocols(compatibleSocket);
+      assertEnabledProtocols(compatibleSocket, "TLSv1", "SSLv3");
+    } finally {
+      compatibleSocket.close();
+    }
 
+    compatibleSocket = createSocketWithEnabledProtocols("TLSv1", "TLSv1.1");
     try {
-      assertTrue(TlsConfiguration.USE_DEFAULT.isCompatible(socket));
-      TlsConfiguration.USE_DEFAULT.configureProtocols(socket);
-      assertEnabledProtocols(socket, defaultProtocols);
+      assertTrue(TlsConfiguration.TLS_V1_0_AND_BELOW.isCompatible(compatibleSocket));
+      TlsConfiguration.TLS_V1_0_AND_BELOW.configureProtocols(compatibleSocket);
+      assertEnabledProtocols(compatibleSocket, "TLSv1");
     } finally {
-      socket.close();
+      compatibleSocket.close();
+    }
+
+    SSLSocket incompatibleSocket = createSocketWithEnabledProtocols("TLSv1.1");
+    try {
+      assertFalse(TlsConfiguration.TLS_V1_0_AND_BELOW.isCompatible(incompatibleSocket));
+    } finally {
+      incompatibleSocket.close();
+    }
+
+    incompatibleSocket = createSocketWithEnabledProtocols("SSLv3");
+    try {
+      assertFalse(TlsConfiguration.TLS_V1_0_AND_BELOW.isCompatible(incompatibleSocket));
+    } finally {
+      incompatibleSocket.close();
     }
 
-    assertTrue(TlsConfiguration.USE_DEFAULT.supportsNpn());
+    assertTrue(TlsConfiguration.TLS_V1_0_AND_BELOW.supportsNpn());
   }
 
   private SSLSocket createSocketWithEnabledProtocols(String... protocols) throws IOException {
diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/TlsFallbackStrategyTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/TlsFallbackStrategyTest.java
index 89fb09c..1b21b68 100644
--- a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/TlsFallbackStrategyTest.java
+++ b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/TlsFallbackStrategyTest.java
@@ -35,7 +35,8 @@ import static org.junit.Assert.assertTrue;
 public class TlsFallbackStrategyTest {
 
   private static final SSLContext sslContext = SslContextBuilder.localhost();
-  private static final String[] TLSV1_AND_SSLV3 = new String[] { "SSLv3", "TLSv1" };
+  private static final String[] TLSV11_TLSV10_AND_SSLV3 =
+      new String[] { "TLSv1.1", "TLSv1", "SSLv3" };
   private static final String[] TLSV1_ONLY = new String[] { "TLSv1" };
   public static final SSLHandshakeException RETRYABLE_EXCEPTION = new SSLHandshakeException(
       "Simulated handshake exception");
@@ -51,7 +52,7 @@ public class TlsFallbackStrategyTest {
 
   @Test
   public void nonRetryableIOException() throws Exception {
-    SSLSocket supportsSslV3 = createSocketWithEnabledProtocols(TLSV1_AND_SSLV3);
+    SSLSocket supportsSslV3 = createSocketWithEnabledProtocols(TLSV11_TLSV10_AND_SSLV3);
     try {
       fallbackStrategy.configureSecureSocket(supportsSslV3, "host", platform);
 
@@ -64,7 +65,7 @@ public class TlsFallbackStrategyTest {
 
   @Test
   public void nonRetryableSSLHandshakeException() throws Exception {
-    SSLSocket supportsSslV3 = createSocketWithEnabledProtocols(TLSV1_AND_SSLV3);
+    SSLSocket supportsSslV3 = createSocketWithEnabledProtocols(TLSV11_TLSV10_AND_SSLV3);
     try {
       fallbackStrategy.configureSecureSocket(supportsSslV3, "host", platform);
 
@@ -80,7 +81,7 @@ public class TlsFallbackStrategyTest {
 
   @Test
   public void retryableSSLHandshakeException() throws Exception {
-    SSLSocket supportsSslV3 = createSocketWithEnabledProtocols(TLSV1_AND_SSLV3);
+    SSLSocket supportsSslV3 = createSocketWithEnabledProtocols(TLSV11_TLSV10_AND_SSLV3);
     try {
       fallbackStrategy.configureSecureSocket(supportsSslV3, "host", platform);
 
@@ -92,27 +93,38 @@ public class TlsFallbackStrategyTest {
   }
 
   @Test
-  public void allFallbacksSupported() throws Exception {
-    SSLSocket supportsSslV3 = createSocketWithEnabledProtocols(TLSV1_AND_SSLV3);
+  public void someFallbacksSupported() throws Exception {
+    SSLSocket socket = createSocketWithEnabledProtocols(TLSV11_TLSV10_AND_SSLV3);
     try {
-      fallbackStrategy.configureSecureSocket(supportsSslV3, "host", platform);
-      assertEnabledProtocols(supportsSslV3, TLSV1_AND_SSLV3);
+      fallbackStrategy.configureSecureSocket(socket, "host", platform);
+      assertEnabledProtocols(socket, TLSV11_TLSV10_AND_SSLV3);
 
       boolean retry = fallbackStrategy.connectionFailed(RETRYABLE_EXCEPTION);
       assertTrue(retry);
     } finally {
-      supportsSslV3.close();
+      socket.close();
     }
 
-    supportsSslV3 = createSocketWithEnabledProtocols(TLSV1_AND_SSLV3);
+    socket = createSocketWithEnabledProtocols(TLSV11_TLSV10_AND_SSLV3);
     try {
-      fallbackStrategy.configureSecureSocket(supportsSslV3, "host", platform);
-      assertEnabledProtocols(supportsSslV3, "SSLv3");
+      fallbackStrategy.configureSecureSocket(socket, "host", platform);
+      assertEnabledProtocols(socket, "TLSv1", "SSLv3");
+
+      boolean retry = fallbackStrategy.connectionFailed(RETRYABLE_EXCEPTION);
+      assertTrue(retry);
+    } finally {
+      socket.close();
+    }
+
+    socket = createSocketWithEnabledProtocols(TLSV11_TLSV10_AND_SSLV3);
+    try {
+      fallbackStrategy.configureSecureSocket(socket, "host", platform);
+      assertEnabledProtocols(socket, "SSLv3");
 
       boolean retry = fallbackStrategy.connectionFailed(RETRYABLE_EXCEPTION);
       assertFalse(retry);
     } finally {
-      supportsSslV3.close();
+      socket.close();
     }
   }
 
@@ -121,7 +133,6 @@ public class TlsFallbackStrategyTest {
     SSLSocket socket = createSocketWithEnabledProtocols(TLSV1_ONLY);
     try {
       fallbackStrategy.configureSecureSocket(socket, "host", platform);
-
       assertEnabledProtocols(socket, TLSV1_ONLY);
 
       boolean retry = fallbackStrategy.connectionFailed(RETRYABLE_EXCEPTION);
diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/TlsConfiguration.java b/okhttp/src/main/java/com/squareup/okhttp/internal/TlsConfiguration.java
index 504844d..c4e3047 100644
--- a/okhttp/src/main/java/com/squareup/okhttp/internal/TlsConfiguration.java
+++ b/okhttp/src/main/java/com/squareup/okhttp/internal/TlsConfiguration.java
@@ -23,10 +23,17 @@ import javax.net.ssl.SSLSocket;
  * A configuration of desired secure socket protocols.
  */
 public class TlsConfiguration {
-  static final String SSL_V3 = "SSLv3";
-
-  public static final TlsConfiguration USE_DEFAULT = new TlsConfiguration(null, true);
-
+  private static final String SSL_V3 = "SSLv3";
+  private static final String TLS_V1_2 = "TLSv1.2";
+  private static final String TLS_V1_1 = "TLSv1.1";
+  private static final String TLS_V1_0 = "TLSv1";
+
+  public static final TlsConfiguration TLS_V1_2_AND_BELOW =
+      new TlsConfiguration(new String[] { TLS_V1_2, TLS_V1_1, TLS_V1_0, SSL_V3 }, true);
+  public static final TlsConfiguration TLS_V1_1_AND_BELOW =
+      new TlsConfiguration(new String[] { TLS_V1_1, TLS_V1_0, SSL_V3 }, true);
+  public static final TlsConfiguration TLS_V1_0_AND_BELOW =
+      new TlsConfiguration(new String[] { TLS_V1_0, SSL_V3 }, true);
   public static final TlsConfiguration SSL_V3_ONLY =
       new TlsConfiguration(new String[] { SSL_V3 }, false /* supportsNpn */);
 
@@ -38,16 +45,15 @@ public class TlsConfiguration {
   /**
    * Creates a {@link TlsConfiguration} with the specified settings.
    *
-   * <p>If {@code protocols} is {@code null} this means "use the default socket configuration".
-   *
-   * <p>If {@code protocols} in non-null it must contain at least one value. The ordering of the
-   * protocols is important: the first protocol specified <em>must</em> be supported by a socket
+   * <p>{@code protocols} must contain at least one value. The ordering of the protocols is
+   * important: the first protocol specified <em>must</em> be enabled by a socket
    * for the {@link #isCompatible(javax.net.ssl.SSLSocket)} method to return {@code true}.
    * The other protocols are considered optional. {@code protocols} must not contain null values.
    */
   private TlsConfiguration(String[] protocols, boolean supportsNpn) {
-    if (protocols != null && protocols.length < 1) {
-      throw new IllegalArgumentException("protocols must contain at least one protocol");
+    if (protocols == null || protocols.length == 0 || contains(protocols, null)) {
+      throw new IllegalArgumentException(
+          "protocols must contain at least one protocol and must not contain nulls");
     }
 
     this.protocols = protocols;
@@ -59,14 +65,9 @@ public class TlsConfiguration {
   }
 
   /**
-   * Returns {@code true} if the socket, as currently configured, supports this configuration.
+   * Returns {@code true} if the socket, as currently configured, supports this TLS configuration.
    */
   public boolean isCompatible(SSLSocket socket) {
-    if (protocols == null) {
-      // No primary protocol means "use default".
-      return true;
-    }
-
     // We use enabled protocols here, not supported, to avoid re-enabling a protocol that has
     // been disabled. Just because something is supported does not make it desirable to use.
     String[] enabledProtocols = socket.getEnabledProtocols();
@@ -74,34 +75,31 @@ public class TlsConfiguration {
   }
 
   public void configureProtocols(SSLSocket socket) {
-    if (protocols != null) {
-      // We use enabled protocols here, not supported, to avoid re-enabling a protocol that has
-      // been disabled. Just because something is supported does not make it desirable to use.
-      String[] enabledProtocols = socket.getEnabledProtocols();
-
-      // Create an array to hold the subset of protocols that are desired, and copy across the
-      // enabled protocols that intersect.
-      String[] desiredProtocols = new String[protocols.length];
-      int desiredIndex = 0;
-      for (int protocolsIndex = 0; protocolsIndex < protocols.length; protocolsIndex++) {
-        String candidateProtocol = protocols[protocolsIndex];
-        if (contains(enabledProtocols, candidateProtocol)) {
-          desiredProtocols[desiredIndex++] = candidateProtocol;
-        } else if (desiredIndex == 0) {
-          // This is checked by isCompatible.
-          throw new AssertionError("primaryProtocol " + candidateProtocol + " is not supported");
-        }
-      }
+    // We use enabled protocols here, not supported, to avoid re-enabling a protocol that has
+    // been disabled. Just because something is supported does not make it desirable to use.
+    String[] enabledProtocols = socket.getEnabledProtocols();
 
-      // Shrink the desiredProtocols array to the correct size.
-      if (desiredIndex < desiredProtocols.length) {
-        String[] desiredCopy = new String[desiredIndex];
-        System.arraycopy(desiredProtocols, 0, desiredCopy, 0, desiredIndex);
-        desiredProtocols = desiredCopy;
+    // Create an array to hold the subset of protocols that are desired, and copy across the
+    // enabled protocols that intersect.
+    String[] desiredProtocols = new String[protocols.length];
+    int desiredIndex = 0;
+    for (String candidateProtocol : protocols) {
+      if (contains(enabledProtocols, candidateProtocol)) {
+        desiredProtocols[desiredIndex++] = candidateProtocol;
+      } else if (desiredIndex == 0) {
+        // This is checked by isCompatible.
+        throw new AssertionError("primaryProtocol " + candidateProtocol + " is not supported");
       }
+    }
 
-      socket.setEnabledProtocols(desiredProtocols);
+    // Shrink the desiredProtocols array to the correct size.
+    if (desiredIndex < desiredProtocols.length) {
+      String[] desiredCopy = new String[desiredIndex];
+      System.arraycopy(desiredProtocols, 0, desiredCopy, 0, desiredIndex);
+      desiredProtocols = desiredCopy;
     }
+
+    socket.setEnabledProtocols(desiredProtocols);
   }
 
   @Override
@@ -114,11 +112,10 @@ public class TlsConfiguration {
 
   private static <T> boolean contains(T[] array, T value) {
     for (T arrayValue : array) {
-      if (value != null && value.equals(arrayValue)) {
+      if (value == arrayValue || (value != null && value.equals(arrayValue))) {
         return true;
       }
     }
     return false;
   }
-
 }
diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/TlsFallbackStrategy.java b/okhttp/src/main/java/com/squareup/okhttp/internal/TlsFallbackStrategy.java
index 6ee5ed5..c0f14a4 100644
--- a/okhttp/src/main/java/com/squareup/okhttp/internal/TlsFallbackStrategy.java
+++ b/okhttp/src/main/java/com/squareup/okhttp/internal/TlsFallbackStrategy.java
@@ -37,7 +37,11 @@ public class TlsFallbackStrategy {
 
   /** Create a new {@link TlsFallbackStrategy}. */
   public static TlsFallbackStrategy create() {
-    return new TlsFallbackStrategy(TlsConfiguration.USE_DEFAULT, TlsConfiguration.SSL_V3_ONLY);
+    return new TlsFallbackStrategy(
+        TlsConfiguration.TLS_V1_2_AND_BELOW,
+        TlsConfiguration.TLS_V1_1_AND_BELOW,
+        TlsConfiguration.TLS_V1_0_AND_BELOW,
+        TlsConfiguration.SSL_V3_ONLY);
   }
 
   /** Use {@link #create()} */