Null Pointer Exception During Certificate Pinning (v. 4.4.1)

[rahul-narkhede]

When creating a certificate pinned connection using OkHttp 4.4.1, we face a Nul Pointer Exception at the following line
https://github.com/square/okhttp/blob/master/okhttp/src/main/java/okhttp3/internal/connection/RealConnection.kt#L393

Bellow is the stack trace

2020-03-24 15:36:44.978 6227-6307/com.example.application E/AndroidRuntime: FATAL EXCEPTION: OkHttp Dispatcher
    Process: com.example.application, PID: 6227
    kotlin.KotlinNullPointerException
        at okhttp3.internal.connection.RealConnection$connectTls$1.invoke(RealConnection.kt:393)
        at okhttp3.internal.connection.RealConnection$connectTls$1.invoke(RealConnection.kt:73)
        at okhttp3.Handshake$peerCertificates$2.invoke(Handshake.kt:53)
        at okhttp3.Handshake$peerCertificates$2.invoke(Handshake.kt:34)
        at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
        at okhttp3.Handshake.peerCertificates(Unknown Source:7)
        at okhttp3.internal.connection.RealConnection$connectTls$2.invoke(RealConnection.kt:399)
        at okhttp3.internal.connection.RealConnection$connectTls$2.invoke(RealConnection.kt:73)
        at okhttp3.CertificatePinner.check$okhttp(CertificatePinner.kt:159)
        at okhttp3.internal.connection.RealConnection.connectTls(RealConnection.kt:398)
        at okhttp3.internal.connection.RealConnection.establishProtocol(RealConnection.kt:325)
        at okhttp3.internal.connection.RealConnection.connect(RealConnection.kt:197)
        at okhttp3.internal.connection.ExchangeFinder.findConnection(ExchangeFinder.kt:235)
        at okhttp3.internal.connection.ExchangeFinder.findHealthyConnection(ExchangeFinder.kt:108)
        at okhttp3.internal.connection.ExchangeFinder.find(ExchangeFinder.kt:76)
        at okhttp3.internal.connection.RealCall.initExchange$okhttp(RealCall.kt:245)
        at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.kt:32)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:100)
        at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.kt:82)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:100)
        at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.kt:83)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:100)
        at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.kt:74)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:100)
        at okhttp3.logging.HttpLoggingInterceptor.intercept(HttpLoggingInterceptor.kt:219)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:100)

How should we create a CertificatePinner with certificateCleaner? Or should the code use ?. operator at the above line rather than !!

[yschimke]

Do you have a test case? It should be set whenever the OkHttpClient is constructed.

Do you have any code that is copying an existing OkHttpClient with some changes e.g. client.newBuilder()...build()

[rahul-narkhede]

private static OkHttpClient.Builder addSocketFactory(final OkHttpClient.Builder clientBuilder) {

        try {
            final ConnectionSpec spec;
            if (ExampleApplication.getBaseUrl().startsWith("http:")) {
                spec = new ConnectionSpec.Builder(ConnectionSpec.CLEARTEXT).build();
            } else {
                spec = new ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS).tlsVersions(TlsVersion.TLS_1_2).build();
            }

            final SSLContext sslContext = SSLContext.getInstance("TLSv1.2");
            sslContext.init(null, null, null);

            clientBuilder.connectionSpecs(Collections.singletonList(spec));

            final SSLSocketFactory socketFactory = new TLSSocketFactoryCompat(sslContext.getSocketFactory());
            clientBuilder.sslSocketFactory(socketFactory);
        } catch (final Exception e) {
            Log.e(TAG, "Problem creating TLS factory: " + e);
        }

        return clientBuilder;
    }

I am using deprecated sslSocketFactory method. But I believe that shouldn't have been a problem. It should have created CertificateChainCleaner object. This is the method we use to add SSL config.

[rahul-narkhede]

When I use clientBuilder.sslSocketFactory(socketFactory, trustManager) I get the following error

03-24 17:40:51.309 3888-3888/com.example.application E/RestClient: Error encountered while fetching the response.
    javax.net.ssl.SSLHandshakeException: SSL handshake aborted: ssl=0xaf5a1400: I/O error during system call, Connection reset by peer
        at com.android.org.conscrypt.NativeCrypto.SSL_do_handshake(Native Method)
        at com.android.org.conscrypt.OpenSSLSocketImpl.startHandshake(OpenSSLSocketImpl.java:302)
        at okhttp3.internal.connection.RealConnection.connectTls(RealConnection.kt:367)
        at okhttp3.internal.connection.RealConnection.establishProtocol(RealConnection.kt:325)
        at okhttp3.internal.connection.RealConnection.connect(RealConnection.kt:197)
        at okhttp3.internal.connection.ExchangeFinder.findConnection(ExchangeFinder.kt:235)
        at okhttp3.internal.connection.ExchangeFinder.findHealthyConnection(ExchangeFinder.kt:108)
        at okhttp3.internal.connection.ExchangeFinder.find(ExchangeFinder.kt:76)
        at okhttp3.internal.connection.RealCall.initExchange$okhttp(RealCall.kt:245)
        at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.kt:32)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:100)
        at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.kt:82)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:100)
        at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.kt:83)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:100)
        at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.kt:74)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:100)
        at okhttp3.logging.HttpLoggingInterceptor.intercept(HttpLoggingInterceptor.kt:219)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:100)

[yschimke]

If you can make a single main method that reproduces it with a test url it will be a lot easier to debug. We should be cutting 4.5 soon, so the easier you can make it to repro, the quicker you can get a fix.

[rahul-narkhede]

After doing some adjustment, I found this statement hidden in logs

Problem creating TLS factory: java.lang.IllegalStateException: Unable to extract the trust manager on AndroidPlatform, sslSocketFactory is class com.example.application.api.TLSSocketFactoryCompat

Where TLSSocketFactoryCompat is same as #2372 (comment)

Using the SocketFactory method with TrustManager as parameter fixes the issue.

[rahul-narkhede]

1 last question though. What is the minimum supported API for android for 4.x?

[yschimke]

Thanks. Can improve this behaviour so I'll keep this open.

[yschimke]

@rahul-narkhede https://square.github.io/okhttp/security/

Android 5.0+ (API level 21+) and on Java 8+.

[yschimke]

After doing some adjustment, I found this statement hidden in logs

Problem creating TLS factory: java.lang.IllegalStateException: Unable to extract the trust manager on AndroidPlatform, sslSocketFactory is class com.example.application.api.TLSSocketFactoryCompat

Where TLSSocketFactoryCompat is same as #2372 (comment)

Using the SocketFactory method with TrustManager as parameter fixes the issue.

You shouldn't need any special code for that yourself. It should be all automatically set on any supported platform.

https://github.com/square/okhttp/blob/okhttp_3.12.x/okhttp/src/main/java/okhttp3/internal/platform/AndroidPlatform.java#L442-L465

[yschimke]

The logic around initialising OkHttpClient is quite awkward. I think we probably need to centralise some of the logic around building the sslSocketFactory, x509TrustManager, certificateChainCleaner, routeDatabase into the OkHttpClient constructor. And make setting a cleartext only connectionSpecs to clear out all other fields.

[yschimke]

Related #5930 shows that we call certificatePinner.withCertificateChainCleaner with a null certificateChainCleaner