simplify code around bundled trust manager

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/crypto/BundledTrustManager.java      | 65 
src/main/java/eu/siacs/conversations/crypto/CombiningTrustManager.java    | 23 
src/main/java/eu/siacs/conversations/crypto/TrustManagers.java            | 27 
src/main/java/eu/siacs/conversations/http/HttpConnectionManager.java      | 72 
src/main/java/eu/siacs/conversations/services/MemorizingTrustManager.java | 20 
5 files changed, 67 insertions(+), 140 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/crypto/BundledTrustManager.java 🔗

@@ -1,65 +0,0 @@
-package eu.siacs.conversations.crypto;
-
-import java.io.IOException;
-import java.io.InputStream;
-import java.security.KeyStore;
-import java.security.KeyStoreException;
-import java.security.NoSuchAlgorithmException;
-import java.security.cert.CertificateException;
-import java.security.cert.X509Certificate;
-
-import javax.net.ssl.X509TrustManager;
-
-public class BundledTrustManager implements X509TrustManager {
-
-    private final X509TrustManager delegate;
-
-    private BundledTrustManager(final KeyStore keyStore)
-            throws NoSuchAlgorithmException, KeyStoreException {
-        this.delegate = TrustManagers.createTrustManager(keyStore);
-    }
-
-    public static Builder builder() throws KeyStoreException {
-        return new Builder();
-    }
-
-    @Override
-    public void checkClientTrusted(final X509Certificate[] chain, final String authType)
-            throws CertificateException {
-        this.delegate.checkClientTrusted(chain, authType);
-    }
-
-    @Override
-    public void checkServerTrusted(final X509Certificate[] chain, final String authType)
-            throws CertificateException {
-        this.delegate.checkServerTrusted(chain, authType);
-    }
-
-    @Override
-    public X509Certificate[] getAcceptedIssuers() {
-        return this.delegate.getAcceptedIssuers();
-    }
-
-    public static class Builder {
-
-        private KeyStore keyStore;
-
-        private Builder() {}
-
-        public Builder loadKeyStore(final InputStream inputStream, final String password)
-                throws CertificateException, IOException, NoSuchAlgorithmException,
-                        KeyStoreException {
-            if (this.keyStore != null) {
-                throw new IllegalStateException("KeyStore has already been loaded");
-            }
-            final KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
-            keyStore.load(inputStream, password.toCharArray());
-            this.keyStore = keyStore;
-            return this;
-        }
-
-        public BundledTrustManager build() throws NoSuchAlgorithmException, KeyStoreException {
-            return new BundledTrustManager(keyStore);
-        }
-    }
-}

src/main/java/eu/siacs/conversations/crypto/CombiningTrustManager.java 🔗

@@ -1,9 +1,9 @@
 package eu.siacs.conversations.crypto;
 
+import android.annotation.SuppressLint;
 import android.util.Log;
-
 import com.google.common.collect.ImmutableList;
-
+import eu.siacs.conversations.Config;
 import java.security.KeyStoreException;
 import java.security.NoSuchAlgorithmException;
 import java.security.cert.CertificateException;
@@ -11,12 +11,10 @@ import java.security.cert.X509Certificate;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
-
 import javax.net.ssl.X509TrustManager;
 
