Don't use broken ALPN on Android 4.4

[ejona86]

As described in Platform.java, ALPN is only supported on Android 4.4 or higher which is currently 40% of the market share. For apps currently using SPDY, this means giving up notable performance for most users.

I couldn't find any reasoning in #1243 for why the NPN removal was useful. What would prohibit restoring NPN support?

[mh-dm]

I guess what would be great is adding back NPN but as a fallback when ALPN support is missing.

[mh-dm]

Actually, if ALPN is buggy in Android 4.4 the fallback is not a good idea. 4d06821#diff-052708b701881930617b926799cba4b8L53

[swankjesse]

@ejona86 yup, good point. As far as I know, NPN works fine on the platforms we supported it on.

[nfuller]

For background, my original pull request: #1201

My pull request had open questions so I think it should be reverted.

Summary:

1. It sounds like the deprecation of NPN in the wild has not gone as fast as was originally hoped. This may be because OpenSSL didn't release a full version that included ALPN support until Oct (15-Oct-2014: OpenSSL 1.0.1j)

2. ALPN support in Android appears to be broken: https://code.google.com/p/android/issues/detail?id=67940. This was referenced in the OkHttp code, but it doesn't look like ALPN was being avoided on Android so I think it was still an issue (albeit documented). I don't know how likely the issue is to occur but it looks catastrophic when it does.

Other points to be aware of:

NPN support in Android is also not perfect. There is an internal Android bug report 18705877 I created in Dec 2014. The bug causes problems if a mix of NPN and non-NPN sockets are being created from the same SSLContext / SocketFactory. My imperfect understanding is that the "should I use NPN?" information is being set on the SSL context, not on the socket. Later sockets from that context that do not use NPN end up failing with:

Caused by: javax.net.ssl.SSLProtocolException: SSL handshake aborted: ssl=0xb8421b40: Failure in SSL library, usually a protocol error
error:140920E3:SSL routines:SSL3_GET_SERVER_HELLO:parse tlsext (external/openssl/ssl/s3_clnt.c:1141 0xb0866024:0x00000000)

        at com.android.org.conscrypt.NativeCrypto.SSL_do_handshake(Native Method)
        at com.android.org.conscrypt.OpenSSLSocketImpl.startHandshake(OpenSSLSocketImpl.java:317)

To quote somebody who knows a lot more about OpenSSL than me: "If the SSL_CTX has an next_proto_select_cb, it will advertise the extension in ClientHello and then if you select nothing (e.g., you don't want NPN for this particular SSL connection), it results in a TLS1_AD_INTERNAL_ERROR."

There may be a workaround for this we can apply in OkHttp for this NPN issue if we return to supporting it. I'd need to look more closely.

Looking to the future: It looks like the most likely scenario is that NPN will be removed from Android in a future release in favor of ALPN. I will chase up the ALPN issue and see if there is a fix coming in a future Android release, but if the ALPN implementation is broken in KitKat it's possible that NPN would be a better choice there.

[swankjesse]

Next steps:
Don't use ALPN on Android 4.4 and earlier; it's broken there.
Restore NPN for versions of Android where it was previously stable.

[ankurbn]

@swankjesse what do you recommend that we use in the interim? I changed my version from 2.2.0 to 2.1.0 and see the following issue:

I/DEBUG ( 1510): #10 pc 000c53d3 /system/lib/libcrypto.so (CRYPTO_malloc+66)
I/DEBUG ( 1510): #11 pc 0001ac4f /system/lib/libssl.so (ssl3_get_new_session_ticket+202)
I/DEBUG ( 1510): #12 pc 000178eb /system/lib/libssl.so (ssl3_connect+1122)
I/DEBUG ( 1510): #13 pc 00027f4b /system/lib/libssl.so (SSL_do_handshake+50)
I/DEBUG ( 1510): #14 pc 0000af49 /system/lib/libjavacrypto.so
I/DEBUG ( 1510): #15 pc 000203cc /system/lib/libdvm.so (dvmPlatformInvoke+112)
I/DEBUG ( 1510): #16 pc 00050eeb /system/lib/libdvm.so (dvmCallJNIMethod(unsigned int const_, JValue_, Method const_, Thread_)+398)
I/DEBUG ( 1510): #17 pc 00029860 /system/lib/libdvm.so
I/DEBUG ( 1510): #18 pc 00030b68 /system/lib/libdvm.so (dvmMterpStd(Thread_)+76)
I/DEBUG ( 1510): #19 pc 0002e200 /system/lib/libdvm.so (dvmInterpret(Thread_, Method const_, JValue_)+184)
I/DEBUG ( 1510): #20 pc 000637cd /system/lib/libdvm.so (dvmInvokeMethod(Object_, Method const_, ArrayObject_, ArrayObject_, ClassObject_, bool)+392)
I/DEBUG ( 1510): #21 pc 0006b6ab /system/lib/libdvm.so
I/DEBUG ( 1510): #22 pc 00029860 /system/lib/libdvm.so
I/DEBUG ( 1510): #23 pc 00030b68 /system/lib/libdvm.so (dvmMterpStd(Thread_)+76)
I/DEBUG ( 1510): #24 pc 0002e200 /system/lib/libdvm.so (dvmInterpret(Thread_, Method const_, JValue_)+184)
I/DEBUG ( 1510): #25 pc 000634e9 /system/lib/libdvm.so (dvmCallMethodV(Thread_, Method const_, Object_, bool, JValue_, std::va_list)+336)
I/DEBUG ( 1510): #26 pc 0006350d /system/lib/libdvm.so (dvmCallMethod(Thread, Method const, Object_, JValue_, ...)+20)
I/DEBUG ( 1510): #27 pc 0007112f /system/lib/libdvm.so
I/DEBUG ( 1510): #28 pc 00029860 /system/lib/libdvm.so
I/DEBUG ( 1510): #29 pc 00030b68 /system/lib/libdvm.so (dvmMterpStd(Thread_)+76)
I/DEBUG ( 1510): #30 pc 0002e200 /system/lib/libdvm.so (dvmInterpret(Thread_, Method const_, JValue_)+184)
I/DEBUG ( 1510): #31 pc 000634e9 /system/lib/libdvm.so (dvmCallMethodV(Thread_, Method const_, Object_, bool, JValue*, std::__va_list)+336)

[nfuller]

I've been chasing up the various bugs on Android and their historic state.

Here's my understanding:

1. On 4.4, ALPN is broken. It is fixed on >= 5.0. Why removing NPN support caused more ALPN issues I don't know, but I assume there is some priority given to NPN or interaction in OpenSSL that were preventing these crashes. Using the Google Play Services dynamic security provider should also find that ALPN works better (since this installs a version of the SSLSocketFactory / SSLSocket implementation that contains the fix).

2. NPN has an issue if you use the same SSLSocketFactory for NPN and non-NPN traffic. There's currently no fix for that.

[swankjesse]

With this PR, I've dropped support for ALPN on Android 4.4:
#1474

Here's the combination of things to run OkHttp tests on an Android emulator:

vogar --verbose \
  --device-dir /data/vogar \
  --classpath /Users/jwilson/.m2/repository/org/bouncycastle/bcprov-jdk15on/1.50/bcprov-jdk15on-1.50.jar \
  --classpath /Users/jwilson/.m2/repository/com/squareup/okio/okio/1.3.0-SNAPSHOT/okio-1.3.0-SNAPSHOT.jar \
  --sourcepath okhttp/src/main/java \
  --sourcepath mockwebserver/src/main/java/ \
  --sourcepath okhttp/target/generated-sources/java-templates/ \
  --classpath okhttp-urlconnection/target/okhttp-urlconnection-2.3.0-SNAPSHOT.jar \
  ./okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/HttpOverHttp20Draft16Test.java

Note that this is incomplete; vogar doesn't implement @Rule, nor does it know about TemporaryDirectory. Possibly-fixed by the new vogar in AOSP. I manually replaced the rule with the manual code it automates. (What a hassle.)

[bsiegel]

It sounds like there was some agreement here on restoring NPN for Android 4.4 and below. Has anything come of this? If I submit a PR is there a chance it could be merged or is NPN a dead issue?

[swankjesse]

No more NPN. If you need it, fork!