Okhttp3 cleanups, docs, and fix timeouts for default client

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/OkHttpClientFactory.java

@@ -1,25 +1,38 @@
-package org.sufficientlysecure.keychain.util;
+/*
+ * Copyright (C) 2016 Michał Kępkowski
+ *
+ * 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
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
 
-import okhttp3.CertificatePinner;
+package org.sufficientlysecure.keychain.util;
-import okhttp3.OkHttpClient;
-import org.sufficientlysecure.keychain.Constants;
 
 import java.io.IOException;
 import java.net.Proxy;
 import java.net.URL;
 import java.util.concurrent.TimeUnit;
 
-/**
+import okhttp3.CertificatePinner;
- * Created by Michał Kępkowski on 11/03/16.
+import okhttp3.OkHttpClient;
- */
+
 public class OkHttpClientFactory {
     private static OkHttpClient client;
 
     public static OkHttpClient getSimpleClient() {
         if (client == null) {
             client = new OkHttpClient.Builder()
-                    .connectTimeout(30000, TimeUnit.MILLISECONDS)
+                    .connectTimeout(5000, TimeUnit.MILLISECONDS)
-                    .readTimeout(45000, TimeUnit.MILLISECONDS)
+                    .readTimeout(25000, TimeUnit.MILLISECONDS)
                     .build();
         }
         return client;
@@ -27,21 +40,23 @@ public class OkHttpClientFactory {
 
     public static OkHttpClient getPinnedSimpleClient(CertificatePinner pinner) {
         return new OkHttpClient.Builder()
-                .connectTimeout(30000, TimeUnit.MILLISECONDS)
+                .connectTimeout(5000, TimeUnit.MILLISECONDS)
-                .readTimeout(45000, TimeUnit.MILLISECONDS)
+                .readTimeout(25000, TimeUnit.MILLISECONDS)
                 .certificatePinner(pinner)
                 .build();
     }
 
-
     public static OkHttpClient getPinnedClient(URL url, Proxy proxy) throws IOException, TlsHelper.TlsHelperException {
 
         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();
     }
@@ -49,9 +64,11 @@ public class OkHttpClientFactory {
     public static OkHttpClient getClient(Proxy proxy) throws IOException, TlsHelper.TlsHelperException {
 
         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();

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/OkHttpKeybaseClient.java

@@ -24,6 +24,7 @@ import okhttp3.OkHttpClient;
 import okhttp3.OkUrlFactory;
 import okhttp3.Request;
 import okhttp3.Response;
+
 import org.sufficientlysecure.keychain.Constants;
 
 import java.io.IOException;
@@ -36,8 +37,6 @@ import java.net.URLConnection;
  */
 public class OkHttpKeybaseClient implements KeybaseUrlConnectionClient {
 
-
-
     @Override
     public Response getUrlResponse(URL url, Proxy proxy, boolean isKeybase) throws IOException {
         OkHttpClient client = null;

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/TlsHelper.java

@@ -19,8 +19,6 @@ package org.sufficientlysecure.keychain.util;
 
 import android.content.res.AssetManager;
 
-
-import okhttp3.OkHttpClient;
 import org.sufficientlysecure.keychain.Constants;
 
 import java.io.ByteArrayInputStream;
@@ -93,7 +91,6 @@ public class TlsHelper {
      * 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
-     * @param builder      OkHttpBuilder to enforce pinning on
      * @throws TlsHelperException
      * @throws IOException
      */
@@ -125,7 +122,6 @@ public class TlsHelper {
             context.init(null, tmf.getTrustManagers(), null);
 
             return context.getSocketFactory();
-            //builder.sslSocketFactory(context.getSocketFactory());
         } catch (CertificateException | KeyStoreException | KeyManagementException | NoSuchAlgorithmException e) {
             throw new TlsHelperException(e);
         }