-import eu.siacs.conversations.Config;
-
-public class CombiningTrustManager implements X509TrustManager {
+@SuppressLint("CustomX509TrustManager")
+public final class CombiningTrustManager implements X509TrustManager {
 
     private final List<X509TrustManager> trustManagers;
 
@@ -32,6 +30,7 @@ public class CombiningTrustManager implements X509TrustManager {
             final X509TrustManager trustManager = iterator.next();
             try {
                 trustManager.checkClientTrusted(chain, authType);
+                return;
             } catch (final CertificateException certificateException) {
                 if (iterator.hasNext()) {
                     continue;
@@ -39,6 +38,7 @@ public class CombiningTrustManager implements X509TrustManager {
                 throw certificateException;
             }
         }
+        throw new CertificateException("No trust managers configured");
     }
 
     @Override
@@ -50,29 +50,20 @@ public class CombiningTrustManager implements X509TrustManager {
                         + " is configured with "
                         + this.trustManagers.size()
                         + " TrustManagers");
-        int i = 0;
         for (final Iterator<X509TrustManager> iterator = this.trustManagers.iterator();
                 iterator.hasNext(); ) {
             final X509TrustManager trustManager = iterator.next();
             try {
                 trustManager.checkServerTrusted(chain, authType);
-                Log.d(
-                        Config.LOGTAG,
-                        "certificate check passed on " + trustManager.getClass().getName()+". chain length was "+chain.length);
                 return;
             } catch (final CertificateException certificateException) {
-                Log.d(
-                        Config.LOGTAG,
-                        "failed to verify in [" + i + "]/" + trustManager.getClass().getName(),
-                        certificateException);
                 if (iterator.hasNext()) {
                     continue;
                 }
                 throw certificateException;
-            } finally {
-                ++i;
             }
         }
+        throw new CertificateException("No trust managers configured");
     }
 
     @Override

src/main/java/eu/siacs/conversations/crypto/TrustManagers.java 🔗

@@ -1,25 +1,23 @@
 package eu.siacs.conversations.crypto;
 
 import android.content.Context;
-
 import androidx.annotation.Nullable;
-
 import com.google.common.collect.Iterables;
-
+import eu.siacs.conversations.R;
 import java.io.IOException;
+import java.io.InputStream;
 import java.security.KeyStore;
 import java.security.KeyStoreException;
 import java.security.NoSuchAlgorithmException;
 import java.security.cert.CertificateException;
 import java.util.Arrays;
-
 import javax.net.ssl.TrustManagerFactory;
 import javax.net.ssl.X509TrustManager;
 
-import eu.siacs.conversations.R;
-
 public final class TrustManagers {
 
+    private static final char[] BUNDLED_KEYSTORE_PASSWORD = "letsencrypt".toCharArray();
+
     private TrustManagers() {
         throw new IllegalStateException("Do not instantiate me");
     }
@@ -40,16 +38,17 @@ public final class TrustManagers {
         return createTrustManager(null);
     }
 
-    public static X509TrustManager defaultWithBundledLetsEncrypt(final Context context)
+    public static X509TrustManager createDefaultWithBundledLetsEncrypt(final Context context)
             throws NoSuchAlgorithmException, KeyStoreException, CertificateException, IOException {
-        final BundledTrustManager bundleTrustManager =
-                BundledTrustManager.builder()
-                        .loadKeyStore(
-                                context.getResources().openRawResource(R.raw.letsencrypt),
-                                "letsencrypt")
-                        .build();
+        final var bundleTrustManager =
+                createWithKeyStore(context.getResources().openRawResource(R.raw.letsencrypt));
         return CombiningTrustManager.combineWithDefault(bundleTrustManager);
     }
 
-
+    private static X509TrustManager createWithKeyStore(final InputStream inputStream)
+            throws CertificateException, IOException, NoSuchAlgorithmException, KeyStoreException {
+        final KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
+        keyStore.load(inputStream, BUNDLED_KEYSTORE_PASSWORD);
+        return TrustManagers.createTrustManager(keyStore);
+    }
 }

src/main/java/eu/siacs/conversations/http/HttpConnectionManager.java 🔗

@@ -5,7 +5,6 @@ import static eu.siacs.conversations.utils.Random.SECURE_RANDOM;
 import android.content.Context;
 import android.os.Build;
 import android.util.Log;
-
 import eu.siacs.conversations.BuildConfig;
 import eu.siacs.conversations.Config;
 import eu.siacs.conversations.crypto.TrustManagers;
@@ -14,14 +13,6 @@ import eu.siacs.conversations.entities.Message;
 import eu.siacs.conversations.services.AbstractConnectionManager;
 import eu.siacs.conversations.services.XmppConnectionService;
 import eu.siacs.conversations.utils.TLSSocketFactory;
-
-import okhttp3.HttpUrl;
-import okhttp3.OkHttpClient;
-import okhttp3.Request;
-import okhttp3.ResponseBody;
-
-import org.apache.http.conn.ssl.StrictHostnameVerifier;
-
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.InetAddress;
@@ -37,9 +28,13 @@ import java.util.List;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
-
 import javax.net.ssl.SSLSocketFactory;
 import javax.net.ssl.X509TrustManager;
+import okhttp3.HttpUrl;
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.ResponseBody;
+import org.apache.http.conn.ssl.StrictHostnameVerifier;
 
 public class HttpConnectionManager extends AbstractConnectionManager {
 
@@ -51,18 +46,20 @@ public class HttpConnectionManager extends AbstractConnectionManager {
     private static final OkHttpClient OK_HTTP_CLIENT;
 
     static {
-        OK_HTTP_CLIENT = new OkHttpClient.Builder()
-                .addInterceptor(chain -> {
-                    final Request original = chain.request();
-                    final Request modified = original.newBuilder()
-                            .header("User-Agent", getUserAgent())
-                            .build();
-                    return chain.proceed(modified);
-                })
-                .build();
+        OK_HTTP_CLIENT =
+                new OkHttpClient.Builder()
+                        .addInterceptor(
+                                chain -> {
+                                    final Request original = chain.request();
+                                    final Request modified =
+                                            original.newBuilder()
+                                                    .header("User-Agent", getUserAgent())
+                                                    .build();
+                                    return chain.proceed(modified);
+                                })
+                        .build();
     }
 
-
     public static String getUserAgent() {
         return String.format("%s/%s", BuildConfig.APP_NAME, BuildConfig.VERSION_NAME);
     }
@@ -74,7 +71,7 @@ public class HttpConnectionManager extends AbstractConnectionManager {
     public static Proxy getProxy() {
         final InetAddress localhost;
         try {
-            localhost = InetAddress.getByAddress(new byte[]{127, 0, 0, 1});
+            localhost = InetAddress.getByAddress(new byte[] {127, 0, 0, 1});
         } catch (final UnknownHostException e) {
             throw new IllegalStateException(e);
         }
@@ -93,7 +90,10 @@ public class HttpConnectionManager extends AbstractConnectionManager {
         synchronized (this.downloadConnections) {
             for (HttpDownloadConnection connection : this.downloadConnections) {
                 if (connection.getMessage() == message) {
-                    Log.d(Config.LOGTAG, message.getConversation().getAccount().getJid().asBareJid() + ": download already in progress");
+                    Log.d(
+                            Config.LOGTAG,
+                            message.getConversation().getAccount().getJid().asBareJid()
+                                    + ": download already in progress");
                     return;
                 }
             }
@@ -107,11 +107,18 @@ public class HttpConnectionManager extends AbstractConnectionManager {
         synchronized (this.uploadConnections) {
             for (HttpUploadConnection connection : this.uploadConnections) {
                 if (connection.getMessage() == message) {
-                    Log.d(Config.LOGTAG, message.getConversation().getAccount().getJid().asBareJid() + ": upload already in progress");
+                    Log.d(
+                            Config.LOGTAG,
+                            message.getConversation().getAccount().getJid().asBareJid()
+                                    + ": upload already in progress");
                     return;
                 }
             }
-            HttpUploadConnection connection = new HttpUploadConnection(message, Method.determine(message.getConversation().getAccount()), this);
+            HttpUploadConnection connection =
+                    new HttpUploadConnection(
+                            message,
+                            Method.determine(message.getConversation().getAccount()),
+                            this);
             connection.init(delay);
             this.uploadConnections.add(connection);
         }
@@ -133,7 +140,8 @@ public class HttpConnectionManager extends AbstractConnectionManager {
         return buildHttpClient(url, account, 30, interactive);
     }
 
-    OkHttpClient buildHttpClient(final HttpUrl url, final Account account, int readTimeout, boolean interactive) {
+    OkHttpClient buildHttpClient(
+            final HttpUrl url, final Account account, int readTimeout, boolean interactive) {
         final String slotHostname = url.host();
         final boolean onionSlot = slotHostname.endsWith(".onion");
         final OkHttpClient.Builder builder = OK_HTTP_CLIENT.newBuilder();
@@ -154,7 +162,8 @@ public class HttpConnectionManager extends AbstractConnectionManager {
             trustManager = mXmppConnectionService.getMemorizingTrustManager().getNonInteractive();
         }
         try {
-            final SSLSocketFactory sf = new TLSSocketFactory(new X509TrustManager[]{trustManager}, SECURE_RANDOM);
+            final SSLSocketFactory sf =
+                    new TLSSocketFactory(new X509TrustManager[] {trustManager}, SECURE_RANDOM);
             builder.sslSocketFactory(sf, trustManager);
             builder.hostnameVerifier(new StrictHostnameVerifier());
         } catch (final KeyManagementException | NoSuchAlgorithmException ignored) {
@@ -179,13 +188,12 @@ public class HttpConnectionManager extends AbstractConnectionManager {
         return body.byteStream();
     }
 
-
     public static OkHttpClient okHttpClient(final Context context) {
         final OkHttpClient.Builder builder = HttpConnectionManager.OK_HTTP_CLIENT.newBuilder();
         try {
             final X509TrustManager trustManager;
             if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.N) {
-                trustManager = TrustManagers.defaultWithBundledLetsEncrypt(context);
+                trustManager = TrustManagers.createDefaultWithBundledLetsEncrypt(context);
             } else {
                 trustManager = TrustManagers.createDefaultTrustManager();
             }
@@ -193,10 +201,10 @@ public class HttpConnectionManager extends AbstractConnectionManager {
                     new TLSSocketFactory(new X509TrustManager[] {trustManager}, SECURE_RANDOM);
             builder.sslSocketFactory(socketFactory, trustManager);
         } catch (final IOException
-                       | KeyManagementException
-                       | NoSuchAlgorithmException
-                       | KeyStoreException
-                       | CertificateException e) {
+                | KeyManagementException
+                | NoSuchAlgorithmException
+                | KeyStoreException
+                | CertificateException e) {
             Log.d(Config.LOGTAG, "not reconfiguring service to work with bundled LetsEncrypt");
             throw new RuntimeException(e);
         }

src/main/java/eu/siacs/conversations/services/MemorizingTrustManager.java 🔗

@@ -39,30 +39,20 @@ import android.preference.PreferenceManager;
 import android.util.Base64;
 import android.util.Log;
 import android.util.SparseArray;
-
 import androidx.appcompat.app.AppCompatActivity;
-
 import com.google.common.base.Charsets;
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.io.ByteStreams;
 import com.google.common.io.CharStreams;
-
 import eu.siacs.conversations.Config;
 import eu.siacs.conversations.R;
-import eu.siacs.conversations.crypto.BundledTrustManager;
-import eu.siacs.conversations.crypto.CombiningTrustManager;
 import eu.siacs.conversations.crypto.TrustManagers;
 import eu.siacs.conversations.crypto.XmppDomainVerifier;
 import eu.siacs.conversations.entities.MTMDecision;
 import eu.siacs.conversations.http.HttpConnectionManager;
 import eu.siacs.conversations.persistance.FileBackend;
 import eu.siacs.conversations.ui.MemorizingActivity;
-
-import org.json.JSONArray;
-import org.json.JSONException;
-import org.json.JSONObject;
-
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
@@ -86,10 +76,12 @@ import java.util.Locale;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Pattern;
-
 import javax.net.ssl.TrustManager;
 import javax.net.ssl.TrustManagerFactory;
 import javax.net.ssl.X509TrustManager;
+import org.json.JSONArray;
+import org.json.JSONException;
+import org.json.JSONObject;
 
 /**
  * A X509 trust manager implementation which asks the user about invalid certificates and memorizes
@@ -115,7 +107,8 @@ public class MemorizingTrustManager {
                     "\\A(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}\\z");
     private static final Pattern PATTERN_IPV6_HEX4DECCOMPRESSED =
             Pattern.compile(
-                    "\\A((?:[0-9A-Fa-f]{1,4}(?::[0-9A-Fa-f]{1,4})*)?) ::((?:[0-9A-Fa-f]{1,4}:)*)(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}\\z");
+                    "\\A((?:[0-9A-Fa-f]{1,4}(?::[0-9A-Fa-f]{1,4})*)?)"
+                        + " ::((?:[0-9A-Fa-f]{1,4}:)*)(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}\\z");
     private static final Pattern PATTERN_IPV6_6HEX4DEC =
             Pattern.compile(
                     "\\A((?:[0-9A-Fa-f]{1,4}:){6,6})(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}\\z");
@@ -176,7 +169,8 @@ public class MemorizingTrustManager {
         this.appTrustManager = getTrustManager(appKeyStore);
         try {
             if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.N) {
-                this.defaultTrustManager = TrustManagers.defaultWithBundledLetsEncrypt(context);
+                this.defaultTrustManager =
+                        TrustManagers.createDefaultWithBundledLetsEncrypt(context);
             } else {
                 this.defaultTrustManager = TrustManagers.createDefaultTrustManager();
             }