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); + } + } }