Assert on effective cipher suite order (#6410)

okhttp-testing-support/src/main/kotlin/okhttp3/testing/PlatformRule.kt

@@ -311,6 +311,10 @@ open class PlatformRule @JvmOverloads constructor(
     assumeThat(Platform.isAndroid).isFalse
   }
 
+  fun assumeJdkVersion(majorVersion: Int) {
+    assumeThat(PlatformVersion.majorVersion).isEqualTo(majorVersion)
+  }
+
   companion object {
     const val PROPERTY_NAME = "okhttp.platform"
     const val CONSCRYPT_PROPERTY = "conscrypt"

okhttp/src/main/kotlin/okhttp3/ConnectionSpec.kt

@@ -20,6 +20,7 @@ import java.util.Objects
 import javax.net.ssl.SSLSocket
 import okhttp3.ConnectionSpec.Builder
 import okhttp3.internal.concat
+import okhttp3.internal.effectiveCipherSuites
 import okhttp3.internal.hasIntersection
 import okhttp3.internal.indexOf
 import okhttp3.internal.intersect
@@ -46,7 +47,7 @@ import okhttp3.internal.intersect
 class ConnectionSpec internal constructor(
   @get:JvmName("isTls") val isTls: Boolean,
   @get:JvmName("supportsTlsExtensions") val supportsTlsExtensions: Boolean,
-  private val cipherSuitesAsString: Array<String>?,
+  internal val cipherSuitesAsString: Array<String>?,
   private val tlsVersionsAsString: Array<String>?
 ) {
 
@@ -106,11 +107,8 @@ class ConnectionSpec internal constructor(
    * Returns a copy of this that omits cipher suites and TLS versions not enabled by [sslSocket].
    */
   private fun supportedSpec(sslSocket: SSLSocket, isFallback: Boolean): ConnectionSpec {
-    var cipherSuitesIntersection = if (cipherSuitesAsString != null) {
+    val socketEnabledCipherSuites = sslSocket.enabledCipherSuites
-      sslSocket.enabledCipherSuites.intersect(cipherSuitesAsString, CipherSuite.ORDER_BY_NAME)
+    var cipherSuitesIntersection: Array<String> = effectiveCipherSuites(socketEnabledCipherSuites)
-    } else {
-      sslSocket.enabledCipherSuites
-    }
 
     val tlsVersionsIntersection = if (tlsVersionsAsString != null) {
       sslSocket.enabledProtocols.intersect(tlsVersionsAsString, naturalOrder())

okhttp/src/main/kotlin/okhttp3/internal/internal.kt

@@ -21,6 +21,7 @@ package okhttp3.internal
 
 import javax.net.ssl.SSLSocket
 import okhttp3.Cache
+import okhttp3.CipherSuite
 import okhttp3.ConnectionSpec
 import okhttp3.Cookie
 import okhttp3.Headers
@@ -43,3 +44,11 @@ fun cacheGet(cache: Cache, request: Request) = cache.get(request)
 
 fun applyConnectionSpec(connectionSpec: ConnectionSpec, sslSocket: SSLSocket, isFallback: Boolean) =
     connectionSpec.apply(sslSocket, isFallback)
+
+fun ConnectionSpec.effectiveCipherSuites(socketEnabledCipherSuites: Array<String>): Array<String> {
+  return if (cipherSuitesAsString != null) {
+    socketEnabledCipherSuites.intersect(cipherSuitesAsString, CipherSuite.ORDER_BY_NAME)
+  } else {
+    socketEnabledCipherSuites
+  }
+}

okhttp/src/test/java/okhttp3/CallHandshakeTest.kt

@@ -19,8 +19,16 @@ import mockwebserver3.MockResponse
 import mockwebserver3.MockWebServer
 import okhttp3.CipherSuite.Companion.TLS_AES_128_GCM_SHA256
 import okhttp3.CipherSuite.Companion.TLS_AES_256_GCM_SHA384
+import okhttp3.CipherSuite.Companion.TLS_CHACHA20_POLY1305_SHA256
+import okhttp3.CipherSuite.Companion.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
 import okhttp3.CipherSuite.Companion.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384
 import okhttp3.CipherSuite.Companion.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
+import okhttp3.CipherSuite.Companion.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256
+import okhttp3.CipherSuite.Companion.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
+import okhttp3.CipherSuite.Companion.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
+import okhttp3.CipherSuite.Companion.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256
+import okhttp3.internal.effectiveCipherSuites
+import okhttp3.internal.platform.Platform
 import okhttp3.testing.PlatformRule
 import okhttp3.tls.internal.TlsUtil.localhost
 import org.assertj.core.api.Assertions.assertThat
@@ -46,7 +54,8 @@ class CallHandshakeTest {
   private lateinit var defaultEnabledCipherSuites: List<String>
   private lateinit var defaultSupportedCipherSuites: List<String>
 
-  val expectedModernTls12CipherSuites = listOf(TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384)
+  val expectedModernTls12CipherSuites =
+    listOf(TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384)
   val expectedModernTls13CipherSuites = listOf(TLS_AES_128_GCM_SHA256, TLS_AES_256_GCM_SHA384)
 
   @BeforeEach
@@ -62,8 +71,10 @@ class CallHandshakeTest {
       .build()
     server.useHttps(handshakeCertificates.sslSocketFactory(), false)
 
-    defaultEnabledCipherSuites = handshakeCertificates.sslSocketFactory().defaultCipherSuites.toList()
+    defaultEnabledCipherSuites =
-    defaultSupportedCipherSuites = handshakeCertificates.sslSocketFactory().supportedCipherSuites.toList()
+      handshakeCertificates.sslSocketFactory().defaultCipherSuites.toList()
+    defaultSupportedCipherSuites =
+      handshakeCertificates.sslSocketFactory().supportedCipherSuites.toList()
   }
 
   @Test
@@ -81,7 +92,8 @@ class CallHandshakeTest {
     // TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
     // TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
     // TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
-    assertThat(handshakeEnabledCipherSuites).containsExactlyElementsOf(expectedConnectionCipherSuites(client))
+    assertThat(handshakeEnabledCipherSuites).containsExactlyElementsOf(
+      expectedConnectionCipherSuites(client))
   }
 
   @Test
@@ -105,7 +117,8 @@ class CallHandshakeTest {
     // TLS_RSA_WITH_AES_256_CBC_SHA
     // TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
     // TLS_RSA_WITH_AES_128_CBC_SHA
-    assertThat(handshakeEnabledCipherSuites).containsExactlyElementsOf(expectedConnectionCipherSuites(client))
+    assertThat(handshakeEnabledCipherSuites).containsExactlyElementsOf(
+      expectedConnectionCipherSuites(client))
   }
 
   @Test
@@ -133,12 +146,14 @@ class CallHandshakeTest {
     // TLS_RSA_WITH_AES_256_CBC_SHA
     // TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
     // TLS_RSA_WITH_AES_128_CBC_SHA
-    assertThat(handshakeEnabledCipherSuites).containsExactlyElementsOf(expectedConnectionCipherSuites(client))
+    assertThat(handshakeEnabledCipherSuites).containsExactlyElementsOf(
+      expectedConnectionCipherSuites(client))
   }
 
   @Test
   fun testHandshakeCipherSuiteOrderingWhenReversed() {
-    val client = makeClient(ConnectionSpec.RESTRICTED_TLS, TlsVersion.TLS_1_2, defaultEnabledCipherSuites.asReversed())
+    val client = makeClient(ConnectionSpec.RESTRICTED_TLS, TlsVersion.TLS_1_2,
+      defaultEnabledCipherSuites.asReversed())
 
     val handshake = makeRequest(client)
 
@@ -146,7 +161,8 @@ class CallHandshakeTest {
     assertThat(handshake.cipherSuite).isIn(expectedModernTls12CipherSuites)
 
     // TODO reversed ciphers
-    assertThat(handshakeEnabledCipherSuites).containsExactlyElementsOf(expectedConnectionCipherSuites(client))
+    assertThat(handshakeEnabledCipherSuites).containsExactlyElementsOf(
+      expectedConnectionCipherSuites(client))
   }
 
   @Test
@@ -154,17 +170,62 @@ class CallHandshakeTest {
     val client = makeClient()
     makeRequest(client)
 
-    val socketOrderedByDefaults = handshakeEnabledCipherSuites.sortedBy { defaultEnabledCipherSuites.indexOf(it) }
+    val socketOrderedByDefaults =
+      handshakeEnabledCipherSuites.sortedBy { defaultEnabledCipherSuites.indexOf(it) }
     assertThat(handshakeEnabledCipherSuites).containsExactlyElementsOf(socketOrderedByDefaults)
   }
 
+  @Test
+  fun advertisedOrderInRestricted() {
+    assertThat(ConnectionSpec.RESTRICTED_TLS.cipherSuites).containsExactly(
+      TLS_AES_128_GCM_SHA256,
+      TLS_AES_256_GCM_SHA384,
+      TLS_CHACHA20_POLY1305_SHA256,
+      TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+      TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+      TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
+      TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
+      TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,
+      TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256
+    )
+  }
+
+  @Test
+  fun effectiveOrderInRestrictedJdk11() {
+    platform.assumeJdkVersion(11)
+
+    val platform = Platform.get()
+    val platformDefaultCipherSuites =
+      platform.newSslSocketFactory(platform.platformTrustManager()).defaultCipherSuites
+    val cipherSuites =
+      ConnectionSpec.RESTRICTED_TLS.effectiveCipherSuites(platformDefaultCipherSuites)
+    assertThat(cipherSuites).containsExactlyElementsOf(listOf(
+      TLS_AES_128_GCM_SHA256,
+      TLS_AES_256_GCM_SHA384,
+      TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
+      TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+      TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
+      TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+
+      // Disabled
+//      TLS_CHACHA20_POLY1305_SHA256,
+//      TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,
+//      TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256
+    ).map { it.javaName })
+  }
+
   private fun expectedConnectionCipherSuites(client: OkHttpClient): Set<String> {
     // TODO correct for the client provided order
 //    return client.connectionSpecs.first().cipherSuites!!.map { it.javaName }.intersect(defaultEnabledCipherSuites)
-    return defaultEnabledCipherSuites.intersect(client.connectionSpecs.first().cipherSuites!!.map { it.javaName })
+    return defaultEnabledCipherSuites.intersect(
+      client.connectionSpecs.first().cipherSuites!!.map { it.javaName })
   }
 
-  private fun makeClient(connectionSpec: ConnectionSpec? = null, tlsVersion: TlsVersion? = null, cipherSuites: List<String>? = null): OkHttpClient {
+  private fun makeClient(
+    connectionSpec: ConnectionSpec? = null,
+    tlsVersion: TlsVersion? = null,
+    cipherSuites: List<String>? = null
+  ): OkHttpClient {
     return this.client.newBuilder()
       .apply {
         if (connectionSpec != null) {