Use cert pinning only if available

This commit is contained in:
Dominik Schürmann
2016-04-09 18:34:00 +02:00
parent 2d762e55da
commit c8e5395d4e
7 changed files with 36 additions and 41 deletions

View File

@@ -106,10 +106,10 @@ public class FacebookKeyserver extends Keyserver {
String request = String.format(FB_KEY_URL_FORMAT, fbUsername);
Log.d(Constants.TAG, "fetching from Facebook with: " + request + " proxy: " + mProxy);
OkHttpClient client = OkHttpClientFactory.getClient(mProxy);
URL url = new URL(request);
OkHttpClient client = OkHttpClientFactory.getClientPinnedIfAvailable(url, mProxy);
Response response = client.newCall(new Request.Builder().url(url).build()).execute();
// contains body both in case of success or failure

View File

@@ -205,7 +205,7 @@ public class HkpKeyserver extends Keyserver {
try {
URL url = new URL(getUrlPrefix() + mHost + ":" + mPort + request);
Log.d(Constants.TAG, "hkp keyserver query: " + url + " Proxy: " + proxy);
OkHttpClient client = OkHttpClientFactory.getPinnedClient(url, proxy);
OkHttpClient client = OkHttpClientFactory.getClientPinnedIfAvailable(url, proxy);
Response response = client.newCall(new Request.Builder().url(url).build()).execute();
String responseBody = response.body().string(); // contains body both in case of success or failure
@@ -396,7 +396,7 @@ public class HkpKeyserver extends Keyserver {
.post(body)
.build();
Response response = OkHttpClientFactory.getPinnedClient(url, mProxy).newCall(request).execute();
Response response = OkHttpClientFactory.getClientPinnedIfAvailable(url, mProxy).newCall(request).execute();
Log.d(Constants.TAG, "response code: " + response.code());
Log.d(Constants.TAG, "answer: " + response.body().string());

View File

@@ -244,7 +244,7 @@ public abstract class LinkedTokenResource extends LinkedResource {
Log.d("Connection to: " + request.url().url().getHost(), "");
OkHttpClient client;
if (pins != null) {
client = OkHttpClientFactory.getPinnedSimpleClient(getCertificatePinner(request.url().url().getHost(), pins));
client = OkHttpClientFactory.getSimpleClientPinned(getCertificatePinner(request.url().url().getHost(), pins));
} else {
client = OkHttpClientFactory.getSimpleClient();
}

View File

@@ -353,8 +353,6 @@ public class AddEditKeyserverDialogFragment extends DialogFragment implements On
Log.d("Converted URL", newKeyserver.toString());
OkHttpClient client = OkHttpClientFactory.getPinnedClient(newKeyserver.toURL(), proxy);
if (onlyTrustedKeyserver
&& TlsHelper.getPinnedSslSocketFactory(newKeyserver.toURL()) == null) {
Log.w(Constants.TAG, "No pinned certificate for this host in OpenKeychain's assets.");
@@ -362,6 +360,8 @@ public class AddEditKeyserverDialogFragment extends DialogFragment implements On
return reason;
}
OkHttpClient client = OkHttpClientFactory.getClientPinnedIfAvailable(newKeyserver.toURL(), proxy);
client.newCall(new Request.Builder().url(newKeyserver.toURL()).build()).execute();
} catch (TlsHelper.TlsHelperException e) {
reason = FailureReason.CONNECTION_FAILED;

View File

@@ -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);
public static OkHttpClient getClient(Proxy proxy) throws IOException, TlsHelper.TlsHelperException {
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);
}
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();
// 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 builder.build();
}
}

View File

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

View File

@@ -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