From 6c68abb710bd9cbe98820ec3bd5bfa310b24c9cc Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Thu, 3 Apr 2025 10:04:03 +0200 Subject: [PATCH] simplify code around bundled trust manager --- .../crypto/BundledTrustManager.java | 65 ----------------- .../crypto/CombiningTrustManager.java | 23 ++---- .../conversations/crypto/TrustManagers.java | 27 ++++--- .../http/HttpConnectionManager.java | 72 ++++++++++--------- .../services/MemorizingTrustManager.java | 20 ++---- 5 files changed, 67 insertions(+), 140 deletions(-) delete mode 100644 src/main/java/eu/siacs/conversations/crypto/BundledTrustManager.java diff --git a/src/main/java/eu/siacs/conversations/crypto/BundledTrustManager.java b/src/main/java/eu/siacs/conversations/crypto/BundledTrustManager.java deleted file mode 100644 index 9eb20cc300f412914dc9b8a2b97ecc72b92ce143..0000000000000000000000000000000000000000 --- a/src/main/java/eu/siacs/conversations/crypto/BundledTrustManager.java +++ /dev/null @@ -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); - } - } -} diff --git a/src/main/java/eu/siacs/conversations/crypto/CombiningTrustManager.java b/src/main/java/eu/siacs/conversations/crypto/CombiningTrustManager.java index 0f3c0e04415e506748908ad818613064bc3cbc50..7126004877cf49388dcf003c29cc29eea7f8b0ed 100644 --- a/src/main/java/eu/siacs/conversations/crypto/CombiningTrustManager.java +++ b/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 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 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 diff --git a/src/main/java/eu/siacs/conversations/crypto/TrustManagers.java b/src/main/java/eu/siacs/conversations/crypto/TrustManagers.java index 4fc11eecf349979d57a77b61c73a5015fcff7e18..f70545fc950065c81a068d210b76e4a8eb5048fb 100644 --- a/src/main/java/eu/siacs/conversations/crypto/TrustManagers.java +++ b/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); + } } diff --git a/src/main/java/eu/siacs/conversations/http/HttpConnectionManager.java b/src/main/java/eu/siacs/conversations/http/HttpConnectionManager.java index 13126f7eb401cc2835671890b67a645708d3d3d5..3bc65eb8bf1b31f0c912d37e5d39de1854e33486 100644 --- a/src/main/java/eu/siacs/conversations/http/HttpConnectionManager.java +++ b/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); } diff --git a/src/main/java/eu/siacs/conversations/services/MemorizingTrustManager.java b/src/main/java/eu/siacs/conversations/services/MemorizingTrustManager.java index d05fa4ac3dfe2b98d3d61f99427a12eea60a4f08..0d036ca9badb596da8cb7eaa9e3c4537c89ab6af 100644 --- a/src/main/java/eu/siacs/conversations/services/MemorizingTrustManager.java +++ b/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(); }