You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by tw...@apache.org on 2025/02/27 22:10:08 UTC
(mina-sshd) branch master updated (1e27daaec -> e32c262e5)
This is an automated email from the ASF dual-hosted git repository.
twolf pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
from 1e27daaec OpenSshCertificate: access options and extensions as maps
new 16fbd1972 KEX: Fix key exchange with host certificate
new baa97441b KEX: guard client against buggy servers
new 69102099c KEX: by default abort on invalid certificates
new 18e30f691 [SSHD-1167] KEX: handle "@cert-authority" in known_hosts files
new e32c262e5 KEX: fix host certificate w/o principals
The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
CHANGES.md | 4 +
.../apache/sshd/common/config/keys/KeyUtils.java | 6 +
.../sshd/common/signature/AbstractSignature.java | 12 +-
.../apache/sshd/common/signature/SignatureDSA.java | 2 +-
.../sshd/common/signature/SignatureECDSA.java | 10 +-
.../apache/sshd/common/signature/SignatureRSA.java | 16 +-
.../eddsa/generic/GenericSignatureEd25519.java | 3 +-
.../java/org/apache/sshd/client/kex/DHGClient.java | 103 +++++++------
.../keyverifier/KnownHostsServerKeyVerifier.java | 20 ++-
.../keyverifier/RequiredServerKeyVerifier.java | 14 +-
.../sshd/client/session/AbstractClientSession.java | 26 +---
.../org/apache/sshd/core/CoreModuleProperties.java | 2 +-
.../signature/KnownHostsCertificateTest.java | 168 +++++++++++++++++++++
.../common/signature/OpenSSHCertificateTest.java | 4 -
.../signature/OpenSshHostCertificateTest.java | 128 ++++++++++++++++
.../sshd/util/test/CoreTestSupportUtils.java | 59 +++++++-
16 files changed, 468 insertions(+), 109 deletions(-)
create mode 100644 sshd-core/src/test/java/org/apache/sshd/common/signature/KnownHostsCertificateTest.java
create mode 100644 sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSshHostCertificateTest.java
(mina-sshd) 03/05: KEX: by default abort on invalid certificates
Posted by tw...@apache.org.
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 69102099c963f4214989b0049322875c9abeeac0
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Sun Feb 16 19:29:23 2025 +0100
KEX: by default abort on invalid certificates
Switch the default of CoreModuleProperties.ABORT_ON_INVALID_CERTIFICATE
from false to true.
Unless we get tons of bug reports due to this I plan to deprecate and
remove this setting altogether. IMO an SSH client should *never* accept
connections if a certificate key exchange is used and the certificate
the server sent is invalid. (Expired, wrongly signed, not listing the
host name or IP in the principals, or whatever.)
---
CHANGES.md | 3 +++
sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/CHANGES.md b/CHANGES.md
index f163fdeaf..323116c15 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -37,5 +37,8 @@
## Potential Compatibility Issues
+Client-side KEX: we've changed the default of the setting `CoreModuleProperties.ABORT_ON_INVALID_CERTIFICATE` from `false` to `true`.
+A client will newly abort an SSH connection if the server presents an invalid OpenSSH host certificate as host key.
+
## Major Code Re-factoring
diff --git a/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java b/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java
index d130648d5..2bec5c3a3 100644
--- a/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java
+++ b/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java
@@ -190,7 +190,7 @@ public final class CoreModuleProperties {
* Defines if we should abort in case we encounter an invalid (e.g. expired) openssh certificate.
*/
public static final Property<Boolean> ABORT_ON_INVALID_CERTIFICATE
- = Property.bool("abort-on-invalid-certificate", false);
+ = Property.bool("abort-on-invalid-certificate", true);
/**
* As per RFC-4256:
(mina-sshd) 05/05: KEX: fix host certificate w/o principals
Posted by tw...@apache.org.
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);
+ }
+ }
}
(mina-sshd) 01/05: KEX: Fix key exchange with host certificate
Posted by tw...@apache.org.
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 16fbd1972fab6b6c3f1d60e77f52434e980cf688
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Mon Feb 3 22:20:36 2025 +0100
KEX: Fix key exchange with host certificate
Previous code only worked for certificates using an RSA key as CA
key. But it is perfectly valid to use some other key type as CA key.
Fix the signature implementations so that the server writes the correct
signature types.
Fix the client to handle not only RSA signatures on certificates.
Fix the tests to actually give the test client and server more
meaningful signature factory orders. Just using BuiltinSignatures.VALUES
places the "-cert@openssh.com" values after the normal ones. They should
come before.
---
.../sshd/common/signature/AbstractSignature.java | 12 +-
.../apache/sshd/common/signature/SignatureDSA.java | 2 +-
.../sshd/common/signature/SignatureECDSA.java | 10 +-
.../apache/sshd/common/signature/SignatureRSA.java | 16 +--
.../eddsa/generic/GenericSignatureEd25519.java | 3 +-
.../java/org/apache/sshd/client/kex/DHGClient.java | 42 +++----
.../common/signature/OpenSSHCertificateTest.java | 4 -
.../signature/OpenSshHostCertificateTest.java | 129 +++++++++++++++++++++
.../sshd/util/test/CoreTestSupportUtils.java | 59 +++++++++-
9 files changed, 220 insertions(+), 57 deletions(-)
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/signature/AbstractSignature.java b/sshd-common/src/main/java/org/apache/sshd/common/signature/AbstractSignature.java
index c687dd723..de645ff58 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/signature/AbstractSignature.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/signature/AbstractSignature.java
@@ -43,11 +43,16 @@ import org.apache.sshd.common.util.security.SecurityUtils;
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public abstract class AbstractSignature implements Signature {
+
private java.security.Signature signatureInstance;
private final String algorithm;
- protected AbstractSignature(String algorithm) {
+ private final String sshAlgorithmName;
+
+ protected AbstractSignature(String algorithm, String sshAlgorithmName) {
this.algorithm = ValidateUtils.checkNotNullAndNotEmpty(algorithm, "No signature algorithm specified");
+ this.sshAlgorithmName = ValidateUtils.checkNotNullAndNotEmpty(sshAlgorithmName,
+ "Missing protocol name of the signature algorithm.");
}
@Override
@@ -55,6 +60,11 @@ public abstract class AbstractSignature implements Signature {
return algorithm;
}
+ @Override
+ public String getSshAlgorithmName(String algo) {
+ return sshAlgorithmName;
+ }
+
/**
* Initializes the internal signature instance
*
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureDSA.java b/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureDSA.java
index b5620be43..3848b14a9 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureDSA.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureDSA.java
@@ -49,7 +49,7 @@ public class SignatureDSA extends AbstractSignature {
}
protected SignatureDSA(String algorithm) {
- super(algorithm);
+ super(algorithm, KeyPairProvider.SSH_DSS);
}
@Override
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureECDSA.java b/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureECDSA.java
index 3939d3d18..88bb470f9 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureECDSA.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureECDSA.java
@@ -41,7 +41,7 @@ public class SignatureECDSA extends AbstractSignature {
public static final String DEFAULT_ALGORITHM = "SHA256withECDSA";
public SignatureECDSA256() {
- super(DEFAULT_ALGORITHM);
+ super(DEFAULT_ALGORITHM, ECCurves.nistp256.getKeyType());
}
}
@@ -49,7 +49,7 @@ public class SignatureECDSA extends AbstractSignature {
public static final String DEFAULT_ALGORITHM = "SHA384withECDSA";
public SignatureECDSA384() {
- super(DEFAULT_ALGORITHM);
+ super(DEFAULT_ALGORITHM, ECCurves.nistp384.getKeyType());
}
}
@@ -57,12 +57,12 @@ public class SignatureECDSA extends AbstractSignature {
public static final String DEFAULT_ALGORITHM = "SHA512withECDSA";
public SignatureECDSA521() {
- super(DEFAULT_ALGORITHM);
+ super(DEFAULT_ALGORITHM, ECCurves.nistp521.getKeyType());
}
}
- protected SignatureECDSA(String algo) {
- super(algo);
+ protected SignatureECDSA(String algo, String sshAlgorithmName) {
+ super(algo, sshAlgorithmName);
}
@Override
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureRSA.java b/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureRSA.java
index 97f9ed978..914cf1e8b 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureRSA.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureRSA.java
@@ -43,26 +43,14 @@ public abstract class SignatureRSA extends AbstractSignature {
public static final NavigableSet<String> SUPPORTED_KEY_TYPES = Collections.unmodifiableNavigableSet(
Stream.of(
KeyPairProvider.SSH_RSA,
- KeyPairProvider.SSH_RSA_CERT,
KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA256_CERT_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_CERT_TYPE_ALIAS)
+ KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS)
.collect(Collectors.toCollection(() -> new TreeSet<>(String.CASE_INSENSITIVE_ORDER))));
private int verifierSignatureSize = -1;
- private final String sshAlgorithmName;
-
protected SignatureRSA(String algorithm, String sshAlgorithmName) {
- super(algorithm);
- this.sshAlgorithmName = ValidateUtils.checkNotNullAndNotEmpty(sshAlgorithmName,
- "Missing protocol name of the signature algorithm.");
- }
-
- @Override
- public String getSshAlgorithmName(String algo) {
- return sshAlgorithmName;
+ super(algorithm, sshAlgorithmName);
}
/**
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/security/eddsa/generic/GenericSignatureEd25519.java b/sshd-common/src/main/java/org/apache/sshd/common/util/security/eddsa/generic/GenericSignatureEd25519.java
index 99c54fa79..943c473d5 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/util/security/eddsa/generic/GenericSignatureEd25519.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/util/security/eddsa/generic/GenericSignatureEd25519.java
@@ -29,8 +29,9 @@ import org.apache.sshd.common.util.ValidateUtils;
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class GenericSignatureEd25519 extends AbstractSignature {
+
public GenericSignatureEd25519(String algorithm) {
- super(algorithm);
+ super(algorithm, KeyPairProvider.SSH_ED25519);
}
@Override
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 3e3412c22..baebc67e6 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
@@ -39,7 +39,6 @@ import org.apache.sshd.common.kex.KexProposalOption;
import org.apache.sshd.common.kex.KeyEncapsulationMethod;
import org.apache.sshd.common.kex.KeyExchange;
import org.apache.sshd.common.kex.KeyExchangeFactory;
-import org.apache.sshd.common.keyprovider.KeyPairProvider;
import org.apache.sshd.common.session.Session;
import org.apache.sshd.common.signature.Signature;
import org.apache.sshd.common.util.GenericUtils;
@@ -235,41 +234,32 @@ public class DHGClient extends AbstractDHClientKeyExchange {
String keyAlg = KeyUtils.getKeyType(signatureKey);
String keyId = openSshKey.getId();
- // allow sha2 signatures for legacy reasons
- String variant = openSshKey.getSignatureAlgorithm();
- if ((!GenericUtils.isEmpty(variant))
- && KeyPairProvider.SSH_RSA.equals(KeyUtils.getCanonicalKeyType(variant))) {
- if (log.isDebugEnabled()) {
- log.debug("verifyCertificate({})[id={}] Allowing to use variant {} instead of {}",
- session, keyId, variant, keyAlg);
- }
- keyAlg = variant;
- } else {
+ if (!OpenSshCertificate.Type.HOST.equals(openSshKey.getType())) {
throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
- "Found invalid signature alg " + variant + " for key ID=" + keyId);
+ "KeyExchange signature verification failed, not a host key (2) " + openSshKey.getType() + " for key ID="
+ + keyId);
}
- Signature verif = ValidateUtils.checkNotNull(
- NamedFactory.create(session.getSignatureFactories(), keyAlg),
- "No KeyExchange CA verifier located for algorithm=%s of key ID=%s", keyAlg, keyId);
- verif.initVerifier(session, signatureKey);
- verif.update(session, openSshKey.getMessage());
-
- if (!verif.verify(session, openSshKey.getSignature())) {
+ if (!OpenSshCertificate.isValidNow(openSshKey)) {
throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
- "KeyExchange CA signature verification failed for key type=" + keyAlg + " of key ID=" + keyId);
+ "KeyExchange signature verification failed, CA expired for key ID=" + keyId);
}
- if (!OpenSshCertificate.Type.HOST.equals(openSshKey.getType())) {
+ String sigAlg = openSshKey.getSignatureAlgorithm();
+ if (!keyAlg.equals(KeyUtils.getCanonicalKeyType(sigAlg))) {
throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
- "KeyExchange signature verification failed, not a host key (2) "
- + openSshKey.getType() + " for key ID="
- + keyId);
+ "Found invalid signature alg " + sigAlg + " for key ID=" + keyId + " using a " + keyAlg + " CA key");
}
- if (!OpenSshCertificate.isValidNow(openSshKey)) {
+ Signature verif = ValidateUtils.checkNotNull(
+ NamedFactory.create(session.getSignatureFactories(), sigAlg),
+ "No KeyExchange CA verifier located for algorithm=%s of key ID=%s", sigAlg, keyId);
+ verif.initVerifier(session, signatureKey);
+ verif.update(session, openSshKey.getMessage());
+
+ if (!verif.verify(session, openSshKey.getSignature())) {
throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
- "KeyExchange signature verification failed, CA expired for key ID=" + keyId);
+ "KeyExchange CA signature verification failed for key type=" + keyAlg + " of key ID=" + keyId);
}
/*
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSSHCertificateTest.java b/sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSSHCertificateTest.java
index 002b10cfe..e47c961d1 100644
--- a/sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSSHCertificateTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSSHCertificateTest.java
@@ -45,10 +45,6 @@ import org.junit.jupiter.api.TestMethodOrder;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
@TestMethodOrder(MethodName.class) // see https://github.com/junit-team/junit/wiki/Parameterized-tests
public class OpenSSHCertificateTest extends BaseTestSupport {
private static SshServer sshd;
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSshHostCertificateTest.java b/sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSshHostCertificateTest.java
new file mode 100644
index 000000000..74369d933
--- /dev/null
+++ b/sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSshHostCertificateTest.java
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sshd.common.signature;
+
+import java.security.KeyPair;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Stream;
+
+import org.apache.sshd.certificate.OpenSshCertificateBuilder;
+import org.apache.sshd.client.SshClient;
+import org.apache.sshd.client.session.ClientSession;
+import org.apache.sshd.common.config.keys.KeyUtils;
+import org.apache.sshd.common.config.keys.OpenSshCertificate;
+import org.apache.sshd.common.keyprovider.KeyPairProvider;
+import org.apache.sshd.server.SshServer;
+import org.apache.sshd.util.test.BaseTestSupport;
+import org.apache.sshd.util.test.CommonTestSupportUtils;
+import org.apache.sshd.util.test.CoreTestSupportUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests for KEX with host certificates.
+ */
+class OpenSshHostCertificateTest extends BaseTestSupport {
+
+ private static SshServer sshd;
+ private static SshClient client;
+ private static int port;
+
+ private KeyPair hostKey;
+ private KeyPair caKey;
+
+ @BeforeAll
+ private static void setupClientAndServer() throws Exception {
+ sshd = CoreTestSupportUtils.setupTestFullSupportServer(OpenSshHostCertificateTest.class);
+ sshd.start();
+ port = sshd.getPort();
+
+ client = CoreTestSupportUtils.setupTestFullSupportClient(OpenSshHostCertificateTest.class);
+ client.start();
+ }
+
+ @AfterAll
+ static void tearDownClientAndServer() throws Exception {
+ if (sshd != null) {
+ try {
+ sshd.stop(true);
+ } finally {
+ sshd = null;
+ }
+ }
+
+ if (client != null) {
+ try {
+ client.stop();
+ } finally {
+ client = null;
+ }
+ }
+ }
+
+ private static Stream<Arguments> certificateAlgorithms() {
+ return Stream.of( //
+ Arguments.of(KeyUtils.RSA_ALGORITHM, 2048, KeyUtils.RSA_ALGORITHM, 2048, "rsa-sha2-512"),
+ Arguments.of(KeyUtils.EC_ALGORITHM, 256, KeyUtils.EC_ALGORITHM, 256, "ecdsa-sha2-nistp256"),
+ Arguments.of(KeyUtils.RSA_ALGORITHM, 2048, KeyUtils.EC_ALGORITHM, 256, "ecdsa-sha2-nistp256"),
+ Arguments.of(KeyUtils.EC_ALGORITHM, 256, KeyUtils.RSA_ALGORITHM, 2048, "rsa-sha2-512"));
+ }
+
+ private void initKeys(String keyType, int keySize, String caType, int caSize, String signatureAlgorithm) 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) //
+ .sign(caKey, signatureAlgorithm);
+ hostKey = hostKeyPair;
+ // new KeyPair(signedCert, hostKeyPair.getPrivate());
+ sshd.setKeyPairProvider(KeyPairProvider.wrap(hostKey));
+ sshd.setHostKeyCertificateProvider(session -> Collections.singletonList(signedCert));
+ client.setServerKeyVerifier((session, address, key) -> {
+ // TODO: we get the CA key here. This is wrong; the serverKeyVerifier should get the OpenSshCertificate and
+ // be responsible itself for checking its CA key against registered trusted CA keys.
+ return KeyUtils.compareKeys(caKey.getPublic(), key);
+ });
+ }
+
+ @ParameterizedTest(name = "test host certificate {0} signed with {2}")
+ @MethodSource("certificateAlgorithms")
+ void testHostCertificate(String keyType, int keySize, String caType, int caSize, String signatureAlgorithm)
+ throws Exception {
+ initKeys(keyType, keySize, caType, caSize, signatureAlgorithm);
+ try (ClientSession s = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT)
+ .getSession()) {
+ s.addPasswordIdentity(getCurrentTestName());
+ s.auth().verify(AUTH_TIMEOUT);
+ }
+ }
+}
diff --git a/sshd-core/src/test/java/org/apache/sshd/util/test/CoreTestSupportUtils.java b/sshd-core/src/test/java/org/apache/sshd/util/test/CoreTestSupportUtils.java
index 686d2de03..74a05bf36 100644
--- a/sshd-core/src/test/java/org/apache/sshd/util/test/CoreTestSupportUtils.java
+++ b/sshd-core/src/test/java/org/apache/sshd/util/test/CoreTestSupportUtils.java
@@ -22,17 +22,19 @@ import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.ServerSocket;
import java.time.Duration;
-import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
import org.apache.sshd.client.ClientBuilder;
import org.apache.sshd.client.SshClient;
import org.apache.sshd.client.config.hosts.HostConfigEntryResolver;
import org.apache.sshd.client.keyverifier.AcceptAllServerKeyVerifier;
import org.apache.sshd.common.NamedFactory;
-import org.apache.sshd.common.helpers.AbstractFactoryManager;
import org.apache.sshd.common.kex.BuiltinDHFactories;
import org.apache.sshd.common.keyprovider.KeyIdentityProvider;
import org.apache.sshd.common.signature.BuiltinSignatures;
+import org.apache.sshd.common.signature.Signature;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.core.CoreModuleProperties;
import org.apache.sshd.server.ServerBuilder;
@@ -106,9 +108,56 @@ public final class CoreTestSupportUtils {
return sshd;
}
- public static <M extends AbstractFactoryManager> M setupFullSignaturesSupport(M manager) {
- manager.setSignatureFactories(new ArrayList<>(BuiltinSignatures.VALUES));
- return manager;
+ @SuppressWarnings("deprecation")
+ public static SshServer setupFullSignaturesSupport(SshServer server) {
+ List<NamedFactory<Signature>> signatures = Stream.of( //
+ BuiltinSignatures.nistp256_cert, //
+ BuiltinSignatures.nistp384_cert, //
+ BuiltinSignatures.nistp521_cert, //
+ BuiltinSignatures.ed25519_cert, //
+ BuiltinSignatures.rsaSHA512_cert, //
+ BuiltinSignatures.rsaSHA256_cert, //
+ BuiltinSignatures.rsa_cert, //
+ BuiltinSignatures.nistp256, //
+ BuiltinSignatures.nistp384, //
+ BuiltinSignatures.nistp521, //
+ BuiltinSignatures.ed25519, //
+ BuiltinSignatures.rsaSHA512, //
+ BuiltinSignatures.rsaSHA256, //
+ BuiltinSignatures.rsa, //
+ BuiltinSignatures.dsa_cert, //
+ BuiltinSignatures.dsa) //
+ .filter(BuiltinSignatures::isSupported) //
+ .collect(Collectors.toList());
+ server.setSignatureFactories(signatures);
+ return server;
+ }
+
+ @SuppressWarnings("deprecation")
+ public static SshClient setupFullSignaturesSupport(SshClient client) {
+ List<NamedFactory<Signature>> signatures = Stream.of( //
+ BuiltinSignatures.nistp256_cert, //
+ BuiltinSignatures.nistp384_cert, //
+ BuiltinSignatures.nistp521_cert, //
+ BuiltinSignatures.ed25519_cert, //
+ BuiltinSignatures.rsaSHA512_cert, //
+ BuiltinSignatures.rsaSHA256_cert, //
+ BuiltinSignatures.rsa_cert, //
+ BuiltinSignatures.nistp256, //
+ BuiltinSignatures.nistp384, //
+ BuiltinSignatures.nistp521, //
+ BuiltinSignatures.ed25519, //
+ BuiltinSignatures.sk_ecdsa_sha2_nistp256, //
+ BuiltinSignatures.sk_ssh_ed25519, //
+ BuiltinSignatures.rsaSHA512, //
+ BuiltinSignatures.rsaSHA256, //
+ BuiltinSignatures.rsa, //
+ BuiltinSignatures.dsa_cert, //
+ BuiltinSignatures.dsa) //
+ .filter(BuiltinSignatures::isSupported) //
+ .collect(Collectors.toList());
+ client.setSignatureFactories(signatures);
+ return client;
}
public static Duration getTimeout(String property, Duration defaultValue) {
(mina-sshd) 04/05: [SSHD-1167] KEX: handle "@cert-authority" in known_hosts files
Posted by tw...@apache.org.
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 18e30f69174bb693a6cc622bcc8d9fdb033546f0
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Sun Feb 16 21:50:37 2025 +0100
[SSHD-1167] KEX: handle "@cert-authority" in known_hosts files
Implement the OpenSSH behavior for checking of certificates: if the
server key is a certificate, only entries with markers @revoked or
@cert-authority are considered. There must be an entry marked as
@cert-authority matching the certificate's CA public key, otherwise the
server key is rejected. Unknown CA keys are never added to the file.
Move the responsibility for this check from AbstractClientSession to
the ServerKeyVerifier. Note that this means that the provided
implementation does *not* fall back to checking the certificate's
public key (i.e., the one that was signed by the CA key). If users
want that (insecure) behavior, they have to provide their own
ServerKeyVerifier that implements it.
---
CHANGES.md | 1 +
.../keyverifier/KnownHostsServerKeyVerifier.java | 20 +++++--
.../keyverifier/RequiredServerKeyVerifier.java | 14 ++++-
.../sshd/client/session/AbstractClientSession.java | 26 ++-------
...ateTest.java => KnownHostsCertificateTest.java} | 68 +++++++++++++++-------
.../signature/OpenSshHostCertificateTest.java | 5 +-
6 files changed, 83 insertions(+), 51 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 323116c15..60c3694c2 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -34,6 +34,7 @@
* [SSHD-1161](https://issues.apache.org/jira/projects/SSHD/issues/SSHD-1161) Support pubkey auth with user certificates (server-side)
* Client-side support was introduced in version 2.8.0 already
+* [SSHD-1167](https://issues.apache.org/jira/projects/SSHD/issues/SSHD-1167) Check host certificates against known_hosts file (implements @<!-- -->cert-authority)
## Potential Compatibility Issues
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/keyverifier/KnownHostsServerKeyVerifier.java b/sshd-core/src/main/java/org/apache/sshd/client/keyverifier/KnownHostsServerKeyVerifier.java
index 88699f503..10d97b1d6 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/keyverifier/KnownHostsServerKeyVerifier.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/keyverifier/KnownHostsServerKeyVerifier.java
@@ -51,6 +51,7 @@ import org.apache.sshd.common.NamedFactory;
import org.apache.sshd.common.config.ConfigFileReaderSupport;
import org.apache.sshd.common.config.keys.AuthorizedKeyEntry;
import org.apache.sshd.common.config.keys.KeyUtils;
+import org.apache.sshd.common.config.keys.OpenSshCertificate;
import org.apache.sshd.common.config.keys.PublicKeyEntry;
import org.apache.sshd.common.config.keys.PublicKeyEntryResolver;
import org.apache.sshd.common.config.keys.UnsupportedSshPublicKey;
@@ -276,11 +277,15 @@ public class KnownHostsServerKeyVerifier
return acceptUnknownHostKey(clientSession, remoteAddress, serverKey);
}
- String serverKeyType = KeyUtils.getKeyType(serverKey);
+ PublicKey keyToCheck = serverKey instanceof OpenSshCertificate
+ ? ((OpenSshCertificate) serverKey).getCaPubKey()
+ : serverKey;
+ boolean isCert = serverKey instanceof OpenSshCertificate;
+ String keyType = KeyUtils.getKeyType(keyToCheck);
List<HostEntryPair> keyMatches = hostMatches.stream()
- .filter(entry -> serverKeyType.equals(entry.getHostEntry().getKeyEntry().getKeyType()))
- .filter(k -> KeyUtils.compareKeys(k.getServerKey(), serverKey))
+ .filter(entry -> keyType.equals(entry.getHostEntry().getKeyEntry().getKeyType()))
+ .filter(k -> KeyUtils.compareKeys(k.getServerKey(), keyToCheck))
.collect(Collectors.toList());
if (keyMatches.stream()
@@ -289,10 +294,17 @@ public class KnownHostsServerKeyVerifier
return false;
}
+ keyMatches = keyMatches.stream()
+ .filter(e -> isCert == "cert-authority".equals(e.getHostEntry().getMarker()))
+ .collect(Collectors.toList());
if (!keyMatches.isEmpty()) {
return true;
}
+ if (isCert) {
+ return false;
+ }
+
Optional<HostEntryPair> anyNonRevokedMatch = hostMatches.stream()
.filter(k -> !"revoked".equals(k.getHostEntry().getMarker()))
.findAny();
@@ -583,7 +595,7 @@ public class KnownHostsServerKeyVerifier
clientSession, remoteAddress, KeyUtils.getFingerPrint(serverKey));
}
- if (delegate.verifyServerKey(clientSession, remoteAddress, serverKey)) {
+ if (delegate.verifyServerKey(clientSession, remoteAddress, serverKey) && !(serverKey instanceof OpenSshCertificate)) {
Path file = getPath();
Collection<HostEntryPair> keys = keysSupplier.get().get();
try {
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/keyverifier/RequiredServerKeyVerifier.java b/sshd-core/src/main/java/org/apache/sshd/client/keyverifier/RequiredServerKeyVerifier.java
index bbdc5c286..47d8c7af1 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/keyverifier/RequiredServerKeyVerifier.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/keyverifier/RequiredServerKeyVerifier.java
@@ -22,6 +22,8 @@ import java.net.SocketAddress;
import java.security.PublicKey;
import org.apache.sshd.client.session.ClientSession;
+import org.apache.sshd.common.config.keys.KeyUtils;
+import org.apache.sshd.common.config.keys.OpenSshCertificate;
import org.apache.sshd.common.util.buffer.BufferUtils;
import org.apache.sshd.common.util.logging.AbstractLoggingBean;
@@ -43,7 +45,17 @@ public class RequiredServerKeyVerifier extends AbstractLoggingBean implements Se
@Override
public boolean verifyServerKey(ClientSession sshClientSession, SocketAddress remoteAddress, PublicKey serverKey) {
- if (requiredKey.equals(serverKey)) {
+ boolean sameKey = KeyUtils.compareKeys(requiredKey, serverKey);
+ if (!sameKey) {
+ PublicKey req = (requiredKey instanceof OpenSshCertificate)
+ ? ((OpenSshCertificate) requiredKey).getCaPubKey()
+ : requiredKey;
+ PublicKey srv = (serverKey instanceof OpenSshCertificate)
+ ? ((OpenSshCertificate) serverKey).getCaPubKey()
+ : serverKey;
+ sameKey = KeyUtils.compareKeys(req, srv);
+ }
+ if (sameKey) {
if (log.isDebugEnabled()) {
log.debug("Server at {} presented expected key: {}", remoteAddress, BufferUtils.toHex(serverKey.getEncoded()));
}
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java b/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java
index 5bde72610..62513c847 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java
@@ -58,7 +58,6 @@ import org.apache.sshd.common.channel.PtyChannelConfigurationHolder;
import org.apache.sshd.common.cipher.BuiltinCiphers;
import org.apache.sshd.common.cipher.CipherNone;
import org.apache.sshd.common.config.keys.KeyUtils;
-import org.apache.sshd.common.config.keys.OpenSshCertificate;
import org.apache.sshd.common.forward.Forwarder;
import org.apache.sshd.common.future.DefaultKeyExchangeFuture;
import org.apache.sshd.common.future.KeyExchangeFuture;
@@ -618,27 +617,10 @@ public abstract class AbstractClientSession extends AbstractSession implements C
remoteAddress = targetServerAddress.toInetSocketAddress();
}
- boolean verified = false;
- if (serverKey instanceof OpenSshCertificate) {
- // check if we trust the CA
- verified = serverKeyVerifier.verifyServerKey(this, remoteAddress, ((OpenSshCertificate) serverKey).getCaPubKey());
- if (log.isDebugEnabled()) {
- log.debug("checkCA({}) key={}-{}, verified={}",
- this, KeyUtils.getKeyType(serverKey), KeyUtils.getFingerPrint(serverKey), verified);
- }
-
- if (!verified) {
- // fallback to actual public host key
- serverKey = ((OpenSshCertificate) serverKey).getCertPubKey();
- }
- }
-
- if (!verified) {
- verified = serverKeyVerifier.verifyServerKey(this, remoteAddress, serverKey);
- if (log.isDebugEnabled()) {
- log.debug("checkKeys({}) key={}-{}, verified={}",
- this, KeyUtils.getKeyType(serverKey), KeyUtils.getFingerPrint(serverKey), verified);
- }
+ boolean verified = serverKeyVerifier.verifyServerKey(this, remoteAddress, serverKey);
+ if (log.isDebugEnabled()) {
+ log.debug("checkKeys({}) key={}-{}, verified={}", this, KeyUtils.getKeyType(serverKey),
+ KeyUtils.getFingerPrint(serverKey), verified);
}
if (!verified) {
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSshHostCertificateTest.java b/sshd-core/src/test/java/org/apache/sshd/common/signature/KnownHostsCertificateTest.java
similarity index 62%
copy from sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSshHostCertificateTest.java
copy to sshd-core/src/test/java/org/apache/sshd/common/signature/KnownHostsCertificateTest.java
index 74369d933..7e40f69c4 100644
--- a/sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSshHostCertificateTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/common/signature/KnownHostsCertificateTest.java
@@ -18,6 +18,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.Collections;
@@ -27,39 +29,48 @@ import java.util.stream.Stream;
import org.apache.sshd.certificate.OpenSshCertificateBuilder;
import org.apache.sshd.client.SshClient;
+import org.apache.sshd.client.keyverifier.AcceptAllServerKeyVerifier;
+import org.apache.sshd.client.keyverifier.KnownHostsServerKeyVerifier;
import org.apache.sshd.client.session.ClientSession;
+import org.apache.sshd.common.SshException;
import org.apache.sshd.common.config.keys.KeyUtils;
import org.apache.sshd.common.config.keys.OpenSshCertificate;
+import org.apache.sshd.common.config.keys.PublicKeyEntry;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
+import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.server.SshServer;
import org.apache.sshd.util.test.BaseTestSupport;
import org.apache.sshd.util.test.CommonTestSupportUtils;
import org.apache.sshd.util.test.CoreTestSupportUtils;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
/**
- * Tests for KEX with host certificates.
+ * Tests for KEX with host certificates with host key validation through a {@link KnownHostsServerKeyVerifier}.
*/
-class OpenSshHostCertificateTest extends BaseTestSupport {
+class KnownHostsCertificateTest extends BaseTestSupport {
private static SshServer sshd;
private static SshClient client;
private static int port;
+ @TempDir
+ private Path tmp;
+
private KeyPair hostKey;
private KeyPair caKey;
@BeforeAll
private static void setupClientAndServer() throws Exception {
- sshd = CoreTestSupportUtils.setupTestFullSupportServer(OpenSshHostCertificateTest.class);
+ sshd = CoreTestSupportUtils.setupTestFullSupportServer(KnownHostsCertificateTest.class);
sshd.start();
port = sshd.getPort();
- client = CoreTestSupportUtils.setupTestFullSupportClient(OpenSshHostCertificateTest.class);
+ client = CoreTestSupportUtils.setupTestFullSupportClient(KnownHostsCertificateTest.class);
client.start();
}
@@ -82,15 +93,12 @@ class OpenSshHostCertificateTest extends BaseTestSupport {
}
}
- private static Stream<Arguments> certificateAlgorithms() {
- return Stream.of( //
- Arguments.of(KeyUtils.RSA_ALGORITHM, 2048, KeyUtils.RSA_ALGORITHM, 2048, "rsa-sha2-512"),
- Arguments.of(KeyUtils.EC_ALGORITHM, 256, KeyUtils.EC_ALGORITHM, 256, "ecdsa-sha2-nistp256"),
- Arguments.of(KeyUtils.RSA_ALGORITHM, 2048, KeyUtils.EC_ALGORITHM, 256, "ecdsa-sha2-nistp256"),
- Arguments.of(KeyUtils.EC_ALGORITHM, 256, KeyUtils.RSA_ALGORITHM, 2048, "rsa-sha2-512"));
+ private static Stream<String> markers() {
+ return Stream.of("rejected", "", null);
}
- private void initKeys(String keyType, int keySize, String caType, int caSize, String signatureAlgorithm) throws Exception {
+ private void initKeys(String keyType, int keySize, String caType, int caSize, String signatureAlgorithm, String marker)
+ throws Exception {
caKey = CommonTestSupportUtils.generateKeyPair(caType, caSize);
KeyPair hostKeyPair = CommonTestSupportUtils.generateKeyPair(keyType, keySize);
// Generate a host certificate.
@@ -108,18 +116,36 @@ class OpenSshHostCertificateTest extends BaseTestSupport {
// new KeyPair(signedCert, hostKeyPair.getPrivate());
sshd.setKeyPairProvider(KeyPairProvider.wrap(hostKey));
sshd.setHostKeyCertificateProvider(session -> Collections.singletonList(signedCert));
- client.setServerKeyVerifier((session, address, key) -> {
- // TODO: we get the CA key here. This is wrong; the serverKeyVerifier should get the OpenSshCertificate and
- // be responsible itself for checking its CA key against registered trusted CA keys.
- return KeyUtils.compareKeys(caKey.getPublic(), key);
+ Path knownHosts = tmp.resolve("known_hosts");
+ if (marker != null) {
+ StringBuilder line = new StringBuilder();
+ if (!GenericUtils.isEmpty(marker)) {
+ line.append('@').append(marker).append(' ');
+ }
+ line.append("[localhost]:").append(port).append(",[127.0.0.1]:").append(port).append(' ');
+ line.append(PublicKeyEntry.toString(caKey.getPublic()));
+ line.append('\n');
+ Files.write(knownHosts, Collections.singletonList(line.toString()));
+ }
+ client.setServerKeyVerifier(new KnownHostsServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE, knownHosts));
+ }
+
+ @ParameterizedTest(name = "test {0} CA key")
+ @MethodSource("markers")
+ void testHostCertificateFails(String marker) throws Exception {
+ initKeys(KeyUtils.EC_ALGORITHM, 256, KeyUtils.EC_ALGORITHM, 256, "ecdsa-sha2-nistp256", marker);
+ assertThrows(SshException.class, () -> {
+ try (ClientSession s = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT)
+ .getSession()) {
+ s.addPasswordIdentity(getCurrentTestName());
+ s.auth().verify(AUTH_TIMEOUT);
+ }
});
}
- @ParameterizedTest(name = "test host certificate {0} signed with {2}")
- @MethodSource("certificateAlgorithms")
- void testHostCertificate(String keyType, int keySize, String caType, int caSize, String signatureAlgorithm)
- throws Exception {
- initKeys(keyType, keySize, caType, caSize, signatureAlgorithm);
+ @Test
+ void testHostCertificateSucceeds() throws Exception {
+ initKeys(KeyUtils.EC_ALGORITHM, 256, KeyUtils.EC_ALGORITHM, 256, "ecdsa-sha2-nistp256", "cert-authority");
try (ClientSession s = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(CONNECT_TIMEOUT)
.getSession()) {
s.addPasswordIdentity(getCurrentTestName());
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSshHostCertificateTest.java b/sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSshHostCertificateTest.java
index 74369d933..8aa52a2cd 100644
--- a/sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSshHostCertificateTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/common/signature/OpenSshHostCertificateTest.java
@@ -109,9 +109,8 @@ class OpenSshHostCertificateTest extends BaseTestSupport {
sshd.setKeyPairProvider(KeyPairProvider.wrap(hostKey));
sshd.setHostKeyCertificateProvider(session -> Collections.singletonList(signedCert));
client.setServerKeyVerifier((session, address, key) -> {
- // TODO: we get the CA key here. This is wrong; the serverKeyVerifier should get the OpenSshCertificate and
- // be responsible itself for checking its CA key against registered trusted CA keys.
- return KeyUtils.compareKeys(caKey.getPublic(), key);
+ return (key instanceof OpenSshCertificate)
+ && KeyUtils.compareKeys(caKey.getPublic(), ((OpenSshCertificate) key).getCaPubKey());
});
}
(mina-sshd) 02/05: KEX: guard client against buggy servers
Posted by tw...@apache.org.
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 baa97441bccb078d648089ef1dd50849a0633cd0
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Sun Feb 16 18:47:53 2025 +0100
KEX: guard client against buggy servers
Make the client abort the connection if a certificate key exchange
had been negotiated but the server sends a plain public key. Also
abort if a certificate is received, but no certificate key exchange
was negotiated.
---
.../org/apache/sshd/common/config/keys/KeyUtils.java | 6 ++++++
.../java/org/apache/sshd/client/kex/DHGClient.java | 19 +++++++++++++------
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/KeyUtils.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/KeyUtils.java
index 28883cb8d..b53a3ea78 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/KeyUtils.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/KeyUtils.java
@@ -1287,4 +1287,10 @@ public final class KeyUtils {
return chosenAlgorithm;
}
}
+
+ public static boolean isCertificateAlgorithm(String algorithm) {
+ synchronized (SIGNATURE_ALGORITHM_MAP) {
+ return SIGNATURE_ALGORITHM_MAP.containsKey(algorithm);
+ }
+ }
}
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 baebc67e6..3c336134e 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
@@ -176,12 +176,21 @@ public class DHGClient extends AbstractDHClientKeyExchange {
}
}
+ String keyAlg = session.getNegotiatedKexParameter(KexProposalOption.SERVERKEYS);
+
+ boolean wantCert = KeyUtils.isCertificateAlgorithm(keyAlg);
+
buffer = new ByteArrayBuffer(k_s);
PublicKey serverKey = buffer.getRawPublicKey();
PublicKey serverPublicHostKey = serverKey;
if (serverKey instanceof OpenSshCertificate) {
OpenSshCertificate openSshKey = (OpenSshCertificate) serverKey;
+ if (!wantCert) {
+ log.error("Got a server key certificate, but negotiated algorithm is {}", keyAlg);
+ throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
+ "OpenSshCertificate found with KEX algorithm " + keyAlg);
+ }
serverPublicHostKey = openSshKey.getCertPubKey();
try {
@@ -195,12 +204,10 @@ public class DHGClient extends AbstractDHClientKeyExchange {
log.info("Ignoring invalid certificate {}", openSshKey.getId(), e);
}
}
- }
-
- String keyAlg = session.getNegotiatedKexParameter(KexProposalOption.SERVERKEYS);
- if (GenericUtils.isEmpty(keyAlg)) {
- throw new SshException("Unsupported server key type: " + serverPublicHostKey.getAlgorithm()
- + "[" + serverPublicHostKey.getFormat() + "]");
+ } else if (wantCert) {
+ log.error("Got a plain public key (not a certificate) for negotiated algorithm {}", keyAlg);
+ throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
+ "Server did not send a certificate with KEX algorithm " + keyAlg);
}
buffer = new ByteArrayBuffer();