workaround for OpenFire: check CN first in self signed certs

Daniel Gultsch created 8 years ago

The self signed certificates created by OpenFire (Not sure if other
certs are affected as well) will crash the Java/Android TLS stack when
accessing getSubjectAlternativeNames() on the the peer certificate.

This usually goes unnoticed in other applications since the
DefaultHostnameVerifier checkes the CN first. That however is a
violation of RFC6125 section 6.4.4 which requires us to check for the
existence of SAN first.

This commit adds a work around where in self signed certificates we
check for the CN first as well. (Avoiding the call to
getSubjectAlternativeNames())

Change summary

src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java | 38 
1 file changed, 32 insertions(+), 6 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java

@@ -16,12 +16,12 @@ import org.bouncycastle.cert.jcajce.JcaX509CertificateHolder;
 
 import java.io.IOException;
 import java.security.cert.Certificate;
+import java.security.cert.CertificateEncodingException;
 import java.security.cert.X509Certificate;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 
-import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLSession;
 
 import de.duenndns.ssl.DomainHostnameVerifier;
@@ -41,6 +41,13 @@ public class XmppDomainVerifier implements DomainHostnameVerifier {
 				return false;
 			}
 			X509Certificate certificate = (X509Certificate) chain[0];
+			if (isSelfSigned(certificate)) {
+				List<String> domains = getCommonNames(certificate);
+				if (domains.size() == 1 && domains.get(0).equals(domain)) {
+					Log.d(LOGTAG,"accepted CN in cert self signed cert for "+domain);
+					return true;
+				}
+			}
 			Collection<List<?>> alternativeNames = certificate.getSubjectAlternativeNames();
 			List<String> xmppAddrs = new ArrayList<>();
 			List<String> srvNames = new ArrayList<>();
@@ -71,11 +78,7 @@ public class XmppDomainVerifier implements DomainHostnameVerifier {
 				}
 			}
 			if (srvNames.size() == 0 && xmppAddrs.size() == 0 && domains.size() == 0) {
-				X500Name x500name = new JcaX509CertificateHolder(certificate).getSubject();
-				RDN[] rdns = x500name.getRDNs(BCStyle.CN);
-				for (int i = 0; i < rdns.length; ++i) {
-					domains.add(IETFUtils.valueToString(x500name.getRDNs(BCStyle.CN)[i].getFirst().getValue()));
-				}
+				domains.addAll(domains);
 			}
 			Log.d(LOGTAG, "searching for " + domain + " in srvNames: " + srvNames + " xmppAddrs: " + xmppAddrs + " domains:" + domains);
 			if (hostname != null) {
@@ -90,6 +93,20 @@ public class XmppDomainVerifier implements DomainHostnameVerifier {
 		}
 	}
 
+	private static List<String> getCommonNames(X509Certificate certificate) {
+		List<String> domains = new ArrayList<>();
+		try {
+			X500Name x500name = new JcaX509CertificateHolder(certificate).getSubject();
+			RDN[] rdns = x500name.getRDNs(BCStyle.CN);
+			for (int i = 0; i < rdns.length; ++i) {
+				domains.add(IETFUtils.valueToString(x500name.getRDNs(BCStyle.CN)[i].getFirst().getValue()));
+			}
+			return domains;
+		} catch (CertificateEncodingException e) {
+			return domains;
+		}
+	}
+
 	private static Pair<String, String> parseOtherName(byte[] otherName) {
 		try {
 			ASN1Primitive asn1Primitive = ASN1Primitive.fromByteArray(otherName);
@@ -133,6 +150,15 @@ public class XmppDomainVerifier implements DomainHostnameVerifier {
 		return false;
 	}
 
+	private boolean isSelfSigned(X509Certificate certificate) {
+		try {
+			certificate.verify(certificate.getPublicKey());
+			return true;
+		} catch (Exception e) {
+			return false;
+		}
+	}
+
 	@Override
 	public boolean verify(String domain, SSLSession sslSession) {
 		return verify(domain,null,sslSession);