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 "-c...@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:d...@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:d...@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) {
