OkHttp changes the global SSL context, breaks other HTTP clients #184

Closed
skyisle opened this Issue May 10, 2013 · 33 comments

Projects

None yet
@skyisle
skyisle commented May 10, 2013

We're enabling SPDY for the shared SSL context, and other HTTP clients like HttpURLConnection don't anticipate this, causing them to freak out and crash the app.

@skyisle's original report...

Here is backtrace.

DEBUG I backtrace:
DEBUG I #00 pc 00022430 /system/lib/libssl.so (SSL_select_next_proto+25)
DEBUG I #1 pc 000222ef /system/lib/libjavacore.so
DEBUG I #2 pc 0002905f /system/lib/libssl.so (ssl_parse_serverhello_tlsext+458)
DEBUG I #3 pc 00015957 /system/lib/libssl.so (ssl3_get_server_hello+894)
DEBUG I #4 pc 00018193 /system/lib/libssl.so (ssl3_connect+618)
DEBUG I #5 pc 000235d7 /system/lib/libssl.so (SSL_connect+18)
DEBUG I #6 pc 0001126b /system/lib/libssl.so (ssl23_connect+1970)
DEBUG I #7 pc 0002350f /system/lib/libssl.so (SSL_do_handshake+66)
DEBUG I #8 pc 00024bc5 /system/lib/libjavacore.so
DEBUG I #9 pc 0001e490 /system/lib/libdvm.so (dvmPlatformInvoke+112)
DEBUG I #10 pc 0004d2b1 /system/lib/libdvm.so (dvmCallJNIMethod(unsigned int const_, JValue_, Method const_, Thread_)+396)
DEBUG I #11 pc 000278a0 /system/lib/libdvm.so
DEBUG I #12 pc 0002b77c /system/lib/libdvm.so (dvmInterpret(Thread_, Method const_, JValue_)+176)
DEBUG I #13 pc 0005fae5 /system/lib/libdvm.so (dvmCallMethodV(Thread_, Method const_, Object_, bool, JValue_, std::va_list)+272)
DEBUG I #14 pc 0005fb0f /system/lib/libdvm.so (dvmCallMethod(Thread
, Method const
, Object_, JValue*, ...)+20)
DEBUG I #15 pc 0005466f /system/lib/libdvm.so
DEBUG I #16 pc 0000e418 /system/lib/libc.so (__thread_entry+72)
DEBUG I #17 pc 0000db0c /system/lib/libc.so (pthread_create+168)
DEBUG I #18 pc 00052f34

@swankjesse
Contributor

Which device is this? Please tell me its a Droid RAZR...

@skyisle
skyisle commented May 10, 2013

Sorry. It's stock galaxy nexus 4.2.2.

@swankjesse
Contributor

Alright, good to know. I'll try to reproduce this. In the interim you can disable SPDY with setTransports("http/1.1").

@andy572
andy572 commented May 15, 2013

