Support host certificate keys (#703)

* Handle @cert-authority in known_hosts. * Fix ClassCastException when receiving an ECDSA-CERT host key. * Mention what exactly is not negotiated. * Verify host key certificates during key exchange. * Unit and integration tests for host key verification. * Show sshd logs when integration test finishes. * Review fixes: extract to private method, change strings.
2025-12-08 08:10:55 +03:00 · 2021-08-17 19:29:02 +07:00
parent 4d9665b6a7
commit a016974743
62 changed files with 724 additions and 35 deletions
--- a/src/main/java/net/schmizz/sshj/Config.java
+++ b/src/main/java/net/schmizz/sshj/Config.java
@@ -90,6 +90,13 @@ public interface Config {
     */
    String getVersion();

+    /**
+     * Returns true if host key certificates should be verified while connecting to the server. It is recommended to
+     * verify them, but can cause connection failures in cases when previous versions of the library could have managed
+     * to connect.
+     */
+    boolean isVerifyHostKeyCertificates();
+
    /**
     * Set the named factories for {@link Cipher}.
     *
@@ -187,4 +194,10 @@ public interface Config {
     * @return The LoggerFactory the SSHClient will use.
     */
    LoggerFactory getLoggerFactory();
+
+    /**
+     * Sets whether the SSH client should verify host key certificates or not.
+     * See {@link #isVerifyHostKeyCertificates()}.
+     */
+    void setVerifyHostKeyCertificates(boolean value);
 }
--- a/src/main/java/net/schmizz/sshj/ConfigImpl.java
+++ b/src/main/java/net/schmizz/sshj/ConfigImpl.java
@@ -47,6 +47,7 @@ public class ConfigImpl

    private boolean waitForServerIdentBeforeSendingClientIdent = false;
    private LoggerFactory loggerFactory;
+    private boolean verifyHostKeyCertificates = true;

    @Override
    public List<Factory.Named<Cipher>> getCipherFactories() {
@@ -177,4 +178,14 @@ public class ConfigImpl
    public void setLoggerFactory(LoggerFactory loggerFactory) {
        this.loggerFactory = loggerFactory;
    }
+
+    @Override
+    public boolean isVerifyHostKeyCertificates() {
+        return verifyHostKeyCertificates;
+    }
+
+    @Override
+    public void setVerifyHostKeyCertificates(boolean value) {
+        verifyHostKeyCertificates = value;
+    }
 }
--- a/src/main/java/net/schmizz/sshj/common/KeyType.java
+++ b/src/main/java/net/schmizz/sshj/common/KeyType.java
@@ -17,12 +17,17 @@ package net.schmizz.sshj.common;

 import com.hierynomus.sshj.common.KeyAlgorithm;
 import com.hierynomus.sshj.signature.Ed25519PublicKey;
+import com.hierynomus.sshj.signature.SignatureEdDSA;
 import com.hierynomus.sshj.userauth.certificate.Certificate;
 import net.i2p.crypto.eddsa.EdDSAPublicKey;
 import net.i2p.crypto.eddsa.spec.EdDSANamedCurveSpec;
 import net.i2p.crypto.eddsa.spec.EdDSANamedCurveTable;
 import net.i2p.crypto.eddsa.spec.EdDSAPublicKeySpec;
 import net.schmizz.sshj.common.Buffer.BufferException;
+import net.schmizz.sshj.signature.Signature;
+import net.schmizz.sshj.signature.SignatureDSA;
+import net.schmizz.sshj.signature.SignatureECDSA;
+import net.schmizz.sshj.signature.SignatureRSA;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;

@@ -36,6 +41,7 @@ import java.security.interfaces.RSAPublicKey;
 import java.security.spec.DSAPublicKeySpec;
 import java.security.spec.RSAPublicKeySpec;
 import java.util.*;
+import java.util.regex.Pattern;

 /** Type of key e.g. rsa, dsa */
 public enum KeyType {
@@ -417,7 +423,7 @@ public enum KeyType {
        return sType;
    }

-    static class CertUtils {
+    public static class CertUtils {

        @SuppressWarnings("unchecked")
        static <T extends PublicKey> Certificate<T> readPubKey(Buffer<?> buf, KeyType innerKeyType) throws GeneralSecurityException {
@@ -461,6 +467,122 @@ public enum KeyType {
                .putBytes(certificate.getSignature());
        }

+        /**
+         * @param certRaw Already serialized host certificate that was received as a packet. Can be restored simply by
+         *                calling {@code new Buffer.PlainBuffer().putPublicKey(cert)}
+         * @param cert A key with a certificate received from a server.
+         * @param hostname A hostname of the server. It is juxtaposed to the principals of the certificate.
+         * @return null if the certificate is valid, an error message if it is not valid.
+         * @throws Buffer.BufferException If something from {@code certRaw} or {@code cert} can't be parsed.
+         */
+        public static String verifyHostCertificate(byte[] certRaw, Certificate<?> cert, String hostname)
+                throws Buffer.BufferException, SSHRuntimeException {
+            String signatureType = new Buffer.PlainBuffer(cert.getSignature()).readString();
+            final Signature signature = Factory.Named.Util.create(ALL_SIGNATURES, signatureType);
+            if (signature == null) {
+                return "Unknown signature algorithm `" + signatureType + "`";
+            }
+
+            // Quotes are from
+            // https://cvsweb.openbsd.org/cgi-bin/cvsweb/~checkout~/src/usr.bin/ssh/PROTOCOL.certkeys?rev=1.19&content-type=text/plain
+
+            // "valid principals" is a string containing zero or more principals as
+            // strings packed inside it. These principals list the names for which this
+            // certificate is valid; hostnames for SSH_CERT_TYPE_HOST certificates and
+            // usernames for SSH_CERT_TYPE_USER certificates. As a special case, a
+            // zero-length "valid principals" field means the certificate is valid for
+            // any principal of the specified type.
+            if (cert.getValidPrincipals() != null && !cert.getValidPrincipals().isEmpty()) {
+                boolean ok = false;
+                for (String principal : cert.getValidPrincipals()) {
+                    ok = matchPattern(hostname, principal);
+                    if (ok) {
+                        break;
+                    }
+                }
+                if (!ok) {
+                    StringBuilder error = new StringBuilder()
+                            .append("Hostname `")
+                            .append(hostname)
+                            .append("` doesn't match any of the principals: `");
+                    String delimiter = "";
+                    for (String principal : cert.getValidPrincipals()) {
+                        error.append(delimiter).append(principal);
+                        delimiter = "`, `";
+                    }
+                    error.append("`");
+                    return error.toString();
+                }
+            }
+
+            // "valid after" and "valid before" specify a validity period for the
+            // certificate. Each represents a time in seconds since 1970-01-01
+            // 00:00:00. A certificate is considered valid if:
+            //  valid after <= current time < valid before
+            Date today = new Date();
+            if (cert.getValidAfter() != null && today.before(cert.getValidAfter())) {
+                return "Certificate is valid after " + cert.getValidAfter() + ", today is " + today;
+            }
+            if (cert.getValidBefore() != null && today.after(cert.getValidBefore())) {
+                return "Certificate is valid before " + cert.getValidBefore() + ", today is " + today;
+            }
+
+            // All critical options supported by OpenSSH relate to the client. Nothing to take from host certificates.
+
+            signature.initVerify(new Buffer.PlainBuffer(cert.getSignatureKey()).readPublicKey());
+            // -4 -- minus the length of the integer holding the length of the signature.
+            signature.update(certRaw, 0, certRaw.length - cert.getSignature().length - 4);
+            if (signature.verify(cert.getSignature())) {
+                return null;
+            } else {
+                return "Signature verification failed";
+            }
+        }
+
+        /**
+         * This method must work exactly as match_pattern from match.c of OpenSSH. If it works differently, consider it
+         * as a bug that must be fixed.
+         */
+        public static boolean matchPattern(String target, String pattern) {
+            StringBuilder regex = new StringBuilder();
+            String endEscape = "";
+            for (int i = 0; i < pattern.length(); ++i) {
+                char p = pattern.charAt(i);
+                if (p == '?' || p == '*') {
+                    regex.append(endEscape);
+                    endEscape = "";
+                    if (p == '?') {
+                        regex.append('.');
+                    } else {
+                        regex.append(".*");
+                    }
+                } else {
+                    if (endEscape.isEmpty()) {
+                        regex.append("\\Q");
+                        endEscape = "\\E";
+                    }
+                    regex.append(p);
+                }
+            }
+            return Pattern.compile(regex.toString()).matcher(target).matches();
+        }
+
+        public static final List<Factory.Named<Signature>> ALL_SIGNATURES = Arrays.asList(
+                new SignatureRSA.FactorySSHRSA(),
+                new SignatureRSA.FactoryCERT(),
+                new SignatureRSA.FactoryRSASHA256(),
+                new SignatureRSA.FactoryRSASHA512(),
+                new SignatureDSA.Factory(),
+                new SignatureDSA.Factory(),
+                new SignatureECDSA.Factory256(),
+                new SignatureECDSA.Factory256(),
+                new SignatureECDSA.Factory384(),
+                new SignatureECDSA.Factory384(),
+                new SignatureECDSA.Factory521(),
+                new SignatureECDSA.Factory521(),
+                new SignatureEdDSA.Factory(),
+                new SignatureEdDSA.Factory());
+
        static boolean isCertificateOfType(Key key, KeyType innerKeyType) {
            if (!(key instanceof Certificate)) {
                return false;
--- a/src/main/java/net/schmizz/sshj/transport/Proposal.java
+++ b/src/main/java/net/schmizz/sshj/transport/Proposal.java
@@ -127,41 +127,42 @@ class Proposal {
    public NegotiatedAlgorithms negotiate(Proposal other)
            throws TransportException {
        return new NegotiatedAlgorithms(
-                firstMatch(this.getKeyExchangeAlgorithms(), other.getKeyExchangeAlgorithms()),
-                firstMatch(this.getHostKeyAlgorithms(), other.getHostKeyAlgorithms()),
-                firstMatch(this.getClient2ServerCipherAlgorithms(), other.getClient2ServerCipherAlgorithms()),
-                firstMatch(this.getServer2ClientCipherAlgorithms(), other.getServer2ClientCipherAlgorithms()),
-                firstMatch(this.getClient2ServerMACAlgorithms(), other.getClient2ServerMACAlgorithms()),
-                firstMatch(this.getServer2ClientMACAlgorithms(), other.getServer2ClientMACAlgorithms()),
-                firstMatch(this.getClient2ServerCompressionAlgorithms(), other.getClient2ServerCompressionAlgorithms()),
-                firstMatch(this.getServer2ClientCompressionAlgorithms(), other.getServer2ClientCompressionAlgorithms()),
+                firstMatch("KeyExchangeAlgorithms",
+                           this.getKeyExchangeAlgorithms(),
+                           other.getKeyExchangeAlgorithms()),
+                firstMatch("HostKeyAlgorithms",
+                           this.getHostKeyAlgorithms(),
+                           other.getHostKeyAlgorithms()),
+                firstMatch("Client2ServerCipherAlgorithms",
+                           this.getClient2ServerCipherAlgorithms(),
+                           other.getClient2ServerCipherAlgorithms()),
+                firstMatch("Server2ClientCipherAlgorithms",
+                           this.getServer2ClientCipherAlgorithms(),
+                           other.getServer2ClientCipherAlgorithms()),
+                firstMatch("Client2ServerMACAlgorithms",
+                           this.getClient2ServerMACAlgorithms(),
+                           other.getClient2ServerMACAlgorithms()),
+                firstMatch("Server2ClientMACAlgorithms",
+                           this.getServer2ClientMACAlgorithms(),
+                           other.getServer2ClientMACAlgorithms()),
+                firstMatch("Client2ServerCompressionAlgorithms",
+                           this.getClient2ServerCompressionAlgorithms(),
+                           other.getClient2ServerCompressionAlgorithms()),
+                firstMatch("Server2ClientCompressionAlgorithms",
+                           this.getServer2ClientCompressionAlgorithms(),
+                           other.getServer2ClientCompressionAlgorithms()),
                other.getHostKeyAlgorithms().containsAll(KeyAlgorithms.SSH_RSA_SHA2_ALGORITHMS)
        );
    }

-    private static String firstMatch(List<String> a, List<String> b)
+    private static String firstMatch(String ofWhat, List<String> a, List<String> b)
            throws TransportException {
        for (String aa : a) {
            if (b.contains(aa)) {
                return aa;
            }
        }
-        throw new TransportException("Unable to reach a settlement: " + a + " and " + b);
-    }
-
-    private static List<String> allMatch(List<String> a, List<String> b) throws TransportException {
-        List<String> res = new ArrayList<String>();
-        for (String aa : a) {
-            if (b.contains(aa)) {
-                res.add(aa);
-            }
-        }
-
-        if (res.isEmpty()) {
-            throw new TransportException("Unable to reach a settlement: " + a + " and " + b);
-        }
-
-        return res;
+        throw new TransportException("Unable to reach a settlement of " + ofWhat + ": " + a + " and " + b);
    }

    private static String toCommaString(List<String> sl) {
--- a/src/main/java/net/schmizz/sshj/transport/kex/AbstractDHG.java
+++ b/src/main/java/net/schmizz/sshj/transport/kex/AbstractDHG.java
@@ -15,6 +15,7 @@
 */
 package net.schmizz.sshj.transport.kex;

+import com.hierynomus.sshj.userauth.certificate.Certificate;
 import net.schmizz.sshj.common.*;
 import net.schmizz.sshj.signature.Signature;
 import net.schmizz.sshj.transport.Transport;
@@ -79,14 +80,52 @@ public abstract class AbstractDHG extends AbstractDH {


        Signature signature = trans.getHostKeyAlgorithm().newSignature();
-        signature.initVerify(hostKey);
+        if (hostKey instanceof Certificate<?>) {
+            signature.initVerify(((Certificate<?>)hostKey).getKey());
+        }
+        else {
+            signature.initVerify(hostKey);
+        }
        signature.update(H, 0, H.length);
        if (!signature.verify(sig))
            throw new TransportException(DisconnectReason.KEY_EXCHANGE_FAILED,
                                         "KeyExchange signature verification failed");
+
+        verifyCertificate(K_S);
+
        return true;
    }

+    private void verifyCertificate(byte[] K_S) throws TransportException {
+        if (hostKey instanceof Certificate<?> && trans.getConfig().isVerifyHostKeyCertificates()) {
+            final Certificate<?> hostKey = (Certificate<?>) this.hostKey;
+            String signatureType, caKeyType;
+            try {
+                signatureType = new Buffer.PlainBuffer(hostKey.getSignature()).readString();
+            } catch (Buffer.BufferException e) {
+                signatureType = null;
+            }
+            try {
+                caKeyType = new Buffer.PlainBuffer(hostKey.getSignatureKey()).readString();
+            } catch (Buffer.BufferException e) {
+                caKeyType = null;
+            }
+            log.debug("Verifying signature of the key with type {} (signature type {}, CA key type {})",
+                      hostKey.getType(), signatureType, caKeyType);
+
+            try {
+                final String certError = KeyType.CertUtils.verifyHostCertificate(K_S, hostKey, trans.getRemoteHost());
+                if (certError != null) {
+                    throw new TransportException(DisconnectReason.KEY_EXCHANGE_FAILED,
+                                                 "KeyExchange certificate check failed: " + certError);
+                }
+            } catch (Buffer.BufferException | SSHRuntimeException e) {
+                throw new TransportException(DisconnectReason.KEY_EXCHANGE_FAILED,
+                                             "KeyExchange certificate check failed", e);
+            }
+        }
+    }
+
    protected abstract void initDH(DHBase dh)
            throws GeneralSecurityException;

--- a/src/main/java/net/schmizz/sshj/transport/kex/AbstractDHGex.java
+++ b/src/main/java/net/schmizz/sshj/transport/kex/AbstractDHGex.java
@@ -16,6 +16,7 @@
 package net.schmizz.sshj.transport.kex;

 import com.hierynomus.sshj.key.KeyAlgorithm;
+import com.hierynomus.sshj.userauth.certificate.Certificate;
 import net.schmizz.sshj.common.*;
 import net.schmizz.sshj.signature.Signature;
 import net.schmizz.sshj.transport.Transport;
@@ -88,7 +89,11 @@ public abstract class AbstractDHGex extends AbstractDH {
        H = digest.digest();
        KeyAlgorithm keyAlgorithm = trans.getHostKeyAlgorithm();
        Signature signature = keyAlgorithm.newSignature();
-        signature.initVerify(hostKey);
+        if (hostKey instanceof Certificate<?>) {
+            signature.initVerify(((Certificate<?>) hostKey).getKey());
+        } else {
+            signature.initVerify(hostKey);
+        }
        signature.update(H, 0, H.length);
        if (!signature.verify(sig))
            throw new TransportException(DisconnectReason.KEY_EXCHANGE_FAILED,
--- a/src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java
+++ b/src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java
@@ -17,6 +17,7 @@ package net.schmizz.sshj.transport.verification;

 import com.hierynomus.sshj.common.KeyAlgorithm;
 import com.hierynomus.sshj.transport.verification.KnownHostMatchers;
+import com.hierynomus.sshj.userauth.certificate.Certificate;
 import net.schmizz.sshj.common.*;
 import org.slf4j.Logger;

@@ -356,18 +357,24 @@ public class OpenSSHKnownHosts
        protected final PublicKey key;
        private final String comment;
        private final KnownHostMatchers.HostMatcher matcher;
+        protected final Logger log;

        public HostEntry(Marker marker, String hostPart, KeyType type, PublicKey key) throws SSHException {
            this(marker, hostPart, type, key, "");
        }

        public HostEntry(Marker marker, String hostPart, KeyType type, PublicKey key, String comment) throws SSHException {
+            this(marker, hostPart, type, key, comment, LoggerFactory.DEFAULT);
+        }
+
+        public HostEntry(Marker marker, String hostPart, KeyType type, PublicKey key, String comment, LoggerFactory loggerFactory) throws SSHException {
            this.marker = marker;
            this.hostPart = hostPart;
            this.type = type;
            this.key = key;
            this.comment = comment;
            this.matcher = KnownHostMatchers.createMatcher(hostPart);
+            this.log = loggerFactory.getLogger(getClass());
        }

        @Override
@@ -387,11 +394,15 @@ public class OpenSSHKnownHosts

        @Override
        public boolean appliesTo(KeyType type, String host) throws IOException {
-            return this.type == type && matcher.match(host);
+            return (this.type == type || (marker == Marker.CA_CERT && type.getParent() != null)) && matcher.match(host);
        }

        @Override
        public boolean verify(PublicKey key) throws IOException {
+            if (marker == Marker.CA_CERT && key instanceof Certificate<?>) {
+                final PublicKey caKey = new Buffer.PlainBuffer(((Certificate<?>) key).getSignatureKey()).readPublicKey();
+                return this.type == KeyType.fromKey(caKey) && getKeyString(caKey).equals(getKeyString(this.key));
+            }
            return getKeyString(key).equals(getKeyString(this.key)) && marker != Marker.REVOKED;
        }