From 731ec3382ef8df566e56061c25ffc0b292521e20 Mon Sep 17 00:00:00 2001 From: Adithya Abraham Philip Date: Sun, 7 Jun 2015 21:44:15 +0530 Subject: fixed certificate pinning --- .../keychain/keyimport/HkpKeyserver.java | 9 +++-- .../keychain/util/TlsHelper.java | 46 +++++++++++++++++----- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/HkpKeyserver.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/HkpKeyserver.java index f18878bb5..da1494ce3 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/HkpKeyserver.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/HkpKeyserver.java @@ -193,7 +193,7 @@ public class HkpKeyserver extends Keyserver { private HttpURLConnection openConnectioan(URL url) throws IOException { HttpURLConnection conn = null; try { - conn = (HttpURLConnection) TlsHelper.openConnection(url); + conn = (HttpURLConnection) TlsHelper.opeanConnection(url); } catch (TlsHelper.TlsHelperException e) { Log.w(Constants.TAG, e); } @@ -212,7 +212,7 @@ public class HkpKeyserver extends Keyserver { * @param proxy * @return */ - private OkHttpClient getClient(URL url, Proxy proxy) { + public static OkHttpClient getClient(URL url, Proxy proxy) throws IOException { OkHttpClient client = new OkHttpClient(); try { @@ -222,7 +222,7 @@ public class HkpKeyserver extends Keyserver { } client.setProxy(proxy); - // TODO: if proxy !=null increase timeout? + // TODO: PHILIP if proxy !=null increase timeout? client.setConnectTimeout(5000, TimeUnit.MILLISECONDS); client.setReadTimeout(25000, TimeUnit.MILLISECONDS); @@ -244,6 +244,7 @@ public class HkpKeyserver extends Keyserver { throw new HttpError(response.code(), responseBody); } } catch (IOException e) { + e.printStackTrace(); throw new QueryFailedException("Keyserver '" + mHost + "' is unavailable. Check your Internet connection!"); } } @@ -421,7 +422,7 @@ public class HkpKeyserver extends Keyserver { * Tries to find a server responsible for a given domain * * @return A responsible Keyserver or null if not found. - * TODO: Add proxy functionality + * TODO: PHILIP Add proxy functionality */ public static HkpKeyserver resolve(String domain) { try { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/TlsHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/TlsHelper.java index b116524ef..58c250cab 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/TlsHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/TlsHelper.java @@ -19,7 +19,6 @@ package org.sufficientlysecure.keychain.util; import android.content.res.AssetManager; -import com.squareup.okhttp.CertificatePinner; import com.squareup.okhttp.OkHttpClient; import org.sufficientlysecure.keychain.Constants; @@ -76,7 +75,7 @@ public class TlsHelper { } } - public static URLConnection openConnection(URL url) throws IOException, TlsHelperException { + public static URLConnection opeanConnection(URL url) throws IOException, TlsHelperException { if (url.getProtocol().equals("https")) { for (String domain : sStaticCA.keySet()) { if (url.getHost().endsWith(domain)) { @@ -87,27 +86,54 @@ public class TlsHelper { return url.openConnection(); } - public static void pinCertificateIfNecessary(OkHttpClient client, URL url) throws TlsHelperException { + public static void pinCertificateIfNecessary(OkHttpClient client, URL url) throws TlsHelperException, IOException { if (url.getProtocol().equals("https")) { for (String domain : sStaticCA.keySet()) { if (url.getHost().endsWith(domain)) { - pinCertificate(sStaticCA.get(domain), domain, client); + pinCertificate(sStaticCA.get(domain), client); } } } } - public static void pinCertificate(byte[] certificate, String hostName, OkHttpClient client) - throws TlsHelperException { + /** + * Modifies the client to accept only requests with a given certificate. Applies to all URLs requested by the + * client. + * Therefore a client that is pinned this way should be used to only make requests to URLs with passed certificate. + * TODO: Refactor - More like SSH StrictHostKeyChecking than pinning? + * + * @param certificate certificate to pin + * @param client OkHttpClient to enforce pinning on + * @throws TlsHelperException + * @throws IOException + */ + private static void pinCertificate(byte[] certificate, OkHttpClient client) + throws TlsHelperException, IOException { + // We don't use OkHttp's CertificatePinner since it depends on a TrustManager to verify it too. 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"); Certificate ca = cf.generateCertificate(new ByteArrayInputStream(certificate)); - String pin = CertificatePinner.pin(ca); - Log.e("PHILIP", "" + ca.getPublicKey() + ":" + pin); - client.setCertificatePinner(new CertificatePinner.Builder().add(hostName, pin).build()); - } catch (CertificateException e) { + // Create a KeyStore containing our trusted CAs + String keyStoreType = KeyStore.getDefaultType(); + KeyStore keyStore = KeyStore.getInstance(keyStoreType); + keyStore.load(null, null); + keyStore.setCertificateEntry("ca", ca); + + // Create a TrustManager that trusts the CAs in our KeyStore + String tmfAlgorithm = TrustManagerFactory.getDefaultAlgorithm(); + TrustManagerFactory tmf = TrustManagerFactory.getInstance(tmfAlgorithm); + tmf.init(keyStore); + + // Create an SSLContext that uses our TrustManager + SSLContext context = SSLContext.getInstance("TLS"); + context.init(null, tmf.getTrustManagers(), null); + + client.setSslSocketFactory(context.getSocketFactory()); + } catch (CertificateException | KeyStoreException | KeyManagementException | NoSuchAlgorithmException e) { throw new TlsHelperException(e); } } -- cgit v1.2.3