fixed certificate pinning

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);
         }
     }