Tom
Tom

Reputation: 23

okhttp doesn't validate pins correctly

Using com.squareup.retrofit2:retrofit:2.0.1 with com.squareup.okhttp3:okhttp:3.2.0 on an AVD with Android 6.0. I'm trying to implement public key pinning using a self signed certificate that is signed by a Root CA. That Root CA is in the system CA trust store.

Using the example provided by okhttp wiki with some small changes:

OkHttpClient client = new OkHttpClient.Builder().certificatePinner(
                new CertificatePinner.Builder()
                        .add(pinningUrl, "sha256/invalidPIN")
                        .build()).build();
Request request = new Request.Builder()
                .url(pinningUrl)
                .build();
Response response = client.newCall(request).execute();

if (!response.isSuccessful()) throw new IOException("Unexpected code " + response);
for (Certificate certificate : response.handshake().peerCertificates()) {
  System.out.println(CertificatePinner.pin(certificate));
}

What happens is that response.isSuccessful returns true, no exception is thrown, although the pin isn't correct. The only thing that is done correctly is the validation of the certificate with the Root CAs in systems CA trust store.

What I've found to be working, is adding this line before the for loop. But that isn't the right approach because the request is already sent, the pinning should work before TLS negotiation is finished. Also this line isn't mentioned in any sample code I've found.

client.certificatePinner().check(pinningUrl, response.handshake().peerCertificates());

throws

javax.net.ssl.SSLPeerUnverifiedException: Certificate pinning failure!

Is there a bug in the sample code provided by okhttp or am I doing something wrong?

Upvotes: 2

Views: 1264

Answers (1)

Jesse Wilson
Jesse Wilson

Reputation: 40593

You’re configuring it incorrectly. Replace pinningUrl with the hostname of the pinning URL. For example, you want example.com instead of http://example.com/. If you’d like to send a PR to make hostname validation more strict, it would be quite welcome.

Upvotes: 3

Related Questions