Confirm client cipher_suites order is maintained

[yschimke]

Looks like we use the SSLSocket ordering over the client.

  private fun supportedSpec(sslSocket: SSLSocket, isFallback: Boolean): ConnectionSpec {
    var cipherSuitesIntersection = if (cipherSuitesAsString != null) {
      sslSocket.enabledCipherSuites.intersect(cipherSuitesAsString, CipherSuite.ORDER_BY_NAME)
    } else {
      sslSocket.enabledCipherSuites
    }

https://tools.ietf.org/html/rfc5246

The cipher suite list, passed from the client to the server in the ClientHello message, contains the combinations of cryptographic algorithms supported by the client in order of the client's preference (favorite choice first). Each cipher suite defines a key exchange algorithm, a bulk encryption algorithm (including secret key length), a MAC algorithm, and a PRF. The server will select a cipher suite or, if no acceptable choices are presented, return a handshake failure alert and close the connection. If the list contains cipher suites the server does not recognize, support, or wish to use, the server MUST ignore those cipher suites, and process the remaining ones as usual.

[yschimke]

My other question from the spec is what is meant by client? The app developer? OkHttp? The JSSE provider?

[yschimke]

From https://stackoverflow.com/questions/64672259/how-can-i-preserve-the-order-of-my-ciphersuite-list-in-okhttp/64676161#64676161

[swankjesse]

If we change which order we prefer, we should make sure our released order is stable from what we've been shipping. There's potential performance regressions from changing which cipher suite is used!

[yschimke]

A theoretical bug ugly fix https://github.com/yschimke/okhttp/pull/6/files

cc @swankjesse

[yschimke]

Is this worth fixing for 4.10? Or should we add test of existing behaviour now, then put out a 4.11.0-RC1 and ask interested parties to test?

[swankjesse]

I have a slight preference to hold until 4.11 just cause 4.10 is already pretty big.

[yschimke]

JDK 13 now defaults to server cipher suite ordering, which may affect testing.

https://seanjmullan.org/blog/2019/08/05/jdk13

[ogolovanov]

Hi.

Its almost 2 years since bugfix by @yschimke ( https://github.com/square/okhttp/pull/6407 ). And that bugfix was delayed for okhttp 4.11 version. I see that 4.11 milestone is closed with "0% completed": https://github.com/square/okhttp/milestone/49

Is okhttp 4.11 going to be released, or?

Thanks.

[yschimke]

I suspect for this change we should make it to 5.x only. To be discussed.

[yschimke]

Fixed in https://github.com/square/okhttp/pull/7452/files