From fd18e0215dd5d58f9edf471d7bf7ae0daf4fc83c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Mon, 4 Dec 2017 16:52:01 +0100 Subject: [PATCH] Refactor certificate pinning for OkHttp 3.9 --- .../keychain/network/OkHttpClientFactory.java | 7 +- .../network/TlsCertificatePinning.java | 86 +++++++++++-------- .../AddEditKeyserverDialogFragment.java | 5 +- 3 files changed, 58 insertions(+), 40 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/network/OkHttpClientFactory.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/network/OkHttpClientFactory.java index a52ce061f..bf9e883c3 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/network/OkHttpClientFactory.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/network/OkHttpClientFactory.java @@ -65,8 +65,11 @@ public class OkHttpClientFactory { // If a pinned cert is available, use it! // NOTE: this fails gracefully back to "no pinning" if no cert is available. - if (url != null && TlsCertificatePinning.getPinnedSslSocketFactory(url) != null) { - builder.sslSocketFactory(TlsCertificatePinning.getPinnedSslSocketFactory(url)); + TlsCertificatePinning tlsCertificatePinning = new TlsCertificatePinning(url); + boolean isHttpsProtocol = "https".equals(url.getProtocol()); + boolean isPinAvailable = tlsCertificatePinning.isPinAvailable(); + if (isHttpsProtocol && isPinAvailable) { + tlsCertificatePinning.pinCertificate(builder); } return builder.build(); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/network/TlsCertificatePinning.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/network/TlsCertificatePinning.java index d1ba2fb20..a6dcbefa1 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/network/TlsCertificatePinning.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/network/TlsCertificatePinning.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2013-2015 Dominik Schürmann + * Copyright (C) 2013-2017 Dominik Schürmann * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -34,16 +34,21 @@ import java.security.NoSuchAlgorithmException; import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; +import java.util.Arrays; import java.util.HashMap; import java.util.Map; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.TrustManager; import javax.net.ssl.TrustManagerFactory; +import javax.net.ssl.X509TrustManager; + +import okhttp3.OkHttpClient; public class TlsCertificatePinning { - private static Map sPinnedCertificates = new HashMap<>(); + private static Map sCertificatePins = new HashMap<>(); /** * Add certificate from assets to pinned certificate map. @@ -61,27 +66,20 @@ public class TlsCertificatePinning { is.close(); - sPinnedCertificates.put(host, baos.toByteArray()); + sCertificatePins.put(host, baos.toByteArray()); } catch (IOException e) { Log.w(Constants.TAG, e); } } - /** - * Use pinned certificate for OkHttpClient if we have one. - * - * @return true, if certificate is available, false if not - */ - public static SSLSocketFactory getPinnedSslSocketFactory(URL url) { - if (url.getProtocol().equals("https")) { - // use certificate PIN from assets if we have one - for (String host : sPinnedCertificates.keySet()) { - if (url.getHost().endsWith(host)) { - return pinCertificate(sPinnedCertificates.get(host)); - } - } - } - return null; + private final URL url; + + public TlsCertificatePinning(URL url) { + this.url = url; + } + + public boolean isPinAvailable() { + return sCertificatePins.containsKey(url.getHost()); } /** @@ -89,39 +87,55 @@ public class TlsCertificatePinning { * Applies to all URLs requested by the builder. * Therefore a builder that is pinned this way should be used to only make requests * to URLs with passed certificate. - * - * @param certificate certificate to pin */ - private static SSLSocketFactory pinCertificate(byte[] certificate) { + void pinCertificate(OkHttpClient.Builder builder) { + Log.d(Constants.TAG, "Pinning certificate for " + url); + // We don't use OkHttp's CertificatePinner since it can not be used to pin self-signed // certificate if such certificate is not accepted by TrustManager. // (Refer to note at end of description: // http://square.github.io/okhttp/javadoc/com/squareup/okhttp/CertificatePinner.html ) // Creating our own TrustManager that trusts only our certificate eliminates the need for certificate pinning try { - // Load CA CertificateFactory cf = CertificateFactory.getInstance("X.509"); + byte[] certificate = sCertificatePins.get(url.getHost()); Certificate ca = cf.generateCertificate(new ByteArrayInputStream(certificate)); - // Create a KeyStore containing our trusted CAs - String keyStoreType = KeyStore.getDefaultType(); - KeyStore keyStore = KeyStore.getInstance(keyStoreType); - keyStore.load(null, null); - keyStore.setCertificateEntry("ca", ca); + KeyStore keyStore = createSingleCertificateKeyStore(ca); + X509TrustManager trustManager = createTrustManager(keyStore); - // Create a TrustManager that trusts the CAs in our KeyStore - String tmfAlgorithm = TrustManagerFactory.getDefaultAlgorithm(); - TrustManagerFactory tmf = TrustManagerFactory.getInstance(tmfAlgorithm); - tmf.init(keyStore); + SSLContext sslContext = SSLContext.getInstance("TLS"); + sslContext.init(null, new TrustManager[]{trustManager}, null); + SSLSocketFactory sslSocketFactory = sslContext.getSocketFactory(); - // Create an SSLContext that uses our TrustManager - SSLContext context = SSLContext.getInstance("TLS"); - context.init(null, tmf.getTrustManagers(), null); - - return context.getSocketFactory(); + builder.sslSocketFactory(sslSocketFactory, trustManager); } catch (CertificateException | KeyStoreException | KeyManagementException | NoSuchAlgorithmException | IOException e) { throw new IllegalStateException(e); } } + + private KeyStore createSingleCertificateKeyStore(Certificate ca) throws KeyStoreException, + CertificateException, NoSuchAlgorithmException, IOException { + String keyStoreType = KeyStore.getDefaultType(); + KeyStore keyStore = KeyStore.getInstance(keyStoreType); + keyStore.load(null, null); + keyStore.setCertificateEntry("ca", ca); + + return keyStore; + } + + private X509TrustManager createTrustManager(KeyStore keyStore) throws NoSuchAlgorithmException, + KeyStoreException { + TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance( + TrustManagerFactory.getDefaultAlgorithm()); + trustManagerFactory.init(keyStore); + TrustManager[] trustManagers = trustManagerFactory.getTrustManagers(); + if (trustManagers.length != 1 || !(trustManagers[0] instanceof X509TrustManager)) { + throw new IllegalStateException("Unexpected default trust managers: " + + Arrays.toString(trustManagers)); + } + + return (X509TrustManager) trustManagers[0]; + } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/AddEditKeyserverDialogFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/AddEditKeyserverDialogFragment.java index 64284a412..a103e975b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/AddEditKeyserverDialogFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/AddEditKeyserverDialogFragment.java @@ -366,8 +366,9 @@ public class AddEditKeyserverDialogFragment extends DialogFragment implements On URI keyserverUriHttp = keyserver.getUrlURI(); // check TLS pinning only for non-Tor keyservers - if (onlyTrustedKeyserver - && TlsCertificatePinning.getPinnedSslSocketFactory(keyserverUriHttp.toURL()) == null) { + TlsCertificatePinning tlsCertificatePinning = new TlsCertificatePinning(keyserverUriHttp.toURL()); + boolean isPinAvailable = tlsCertificatePinning.isPinAvailable(); + if (onlyTrustedKeyserver && !isPinAvailable) { Log.w(Constants.TAG, "No pinned certificate for this host in OpenKeychain's assets."); reason = VerifyReturn.NO_PINNED_CERTIFICATE; return reason;