From 786a6c4c2aa0d0f992014d704ab0eabaa891e3b6 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Wed, 1 Mar 2023 18:02:14 +0100 Subject: [PATCH] put trust manager framework in place --- .../im/conversations/android/AppSettings.java | 10 +++ .../android/ui/model/SetupViewModel.java | 58 ++++++++++++ .../android/xmpp/XmppConnection.java | 8 +- .../android/xmpp/manager/TrustManager.java | 89 ++++++++++++++++++- app/src/main/res/values/defaults.xml | 1 + app/src/main/res/xml/preferences_security.xml | 1 + 6 files changed, 159 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/im/conversations/android/AppSettings.java b/app/src/main/java/im/conversations/android/AppSettings.java index 0e4e47d8a..83d085017 100644 --- a/app/src/main/java/im/conversations/android/AppSettings.java +++ b/app/src/main/java/im/conversations/android/AppSettings.java @@ -11,6 +11,8 @@ public class AppSettings { public static final String PREFERENCE_KEY_RINGTONE = "call_ringtone"; public static final String PREFERENCE_KEY_BTBV = "btbv"; + public static final String PREFERENCE_KEY_TRUST_SYSTEM_CA_STORE = "trust_system_ca_store"; + private final Context context; public AppSettings(final Context context) { @@ -42,4 +44,12 @@ public class AppSettings { return sharedPreferences.getBoolean( PREFERENCE_KEY_BTBV, context.getResources().getBoolean(R.bool.btbv)); } + + public boolean isTrustSystemCAStore() { + final SharedPreferences sharedPreferences = + PreferenceManager.getDefaultSharedPreferences(context); + return sharedPreferences.getBoolean( + PREFERENCE_KEY_TRUST_SYSTEM_CA_STORE, + context.getResources().getBoolean(R.bool.btbv)); + } } diff --git a/app/src/main/java/im/conversations/android/ui/model/SetupViewModel.java b/app/src/main/java/im/conversations/android/ui/model/SetupViewModel.java index b8c94c2ca..228fb7b8e 100644 --- a/app/src/main/java/im/conversations/android/ui/model/SetupViewModel.java +++ b/app/src/main/java/im/conversations/android/ui/model/SetupViewModel.java @@ -7,12 +7,14 @@ import androidx.lifecycle.LiveData; import androidx.lifecycle.MutableLiveData; import androidx.lifecycle.Transformations; import com.google.common.base.CharMatcher; +import com.google.common.base.Optional; import com.google.common.base.Strings; import com.google.common.primitives.Ints; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.MoreExecutors; +import com.google.common.util.concurrent.SettableFuture; import im.conversations.android.R; import im.conversations.android.database.model.Account; import im.conversations.android.database.model.Connection; @@ -20,10 +22,13 @@ import im.conversations.android.repository.AccountRepository; import im.conversations.android.ui.Event; import im.conversations.android.util.ConnectionStates; import im.conversations.android.xmpp.ConnectionException; +import im.conversations.android.xmpp.ConnectionPool; import im.conversations.android.xmpp.ConnectionState; import im.conversations.android.xmpp.XmppConnection; +import im.conversations.android.xmpp.manager.TrustManager; import java.util.Arrays; import java.util.Locale; +import java.util.function.Function; import org.jetbrains.annotations.NotNull; import org.jxmpp.jid.BareJid; import org.jxmpp.jid.impl.JidCreate; @@ -50,6 +55,17 @@ public class SetupViewModel extends AndroidViewModel { private final MutableLiveData> redirection = new MutableLiveData<>(); + private final MutableLiveData trustDecision = new MutableLiveData<>(); + + private final Function> trustDecisionCallback = + fingerprint -> { + final SettableFuture settableFuture = SettableFuture.create(); + final var trustDecision = new TrustDecision(fingerprint, settableFuture); + LOGGER.debug("posting trust decision"); + this.trustDecision.postValue(trustDecision); + return settableFuture; + }; + private final AccountRepository accountRepository; private Account account; @@ -134,6 +150,7 @@ public class SetupViewModel extends AndroidViewModel { decideNextStep(Target.ENTER_ADDRESS, account); return true; } else { + this.unregisterTrustDecisionCallback(); this.account = null; // when the XMPP address changes we want to reset connection info too @@ -187,9 +204,34 @@ public class SetupViewModel extends AndroidViewModel { private void setAccount(@NonNull final Account account) { this.account = account; + this.registerTrustDecisionCallback(); this.decideNextStep(Target.ENTER_ADDRESS, account); } + private Optional getTrustManager() { + final var account = this.account; + if (account == null) { + return Optional.absent(); + } + return ConnectionPool.getInstance(getApplication()) + .get(account) + .transform(xc -> xc.getManager(TrustManager.class)); + } + + private void registerTrustDecisionCallback() { + final var optionalTrustManager = getTrustManager(); + if (optionalTrustManager.isPresent()) { + optionalTrustManager.get().setUserInterfaceCallback(this.trustDecisionCallback); + } + } + + private void unregisterTrustDecisionCallback() { + final var optionalTrustManager = getTrustManager(); + if (optionalTrustManager.isPresent()) { + optionalTrustManager.get().removeUserInterfaceCallback(this.trustDecisionCallback); + } + } + private void decideNextStep(final Target current, @NonNull final Account account) { final ListenableFuture connectedFuture = this.accountRepository.getConnectedFuture(account); @@ -336,6 +378,7 @@ public class SetupViewModel extends AndroidViewModel { public void cancelSetup() { final var account = this.account; if (account != null) { + this.unregisterTrustDecisionCallback(); this.account = null; this.accountRepository.deleteAccountAsync(account); } @@ -345,10 +388,25 @@ public class SetupViewModel extends AndroidViewModel { return this.redirection; } + public void onCleared() { + super.onCleared(); + this.unregisterTrustDecisionCallback(); + } + public enum Target { ENTER_ADDRESS, ENTER_PASSWORD, ENTER_HOSTNAME, DONE } + + public static class TrustDecision { + public final byte[] fingerprint; + public final SettableFuture decision; + + public TrustDecision(byte[] fingerprint, SettableFuture decision) { + this.fingerprint = fingerprint; + this.decision = decision; + } + } } diff --git a/app/src/main/java/im/conversations/android/xmpp/XmppConnection.java b/app/src/main/java/im/conversations/android/xmpp/XmppConnection.java index c0e16f3fe..2ec6aa226 100644 --- a/app/src/main/java/im/conversations/android/xmpp/XmppConnection.java +++ b/app/src/main/java/im/conversations/android/xmpp/XmppConnection.java @@ -28,7 +28,6 @@ import im.conversations.android.database.model.Credential; import im.conversations.android.dns.Resolver; import im.conversations.android.socks.SocksSocketFactory; import im.conversations.android.tls.SSLSockets; -import im.conversations.android.tls.TrustManagers; import im.conversations.android.tls.XmppDomainVerifier; import im.conversations.android.util.PendingItem; import im.conversations.android.xml.Element; @@ -39,6 +38,7 @@ import im.conversations.android.xml.XmlReader; import im.conversations.android.xmpp.manager.AbstractManager; import im.conversations.android.xmpp.manager.CarbonsManager; import im.conversations.android.xmpp.manager.DiscoManager; +import im.conversations.android.xmpp.manager.TrustManager; import im.conversations.android.xmpp.model.Extension; import im.conversations.android.xmpp.model.StreamElement; import im.conversations.android.xmpp.model.bind2.BindInlineFeatures; @@ -510,13 +510,9 @@ public class XmppConnection implements Runnable { } else { keyManager = new KeyManager[] {new MyKeyManager(context, credential)}; } - final String domain = account.address.getDomain().toString(); - // TODO we used to use two different trust managers; interactive and non interactive (to - // trigger SSL cert prompts) - // we need a better solution for this using live data or similar sc.init( keyManager, - new X509TrustManager[] {TrustManagers.getTrustManager()}, + new X509TrustManager[] {getManager(TrustManager.class)}, Conversations.SECURE_RANDOM); return sc.getSocketFactory(); } diff --git a/app/src/main/java/im/conversations/android/xmpp/manager/TrustManager.java b/app/src/main/java/im/conversations/android/xmpp/manager/TrustManager.java index 369104614..1b047dee2 100644 --- a/app/src/main/java/im/conversations/android/xmpp/manager/TrustManager.java +++ b/app/src/main/java/im/conversations/android/xmpp/manager/TrustManager.java @@ -1,16 +1,35 @@ package im.conversations.android.xmpp.manager; +import android.annotation.SuppressLint; import android.content.Context; +import com.google.common.base.Joiner; +import com.google.common.base.Throwables; +import com.google.common.collect.Lists; +import com.google.common.hash.Hashing; +import com.google.common.primitives.Bytes; +import com.google.common.util.concurrent.ListenableFuture; import im.conversations.android.AppSettings; +import im.conversations.android.tls.TrustManagers; import im.conversations.android.xmpp.XmppConnection; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.function.Function; import javax.net.ssl.X509TrustManager; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +@SuppressLint("CustomX509TrustManager") public class TrustManager extends AbstractManager implements X509TrustManager { + private static final Logger LOGGER = LoggerFactory.getLogger(TrustManager.class); + private final AppSettings appSettings; + private Function> userInterfaceCallback; + public TrustManager(final Context context, final XmppConnection connection) { super(context, connection); this.appSettings = new AppSettings(context); @@ -18,14 +37,80 @@ public class TrustManager extends AbstractManager implements X509TrustManager { @Override public void checkClientTrusted(final X509Certificate[] chain, final String authType) - throws CertificateException {} + throws CertificateException { + throw new CertificateException( + "This implementation has no support for client certificates"); + } @Override public void checkServerTrusted(final X509Certificate[] chain, final String authType) - throws CertificateException {} + throws CertificateException { + if (chain.length == 0) { + throw new CertificateException("Certificate chain is zero length"); + } + for (final X509Certificate certificate : chain) { + certificate.checkValidity(); + } + final boolean isTrustSystemCaStore = appSettings.isTrustSystemCAStore(); + if (isTrustSystemCaStore && isServerTrustedInSystemCaStore(chain, authType)) { + return; + } + final X509Certificate certificate = chain[0]; + final byte[] fingerprint = Hashing.sha256().hashBytes(certificate.getEncoded()).asBytes(); + LOGGER.info("Looking up {} in database", fingerprint(fingerprint)); + final var callback = this.userInterfaceCallback; + if (callback == null) { + throw new CertificateException( + "No user interface registered. Can not trust certificate"); + } + final ListenableFuture futureDecision = callback.apply(fingerprint); + final boolean decision; + try { + decision = Boolean.TRUE.equals(futureDecision.get(10, TimeUnit.SECONDS)); + } catch (final ExecutionException | InterruptedException | TimeoutException e) { + throw new CertificateException( + "Timeout waiting for user response", Throwables.getRootCause(e)); + } + if (decision) { + return; + } + throw new CertificateException("User did not trust certificate"); + } + + private boolean isServerTrustedInSystemCaStore( + final X509Certificate[] chain, final String authType) { + final var trustManager = TrustManagers.getTrustManager(); + if (trustManager == null) { + return false; + } + try { + trustManager.checkServerTrusted(chain, authType); + return true; + } catch (final CertificateException e) { + return false; + } + } @Override public X509Certificate[] getAcceptedIssuers() { return new X509Certificate[0]; } + + public static String fingerprint(final byte[] bytes) { + return Joiner.on(':') + .join(Lists.transform(Bytes.asList(bytes), b -> String.format("%02X", b))); + } + + public void setUserInterfaceCallback( + final Function> callback) { + this.userInterfaceCallback = callback; + } + + public void removeUserInterfaceCallback( + final Function> callback) { + if (this.userInterfaceCallback == callback) { + LOGGER.info("Remove user interface callback"); + this.userInterfaceCallback = null; + } + } } diff --git a/app/src/main/res/values/defaults.xml b/app/src/main/res/values/defaults.xml index 85b959357..6dc3029cc 100644 --- a/app/src/main/res/values/defaults.xml +++ b/app/src/main/res/values/defaults.xml @@ -5,6 +5,7 @@ false true true + true default_on content://settings/system/ringtone 144 diff --git a/app/src/main/res/xml/preferences_security.xml b/app/src/main/res/xml/preferences_security.xml index e02fdc8fb..4fd7098ac 100644 --- a/app/src/main/res/xml/preferences_security.xml +++ b/app/src/main/res/xml/preferences_security.xml @@ -20,6 +20,7 @@