Okhttp3 cleanups, docs, and fix timeouts for default client

3 files changed, 36 insertions, 24 deletions

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 cbbbf6e71..f3606aa2f 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/OkHttpClientFactory.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/OkHttpClientFactory.java
@@ -1,57 +1,74 @@
-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;
-import okhttp3.OkHttpClient;
-import org.sufficientlysecure.keychain.Constants;
+package org.sufficientlysecure.keychain.util;
 
 import java.io.IOException;
 import java.net.Proxy;
 import java.net.URL;
 import java.util.concurrent.TimeUnit;
 
-/**
- * Created by Michał Kępkowski on 11/03/16.
- */
+import okhttp3.CertificatePinner;
+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)
-                    .readTimeout(45000, TimeUnit.MILLISECONDS)
+    public static OkHttpClient getSimpleClient() {
+        if (client == null) {
+            client = new OkHttpClient.Builder()
+                    .connectTimeout(5000, TimeUnit.MILLISECONDS)
+                    .readTimeout(25000, TimeUnit.MILLISECONDS)
                     .build();
         }
         return client;
     }
 
-    public static OkHttpClient getPinnedSimpleClient(CertificatePinner pinner){
+    public static OkHttpClient getPinnedSimpleClient(CertificatePinner pinner) {
         return new OkHttpClient.Builder()
-                .connectTimeout(30000, TimeUnit.MILLISECONDS)
-                .readTimeout(45000, TimeUnit.MILLISECONDS)
+                .connectTimeout(5000, 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();
     }
 
-    public static OkHttpClient getClient( Proxy proxy) throws IOException, TlsHelper.TlsHelperException {
+    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();
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 afe688bbe..9b7b31d68 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/OkHttpKeybaseClient.java
+++ b/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;
@@ -58,7 +57,7 @@ public class OkHttpKeybaseClient implements KeybaseUrlConnectionClient {
         Request request = new Request.Builder()
                 .url(url).build();
         okhttp3.Response okResponse = client.newCall(request).execute();
-        return new Response(okResponse.body().byteStream(),okResponse.code(),okResponse.message(), okResponse.headers().toMultimap());
+        return new Response(okResponse.body().byteStream(), okResponse.code(), okResponse.message(), okResponse.headers().toMultimap());
     }
 
     @Override
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 c23985ac0..77ed6fe0b 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/TlsHelper.java
+++ b/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);
         }