Okhttp: OkHttp fails to set TLS v1.1 and v1.2 on JDK 7

[panga]

DESCRIPTION:
OkHttp creates a SslSocketFactory using SSLContext.getInstance("TLS") to retrieve default TLS implementation from JDK.
The last public version of JDK 7 (update 80) uses TLS v1 as default.
This behavior has changed in new commercial versions by Oracle JDK 7 update 131 to use TLS v1.2 as default (the same behavior of JDK 8).

PROBLEM:
When running my code on last JDK 7 public version (update 80), I'm not able to connect to servers that uses TLS v1.1 or TLS v1.2.
The connectionsSpecs.tlsVersions feature does nothing because SSLContext.getInstance("TLS") retrieves v1 implementation.

SOLUTION:
Change SSLContext.getInstance("TLS") to SSLContext.getInstance("TLSv1.2") so the JDK 7 will also retrieve TLS v1.2 by default and connectionsSpecs.tlsVersions feature will work as expected as TLS v1.2 can downgrade gracefully to previous versions.

Line to change:
https://github.com/square/okhttp/blob/c3d6607a14f66eae3dedc927a3eff642d1493caa/okhttp/src/main/java/okhttp3/internal/platform/Platform.java#L269

[yschimke]

Is this documented somewhere? Is it safe to always use this on all platforms? or should it fallback to TLS?

[panga]

@yschimke Yes, I'm sure this protocol version is implemented on JDK supported platforms and Android as well.

Most importantly is that this will become web default as the PCI compliance is dropping SSL/TLSv1 on end of month.

That's not a problem for JDK 8+ users, but we still have many enterprise customers running JDK 7.

https://blog.pcisecuritystandards.org/are-you-ready-for-30-june-2018-sayin-goodbye-to-ssl-early-tls

[yschimke]

https://www.java.com/en/configure_crypto.html#enableTLSv1_2
http://www.oracle.com/technetwork/java/javase/7u131-relnotes-3338543.html

IIUC

7 u131 - uses up to TLSv1.2
7 u95 - uses jdk.tls.client.protocols to decide versions
7 u80 - uses TLSv1, but we could opt into TLSv2 by requesting "TLSv1.2"

So clients JDK who upgrade will get more modern TLS, so not an urgent problem. But we can increase security for older clients.

[panga]

@yschimke they can't.

7u80 is the last public version available.
http://www.oracle.com/technetwork/java/javase/downloads/java-archive-downloads-javase7-521261.html

7u131 is an Oracle paid version!

[yschimke]

$ /Library/Java/JavaVirtualMachines/jdk1.8.0_162.jdk/Contents/Home/bin/java -classpath okhttp/target/classes TLS
Testing: TLS
[TLSv1, TLSv1.1, TLSv1.2]
Testing: TLSv1.2
[TLSv1, TLSv1.1, TLSv1.2]
Testing: TLSv1.1
[TLSv1, TLSv1.1]
Testing: TLSv1
[TLSv1]

$ /Library/Java/JavaVirtualMachines/jdk1.7.0_80.jdk/Contents/Home/bin/java -classpath okhttp/target/classes TLS
Testing: TLS
[TLSv1]
Testing: TLSv1.2
[TLSv1, TLSv1.1, TLSv1.2]
Testing: TLSv1.1
[TLSv1, TLSv1.1]
Testing: TLSv1
[TLSv1]

$ /Library/Java/JavaVirtualMachines/jdk-10.0.1.jdk/Contents/Home/bin/java -classpath okhttp/target/classes TLS
Testing: TLS
[TLSv1, TLSv1.1, TLSv1.2]
Testing: TLSv1.2
[TLSv1, TLSv1.1, TLSv1.2]
Testing: TLSv1.1
[TLSv1, TLSv1.1]
Testing: TLSv1
[TLSv1]

[yschimke]

https://github.com/square/okhttp/pull/4089

[yschimke]

Do you know what the advice on Android should be? Client developer can opt into the Google Play provider, and I suspect that Android doesn't have same quirks for "TLS".

[panga]

Thanks @yschimke

If you want I can provide a simple test case that fails with TLS handshake in JDK 7:

@Test
public void tls12Test() throws IOException {
        OkHttpClient client = new OkHttpClient();
        Request request = new Request.Builder().url("https://www.salesforce.com").build();
        Response response = client.newCall(request).execute();
        Assert.assertEquals(200, response.code());
}

[panga]

Do you know what the advice on Android should be? Client developer can opt into the Google Play provider, and I suspect that Android doesn't have same quirks for "TLS".

Android also supports "TLSv1.2" SSLContext, look at all hacks mentioned in #2372.

[yschimke]

My concern is more if on newer Android, whether an earlier preferred Android provider only supports TLS and specifying TLSv1.2 drop to a less preferred provider.

This sort of change feels safer if it is tightly scoped e.g. this fix only affects JVM 1.7 u80-130

Or we will regret this when newer JVMs support TLSv1.3, although that might require API changes anyway.

[yschimke]

@panga could you avoid a test that hits a remote service? Maybe a test that runs against Mock webserver set to only support certain protocols.

[yschimke]

It sucks you can't check supported protocol versions until after init, so we can't get TLS first, and then check and try TLSv1.2 just in case.

[panga]

This sort of change feels safer if it is tightly scoped e.g. this fix only affects JVM 1.7 u80-130

Totally agree. This change must check for Java 1.7 in case of Java and Android API version < 23 in case of Android. That fix both old JDK's and Android's without impact in future.

[yschimke]

You should submit a PR next time, this felt a bit like

[panga]

Hahaha!

I'll submit on next time for sure!

[yschimke]

Feel free to take this over if you want to complete and write the tests etc.

[yschimke]

Can you test with a snapshot build? https://oss.jfrog.org/artifactory/libs-snapshot/com/squareup/okhttp3/okhttp/3.11.0-SNAPSHOT/

[panga]

@yschimke yes, I'll test that snapshot early Monday and paste here the results.

Thanks again!

[panga]

@yschimke Tests are green now!