From c8e5395d4e3c3dcc349ebe6bb300016f44d430d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Sat, 9 Apr 2016 18:34:00 +0200 Subject: Use cert pinning only if available --- .../keychain/util/OkHttpClientFactory.java | 47 +++++++++++----------- .../keychain/util/OkHttpKeybaseClient.java | 9 +---- .../keychain/util/TlsHelper.java | 7 ++-- 3 files changed, 29 insertions(+), 34 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/OkHttpClientFactory.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/OkHttpClientFactory.java index f3606aa2f..ea2ae8368 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/OkHttpClientFactory.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/OkHttpClientFactory.java @@ -38,7 +38,7 @@ public class OkHttpClientFactory { return client; } - public static OkHttpClient getPinnedSimpleClient(CertificatePinner pinner) { + public static OkHttpClient getSimpleClientPinned(CertificatePinner pinner) { return new OkHttpClient.Builder() .connectTimeout(5000, TimeUnit.MILLISECONDS) .readTimeout(25000, TimeUnit.MILLISECONDS) @@ -46,32 +46,31 @@ public class OkHttpClientFactory { .build(); } - public static OkHttpClient getPinnedClient(URL url, Proxy proxy) throws IOException, TlsHelper.TlsHelperException { + public static OkHttpClient getClientPinnedIfAvailable(URL url, Proxy proxy) throws IOException, + TlsHelper.TlsHelperException { + OkHttpClient.Builder builder = new OkHttpClient.Builder(); - return new OkHttpClient.Builder() - // don't follow any redirects for keyservers, as discussed in the security audit - .followRedirects(false) - .followSslRedirects(false) - .proxy(proxy) - // higher timeouts for Tor - .connectTimeout(30000, TimeUnit.MILLISECONDS) - .readTimeout(45000, TimeUnit.MILLISECONDS) - // use pinned cert with SocketFactory - .sslSocketFactory(TlsHelper.getPinnedSslSocketFactory(url)) - .build(); - } + // don't follow any redirects for keyservers, as discussed in the security audit + builder.followRedirects(false) + .followSslRedirects(false); + + if (proxy != null) { + // set proxy and higher timeouts for Tor + builder.proxy(proxy); + builder.connectTimeout(30000, TimeUnit.MILLISECONDS) + .readTimeout(45000, TimeUnit.MILLISECONDS); + } else { + builder.connectTimeout(5000, TimeUnit.MILLISECONDS) + .readTimeout(25000, TimeUnit.MILLISECONDS); + } - public static OkHttpClient getClient(Proxy proxy) throws IOException, TlsHelper.TlsHelperException { + // If a pinned cert is available, use it! + // NOTE: this fails gracefully back to "no pinning" if no cert is available. + if (url != null && TlsHelper.getPinnedSslSocketFactory(url) != null) { + builder.sslSocketFactory(TlsHelper.getPinnedSslSocketFactory(url)); + } - return new OkHttpClient.Builder() - // don't follow any redirects for keyservers, as discussed in the security audit - .followRedirects(false) - .followSslRedirects(false) - .proxy(proxy) - // higher timeouts for Tor - .connectTimeout(30000, TimeUnit.MILLISECONDS) - .readTimeout(45000, TimeUnit.MILLISECONDS) - .build(); + return builder.build(); } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/OkHttpKeybaseClient.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/OkHttpKeybaseClient.java index 9b7b31d68..8d3eb6963 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/OkHttpKeybaseClient.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/OkHttpKeybaseClient.java @@ -21,16 +21,13 @@ package org.sufficientlysecure.keychain.util; import com.textuality.keybase.lib.KeybaseUrlConnectionClient; import okhttp3.OkHttpClient; -import okhttp3.OkUrlFactory; import okhttp3.Request; -import okhttp3.Response; import org.sufficientlysecure.keychain.Constants; import java.io.IOException; import java.net.Proxy; import java.net.URL; -import java.net.URLConnection; /** * Wrapper for Keybase Lib @@ -42,10 +39,8 @@ public class OkHttpKeybaseClient implements KeybaseUrlConnectionClient { OkHttpClient client = null; try { - if (isKeybase && proxy != null) { - client = OkHttpClientFactory.getPinnedClient(url, proxy); - } else if (proxy != null) { - client = OkHttpClientFactory.getClient(proxy); + if (proxy != null) { + client = OkHttpClientFactory.getClientPinnedIfAvailable(url, proxy); } else { client = OkHttpClientFactory.getSimpleClient(); } 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 77ed6fe0b..fe62eff55 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/TlsHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/TlsHelper.java @@ -86,9 +86,10 @@ public class TlsHelper { } /** - * Modifies the builder to accept only requests with a given certificate. 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. + * Modifies the builder to accept only requests with a given certificate. + * 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 * @throws TlsHelperException -- cgit v1.2.3