This is an automated email from the ASF dual-hosted git repository.

twolf pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit e32c262e5a8c2f410b61636bb1a11ccd51a06a85
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Tue Feb 18 00:33:47 2025 +0100

    KEX: fix host certificate w/o principals
    
    A host certificate with empty principals is valid for any host.[1]
    
    [1] 
https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys
---
 .../java/org/apache/sshd/client/kex/DHGClient.java | 42 ++++++++++++----------
 .../signature/KnownHostsCertificateTest.java       | 25 +++++++++----
 2 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java 
b/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java
index 3c336134e..bab1928f0 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java
@@ -269,27 +269,33 @@ public class DHGClient extends 
AbstractDHClientKeyExchange {
                     "KeyExchange CA signature verification failed for key 
type=" + keyAlg + " of key ID=" + keyId);
         }
 
-        /*
-         * We compare only the connect address against the principals and do 
not do any reverse DNS lookups. If one
-         * wants to connect with the IP it has to be included in the 
principals list of the certificate.
-         */
-        SocketAddress connectSocketAddress = 
getClientSession().getConnectAddress();
-        if (connectSocketAddress instanceof SshdSocketAddress) {
-            connectSocketAddress = ((SshdSocketAddress) 
connectSocketAddress).toInetSocketAddress();
-        }
+        // "As a special case, a zero-length "valid principals" field means 
the certificate is valid for
+        // any principal of the specified type."
+        // See 
https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys
+        //
+        // Empty principals in a host certificate mean the certificate is 
valid for any host.
+        Collection<String> principals = openSshKey.getPrincipals();
+        if (!GenericUtils.isEmpty(principals)) {
+            /*
+             * We compare only the connect address against the principals and 
do not do any reverse DNS lookups. If one
+             * wants to connect with the IP it has to be included in the 
principals list of the certificate.
+             */
+            SocketAddress connectSocketAddress = 
getClientSession().getConnectAddress();
+            if (connectSocketAddress instanceof SshdSocketAddress) {
+                connectSocketAddress = ((SshdSocketAddress) 
connectSocketAddress).toInetSocketAddress();
+            }
 
-        if (connectSocketAddress instanceof InetSocketAddress) {
-            String hostName = ((InetSocketAddress) 
connectSocketAddress).getHostString();
-            Collection<String> principals = openSshKey.getPrincipals();
-            if (GenericUtils.isEmpty(principals) || 
(!principals.contains(hostName))) {
+            if (connectSocketAddress instanceof InetSocketAddress) {
+                String hostName = ((InetSocketAddress) 
connectSocketAddress).getHostString();
+                if (GenericUtils.isEmpty(principals) || 
(!principals.contains(hostName))) {
+                    throw new 
SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
+                            "KeyExchange signature verification failed, 
invalid principal " + hostName + " for key ID=" + keyId
+                                                                               
              + " - allowed=" + principals);
+                }
+            } else {
                 throw new 
SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
-                        "KeyExchange signature verification failed, invalid 
principal "
-                                                                               
          + hostName + " for key ID=" + keyId
-                                                                               
          + " - allowed=" + principals);
+                        "KeyExchange signature verification failed, could not 
determine connect host for key ID=" + keyId);
             }
-        } else {
-            throw new 
SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
-                    "KeyExchange signature verification failed, could not 
determine connect host for key ID=" + keyId);
         }
 
         if (!GenericUtils.isEmpty(openSshKey.getCriticalOptions())) {
diff --git 
a/sshd-core/src/test/java/org/apache/sshd/common/signature/KnownHostsCertificateTest.java
 
b/sshd-core/src/test/java/org/apache/sshd/common/signature/KnownHostsCertificateTest.java
index 7e40f69c4..4cf041fe0 100644
--- 
a/sshd-core/src/test/java/org/apache/sshd/common/signature/KnownHostsCertificateTest.java
+++ 
b/sshd-core/src/test/java/org/apache/sshd/common/signature/KnownHostsCertificateTest.java
@@ -21,9 +21,8 @@ package org.apache.sshd.common.signature;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.security.KeyPair;
-import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
-import java.util.List;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Stream;
 
@@ -99,18 +98,21 @@ class KnownHostsCertificateTest extends BaseTestSupport {
 
     private void initKeys(String keyType, int keySize, String caType, int 
caSize, String signatureAlgorithm, String marker)
             throws Exception {
+        initKeys(keyType, keySize, caType, caSize, signatureAlgorithm, marker, 
"localhost", "127.0.0.1");
+    }
+
+    private void initKeys(
+            String keyType, int keySize, String caType, int caSize, String 
signatureAlgorithm, String marker,
+            String... principals) throws Exception {
         caKey = CommonTestSupportUtils.generateKeyPair(caType, caSize);
         KeyPair hostKeyPair = CommonTestSupportUtils.generateKeyPair(keyType, 
keySize);
         // Generate a host certificate.
-        List<String> principals = new ArrayList<>();
-        principals.add("localhost");
-        principals.add("127.0.0.1");
         OpenSshCertificate signedCert = 
OpenSshCertificateBuilder.hostCertificate() //
                 .serial(System.currentTimeMillis()) //
                 .publicKey(hostKeyPair.getPublic()) //
                 .id("test-cert-" + signatureAlgorithm) //
                 .validBefore(System.currentTimeMillis() + 
TimeUnit.HOURS.toMillis(1)) //
-                .principals(principals) //
+                .principals(Arrays.asList(principals)) //
                 .sign(caKey, signatureAlgorithm);
         hostKey = hostKeyPair;
         // new KeyPair(signedCert, hostKeyPair.getPrivate());
@@ -152,4 +154,15 @@ class KnownHostsCertificateTest extends BaseTestSupport {
             s.auth().verify(AUTH_TIMEOUT);
         }
     }
+
+    @Test
+    void testHostCertificateWithoutPrincipalsSucceeds() throws Exception {
+        initKeys(KeyUtils.EC_ALGORITHM, 256, KeyUtils.EC_ALGORITHM, 256, 
"ecdsa-sha2-nistp256", "cert-authority",
+                new String[0]);
+        try (ClientSession s = client.connect(getCurrentTestName(), 
TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT)
+                .getSession()) {
+            s.addPasswordIdentity(getCurrentTestName());
+            s.auth().verify(AUTH_TIMEOUT);
+        }
+    }
 }

Reply via email to