Crashes with SSL on Stock HTC One X (Android 4.1.1) too :(

@mauimauer

Same here...this seems to be happening on all my devices > 4.1

@lexer
Contributor
lexer commented May 30, 2013

Nexus 4. OS 4.2.2

@lexer
Contributor
lexer commented May 30, 2013

Workaround is to use only OkHttp connection. Possible problems that 3rd party libraries will still use Android UrlConnection that could cause this crushes.

@andy572
andy572 commented May 30, 2013

It has nothing to do with URLConnection - the underlaying libSSL is crashing in
method "SSL_select_next_proto" ... there is possibly a null pointer, for me it seems libSSL get a empty object
when trying to validate a certificate.

The workaround is to force http/1.1 and not to use http 2.0:
ArrayList list = new ArrayList();
list.add("http/1.1");
client.setTransports(list);

@codebutler

I started running into this on a Nexus 4 after adding Google Analytics to an app that already was using OkHttp (Note the thread name "GAThread": https://gist.github.com/codebutler/5681044). Unfortunately there doesn't seem to be a way to change the HTTPClient used by GA, so "just use OkHttp everywhere" isn't an available workaround.

Please let me know if I can help with debugging.

@mauimauer

Doesn't seem to help in my case. I am using OkHttp as transport for both my own API (using Volley) and for gapi-client. Still crashes when SSL endpoints are involved.

@swankjesse
Contributor

Thanks for reporting. I'm able to reproduce this.

@swankjesse
Contributor

Yeah, looks like this occurs consistently if you use OkHttp and HttpURLConnection in the same application.

@swankjesse
Contributor

So the problem is that Java and Android programs default to using a single global SSL context, accessible to our HTTP clients as the SSLSocketFactory. When OkHttp enables NPN for its own SPDY-related stuff, it ends up turning on NPN for the singleton shared SSL socket factory. Later when HttpURLConnection attempts to do SSL, it freaks out and crashes because NPN is enabled when it isn't expected to be.

A simple immediate workaround for OkHttp is to configure the client to create its own SSL context rather than using the system default. The code to set that up looks like this:

      OkHttpClient okHttpClient = new OkHttpClient();
      SSLContext sslContext;
      try {
        sslContext = SSLContext.getInstance("TLS");
        sslContext.init(null, null, null);
      } catch (GeneralSecurityException e) {
        throw new AssertionError(); // The system has no TLS. Just give up.
      }
      okHttpClient.setSslSocketFactory(sslContext.getSocketFactory());

I'll investigate changing OkHttp to prefer its own SSL context over the system default. The significant drawback of this approach is that apps that customize the global SSL context will lose these customizations. This could break important features like certificate pinning! So I should be extremely careful if I'm changing behavior here.

@andy572
andy572 commented Jun 1, 2013

the workaround is working nice, now i can use spdy and my app is very fast with internet connections.
The problem was as described, i use the twitter4j jar library which is using a httpurlconnection :)

@JakeWharton
Member

The pull request to allow installing OkHttp as the backer for URL#openConnection will also mitigate.

@swankjesse
Contributor

Yup. This is the official workaround:

  URL.setURLStreamHandlerFactory(new OkHttpClient());
@kkocel
Contributor
kkocel commented Jun 24, 2013

Is there any plan / way to fix this issue?

@swankjesse
Contributor

@kkocel yup. Plan is to switch OkHttp to prefer a private default SSL context rather than the shared singleton context. Unfortunately that's an API incompatible change so it'll need to be on a major release. I'm planning a major 2.0 release within 3 months.

@brycehendrix

The workaround did resolve the issue I was having with bugsense causing the crash, as long as I did it prior to calling BugSenseHandler.initAndStartSession()

@orip
orip commented Jul 23, 2013

In our case, running URL.setURLStreamHandlerFactory as early as possible (in MyApplication.onCreate) didn't seem to fix the problem.

It was easy enough to convert all our uses of OkHttpClient to create their own SSL context but converting Picasso was a little trickier, so now we have PicassoUtils.with instead of Picasso.with. Works with Picasso 1.1.1 (easy to make it work with Picasso HEAD).

@brycehendrix

I take back what I said, I'm still having problems using BugSense and OkHttp together, though less frequently. In my Application class I overload onCreate:

@Override
public void onCreate(){
super.onCreate();

URL.setURLStreamHandlerFactory(new OkHttpClient());
BugSenseHandler.initAndStartSession(this, "apikey");

}

Sometimes when an unhandled exception is thrown, BugSense will catch it, then try to report it via SSL and fail with the SSL error. It doesn't always happen, which makes it hard to track down. Maybe the original SSL context is released?

@tmszdmsk tmszdmsk added a commit to tmszdmsk/arij that referenced this issue Jul 31, 2013
@tmszdmsk tmszdmsk #8 added PoC QuickSearch.jspa calling.
Only jql like inputs are handled.
Wanted to use OkHttp library by retrofit, but there is nasty unresolved for few months bug square/okhttp#184 which doesn't work very well when some parts (e.g. analytics library in that case) doesn't use him.
Fixed bug with calling quicksearch two times when using hardware keyboard ENTER button (key down and key up).
Extracted AuthorizationHeaderGenerator because it's used also with this QuerySearch call.
a95dd8c
@robUx4
Contributor
robUx4 commented Aug 7, 2013

I am also having issues with Google Analytics when mixed with okhttp. I still get some crashes with URL.setURLStreamHandlerFactory() but not with this hack

@swankjesse
Contributor

For 2.0 we should change the default to not use the global SSL context.

@MichaelEvans
Contributor

Is there any suggested workaround for those using Picasso as well? Is https://github.com/square/okhttp/issues/184#issuecomment-19521268 the recommended answer for now?

@timothyasp timothyasp added a commit to iFixit/iFixitAndroid that referenced this issue Nov 13, 2013
@timothyasp timothyasp Patch/Hack to prevent segfault from OkHttp
square/okhttp#184

Because OkHttp uses the shared SSLContext, which also happens to be used by the
Google Analytics library, it would silently and abruptly cause an segfault
breaking everything.  The above issue mentions a workaround creating a new
sslcontext and setting the OkHttpClient to use that one instead of the default
sslcontext fixes it.
0033b4b
@UweTrottmann UweTrottmann added a commit to UweTrottmann/SeriesGuide that referenced this issue Dec 13, 2013
@UweTrottmann UweTrottmann Fix libssl crash because of OkHttp changing the global SSL context.
- See related square/okhttp#184.
4c0e925
@UweTrottmann UweTrottmann added a commit to UweTrottmann/getglue-java that referenced this issue Dec 21, 2013
@UweTrottmann UweTrottmann Use private SSL context with OkHttpClient to avoid libssl crash. 1be1a6d
@skykelsey

Is there a fix scheduled for this? A lot of people are using this library, and since it doesn't play nicely with other libraries, it should be considered a major bug.

@adriancole
Collaborator

@swankjesse WDYT about a default for this. A naive approach would be to plug the following into OkHttpClient.copyWithDefaults(). Another could be to handle it as a Platform hook.

    if (result.sslSocketFactory == null) {
      try {
        SSLContext sslContext = SSLContext.getInstance("TLS");
        sslContext.init(null, null, null);
        result.sslSocketFactory = sslContext.getSocketFactory();
      } catch (GeneralSecurityException e) {
        throw new AssertionError(); // The system has no TLS. Just give up.
      }
    }
@swankjesse
Contributor

Yes, but with two small changes:
If you create an OkHttpClient and don't configure anything, all HTTPS requests created with that client share an SSL Socket factory.

If you create an OkHttpClient and install your own SSLSocketFactory, then we don't waste time creating an unused SSLSocketFactory. (Ie. the SSLSocketFactory above must be created lazily.)

@swankjesse
Contributor

(Not sharing an SSLSocketFactory is bad because requests that don't share an SSLSocketFactory can't share a connection in the connection pool.)

@swankjesse swankjesse closed this in #518 Feb 9, 2014
@ceram1
ceram1 commented Feb 26, 2014

But at now, it crashes.

I tested in
Samsung Galaxy Note 4.3 and LG Optimus G 4.1

I use volley with okhttp stack and google analytics.
"GAThread" crashes with okhttp

When will 2.0 released??

@swankjesse
Contributor

@ceram1 I was hoping soon, but we've made some API changes that make it tricky. I'll talk to @JakeWharton about doing an RC1 or something.

@tpurtell tpurtell added a commit to tpurtell/ModernHttpClient-1 that referenced this issue Feb 28, 2014
@tpurtell tpurtell add workaround of having a private SSL context so crashes don't occur…
… when using a library like good analytics along side OkHttp square/okhttp#184 . an alternative might be to update the okhttp library, they seemed to have finally closed the issue...
ec6a0cb
@dabluck dabluck referenced this issue in square/retrofit Jun 4, 2014
Closed

Native crash at /system/lib/libssl.so #501

@aried3r aried3r referenced this issue in Catrobat/Catroid Oct 28, 2014
Closed

Fix tls version #1133

@DanMorganiOS

If you come across this thread with a PhoneGap/Cordova application check here for the issue: https://issues.apache.org/jira/browse/CB-7925

@jimmyhmiller

I know this was closed a long time ago. But I'm having trouble with okhttp3 and the facebook sdk. I was wondering if this could still be causing an issue, or if there could be some other cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment