default ssl socket factory set after cache install is ignored on 2.2

[jaypatrickhoward]

I'm doing the following:

1. Installing HttpResponseCache (1.3 latest), then
2. Setting a default SSL socket factory via HttpsURLConnection.setDefaultSSLSocketFactory() in order to do two-way SSL authentication.

This works fine in 2.3 and above. In 2.2, however, when HttpResponseCache is installed, SSL connections ignore the default SSL socket factory I set.

Any thoughts on how to work around this? Would it be possible to patch HttpResponseCache so that the behavior on 2.2 matches the behavior on 2.3+?

[jaypatrickhoward]

I tried setting the SSLSocketFactory manually on each HttpsURLConnection, as opposed to just setting the system default, and it still didn't work. I'm pretty sure (but not certain) that this problem occurs even when loading locally cached content. Since I can't figure out a workaround I'm going to have to disable HttpResponseCache installation on Android versions prior to Gingerbread (2.3).

To test:

1. Create the class LoggingSSLSocketFactory that extends SSLSocketFactory and delegates all calls to super, but logs each method call.
2. Create an instance of LoggingSSLSocketFactory and set it as the system default by passing it to HttpsURLConnection.setDefaultSSLSocketFactory().
3. Install HttpResponseCache.
4. Open a connection to a "https" URL and try to read from it. You should not see any log statements from the LoggingSSLSocketFactory you set as the system default.

[jaypatrickhoward]

I spent some time debugging this today and my original description of the issue is not accurate.

The HttpResponseCache code does in fact honor the default SSLSocketFactory set on HttpsURLConnection.

What's happening is that HttpConnection.setupSecureSocket() (from HttpResponseCache) is failing to set advanced TLS options on the SSLSocket. This causes it to fall back to SSLv3, which I verified is supported by the underlying socket implementation. However, when startHandshake() is called, I see this in the log:

08-30 16:12:12.774: E/NativeCrypto(2918): Unknown error 1 during connect

So setupSecureSocket() must be configuring the SSLSocket incorrectly somehow, since this "works" when HttpResponseCache isn't installed.

A search for "Unknown error 1 during connect" turned up several Stack Overflow posts with a very similar error in relation to SSL calls, but no solutions or insight into what's causing it.

Somehow, however, this "works" when HttpResponseCache isn't in the picture. I will try to figure out how the Android 2.2 version of HttpConnection sets up its SSLSocket.

[jaypatrickhoward]

All the Android HttpConnection source code I can find online looks identical to the version in your library.

[jaypatrickhoward]

More info:

The SSLSocketFactory I set as the default is created from a SSLContext that was instantiated with "TLS" as the protocol, despite TLS apparently not being supported by the underlying SocketImpl class. I'm guessing this is the root of the problem. Still, no idea how or why it works when no HttpResponseCache is installed.

I'm going to try to modify my code to use the protocol of the default SSLContext (SSLContext.getDefault().getProtocol()) instead of using a hard-coded value of "TLS" and see if that fixes the issue.

[jaypatrickhoward]

Red herring. Getting a SSLContext instance via getInstance("TLS") is the correct way. The bug may simply be that setupSecureSocket() falls back to SSLv3 unnecessarily when the methods it wants to see on unverifiedSocket aren't there.

[jaypatrickhoward]

This is in fact the case. When I modify HttpConnection.setupSecureSocket() to not fall back to SSLv3 when the methods it wants to call (setEnabledCompressionMethods, setUseSessionTickets, setHostname) on the SSLSocket don't exist then everything seems to work fine.

[candrews]

@jaypatrickhoward I'm not sure I'm following your last comment - are you saying that you found a workaround? If so, could you submit a merge request? If not... could you explain? My apologies for my density!

[jaypatrickhoward]

I did find a work around, but I don't know enough about the SSL/TLS logic to know whether it's advisable. The change is to com.integralblue.httpresponsecache.compat.libcore.net.http.HttpConnection.setupSecureSocket(). On this particular device, the SSLSocket class returned by sslSocketFactory.createSocket() lacks the methods the TLS branch attempts to invoke via reflection (e.g. setEnabledCompressionMethods). This causes it to fall back to SSLv3, which then (for some reason) doesn't work on this device.

When I let those method invocations fail silently (without falling back to SSLv3) it seems to work.

This is probably a really dumb question, but are all those compat classes really necessary? I get that HttpResponseCache has to provide custom implementations of HttpURLConnectionImpl and HttpsURLConnectionImpl in order to perform the caching, but is it really necessary to provide custom versions of all the rest? For instance, if we were able to use the "stock" HttpConnection class then this problem might just disappear.

[candrews]

Unfortunately, they are necessary. There are a few reasons:
The stock classes aren't visible (application code cannot see/access them)
They don't exist, or have different features/bugs/methods for different versions of Android, so even if I could access them and use them, then applications would get different behaviors and features on different versions of Android, which is one of the problems I'd like to avoid by using this library
I think a neat feature of this library is that it doesn't have to be used on Android - it'll work on any JVM >= 5

[jaypatrickhoward]

Hmm. Sounds reasonable. I'm going to try to create a wrapper class that extends the underlying SSLSocket implementation. (Though if this class isn't visible then that's not going to work). My class will delegate each method call to its wrapped instance but will log each one. That way I'll be able to tell exactly how your compat code differs from the stock code with respect to how it configures SSLSockets before calling their startHandshake() method.

Another thought: I'm actually not certain this is a 2.2 issue per se. It may just be an issue with this specific device. Unfortunately I don't have any other 2.2 devices to test with. I do have a DeviceAnywhere account, though; I may get on there and see if I can find one.

[jaypatrickhoward]

The more I think about it, the more I think HttpConnection shouldn't automatically fall back to SSLv3 when those "advanced" TLS methods aren't present. Here's why. It has no way to know what the implementation of SSLSocket is that it gets back from the SSLSocketFactory. For instance, maybe the user set his own custom SSLSocketFactory that constructs custom SSLSocket implementations that lack those methods, but still support that functionality (e.g. compression). Maybe my custom SSLSocketImpl supports TLS but has has ZLIB compression "hard-coded" and so lacks that setter.

For that matter, I question whether HttpConnection should even do this kind of configuration even when it IS possible, since the caller may not want those values changed. For instance, maybe I set a custom SSLSocketFactory that configures the sockets it produces to use something other than ZLIB compression. The code in your HttpConnection class is going to overwrite my custom value with "ZLIB".

By the way, here's what I got from my logging wrapper classes (I could only overwrite the methods from SSLSocketFactory and SSLSocket, not those from the underlying implementations):

2.2 with cache installed:

08-31 08:39:07.122: E/LoggingSSLSocketFactoryImpl(5660): LoggingSSLSocketFactoryImpl(SSLSocketFactory delegate)
08-31 08:39:07.754: E/LoggingSSLSocketFactoryImpl(5660): createSocket(Socket arg0, String arg1, int arg2, boolean arg3)
08-31 08:39:07.862: E/LoggingSSLSocketImpl(5660): LoggingSSLSocketImpl(SSLSocket delegate)
08-31 08:39:07.872: E/LoggingSSLSocketImpl(5660): setEnabledProtocols(String[] arg0)
08-31 08:39:07.872: E/LoggingSSLSocketImpl(5660): startHandshake()

2.2 w/o cache installed:

08-31 08:42:50.402: E/LoggingSSLSocketFactoryImpl(5729): LoggingSSLSocketFactoryImpl(SSLSocketFactory delegate)
08-31 08:42:50.642: E/LoggingSSLSocketFactoryImpl(5729): createSocket(Socket arg0, String arg1, int arg2, boolean arg3)
08-31 08:42:50.712: E/LoggingSSLSocketImpl(5729): LoggingSSLSocketImpl(SSLSocket delegate)
08-31 08:42:50.712: E/LoggingSSLSocketImpl(5729): setUseClientMode(boolean arg0)
08-31 08:42:50.722: E/LoggingSSLSocketImpl(5729): startHandshake()

Since I'm using a client certificate, it may be the lack of setUseClientMode() that's causing the problem.

[jaypatrickhoward]

Manually setting UseClientMode = true in my custom SSLSocketFactory had no effect when the cache is installed. I still get the failure. So that's not the issue.

If you'll notice, the stock code never changes the enabled protocols. It asks the SSLSocketFactory for a SSLSocket and assumes that the factory has correctly set the enabled protocols. The only way it modifies the SSLSocket's configuration prior to calling startHandshake() is to call setUseClientMode().

I wonder whether the HttpConnection.setupSecureSocket() should just be:

public void setupSecureSocket(SSLSocketFactory sslSocketFactory, boolean tlsTolerant) throws IOException {
    unverifiedSocket = (SSLSocket) sslSocketFactory.createSocket(socket, address.uriHost, address.uriPort, true);
    unverifiedSocket.startHandshake();
}

Just assume the SSLSocketFactory performs all the configuration it wants to do in its createSocket() method.

[jaypatrickhoward]

Actually that causes problems. This doesn't:

public void setupSecureSocket(SSLSocketFactory sslSocketFactory, boolean tlsTolerant) throws IOException {
    unverifiedSocket = (SSLSocket) sslSocketFactory.createSocket(socket, address.uriHost, address.uriPort, true);
    if (tlsTolerant) {
        // do nothing
    } else {
        unverifiedSocket.setEnabledProtocols(new String [] { "SSLv3" });
    }
    unverifiedSocket.startHandshake();
}

[jaypatrickhoward]

Alternately, if you want to keep the previous functionality in place when those methods exist, just surround each method invocation in its own try/catch block and swallow the exception if the method doesn't exist.

[candrews]

I think I follow what you're saying - can you submit a merge request against head?

3 methods are called on the SSLSocket: setEnabledCompressionMethods, setUseSessionTickets, setHostname. So at least one of those is missing on <= 2.2... do you know which ones?

[jaypatrickhoward]

All the methods are missing on this device. The phone runs Android 2.2, but I'm not sure whether it's an OS issue or a device issue. There may be other devices running 2.2 that have support for advanced TLS configuration. Either way it probably deserves a fix. I see two options:

1. Remove those method calls altogether (i.e. the "do nothing" version I posted above), or
2. Call the methods only if they're available, otherwise fail silently instead of falling back to SSLv3.

Option #2 has the benefit of retaining "current behavior" on devices where these methods are present. What I wonder about is whether "current behavior" is actually "good behavior". I say that because it's somewhat unintuitive that the act of installing HttpResponseCache should override my SSL settings. Consider the situation where I set a default SSLSocketFactory that produces SSLSockets with ZLIB compression disabled. The HttpResponseCache code as it currently stands would re-enable ZLIB compression for every SSL connection it makes, despite my attempts to turn it off.

Let me know which option you prefer and I'll try to merge in a change. (I'm not familiar with Git and/or Github, so I'll have to figure out how to do that.)