Okhttp: Customize KeyStore for HandshakeCertificates

[MGaetan89]

Hello,

I am trying to establish a connection with my server by using two way SSL.
For that, I'm using HandshakeCertificates as follow:

val deviceCertificate = keyStore.getCertificate(CERTIFICATE_ALIAS) as X509Certificate
val publicKey = deviceCertificate.publicKey
val privateKey = getKeyStore().getKey(KEY_ALIAS, null) as PrivateKey
val keyPair = KeyPair(publicKey, privateKey)
val heldCertificate = HeldCertificate(keyPair, deviceCertificate)

handshakeBuilder.heldCertificate(heldCertificate)

Which I then set on my OkHttpClient with:

val handshake = handshakeBuilder.build()

clientBuilder.sslSocketFactory(handshake.sslSocketFactory(), handshake.trustManager)

But when I run my app, it crashes with:

java.security.KeyStoreException: java.lang.NullPointerException: Attempt to invoke virtual method 'int java.lang.String.length()' on a null object reference
        at com.android.org.bouncycastle.jcajce.provider.keystore.bc.BcKeyStoreSpi.engineSetKeyEntry(BcKeyStoreSpi.java:689)
        at java.security.KeyStore.setKeyEntry(KeyStore.java:1179)
        at okhttp3.tls.internal.TlsUtil.newKeyManager(TlsUtil.kt:95)
        at okhttp3.tls.HandshakeCertificates$Builder.build(HandshakeCertificates.kt:175)
        at com.myapp.common.security.data.CommonSecurityProvider$Companion.getSslSocketFactoryConfig(CommonSecurityProvider.kt:112)
        at com.myapp.common.network.di.CommonNetworkModuleKt$commonNetworkModule$1$7.invoke(CommonNetworkModule.kt:64)
        at com.myapp.common.network.di.CommonNetworkModuleKt$commonNetworkModule$1$7.invoke(Unknown Source:4)
        at org.koin.core.instance.InstanceFactory.create(InstanceFactory.kt:50)
            ... 139 more

From my investigation, it could be because OkHttp is using its own KeyStore when building the HandshakeCertificates, while my app is using AndroidKeyStore. Is there a way to customize this? Or am I doing something wrong?

https://github.com/square/okhttp/blob/4c595553fbcbfc316d24a74197aa753dbc05da9f/okhttp-tls/src/main/kotlin/okhttp3/tls/HandshakeCertificates.kt#L175-L176

[yschimke]

Dupe of https://github.com/square/okhttp/issues/5684.
https://stackoverflow.com/questions/59422820/android-okhttp3-error-with-client-certificate

Bouncy Castle is throwing away the root cause exception

        try
        {
            table.put(alias, new StoreEntry(alias, key, password, chain));
        }
        catch (Exception e)
        {
            throw new KeyStoreException(e.toString());
        }

[yschimke]

I'd like to understand why it's happening, how we are triggering. But ultimately I suspect it's a bouncycastle issue on Android. Worth raising with them.

Can you easily test with BouncyCastle betas?

https://downloads.bouncycastle.org/betas/bcprov-jdk15on-166b10.jar
https://downloads.bouncycastle.org/betas/bcpkix-jdk15on-166b10.jar
https://downloads.bouncycastle.org/betas/bctls-jdk15on-166b10.jar

[yschimke]

Poor mans attempt at a reproduction https://github.com/square/okhttp/pull/6118/files

[yschimke]

[edit: not sure if it's an AOSP issue instead of bc-java]

Closing out here and will track on https://github.com/bcgit/bc-java/issues/723

I'll reopen this if BC closes out their issue, or we can repro here. But currently not much we can do with this report, it's failing outside of our codebase.

Separately please re-open if you can provide a minimal reproduction. And thanks for reporting, I'd love to get to the bottom of this.

[MGaetan89]

Thanks for your time on this issue.

From our investigation, the NullPointerException is coming from here: https://cs.android.com/android/platform/superproject/+/master:external/bouncycastle/repackaged/bcprov/src/main/java/com/android/org/bouncycastle/jcajce/provider/keystore/bc/BcKeyStoreSpi.java;l=413?q=BcKeyStoreSpi&ss=android%2Fplatform%2Fsuperproject

I'll try to use the beta versions of Bouncy Castle, or provide a minimal failing project to reproduce the issue.

For my original question, is there a way (or is it planned), to be able to set the KeyStore type in HandshakeCertificates, instead of passing null to newKeyManager and newTrustManager?

[yschimke]

@MGaetan89 not actively planned as far as I know, but with a clean repro and ideally a PR, we can work out the right API for this.

There isn't much code in okhttp-tls, and it's on the periphery of the library, so I assume you can make a temporary copy and get it working.

[yschimke]

@MGaetan89 if it's happening in the AOSP copy of bc-java, I guess you can't use a beta. That is probably a dead end, sorry.

[slovi]

@MGaetan89 Hi Gaetan, I have the same problem. Did you find any solution?

I investigated that and the problem most likely seems to be in this class:

https://android.googlesource.com/platform/external/conscrypt/+/dac92c6/src/main/java/org/conscrypt/OpenSSLRSAPrivateKey.java#195

getFormat() method returns null if key is "engineBased". Does anybody know what does it mean and how to instantiate PrivateKey which is guaranteed to be exportable? (otherwise I cannot store it as a keystore entry).

[yschimke]

Is this happening with 4.8.1 still?

[MGaetan89]

I've just check again with OkHttp 4.8.1, and I still have the same issue.

[yschimke]

@MGaetan89 I think I finally understand this enough to attempt a proper fix

This is the terrible workaround I've tested with https://gist.github.com/yschimke/4a1eda31c584a64549c790d887f275e0