From 7cc9c491db2eb371a79b98203b5ee294486200c5 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Fri, 22 Nov 2024 20:25:23 +0100 Subject: [PATCH] GH-636: Handle unknown key types in known_hosts Harden the parser so that it can parse known_host and authorized_key lines with unknown key types. Introduce a new UnsupportedSshPublicKey class to be able to deal with such entries later on when the server host key is compared. (An alternative would have been not to create PublicKeys from known_host lines at all but to serialize the given server key into string form and then just compare against the string from the known_host line. But that is not possible without breaking API...) Such an UnsupportedSshPublicKey supports getting its key type, its raw key data, its fingerprint, and it can be written into a Buffer. --- CHANGES.md | 1 + .../client/config/hosts/KnownHostEntry.java | 5 +- .../config/keys/AuthorizedKeyEntry.java | 32 ++++- .../sshd/common/config/keys/KeyUtils.java | 11 +- .../config/keys/OpenSshCertificate.java | 9 +- .../common/config/keys/PublicKeyEntry.java | 34 +++--- .../config/keys/PublicKeyEntryResolver.java | 16 +++ .../sshd/common/config/keys/SshPublicKey.java | 37 ++++++ .../config/keys/UnsupportedSshPublicKey.java | 91 +++++++++++++++ .../config/keys/u2f/SecurityKeyPublicKey.java | 4 +- .../config/keys/u2f/SkED25519PublicKey.java | 7 ++ .../config/keys/u2f/SkEcdsaPublicKey.java | 7 ++ .../sshd/common/util/buffer/Buffer.java | 5 +- .../config/hosts/KnownHostEntryTest.java | 55 +++++++++ .../KnownHostsServerKeyVerifier.java | 8 +- .../KnownHostsServerKeyVerifierTest.java | 8 -- .../KnownHostsUnsupportedKeysTest.java | 109 ++++++++++++++++++ 17 files changed, 393 insertions(+), 46 deletions(-) create mode 100644 sshd-common/src/main/java/org/apache/sshd/common/config/keys/SshPublicKey.java create mode 100644 sshd-common/src/main/java/org/apache/sshd/common/config/keys/UnsupportedSshPublicKey.java create mode 100644 sshd-common/src/test/java/org/apache/sshd/client/config/hosts/KnownHostEntryTest.java create mode 100644 sshd-core/src/test/java/org/apache/sshd/client/keyverifier/KnownHostsUnsupportedKeysTest.java diff --git a/CHANGES.md b/CHANGES.md index bc90e48d8..d13d04263 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -43,6 +43,7 @@ * [GH-618](https://github.com/apache/mina-sshd/issues/618) Fix reading an `OpenSshCertificate` from a `Buffer` * [GH-626](https://github.com/apache/mina-sshd/issues/626) Enable `Streaming.Async` for `ChannelDirectTcpip` * [GH-628](https://github.com/apache/mina-sshd/issues/628) SFTP: fix reading directories with trailing blanks in the name +* [GH-636](https://github.com/apache/mina-sshd/issues/636) Fix handling of unsupported key types in `known_hosts` file ## New Features diff --git a/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/KnownHostEntry.java b/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/KnownHostEntry.java index fffeb3c52..a0c35d666 100644 --- a/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/KnownHostEntry.java +++ b/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/KnownHostEntry.java @@ -252,9 +252,8 @@ public static E parseKnownHostEntry(E entry, String d entry.setHashedEntry(null); entry.setPatterns(parsePatterns(GenericUtils.split(hostPattern, ','))); } - - AuthorizedKeyEntry key = ValidateUtils.checkNotNull(AuthorizedKeyEntry.parseAuthorizedKeyEntry(line), - "No valid key entry recovered from line=%s", data); + AuthorizedKeyEntry key = PublicKeyEntry.parsePublicKeyEntry(new AuthorizedKeyEntry(), + ValidateUtils.checkNotNullAndNotEmpty(line, "No valid key entry recovered from line=%s", data)); entry.setKeyEntry(key); return entry; } diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/AuthorizedKeyEntry.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/AuthorizedKeyEntry.java index dc635ddeb..237c94354 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/AuthorizedKeyEntry.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/AuthorizedKeyEntry.java @@ -304,14 +304,36 @@ public static AuthorizedKeyEntry parseAuthorizedKeyEntry( decoder = KeyUtils.getPublicKeyEntryDecoder(keyType); } - AuthorizedKeyEntry entry; + AuthorizedKeyEntry entry = null; // assume this is due to the fact that it starts with login options if (decoder == null) { Map.Entry comps = resolveEntryComponents(line); - entry = parseAuthorizedKeyEntry(comps.getValue()); - ValidateUtils.checkTrue(entry != null, "Bad format (no key data after login options): %s", line); - entry.setLoginOptions(parseLoginOptions(comps.getKey())); - } else { + String keyData = comps.getValue(); + String options = comps.getKey(); + if (keyData.startsWith("AAAA")) { + // OpenSSH known_hosts is defined to use base64, and the key data contains the binary encoded string for + // the key type again. So the base64 data always starts off with the uint32 length of the key type, with + // always starts with at least 3 zero bytes (assuming the key type has less than 256 characters). 3 zero + // bytes yield 4 A's in base64. + // + // So here we know that resolveEntryComponents() read ahead one token too far. This may happen if we + // don't support the key type (we have no decoder for it). + int i = options.lastIndexOf(' '); + if (i < 0) { + // options must be equal to keyType. Just handle the original line. + keyData = null; + } else { + keyData = options.substring(i + 1) + ' ' + keyData; + options = options.substring(0, i); + } + } + if (keyData != null) { + entry = parseAuthorizedKeyEntry(keyData, resolver); + ValidateUtils.checkTrue(entry != null, "Bad format (no key data after login options): %s", line); + entry.setLoginOptions(parseLoginOptions(options)); + } + } + if (entry == null) { String encData = (endPos < (line.length() - 1)) ? line.substring(0, endPos).trim() : line; String comment = (endPos < (line.length() - 1)) ? line.substring(endPos + 1).trim() : null; entry = parsePublicKeyEntry(new AuthorizedKeyEntry(), encData, resolver); 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 15b296c96..9f58f0046 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 @@ -859,6 +859,8 @@ public static String getKeyType(KeyPair kp) { public static String getKeyType(Key key) { if (key == null) { return null; + } else if (key instanceof SshPublicKey) { + return ((SshPublicKey) key).getKeyType(); } else if (key instanceof DSAKey) { return KeyPairProvider.SSH_DSS; } else if (key instanceof RSAKey) { @@ -872,14 +874,8 @@ public static String getKeyType(Key key) { } else { return curve.getKeyType(); } - } else if (key instanceof SkEcdsaPublicKey) { - return SkECDSAPublicKeyEntryDecoder.KEY_TYPE; } else if (SecurityUtils.EDDSA.equalsIgnoreCase(key.getAlgorithm())) { return KeyPairProvider.SSH_ED25519; - } else if (key instanceof SkED25519PublicKey) { - return SkED25519PublicKeyEntryDecoder.KEY_TYPE; - } else if (key instanceof OpenSshCertificate) { - return ((OpenSshCertificate) key).getKeyType(); } return null; @@ -1063,6 +1059,9 @@ public static boolean compareKeyPairs(KeyPair k1, KeyPair k2) { } public static boolean compareKeys(PublicKey k1, PublicKey k2) { + if (Objects.equals(k1, k2)) { + return true; + } if ((k1 instanceof RSAPublicKey) && (k2 instanceof RSAPublicKey)) { return compareRSAKeys(RSAPublicKey.class.cast(k1), RSAPublicKey.class.cast(k2)); } else if ((k1 instanceof DSAPublicKey) && (k2 instanceof DSAPublicKey)) { diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificate.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificate.java index 799568f71..d345a3aac 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificate.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificate.java @@ -35,7 +35,7 @@ * @author Apache MINA SSHD Project * @see PROTOCOL.certkeys */ -public interface OpenSshCertificate extends PublicKey, PrivateKey { +public interface OpenSshCertificate extends SshPublicKey, PrivateKey { /** * {@link OpenSshCertificate}s have a type indicating whether the certificate if for a host key (certifying a host @@ -90,13 +90,6 @@ public static Type fromCode(int code) { */ byte[] getNonce(); - /** - * Retrieves the SSH key type of this certificate. - * - * @return the key type, for instance "ssh-rsa-cert-v01@openssh.com" - */ - String getKeyType(); - /** * Retrieves the certified public key. * diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/PublicKeyEntry.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/PublicKeyEntry.java index 8e7b77ef6..42c4257fd 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/PublicKeyEntry.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/PublicKeyEntry.java @@ -482,24 +482,32 @@ public static A appendPublicKeyEntry( return sb; } - @SuppressWarnings("unchecked") - PublicKeyEntryDecoder decoder - = (PublicKeyEntryDecoder) KeyUtils.getPublicKeyEntryDecoder(key); - if (decoder == null) { - throw new StreamCorruptedException("Cannot retrieve decoder for key=" + key.getAlgorithm()); - } + String keyType; + byte[] bytes; - try (ByteArrayOutputStream s = new ByteArrayOutputStream(Byte.MAX_VALUE)) { - String keyType = decoder.encodePublicKey(s, key); - byte[] bytes = s.toByteArray(); - if (encoder == null) { - encoder = resolveKeyDataEntryResolver(keyType); + if (key instanceof UnsupportedSshPublicKey) { + UnsupportedSshPublicKey unsupported = (UnsupportedSshPublicKey) key; + keyType = unsupported.getKeyType(); + bytes = unsupported.getKeyData(); + } else { + @SuppressWarnings("unchecked") + PublicKeyEntryDecoder decoder = (PublicKeyEntryDecoder) KeyUtils + .getPublicKeyEntryDecoder(key); + if (decoder == null) { + throw new StreamCorruptedException("Cannot retrieve decoder for key=" + key.getAlgorithm()); } - String encData = encoder.encodeEntryKeyData(bytes); - sb.append(keyType).append(' ').append(encData); + try (ByteArrayOutputStream s = new ByteArrayOutputStream(Byte.MAX_VALUE)) { + keyType = decoder.encodePublicKey(s, key); + bytes = s.toByteArray(); + } + } + if (encoder == null) { + encoder = resolveKeyDataEntryResolver(keyType); } + String encData = encoder.encodeEntryKeyData(bytes); + sb.append(keyType).append(' ').append(encData); return sb; } diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/PublicKeyEntryResolver.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/PublicKeyEntryResolver.java index f998e1d10..eeb9c56a7 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/PublicKeyEntryResolver.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/PublicKeyEntryResolver.java @@ -64,6 +64,22 @@ public String toString() { } }; + /** + * A resolver that returns an {@link UnsupportedSshPublicKey} for any input. + */ + PublicKeyEntryResolver UNSUPPORTED = new PublicKeyEntryResolver() { + @Override + public PublicKey resolve(SessionContext session, String keyType, byte[] keyData, Map headers) + throws IOException, GeneralSecurityException { + return new UnsupportedSshPublicKey(keyType, keyData); + } + + @Override + public String toString() { + return "UNSUPPORTED"; + } + }; + /** * @param session The {@link SessionContext} for invoking this load command - may be {@code null} * if not invoked within a session context (e.g., offline tool or session unknown). diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/SshPublicKey.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/SshPublicKey.java new file mode 100644 index 000000000..dcf4a7ce8 --- /dev/null +++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/SshPublicKey.java @@ -0,0 +1,37 @@ +/* + * 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.config.keys; + +import java.security.PublicKey; + +/** + * A {@link PublicKey} that has an SSH key type. + * + * @author Apache MINA SSHD Project + */ +public interface SshPublicKey extends PublicKey { + + /** + * Retrieves the SSH key type. + * + * @return the SSH key type, never {@code null}. + */ + String getKeyType(); + +} diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/UnsupportedSshPublicKey.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/UnsupportedSshPublicKey.java new file mode 100644 index 000000000..64d695e32 --- /dev/null +++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/UnsupportedSshPublicKey.java @@ -0,0 +1,91 @@ +/* + * 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.config.keys; + +import java.util.Arrays; +import java.util.Objects; + +/** + * A representation of an unsupported SSH public key -- just a key type and the raw key data. + * + * @author Apache MINA SSHD Project + */ +public class UnsupportedSshPublicKey implements SshPublicKey { + + private static final long serialVersionUID = -4870624671501562706L; + + private final String keyType; + + private final byte[] keyData; + + public UnsupportedSshPublicKey(String keyType, byte[] keyData) { + this.keyType = keyType; + this.keyData = keyData.clone(); + } + + @Override + public String getAlgorithm() { + // Won't match any JCE algorithm. + return getKeyType(); + } + + @Override + public String getFormat() { + // We cannot produce an encoding for an unsupported key. + return null; + } + + @Override + public byte[] getEncoded() { + // We cannot produce an encoding for an unsupported key. + return null; + } + + @Override + public String getKeyType() { + return keyType; + } + + /** + * Retrieves the raw key bytes (serialized form). + * + * @return the key bytes + */ + public byte[] getKeyData() { + return keyData.clone(); + } + + @Override + public int hashCode() { + return Arrays.hashCode(keyData) * 31 + Objects.hash(keyType); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof UnsupportedSshPublicKey)) { + return false; + } + UnsupportedSshPublicKey other = (UnsupportedSshPublicKey) obj; + return Arrays.equals(keyData, other.keyData) && Objects.equals(keyType, other.keyType); + } + +} diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/u2f/SecurityKeyPublicKey.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/u2f/SecurityKeyPublicKey.java index 314d84853..8144c2edf 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/u2f/SecurityKeyPublicKey.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/u2f/SecurityKeyPublicKey.java @@ -20,7 +20,9 @@ import java.security.PublicKey; -public interface SecurityKeyPublicKey extends PublicKey { +import org.apache.sshd.common.config.keys.SshPublicKey; + +public interface SecurityKeyPublicKey extends SshPublicKey { String getAppName(); boolean isNoTouchRequired(); diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/u2f/SkED25519PublicKey.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/u2f/SkED25519PublicKey.java index bae73c3a1..ebac6a583 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/u2f/SkED25519PublicKey.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/u2f/SkED25519PublicKey.java @@ -21,6 +21,7 @@ import java.util.Objects; import net.i2p.crypto.eddsa.EdDSAPublicKey; +import org.apache.sshd.common.config.keys.impl.SkED25519PublicKeyEntryDecoder; public class SkED25519PublicKey implements SecurityKeyPublicKey { @@ -43,6 +44,11 @@ public String getAlgorithm() { return ALGORITHM; } + @Override + public String getKeyType() { + return SkED25519PublicKeyEntryDecoder.KEY_TYPE; + } + @Override public String getFormat() { return null; @@ -99,4 +105,5 @@ public boolean equals(Object obj) { && this.noTouchRequired == other.noTouchRequired && Objects.equals(this.delegatePublicKey, other.delegatePublicKey); } + } diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/u2f/SkEcdsaPublicKey.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/u2f/SkEcdsaPublicKey.java index 666ae11ad..1937bdc4a 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/u2f/SkEcdsaPublicKey.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/u2f/SkEcdsaPublicKey.java @@ -21,6 +21,8 @@ import java.security.interfaces.ECPublicKey; import java.util.Objects; +import org.apache.sshd.common.config.keys.impl.SkECDSAPublicKeyEntryDecoder; + public class SkEcdsaPublicKey implements SecurityKeyPublicKey { public static final String ALGORITHM = "ECDSA-SK"; @@ -42,6 +44,11 @@ public String getAlgorithm() { return ALGORITHM; } + @Override + public String getKeyType() { + return SkECDSAPublicKeyEntryDecoder.KEY_TYPE; + } + @Override public String getFormat() { return null; diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java index 803c3cb24..275280880 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java @@ -61,6 +61,7 @@ import org.apache.sshd.common.cipher.ECCurves; import org.apache.sshd.common.config.keys.KeyUtils; import org.apache.sshd.common.config.keys.OpenSshCertificate; +import org.apache.sshd.common.config.keys.UnsupportedSshPublicKey; import org.apache.sshd.common.config.keys.u2f.SecurityKeyPublicKey; import org.apache.sshd.common.keyprovider.KeyPairProvider; import org.apache.sshd.common.util.GenericUtils; @@ -1015,7 +1016,9 @@ public void putRawPublicKey(PublicKey key) { public void putRawPublicKeyBytes(PublicKey key) { Objects.requireNonNull(key, "No key"); - if (key instanceof RSAPublicKey) { + if (key instanceof UnsupportedSshPublicKey) { + putRawBytes(((UnsupportedSshPublicKey) key).getKeyData()); + } else if (key instanceof RSAPublicKey) { RSAPublicKey rsaPub = (RSAPublicKey) key; putMPInt(rsaPub.getPublicExponent()); diff --git a/sshd-common/src/test/java/org/apache/sshd/client/config/hosts/KnownHostEntryTest.java b/sshd-common/src/test/java/org/apache/sshd/client/config/hosts/KnownHostEntryTest.java new file mode 100644 index 000000000..d7afffef3 --- /dev/null +++ b/sshd-common/src/test/java/org/apache/sshd/client/config/hosts/KnownHostEntryTest.java @@ -0,0 +1,55 @@ +/* + * 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.client.config.hosts; + +import java.io.StringReader; +import java.security.PublicKey; +import java.util.List; + +import org.apache.sshd.common.config.keys.AuthorizedKeyEntry; +import org.apache.sshd.common.config.keys.PublicKeyEntry; +import org.apache.sshd.common.config.keys.PublicKeyEntryResolver; +import org.apache.sshd.common.config.keys.UnsupportedSshPublicKey; +import org.apache.sshd.util.test.JUnitTestSupport; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +@Tag("NoIoTestCase") +class KnownHostEntryTest extends JUnitTestSupport { + + @Test + void testLine() throws Exception { + List entries = KnownHostEntry.readKnownHostEntries( + new StringReader( + "[127.0.0.1]:2222 ssh-ed448 AAAAC3NzaC1lZDI1NTE5AAAAIPu6ntmyfSOkqLl3qPxD5XxwW7OONwwSG3KO+TGn+PFu"), + true); + assertNotNull(entries); + assertEquals(1, entries.size()); + KnownHostEntry entry = entries.get(0); + AuthorizedKeyEntry keyEntry = entry.getKeyEntry(); + assertNotNull(keyEntry); + assertEquals("ssh-ed448", keyEntry.getKeyType()); + PublicKey pk = keyEntry.resolvePublicKey(null, PublicKeyEntryResolver.UNSUPPORTED); + assertTrue(pk instanceof UnsupportedSshPublicKey); + UnsupportedSshPublicKey sshKey = (UnsupportedSshPublicKey) pk; + assertEquals("ssh-ed448", sshKey.getKeyType()); + assertEquals("ssh-ed448 AAAAC3NzaC1lZDI1NTE5AAAAIPu6ntmyfSOkqLl3qPxD5XxwW7OONwwSG3KO+TGn+PFu", + PublicKeyEntry.toString(sshKey)); + } +} 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 fe079fb2b..c07126ab3 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 @@ -53,6 +53,7 @@ import org.apache.sshd.common.config.keys.KeyUtils; import org.apache.sshd.common.config.keys.PublicKeyEntry; import org.apache.sshd.common.config.keys.PublicKeyEntryResolver; +import org.apache.sshd.common.config.keys.UnsupportedSshPublicKey; import org.apache.sshd.common.mac.Mac; import org.apache.sshd.common.random.Random; import org.apache.sshd.common.session.SessionContext; @@ -263,7 +264,7 @@ protected PublicKey resolveHostKey( } protected PublicKeyEntryResolver getFallbackPublicKeyEntryResolver() { - return PublicKeyEntryResolver.IGNORING; + return PublicKeyEntryResolver.UNSUPPORTED; } protected boolean acceptKnownHostEntries( @@ -350,6 +351,11 @@ protected void updateModifiedServerKey( ClientSession clientSession, SocketAddress remoteAddress, HostEntryPair match, PublicKey actual, Path file, Collection knownHosts) throws Exception { + if (match.getServerKey() instanceof UnsupportedSshPublicKey) { + // Never replace unsupported keys, always add. + updateKnownHostsFile(clientSession, remoteAddress, actual, file, knownHosts); + return; + } KnownHostEntry entry = match.getHostEntry(); String matchLine = ValidateUtils.checkNotNullAndNotEmpty(entry.getConfigLine(), "No entry config line"); String newLine = prepareModifiedServerKeyLine( diff --git a/sshd-core/src/test/java/org/apache/sshd/client/keyverifier/KnownHostsServerKeyVerifierTest.java b/sshd-core/src/test/java/org/apache/sshd/client/keyverifier/KnownHostsServerKeyVerifierTest.java index e3ae6a454..3e5fe55bf 100644 --- a/sshd-core/src/test/java/org/apache/sshd/client/keyverifier/KnownHostsServerKeyVerifierTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/client/keyverifier/KnownHostsServerKeyVerifierTest.java @@ -63,14 +63,6 @@ import org.junit.jupiter.api.TestMethodOrder; import org.mockito.Mockito; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; - /** * @author Apache MINA SSHD Project */ diff --git a/sshd-core/src/test/java/org/apache/sshd/client/keyverifier/KnownHostsUnsupportedKeysTest.java b/sshd-core/src/test/java/org/apache/sshd/client/keyverifier/KnownHostsUnsupportedKeysTest.java new file mode 100644 index 000000000..dedd9015d --- /dev/null +++ b/sshd-core/src/test/java/org/apache/sshd/client/keyverifier/KnownHostsUnsupportedKeysTest.java @@ -0,0 +1,109 @@ +/* + * 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.client.keyverifier; + +import java.net.SocketAddress; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.KeyPair; +import java.security.PublicKey; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.sshd.client.config.hosts.KnownHostEntry; +import org.apache.sshd.client.session.ClientSession; +import org.apache.sshd.common.config.keys.AuthorizedKeyEntry; +import org.apache.sshd.common.config.keys.KeyUtils; +import org.apache.sshd.common.config.keys.PublicKeyEntryResolver; +import org.apache.sshd.common.config.keys.UnsupportedSshPublicKey; +import org.apache.sshd.common.util.net.SshdSocketAddress; +import org.apache.sshd.util.test.CommonTestSupportUtils; +import org.apache.sshd.util.test.JUnitTestSupport; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mockito; + +@Tag("NoIoTestCase") +class KnownHostsUnsupportedKeysTest extends JUnitTestSupport { + + @TempDir + private Path tmp; + + private boolean invokeVerifier(ServerKeyVerifier verifier, SocketAddress hostIdentity, PublicKey serverKey) { + ClientSession session = Mockito.mock(ClientSession.class); + Mockito.when(session.getConnectAddress()).thenReturn(hostIdentity); + Mockito.when(session.toString()).thenReturn(getCurrentTestName() + "[" + hostIdentity + "]"); + return verifier.verifyServerKey(session, hostIdentity, serverKey); + } + + @Test + void unknownExistingKey() throws Exception { + Path knownHosts = tmp.resolve("known_hosts"); + List lines = new ArrayList<>(); + lines.add("[127.0.0.1]:2222 ssh-ed448 AAAAC3NzaC1lZDI1NTE5AAAAIPu6ntmyfSOkqLl3qPxD5XxwW7OONwwSG3KO+TGn+PFu"); + lines.add( + "[127.0.0.1]:2222 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBCbZVVpqEHGLNWMqMeyU1VbWb91XteoamVcgpy4yxNVbZffb5IDdbo1ons/y9KAhcub6LZeLrvXzVUZbXCZiUkg="); + Files.write(knownHosts, lines); + KnownHostsServerKeyVerifier verifier = new KnownHostsServerKeyVerifier(RejectAllServerKeyVerifier.INSTANCE, knownHosts); + KnownHostEntry knownHost = KnownHostEntry.parseKnownHostEntry(lines.get(1)); + AuthorizedKeyEntry keyEntry = knownHost.getKeyEntry(); + assertNotNull(keyEntry); + PublicKey key = keyEntry.resolvePublicKey(null, PublicKeyEntryResolver.FAILING); + assertTrue(invokeVerifier(verifier, new SshdSocketAddress("127.0.0.1", 2222), key)); + } + + @Test + void unknownNewKey() throws Exception { + KeyPair kp = CommonTestSupportUtils.generateKeyPair(KeyUtils.RSA_ALGORITHM, 1024); + PublicKey newKey = kp.getPublic(); + Path knownHosts = tmp.resolve("known_hosts"); + List lines = new ArrayList<>(); + lines.add("[127.0.0.1]:2222 ssh-ed448 AAAAC3NzaC1lZDI1NTE5AAAAIPu6ntmyfSOkqLl3qPxD5XxwW7OONwwSG3KO+TGn+PFu"); + Files.write(knownHosts, lines); + AtomicInteger numberOfCalls = new AtomicInteger(); + KnownHostsServerKeyVerifier verifier = new KnownHostsServerKeyVerifier(RejectAllServerKeyVerifier.INSTANCE, + knownHosts) { + @Override + public boolean acceptModifiedServerKey( + ClientSession clientSession, SocketAddress remoteAddress, + KnownHostEntry entry, PublicKey expected, PublicKey actual) throws Exception { + numberOfCalls.incrementAndGet(); + assertSame(newKey, actual, "Mismatched actual key for " + remoteAddress); + assertTrue(expected instanceof UnsupportedSshPublicKey); + String fingerprint = KeyUtils.getFingerPrint(expected); + assertNotNull(fingerprint); + assertTrue(fingerprint.length() > 0); + return true; + } + }; + assertTrue(invokeVerifier(verifier, new SshdSocketAddress("127.0.0.1", 2222), newKey)); + assertEquals(1, numberOfCalls.get()); + // Load the file again. We should have two entries now, and the second one should be our newKey + List newEntries = KnownHostEntry.readKnownHostEntries(knownHosts); + assertNotNull(newEntries); + assertEquals(2, newEntries.size()); + KnownHostEntry knownHost = newEntries.get(1); + AuthorizedKeyEntry keyEntry = knownHost.getKeyEntry(); + assertNotNull(keyEntry); + PublicKey key = keyEntry.resolvePublicKey(null, PublicKeyEntryResolver.FAILING); + assertTrue(KeyUtils.compareKeys(newKey, key)); + } +}