Detailed changes
@@ -1,10 +1,11 @@
package eu.siacs.conversations.crypto;
import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.SSLPeerUnverifiedException;
import javax.net.ssl.SSLSession;
public interface DomainHostnameVerifier extends HostnameVerifier {
- boolean verify(String domain, String hostname, SSLSession sslSession);
+ boolean verify(String domain, String hostname, SSLSession sslSession) throws SSLPeerUnverifiedException;
}
@@ -3,6 +3,8 @@ package eu.siacs.conversations.crypto;
import android.util.Log;
import android.util.Pair;
+import com.google.common.collect.ImmutableList;
+
import org.bouncycastle.asn1.ASN1Primitive;
import org.bouncycastle.asn1.DERIA5String;
import org.bouncycastle.asn1.DERTaggedObject;
@@ -18,15 +20,17 @@ import java.io.IOException;
import java.net.IDN;
import java.security.cert.Certificate;
import java.security.cert.CertificateEncodingException;
+import java.security.cert.CertificateParsingException;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Locale;
+import javax.net.ssl.SSLPeerUnverifiedException;
import javax.net.ssl.SSLSession;
-public class XmppDomainVerifier implements DomainHostnameVerifier {
+public class XmppDomainVerifier {
private static final String LOGTAG = "XmppDomainVerifier";
@@ -94,68 +98,93 @@ public class XmppDomainVerifier implements DomainHostnameVerifier {
return false;
}
- @Override
- public boolean verify(final String unicodeDomain,final String unicodeHostname, SSLSession sslSession) {
+ public boolean verify(final String unicodeDomain, final String unicodeHostname, SSLSession sslSession) throws SSLPeerUnverifiedException {
final String domain = IDN.toASCII(unicodeDomain);
final String hostname = unicodeHostname == null ? null : IDN.toASCII(unicodeHostname);
- try {
- final Certificate[] chain = sslSession.getPeerCertificates();
- if (chain.length == 0 || !(chain[0] instanceof X509Certificate)) {
- return false;
- }
- final X509Certificate certificate = (X509Certificate) chain[0];
- final List<String> commonNames = getCommonNames(certificate);
- if (isSelfSigned(certificate)) {
- if (commonNames.size() == 1 && matchDomain(domain, commonNames)) {
- Log.d(LOGTAG, "accepted CN in self signed cert as work around for " + domain);
- return true;
- }
- }
- final Collection<List<?>> alternativeNames = certificate.getSubjectAlternativeNames();
- final List<String> xmppAddrs = new ArrayList<>();
- final List<String> srvNames = new ArrayList<>();
- final List<String> domains = new ArrayList<>();
- if (alternativeNames != null) {
- for (List<?> san : alternativeNames) {
- final Integer type = (Integer) san.get(0);
- if (type == 0) {
- final Pair<String, String> otherName = parseOtherName((byte[]) san.get(1));
- if (otherName != null && otherName.first != null && otherName.second != null) {
- switch (otherName.first) {
- case SRV_NAME:
- srvNames.add(otherName.second.toLowerCase(Locale.US));
- break;
- case XMPP_ADDR:
- xmppAddrs.add(otherName.second.toLowerCase(Locale.US));
- break;
- default:
- Log.d(LOGTAG, "oid: " + otherName.first + " value: " + otherName.second);
- }
- }
- } else if (type == 2) {
- final Object value = san.get(1);
- if (value instanceof String) {
- domains.add(((String) value).toLowerCase(Locale.US));
- }
- }
- }
- }
- if (srvNames.size() == 0 && xmppAddrs.size() == 0 && domains.size() == 0) {
- domains.addAll(commonNames);
+ final Certificate[] chain = sslSession.getPeerCertificates();
+ if (chain.length == 0 || !(chain[0] instanceof X509Certificate)) {
+ return false;
+ }
+ final X509Certificate certificate = (X509Certificate) chain[0];
+ final List<String> commonNames = getCommonNames(certificate);
+ if (isSelfSigned(certificate)) {
+ if (commonNames.size() == 1 && matchDomain(domain, commonNames)) {
+ Log.d(LOGTAG, "accepted CN in self signed cert as work around for " + domain);
+ return true;
}
- Log.d(LOGTAG, "searching for " + domain + " in srvNames: " + srvNames + " xmppAddrs: " + xmppAddrs + " domains:" + domains);
+ }
+ try {
+ final ValidDomains validDomains = parseValidDomains(certificate);
+ Log.d(LOGTAG, "searching for " + domain + " in srvNames: " + validDomains.srvNames + " xmppAddrs: " + validDomains.xmppAddrs + " domains:" + validDomains.domains);
if (hostname != null) {
Log.d(LOGTAG, "also trying to verify hostname " + hostname);
}
- return xmppAddrs.contains(domain)
- || srvNames.contains("_xmpp-client." + domain)
- || matchDomain(domain, domains)
- || (hostname != null && matchDomain(hostname, domains));
+ return validDomains.xmppAddrs.contains(domain)
+ || validDomains.srvNames.contains("_xmpp-client." + domain)
+ || matchDomain(domain, validDomains.domains)
+ || (hostname != null && matchDomain(hostname, validDomains.domains));
} catch (final Exception e) {
return false;
}
}
+ public static ValidDomains parseValidDomains(final X509Certificate certificate) throws CertificateParsingException {
+ final List<String> commonNames = getCommonNames(certificate);
+ final Collection<List<?>> alternativeNames = certificate.getSubjectAlternativeNames();
+ final List<String> xmppAddrs = new ArrayList<>();
+ final List<String> srvNames = new ArrayList<>();
+ final List<String> domains = new ArrayList<>();
+ if (alternativeNames != null) {
+ for (List<?> san : alternativeNames) {
+ final Integer type = (Integer) san.get(0);
+ if (type == 0) {
+ final Pair<String, String> otherName = parseOtherName((byte[]) san.get(1));
+ if (otherName != null && otherName.first != null && otherName.second != null) {
+ switch (otherName.first) {
+ case SRV_NAME:
+ srvNames.add(otherName.second.toLowerCase(Locale.US));
+ break;
+ case XMPP_ADDR:
+ xmppAddrs.add(otherName.second.toLowerCase(Locale.US));
+ break;
+ default:
+ Log.d(LOGTAG, "oid: " + otherName.first + " value: " + otherName.second);
+ }
+ }
+ } else if (type == 2) {
+ final Object value = san.get(1);
+ if (value instanceof String) {
+ domains.add(((String) value).toLowerCase(Locale.US));
+ }
+ }
+ }
+ }
+ if (srvNames.size() == 0 && xmppAddrs.size() == 0 && domains.size() == 0) {
+ domains.addAll(commonNames);
+ }
+ return new ValidDomains(xmppAddrs, srvNames, domains);
+ }
+
+ public static final class ValidDomains {
+ final List<String> xmppAddrs;
+ final List<String> srvNames;
+ final List<String> domains;
+
+ private ValidDomains(List<String> xmppAddrs, List<String> srvNames, List<String> domains) {
+ this.xmppAddrs = xmppAddrs;
+ this.srvNames = srvNames;
+ this.domains = domains;
+ }
+
+ public List<String> all() {
+ ImmutableList.Builder<String> all = new ImmutableList.Builder<>();
+ all.addAll(xmppAddrs);
+ all.addAll(srvNames);
+ all.addAll(domains);
+ return all.build();
+ }
+ }
+
private boolean isSelfSigned(X509Certificate certificate) {
try {
certificate.verify(certificate.getPublicKey());
@@ -164,9 +193,4 @@ public class XmppDomainVerifier implements DomainHostnameVerifier {
return false;
}
}
-
- @Override
- public boolean verify(String domain, SSLSession sslSession) {
- return verify(domain, null, sslSession);
- }
}
@@ -42,6 +42,7 @@ 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.io.CharStreams;
import org.json.JSONArray;
@@ -61,12 +62,13 @@ import java.security.NoSuchAlgorithmException;
import java.security.cert.Certificate;
import java.security.cert.CertificateEncodingException;
import java.security.cert.CertificateException;
-import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateParsingException;
import java.security.cert.X509Certificate;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.List;
+import java.util.Locale;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Pattern;
@@ -76,6 +78,7 @@ import javax.net.ssl.TrustManagerFactory;
import javax.net.ssl.X509TrustManager;
import eu.siacs.conversations.R;
+import eu.siacs.conversations.crypto.XmppDomainVerifier;
import eu.siacs.conversations.entities.MTMDecision;
import eu.siacs.conversations.http.HttpConnectionManager;
import eu.siacs.conversations.persistance.FileBackend;
@@ -93,6 +96,8 @@ import eu.siacs.conversations.ui.MemorizingActivity;
*/
public class MemorizingTrustManager {
+ private static final SimpleDateFormat DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd", Locale.US);
+
final static String DECISION_INTENT = "de.duenndns.ssl.DECISION";
public final static String DECISION_INTENT_ID = DECISION_INTENT + ".decisionId";
public final static String DECISION_INTENT_CERT = DECISION_INTENT + ".cert";
@@ -521,14 +526,24 @@ public class MemorizingTrustManager {
return myId;
}
- private void certDetails(StringBuffer si, X509Certificate c) {
- SimpleDateFormat validityDateFormater = new SimpleDateFormat("yyyy-MM-dd");
+ private void certDetails(final StringBuffer si, final X509Certificate c, final boolean showValidFor) {
+
si.append("\n");
- si.append(c.getSubjectDN().toString());
+ if (showValidFor) {
+ try {
+ si.append("Valid for: ");
+ si.append(Joiner.on(", ").join(XmppDomainVerifier.parseValidDomains(c).all()));
+ } catch (final CertificateParsingException e) {
+ si.append("Unable to parse Certificate");
+ }
+ si.append("\n");
+ } else {
+ si.append(c.getSubjectDN());
+ }
si.append("\n");
- si.append(validityDateFormater.format(c.getNotBefore()));
+ si.append(DATE_FORMAT.format(c.getNotBefore()));
si.append(" - ");
- si.append(validityDateFormater.format(c.getNotAfter()));
+ si.append(DATE_FORMAT.format(c.getNotAfter()));
si.append("\nSHA-256: ");
si.append(certHash(c, "SHA-256"));
si.append("\nSHA-1: ");
@@ -541,7 +556,7 @@ public class MemorizingTrustManager {
private String certChainMessage(final X509Certificate[] chain, CertificateException cause) {
Throwable e = cause;
LOGGER.log(Level.FINE, "certChainMessage for " + e);
- StringBuffer si = new StringBuffer();
+ final StringBuffer si = new StringBuffer();
if (e.getCause() != null) {
e = e.getCause();
// HACK: there is no sane way to check if the error is a "trust anchor
@@ -556,8 +571,9 @@ public class MemorizingTrustManager {
si.append(master.getString(R.string.mtm_connect_anyway));
si.append("\n\n");
si.append(master.getString(R.string.mtm_cert_details));
- for (X509Certificate c : chain) {
- certDetails(si, c);
+ si.append('\n');
+ for(int i = 0; i < chain.length; ++i) {
+ certDetails(si, chain[i], i == 0);
}
return si.toString();
}
@@ -46,6 +46,7 @@ import java.util.regex.Matcher;
import javax.net.ssl.KeyManager;
import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLPeerUnverifiedException;
import javax.net.ssl.SSLSocket;
import javax.net.ssl.SSLSocketFactory;
import javax.net.ssl.X509KeyManager;
@@ -826,10 +827,15 @@ public class XmppConnection implements Runnable {
SSLSocketHelper.setHostname(sslSocket, IDN.toASCII(account.getServer()));
SSLSocketHelper.setApplicationProtocol(sslSocket, "xmpp-client");
final XmppDomainVerifier xmppDomainVerifier = new XmppDomainVerifier();
- if (!xmppDomainVerifier.verify(account.getServer(), this.verifiedHostname, sslSocket.getSession())) {
- Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": TLS certificate domain verification failed");
+ try {
+ if (!xmppDomainVerifier.verify(account.getServer(), this.verifiedHostname, sslSocket.getSession())) {
+ Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": TLS certificate domain verification failed");
+ FileBackend.close(sslSocket);
+ throw new StateChangingException(Account.State.TLS_ERROR_DOMAIN);
+ }
+ } catch (final SSLPeerUnverifiedException e) {
FileBackend.close(sslSocket);
- throw new StateChangingException(Account.State.TLS_ERROR_DOMAIN);
+ throw new StateChangingException(Account.State.TLS_ERROR);
}
return sslSocket;
}