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