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
The following commit(s) were added to refs/heads/master by this push: new f1994bd3b [SSHD-1259] Match key type in known_hosts lookup (#368) f1994bd3b is described below commit f1994bd3bee631ae828def8fe9e9ab35310da8cd Author: FliegenKLATSCH <ch...@koras.de> AuthorDate: Mon May 29 11:00:11 2023 +0200 [SSHD-1259] Match key type in known_hosts lookup (#368) Consider key types, and consider all keys for a host. Also handle the revoked flag: do not accept server keys that are marked as revoked in the known_hosts files. --- .../keyverifier/KnownHostsServerKeyVerifier.java | 106 ++++++------ .../keyverifier/ModifiedServerKeyAcceptor.java | 4 +- .../KnownHostsServerKeyVerifierTest.java | 186 ++++++++++++++------- .../org/apache/sshd/client/keyverifier/known_hosts | 3 + 4 files changed, 186 insertions(+), 113 deletions(-) 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 97de87e5c..fe079fb2b 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 @@ -36,9 +36,11 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.TreeSet; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Supplier; +import java.util.stream.Collectors; import org.apache.sshd.client.config.hosts.KnownHostEntry; import org.apache.sshd.client.config.hosts.KnownHostHashValue; @@ -267,36 +269,63 @@ public class KnownHostsServerKeyVerifier protected boolean acceptKnownHostEntries( ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey, Collection<HostEntryPair> knownHosts) { - // TODO allow for several candidates and check if ANY of them matches the key and has 'revoked' marker - HostEntryPair match = findKnownHostEntry(clientSession, remoteAddress, knownHosts); - if (match == null) { + + List<HostEntryPair> hostMatches = findKnownHostEntries(clientSession, remoteAddress, knownHosts); + if (hostMatches.isEmpty()) { return acceptUnknownHostKey(clientSession, remoteAddress, serverKey); } - KnownHostEntry entry = match.getHostEntry(); - PublicKey expected = match.getServerKey(); - if (KeyUtils.compareKeys(expected, serverKey)) { - return acceptKnownHostEntry(clientSession, remoteAddress, serverKey, entry); + String serverKeyType = KeyUtils.getKeyType(serverKey); + + List<HostEntryPair> keyMatches = hostMatches.stream() + .filter(entry -> serverKeyType.equals(entry.getHostEntry().getKeyEntry().getKeyType())) + .filter(k -> KeyUtils.compareKeys(k.getServerKey(), serverKey)) + .collect(Collectors.toList()); + + if (keyMatches.stream() + .anyMatch(k -> "revoked".equals(k.getHostEntry().getMarker()))) { + log.debug("acceptKnownHostEntry({})[{}] key={}-{} marked as revoked", + clientSession, remoteAddress, KeyUtils.getKeyType(serverKey), KeyUtils.getFingerPrint(serverKey)); + return false; } + if (!keyMatches.isEmpty()) { + return true; + } + + Optional<HostEntryPair> anyNonRevokedMatch = hostMatches.stream() + .filter(k -> !"revoked".equals(k.getHostEntry().getMarker())) + .findAny(); + + if (!anyNonRevokedMatch.isPresent()) { + return acceptUnknownHostKey(clientSession, remoteAddress, serverKey); + } + + KnownHostEntry entry = anyNonRevokedMatch.get().getHostEntry(); + PublicKey expected = anyNonRevokedMatch.get().getServerKey(); + try { - if (!acceptModifiedServerKey(clientSession, remoteAddress, entry, expected, serverKey)) { - return false; + if (acceptModifiedServerKey(clientSession, remoteAddress, entry, expected, serverKey)) { + updateModifiedServerKey(clientSession, remoteAddress, serverKey, knownHosts, anyNonRevokedMatch.get()); + return true; } } catch (Throwable t) { warn("acceptKnownHostEntries({})[{}] failed ({}) to accept modified server key: {}", clientSession, remoteAddress, t.getClass().getSimpleName(), t.getMessage(), t); - return false; } + return false; + } + + protected void updateModifiedServerKey( + ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey, Collection<HostEntryPair> knownHosts, + HostEntryPair match) { Path file = getPath(); try { updateModifiedServerKey(clientSession, remoteAddress, match, serverKey, file, knownHosts); } catch (Throwable t) { handleModifiedServerKeyUpdateFailure(clientSession, remoteAddress, match, serverKey, file, knownHosts, t); } - - return true; } /** @@ -460,74 +489,45 @@ public class KnownHostsServerKeyVerifier clientSession, remoteAddress, reason.getClass().getSimpleName(), match, reason.getMessage(), reason); } - /** - * Invoked <U>after</U> known host entry located and keys match - by default checks that entry has not been revoked - * - * @param clientSession The {@link ClientSession} - * @param remoteAddress The remote host address - * @param serverKey The presented server {@link PublicKey} - * @param entry The {@link KnownHostEntry} value - if {@code null} then no known matching host entry was - * found - default will call - * {@link #acceptUnknownHostKey(ClientSession, SocketAddress, PublicKey)} - * @return {@code true} if OK to accept the server - */ - protected boolean acceptKnownHostEntry( - ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey, KnownHostEntry entry) { - if (entry == null) { // not really expected, but manage it - return acceptUnknownHostKey(clientSession, remoteAddress, serverKey); - } - - if ("revoked".equals(entry.getMarker())) { - log.debug("acceptKnownHostEntry({})[{}] key={}-{} marked as {}", - clientSession, remoteAddress, KeyUtils.getKeyType(serverKey), KeyUtils.getFingerPrint(serverKey), - entry.getMarker()); - return false; - } - - if (log.isDebugEnabled()) { - log.debug("acceptKnownHostEntry({})[{}] matched key={}-{}", - clientSession, remoteAddress, KeyUtils.getKeyType(serverKey), KeyUtils.getFingerPrint(serverKey)); - } - return true; - } - - protected HostEntryPair findKnownHostEntry( + protected List<HostEntryPair> findKnownHostEntries( ClientSession clientSession, SocketAddress remoteAddress, Collection<HostEntryPair> knownHosts) { if (GenericUtils.isEmpty(knownHosts)) { - return null; + return Collections.emptyList(); } Collection<SshdSocketAddress> candidates = resolveHostNetworkIdentities(clientSession, remoteAddress); boolean debugEnabled = log.isDebugEnabled(); if (debugEnabled) { - log.debug("findKnownHostEntry({})[{}] host network identities: {}", + log.debug("findKnownHostEntries({})[{}] host network identities: {}", clientSession, remoteAddress, candidates); } if (GenericUtils.isEmpty(candidates)) { - return null; + return Collections.emptyList(); } - for (HostEntryPair match : knownHosts) { - KnownHostEntry entry = match.getHostEntry(); + List<HostEntryPair> matches = new ArrayList<>(); + for (HostEntryPair line : knownHosts) { + KnownHostEntry entry = line.getHostEntry(); for (SshdSocketAddress host : candidates) { try { if (entry.isHostMatch(host.getHostName(), host.getPort())) { if (debugEnabled) { - log.debug("findKnownHostEntry({})[{}] matched host={} for entry={}", + log.debug("findKnownHostEntries({})[{}] matched host={} for entry={}", clientSession, remoteAddress, host, entry); } - return match; + matches.add(line); + break; } } catch (RuntimeException | Error e) { - warn("findKnownHostEntry({})[{}] failed ({}) to check host={} for entry={}: {}", + warn("findKnownHostEntries({})[{}] failed ({}) to check host={} for entry={}: {}", clientSession, remoteAddress, e.getClass().getSimpleName(), host, entry.getConfigLine(), e.getMessage(), e); } } } - return null; // no match found + return matches; } /** diff --git a/sshd-core/src/main/java/org/apache/sshd/client/keyverifier/ModifiedServerKeyAcceptor.java b/sshd-core/src/main/java/org/apache/sshd/client/keyverifier/ModifiedServerKeyAcceptor.java index 966a970ed..c3a27ef74 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/keyverifier/ModifiedServerKeyAcceptor.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/keyverifier/ModifiedServerKeyAcceptor.java @@ -35,8 +35,8 @@ public interface ModifiedServerKeyAcceptor { * * @param clientSession The {@link ClientSession} * @param remoteAddress The remote host address - * @param entry The original {@link KnownHostEntry} whose key did not match - * @param expected The expected server {@link PublicKey} + * @param entry Any original {@link KnownHostEntry} whose key did not match + * @param expected Any expected server {@link PublicKey} * @param actual The presented server {@link PublicKey} * @return {@code true} if accept the server key anyway * @throws Exception if cannot process the request - equivalent to {@code false} return value 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 67cdba0c8..9c323b855 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 @@ -28,8 +28,10 @@ import java.nio.file.Paths; import java.nio.file.StandardCopyOption; import java.security.KeyPair; import java.security.PublicKey; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.TreeMap; import java.util.concurrent.atomic.AtomicInteger; @@ -50,7 +52,6 @@ import org.apache.sshd.common.config.keys.PublicKeyEntryResolver; import org.apache.sshd.common.mac.Mac; import org.apache.sshd.common.random.JceRandomFactory; import org.apache.sshd.common.util.GenericUtils; -import org.apache.sshd.common.util.MapEntryUtils; import org.apache.sshd.common.util.ValidateUtils; import org.apache.sshd.common.util.net.SshdSocketAddress; import org.apache.sshd.util.test.BaseTestSupport; @@ -70,8 +71,8 @@ import org.mockito.Mockito; @Category({ NoIoTestCase.class }) public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { private static final String HASHED_HOST = "192.168.1.61"; - private static final Map<SshdSocketAddress, PublicKey> HOST_KEYS = new TreeMap<>(SshdSocketAddress.BY_HOST_AND_PORT); - private static Map<SshdSocketAddress, KnownHostEntry> hostsEntries; + private static final Map<SshdSocketAddress, List<PublicKey>> HOST_KEYS = new TreeMap<>(SshdSocketAddress.BY_HOST_AND_PORT); + private static Map<SshdSocketAddress, List<KnownHostEntry>> hostsEntries; private static Path entriesFile; public KnownHostsServerKeyVerifierTest() { @@ -87,12 +88,13 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { hostsEntries = loadEntries(entriesFile); // Cannot use forEach because of the potential IOException/GeneralSecurityException being thrown - for (Map.Entry<SshdSocketAddress, KnownHostEntry> ke : hostsEntries.entrySet()) { - SshdSocketAddress hostIdentity = ke.getKey(); - KnownHostEntry entry = ke.getValue(); - AuthorizedKeyEntry authEntry = ValidateUtils.checkNotNull(entry.getKeyEntry(), "No key extracted from %s", entry); - PublicKey key = authEntry.resolvePublicKey(null, Collections.emptyMap(), PublicKeyEntryResolver.FAILING); - assertNull("Multiple keys for host=" + hostIdentity, HOST_KEYS.put(hostIdentity, key)); + for (Map.Entry<SshdSocketAddress, List<KnownHostEntry>> entry : hostsEntries.entrySet()) { + for (KnownHostEntry knownHostEntry : entry.getValue()) { + AuthorizedKeyEntry authEntry + = ValidateUtils.checkNotNull(knownHostEntry.getKeyEntry(), "No key extracted from %s", entry.getKey()); + PublicKey key = authEntry.resolvePublicKey(null, Collections.emptyMap(), PublicKeyEntryResolver.FAILING); + HOST_KEYS.computeIfAbsent(entry.getKey(), k -> new ArrayList<>()).add(key); + } } } @@ -120,15 +122,22 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { ClientFactoryManager manager = Mockito.mock(ClientFactoryManager.class); Mockito.when(manager.getRandomFactory()).thenReturn((Factory) JceRandomFactory.INSTANCE); - HOST_KEYS.entrySet().parallelStream().forEach(line -> { - KnownHostEntry entry = hostsEntries.get(line.getKey()); + HOST_KEYS.forEach((host, list) -> list.forEach(publicKey -> { + KnownHostEntry entry = hostsEntries.get(host).stream() + .filter(key -> key.getKeyEntry().getKeyType().equals(KeyUtils.getKeyType(publicKey))) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Missing updated key for " + KeyUtils.getKeyType(publicKey))); ClientSession session = Mockito.mock(ClientSession.class); Mockito.when(session.getFactoryManager()).thenReturn(manager); - Mockito.when(session.getConnectAddress()).thenReturn(line.getKey()); - assertTrue("Failed to validate server=" + entry, verifier.verifyServerKey(session, line.getKey(), line.getValue())); - }); + Mockito.when(session.getConnectAddress()).thenReturn(host); + if ("revoked".equals(entry.getMarker())) { + assertFalse("Failed to validate server=" + entry, verifier.verifyServerKey(session, host, publicKey)); + } else { + assertTrue("Failed to validate server=" + entry, verifier.verifyServerKey(session, host, publicKey)); + } + })); } @Test @@ -154,13 +163,20 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { }; - HOST_KEYS.forEach((hostIdentity, serverKey) -> { - KnownHostEntry entry = hostsEntries.get(hostIdentity); + HOST_KEYS.forEach((host, list) -> list.forEach(hostKey -> { + KnownHostEntry entry = hostsEntries.get(host).stream() + .filter(key -> key.getKeyEntry().getKeyType().equals(KeyUtils.getKeyType(hostKey))) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Missing updated key for " + KeyUtils.getKeyType(hostKey))); outputDebugMessage("Verify host=%s", entry); - assertTrue("Failed to verify server=" + entry, invokeVerifier(verifier, hostIdentity, serverKey)); + if ("revoked".equals(entry.getMarker())) { + assertFalse("Failed to verify server=" + entry, invokeVerifier(verifier, host, hostKey)); + } else { + assertTrue("Failed to verify server=" + entry, invokeVerifier(verifier, host, hostKey)); + } assertEquals("Unexpected delegate invocation for host=" + entry, 0, delegateCount.get()); assertEquals("Unexpected update invocation for host=" + entry, 0, updateCount.get()); - }); + })); } @Test @@ -188,10 +204,12 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { int verificationCount = 0; // Cannot use forEach because the verification count variable is not effectively final - for (Map.Entry<SshdSocketAddress, PublicKey> ke : HOST_KEYS.entrySet()) { - SshdSocketAddress hostIdentity = ke.getKey(); - PublicKey serverKey = ke.getValue(); - KnownHostEntry entry = hostsEntries.get(hostIdentity); + for (SshdSocketAddress hostIdentity : HOST_KEYS.keySet()) { + PublicKey serverKey = HOST_KEYS.get(hostIdentity).get(0); + KnownHostEntry entry = hostsEntries.get(hostIdentity).stream() + .filter(key -> key.getKeyEntry().getKeyType().equals(KeyUtils.getKeyType(serverKey))) + .findAny() + .orElseThrow(() -> new IllegalStateException("Missing updated key for " + KeyUtils.getKeyType(serverKey))); outputDebugMessage("Verify host=%s", entry); assertTrue("Failed to verify server=" + entry, invokeVerifier(verifier, hostIdentity, serverKey)); verificationCount++; @@ -200,12 +218,21 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { } // make sure we have all the original entries and ONLY them - Map<SshdSocketAddress, KnownHostEntry> updatedEntries = loadEntries(path); - hostsEntries.forEach((hostIdentity, expected) -> { - KnownHostEntry actual = updatedEntries.remove(hostIdentity); - assertNotNull("No updated entry for host=" + hostIdentity, actual); + Map<SshdSocketAddress, List<KnownHostEntry>> updatedEntries = loadEntries(path); + hostsEntries.keySet().forEach(hostIdentity -> { + KnownHostEntry expected = hostsEntries.get(hostIdentity).get(0); + + KnownHostEntry actual = updatedEntries.get(hostIdentity).stream() + .filter(key -> key.getKeyEntry().getKeyType().equals(expected.getKeyEntry().getKeyType())) + .findAny() + .orElseThrow(() -> new IllegalStateException("Missing updated key for " + expected)); + + assertTrue("No updated entry for host=" + hostIdentity, updatedEntries.get(hostIdentity).remove(actual)); + if (updatedEntries.get(hostIdentity).isEmpty()) { + updatedEntries.remove(hostIdentity); + } - String expLine = expected.getConfigLine(); + String expLine = expected.getConfigLine().replace("@revoked ", ""); // if original is a list or hashed then replace them with the expected host if ((expLine.indexOf(',') > 0) || (expLine.indexOf(KnownHostHashValue.HASHED_HOST_DELIMITER) >= 0)) { int pos = expLine.indexOf(' '); @@ -242,12 +269,16 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { ClientSession session = Mockito.mock(ClientSession.class); Mockito.when(session.getFactoryManager()).thenReturn(manager); - HOST_KEYS.forEach((hostIdentity, serverKey) -> { - KnownHostEntry entry = hostsEntries.get(hostIdentity); + HOST_KEYS.keySet().forEach(host -> { + PublicKey serverKey = HOST_KEYS.get(host).get(0); + KnownHostEntry entry = hostsEntries.get(host).stream() + .filter(key -> key.getKeyEntry().getKeyType().equals(KeyUtils.getKeyType(serverKey))) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Missing updated key for " + KeyUtils.getKeyType(serverKey))); outputDebugMessage("Write host=%s", entry); - Mockito.when(session.getConnectAddress()).thenReturn(hostIdentity); - assertTrue("Failed to validate server=" + entry, verifier.verifyServerKey(session, hostIdentity, serverKey)); + Mockito.when(session.getConnectAddress()).thenReturn(host); + assertTrue("Failed to validate server=" + entry, verifier.verifyServerKey(session, host, serverKey)); }); // force re-read to ensure all values are hashed @@ -259,12 +290,16 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { verifier.setLoadedHostsEntries(keys); // make sure can still validate the original hosts - HOST_KEYS.forEach((hostIdentity, serverKey) -> { - KnownHostEntry entry = hostsEntries.get(hostIdentity); + HOST_KEYS.keySet().forEach(host -> { + PublicKey serverKey = HOST_KEYS.get(host).get(0); + KnownHostEntry entry = hostsEntries.get(host).stream() + .filter(key -> key.getKeyEntry().getKeyType().equals(KeyUtils.getKeyType(serverKey))) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Missing updated key for " + KeyUtils.getKeyType(serverKey))); outputDebugMessage("Re-validate host=%s", entry); - Mockito.when(session.getConnectAddress()).thenReturn(hostIdentity); - assertTrue("Failed to re-validate server=" + entry, verifier.verifyServerKey(session, hostIdentity, serverKey)); + Mockito.when(session.getConnectAddress()).thenReturn(host); + assertTrue("Failed to re-validate server=" + entry, verifier.verifyServerKey(session, host, serverKey)); }); } @@ -273,7 +308,8 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { KeyPair kp = CommonTestSupportUtils.generateKeyPair(KeyUtils.RSA_ALGORITHM, 1024); PublicKey modifiedKey = kp.getPublic(); AtomicInteger acceptCount = new AtomicInteger(0); - ServerKeyVerifier verifier + AtomicInteger unknownCount = new AtomicInteger(0); + KnownHostsServerKeyVerifier verifier = new KnownHostsServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE, createKnownHostsCopy()) { @Override public boolean acceptModifiedServerKey( @@ -281,20 +317,41 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { KnownHostEntry entry, PublicKey expected, PublicKey actual) throws Exception { acceptCount.incrementAndGet(); + assertNull("Unexpected marker for " + remoteAddress, entry.getMarker()); assertSame("Mismatched actual key for " + remoteAddress, modifiedKey, actual); return super.acceptModifiedServerKey(clientSession, remoteAddress, entry, expected, actual); } + + @Override + protected boolean acceptUnknownHostKey( + ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey) { + unknownCount.incrementAndGet(); + return false; + } }; int validationCount = 0; + int validUnknownCount = 0; // Cannot use forEach because the validation count variable is not effectively final - for (Map.Entry<SshdSocketAddress, KnownHostEntry> ke : hostsEntries.entrySet()) { + for (Map.Entry<SshdSocketAddress, List<KnownHostEntry>> ke : hostsEntries.entrySet()) { SshdSocketAddress hostIdentity = ke.getKey(); - KnownHostEntry entry = ke.getValue(); - outputDebugMessage("Verify host=%s", entry); - assertFalse("Unexpected to verification success for " + entry, invokeVerifier(verifier, hostIdentity, modifiedKey)); - validationCount++; - assertEquals("Mismatched invocation count for host=" + entry, validationCount, acceptCount.get()); + for (KnownHostEntry entry : ke.getValue()) { + outputDebugMessage("Verify host=%s", entry); + assertFalse("Unexpected to verification success for " + entry, + invokeVerifier(verifier, hostIdentity, modifiedKey)); + long acceptedCount = ke.getValue().stream() + .filter(k -> !"revoked".equals(k.getMarker())) + .count(); + if (acceptedCount == 0) { + validUnknownCount++; + } else { + validationCount++; + } + assertEquals("Mismatched invocation count (acceptModifiedServerKey) for host=" + entry, validationCount, + acceptCount.get()); + assertEquals("Mismatched invocation count (acceptUnknownHostKey) host=" + entry, validUnknownCount, + unknownCount.get()); + } } } @@ -303,6 +360,7 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { KeyPair kp = CommonTestSupportUtils.generateKeyPair(KeyUtils.RSA_ALGORITHM, 1024); PublicKey modifiedKey = kp.getPublic(); Path path = createKnownHostsCopy(); + Files.deleteIfExists(path); ServerKeyVerifier verifier = new KnownHostsServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE, path) { @Override public boolean acceptModifiedServerKey( @@ -314,18 +372,26 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { } }; - hostsEntries.forEach((host, entry) -> { - outputDebugMessage("Verify host=%s", entry); - assertTrue("Failed to verify " + entry, invokeVerifier(verifier, host, modifiedKey)); + hostsEntries.forEach((host, list) -> { + outputDebugMessage("Verify host=%s", host); + assertTrue("Failed to verify " + host, invokeVerifier(verifier, host, modifiedKey)); }); String expected = PublicKeyEntry.toString(modifiedKey); - Map<SshdSocketAddress, KnownHostEntry> updatedKeys = loadEntries(path); - hostsEntries.forEach((hostIdentity, original) -> { - KnownHostEntry updated = updatedKeys.remove(hostIdentity); - assertNotNull("No updated entry for " + original, updated); + Map<SshdSocketAddress, List<KnownHostEntry>> updatedKeys = loadEntries(path); + hostsEntries.keySet().forEach(host -> { + KnownHostEntry updated = updatedKeys.get(host).stream() + .filter(key -> key.getKeyEntry().getKeyType().equals(KeyUtils.getKeyType(modifiedKey))) + .findAny() + .orElseThrow( + () -> new IllegalStateException("Missing updated key for " + KeyUtils.getKeyType(modifiedKey))); + updatedKeys.get(host).remove(updated); + if (updatedKeys.get(host).isEmpty()) { + updatedKeys.remove(host); + } String actual = updated.getConfigLine(); + assertNotNull("No updated entry for " + hostsEntries.get(host), actual); int pos = actual.indexOf(' '); if (actual.charAt(0) == KnownHostEntry.MARKER_INDICATOR) { for (pos++; pos < actual.length(); pos++) { @@ -337,7 +403,7 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { } actual = GenericUtils.trimToEmpty(actual.substring(pos + 1)); - assertEquals("Mismatched updated value for host=" + hostIdentity, expected, actual); + assertEquals("Mismatched updated value for host=" + host, expected, actual); }); assertTrue("Unexpected extra updated entries: " + updatedKeys, updatedKeys.isEmpty()); @@ -395,8 +461,8 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { boolean accepted2 = invokeVerifier(verifier, address2, serverKey2); assertTrue("Accepted on port=" + port2 + " ?", accepted2); - Map<SshdSocketAddress, KnownHostEntry> updatedKeys = loadEntries(path); - assertEquals("Mismatched total entries count", 2, MapEntryUtils.size(updatedKeys)); + Map<SshdSocketAddress, List<KnownHostEntry>> updatedKeys = loadEntries(path); + assertEquals("Mismatched total entries count", 2, updatedKeys.size()); } private Path createKnownHostsCopy() throws IOException { @@ -418,21 +484,25 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { return verifier.verifyServerKey(session, hostIdentity, serverKey); } - private static Map<SshdSocketAddress, KnownHostEntry> loadEntries(Path file) throws IOException { + private static Map<SshdSocketAddress, List<KnownHostEntry>> loadEntries(Path file) throws IOException { Collection<KnownHostEntry> entries = KnownHostEntry.readKnownHostEntries(file); if (GenericUtils.isEmpty(entries)) { return Collections.emptyMap(); } - Map<SshdSocketAddress, KnownHostEntry> hostsMap = new TreeMap<>(SshdSocketAddress.BY_HOST_AND_PORT); + Map<SshdSocketAddress, List<KnownHostEntry>> hostsMap = new TreeMap<>(SshdSocketAddress.BY_HOST_AND_PORT); for (KnownHostEntry entry : entries) { String line = entry.getConfigLine(); outputDebugMessage("loadTestLines(%s) processing %s", file, line); // extract hosts - int pos = line.indexOf(' '); - String patterns = line.substring(0, pos); + int markerOffset = 0; + if (line.startsWith("@")) { + markerOffset = line.indexOf(' ') + 1; + } + int pos = line.indexOf(' ', markerOffset); + String patterns = line.substring(markerOffset, pos); if (entry.getHashedEntry() != null) { - assertNull("Multiple hashed entries in file", hostsMap.put(new SshdSocketAddress(HASHED_HOST, 0), entry)); + hostsMap.computeIfAbsent(new SshdSocketAddress(HASHED_HOST, 0), k -> new ArrayList<>()).add(entry); } else { String[] addrs = GenericUtils.split(patterns, ','); for (String a : addrs) { @@ -444,7 +514,7 @@ public class KnownHostsServerKeyVerifierTest extends BaseTestSupport { port = Integer.parseInt(a.substring(pos + 2)); a = a.substring(1, pos); } - assertNull("Multiple entries for address=" + a, hostsMap.put(new SshdSocketAddress(a, port), entry)); + hostsMap.computeIfAbsent(new SshdSocketAddress(a, port), k -> new ArrayList<>()).add(entry); } } } diff --git a/sshd-core/src/test/resources/org/apache/sshd/client/keyverifier/known_hosts b/sshd-core/src/test/resources/org/apache/sshd/client/keyverifier/known_hosts index 50a2944b8..b232c3820 100644 --- a/sshd-core/src/test/resources/org/apache/sshd/client/keyverifier/known_hosts +++ b/sshd-core/src/test/resources/org/apache/sshd/client/keyverifier/known_hosts @@ -4,7 +4,10 @@ server.sshd.apache.org,10.23.222.240 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbml # comment 10.23.222.127 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBCbZVVpqEHGLNWMqMeyU1VbWb91XteoamVcgpy4yxNVbZffb5IDdbo1ons/y9KAhcub6LZeLrvXzVUZbXCZiUkg= +10.23.222.127 ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAvQK9DhBSStbi2CaZNOo5vHy9Nga/hLCnO19tL6i5U/5Nhh5Y8W+tL+AA8hqD/doLBnyaEv2xjfzECwKlStc9HWx6EcJ+9B1rA4+5HztuUEWxuozNnkvcScjTBBqEd7fPPt0INI+pSZRYa2InEBBUUHTt1YaDEXamM/j4RVKovEH7Efgq9VUti148ZG90/w9V5ZT1o6yhuOw9UbME/eHIbS2E9P/Gy33OhkAgTLyOfCZAdJiYvcvFXqNWSKVFx3H5hSolh9ppxVFVFj2hW6QtvgYTphLU0ccHOWTBd/UToG4Xd9GjgSoD1pAI9e0NwBOsUfiqSzO99wqpzs6erfwelQ== 10.163.4.61 ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAvQK9DhBSStbi2CaZNOo5vHy9Nga/hLCnO19tL6i5U/5Nhh5Y8W+tL+AA8hqD/doLBnyaEv2xjfzECwKlStc9HWx6EcJ+9B1rA4+5HztuUEWxuozNnkvcScjTBBqEd7fPPt0INI+pSZRYa2InEBBUUHTt1YaDEXamM/j4RVKovEH7Efgq9VUti148ZG90/w9V5ZT1o6yhuOw9UbME/eHIbS2E9P/Gy33OhkAgTLyOfCZAdJiYvcvFXqNWSKVFx3H5hSolh9ppxVFVFj2hW6QtvgYTphLU0ccHOWTBd/UToG4Xd9GjgSoD1pAI9e0NwBOsUfiqSzO99wqpzs6erfwelQ== +@revoked 10.163.4.61 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBCbZVVpqEHGLNWMqMeyU1VbWb91XteoamVcgpy4yxNVbZffb5IDdbo1ons/y9KAhcub6LZeLrvXzVUZbXCZiUkg= +@revoked 10.163.4.62 ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAvQK9DhBSStbi2CaZNOo5vHy9Nga/hLCnO19tL6i5U/5Nhh5Y8W+tL+AA8hqD/doLBnyaEv2xjfzECwKlStc9HWx6EcJ+9B1rA4+5HztuUEWxuozNnkvcScjTBBqEd7fPPt0INI+pSZRYa2InEBBUUHTt1YaDEXamM/j4RVKovEH7Efgq9VUti148ZG90/w9V5ZT1o6yhuOw9UbME/eHIbS2E9P/Gy33OhkAgTLyOfCZAdJiYvcvFXqNWSKVFx3H5hSolh9ppxVFVFj2hW6QtvgYTphLU0ccHOWTBd/UToG4Xd9GjgSoD1pAI9e0NwBOsUfiqSzO99wqpzs6erfwelQ== 192.168.174.129 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBJvwXQ0Wc7TdhmjaTdkrHJvatWrw/6t6W1Hgh2nTauN+rhsMKYbmQeFTnzsP1NctWzQXwsqIOGcXIMNVhT92jgQ=