only save certificates passing hostname verification as ca validated, and retain not yet valid certificates
What the developer wrote
Authored by Craig Raw
only save certificates passing hostname verification as ca validated, and retain not yet valid certificates
What changed, and why it matters
This commit tightens how Sparrow Wallet saves and reuses TLS certificates for private servers. Previously, any certificate that passed a basic CA check was saved as 'CA validated' even if it didn't actually match the server's hostname. That could let a malicious or misconfigured server trick the wallet into trusting the wrong certificate. Now the app also checks that the certificate's hostname matches the server, and it no longer deletes certificates that are merely 'not yet valid' (which can happen if the user's computer clock is wrong).
Users relying on private Electrum servers over TLS should upgrade. Review any previously saved CA-validated certificates, as older versions may have trusted certificates that did not match the server hostname. Ensure system clocks are correct, as not-yet-valid certificates are now retained rather than deleted.
Security signals we found
Missing hostname verification before saving CA-trusted certificate
Broad exception handling previously caused deletion of saved certificates on any CertificateException
Certificate pinning/trust-store logic for private Electrum servers
TLS trust decision made without server identity validation
Evidence from the diff
The patch modifies TcpOverTlsTransport.java. It renames isCaSigned() to isCaValidated() and adds hostname verification against the certificate’s Subject Alternative Names (DNS and IP) before saving a certificate as CA validated. It also narrows the exception handling for saved certificates from the broad CertificateException to CertificateNotYetValidException, so expired or otherwise invalid certs are no longer silently deleted; only not-yet-valid certs are retained with a warning. Onion addresses are exempt from hostname matching.
Changed components
src/main/java/com/sparrowwallet/sparrow/net/TcpOverTlsTransport.javaTLS certificate validation and storage for private Electrum serversInspect captured patch +38 / −6
diff --git a/src/main/java/com/sparrowwallet/sparrow/net/TcpOverTlsTransport.java b/src/main/java/com/sparrowwallet/sparrow/net/TcpOverTlsTransport.java
index 01fef26..b8f8f27 100644
--- a/src/main/java/com/sparrowwallet/sparrow/net/TcpOverTlsTransport.java
+++ b/src/main/java/com/sparrowwallet/sparrow/net/TcpOverTlsTransport.java
@@ -14,10 +14,14 @@ import java.net.InetSocketAddress;
import java.security.*;
import java.security.cert.*;
import java.security.cert.Certificate;
+import java.util.Collection;
+import java.util.List;
public class TcpOverTlsTransport extends TcpTransport {
private static final Logger log = LoggerFactory.getLogger(TcpOverTlsTransport.class);
public static final int PAD_TO_MULTIPLE_OF_BYTES = 96;
+ private static final int GENERAL_NAME_DNS = 2;
+ private static final int GENERAL_NAME_IP_ADDRESS = 7;
protected final SSLSocketFactory sslSocketFactory;
protected final boolean usingCaTrust;
@@ -112,9 +116,9 @@ public class TcpOverTlsTransport extends TcpTransport {
return getTrustManagers(null, host);
}
//Allow expired certificates for private servers where users may not have the expertise to renew
- } catch(CertificateException e) {
- crtFile.delete();
- return getTrustManagers(null, host);
+ } catch(CertificateNotYetValidException e) {
+ //A certificate that was valid when saved or configured can only appear to be not yet valid if the system clock is incorrect - retain it
+ log.warn("Certificate for " + host + " at " + crtFile.getAbsolutePath() + " is not yet valid, check the system clock is correct");
}
}
@@ -140,7 +144,7 @@ public class TcpOverTlsTransport extends TcpTransport {
try {
Certificate[] certs = event.getPeerCertificates();
if(certs.length > 0) {
- if(isCaSigned(certs)) {
+ if(isCaValidated(certs, server)) {
Storage.saveCaCertificate(server.getHost(), certs[0]);
} else {
Storage.saveCertificate(server.getHost(), certs[0]);
@@ -171,7 +175,7 @@ public class TcpOverTlsTransport extends TcpTransport {
return tmf.getTrustManagers();
}
- private static boolean isCaSigned(Certificate[] certs) {
+ private static boolean isCaValidated(Certificate[] certs, HostAndPort server) {
try {
TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
tmf.init((KeyStore)null);
@@ -195,12 +199,40 @@ public class TcpOverTlsTransport extends TcpTransport {
String authType = x509Certs[0].getPublicKey().getAlgorithm().equals("EC") ? "ECDHE_ECDSA" : "RSA";
defaultTm.checkServerTrusted(x509Certs, authType);
- return true;
+
+ //Match the hostname as CA validated connections do, so that a certificate saved here can be validated when it is used
+ return Protocol.isOnionAddress(server) || matchesHostname(x509Certs[0], server.getHost());
} catch(Exception e) {
return false;
}
}
+ private static boolean matchesHostname(X509Certificate cert, String host) throws CertificateParsingException {
+ Collection<List<?>> subjectAltNames = cert.getSubjectAlternativeNames();
+ if(subjectAltNames == null) {
+ return false;
+ }
+
+ for(List<?> subjectAltName : subjectAltNames) {
+ if(subjectAltName.size() > 1 && subjectAltName.get(0) instanceof Integer type && subjectAltName.get(1) instanceof String name) {
+ if((type == GENERAL_NAME_DNS && matchesDnsName(name, host)) || (type == GENERAL_NAME_IP_ADDRESS && name.equalsIgnoreCase(host))) {
+ return true;
+ }
+ }
+ }
+
+ return false;
+ }
+
+ private static boolean matchesDnsName(String dnsName, String host) {
+ if(dnsName.startsWith("*.")) {
+ int index = host.indexOf('.');
+ return index > 0 && host.substring(index + 1).equalsIgnoreCase(dnsName.substring(2));
+ }
+
+ return dnsName.equalsIgnoreCase(host);
+ }
+
@Override
protected int getDefaultPort() {
return Protocol.SSL.getDefaultPort();
Why this scored 59/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.