Repository: mina-sshd Updated Branches: refs/heads/master 326725da2 -> 422e7428c
[SSHD-852] Make sure non-standard port and hash are correctly written to known-hosts file Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/422e7428 Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/422e7428 Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/422e7428 Branch: refs/heads/master Commit: 422e7428c15e1cebada9f6ab21d06d8b2b70fd21 Parents: 326725d Author: Lyor Goldstein <[email protected]> Authored: Fri Oct 19 20:33:29 2018 +0300 Committer: Lyor Goldstein <[email protected]> Committed: Fri Oct 19 20:34:23 2018 +0300 ---------------------------------------------------------------------- .../client/config/hosts/HostPatternValue.java | 14 ++-- .../client/config/hosts/KnownHostEntry.java | 11 +--- .../client/config/hosts/KnownHostHashValue.java | 38 +++++++++-- .../config/hosts/KnownHostHashValueTest.java | 27 +++++--- .../KnownHostsServerKeyVerifier.java | 67 ++++++++++---------- 5 files changed, 92 insertions(+), 65 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/422e7428/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/HostPatternValue.java ---------------------------------------------------------------------- diff --git a/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/HostPatternValue.java b/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/HostPatternValue.java index 20d682f..50a3820 100644 --- a/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/HostPatternValue.java +++ b/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/HostPatternValue.java @@ -19,6 +19,7 @@ package org.apache.sshd.client.config.hosts; +import java.io.IOException; import java.util.regex.Pattern; import org.apache.sshd.common.util.GenericUtils; @@ -82,14 +83,11 @@ public class HostPatternValue { } int portValue = getPort(); - if (portValue > 0) { - sb.append(HostPatternsHolder.NON_STANDARD_PORT_PATTERN_ENCLOSURE_START_DELIM); - } - sb.append(purePattern); - if (portValue > 0) { - sb.append(HostPatternsHolder.NON_STANDARD_PORT_PATTERN_ENCLOSURE_END_DELIM); - sb.append(HostPatternsHolder.PORT_VALUE_DELIMITER); - sb.append(portValue); + try { + KnownHostHashValue.appendHostPattern(sb, purePattern, portValue); + } catch (IOException e) { + throw new RuntimeException("Unexpected (" + e.getClass().getSimpleName() + ") failure" + + " to append host pattern of " + purePattern + ":" + portValue + ": " + e.getMessage(), e); } return sb.toString(); http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/422e7428/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/KnownHostEntry.java ---------------------------------------------------------------------- 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 c6f0150..4a8e9ca 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 @@ -126,17 +126,8 @@ public class KnownHostEntry extends HostPatternsHolder { return true; } - String address; - if ((port > 0) && (port != ConfigFileReaderSupport.DEFAULT_PORT)) { - address = HostPatternsHolder.NON_STANDARD_PORT_PATTERN_ENCLOSURE_START_DELIM - + host + HostPatternsHolder.NON_STANDARD_PORT_PATTERN_ENCLOSURE_END_DELIM - + HostPatternsHolder.PORT_VALUE_DELIMITER + port; - } else { - address = host; - } - KnownHostHashValue hash = getHashedEntry(); - return (hash != null) && hash.isHostMatch(address); + return (hash != null) && hash.isHostMatch(host, port); } @Override http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/422e7428/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/KnownHostHashValue.java ---------------------------------------------------------------------- diff --git a/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/KnownHostHashValue.java b/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/KnownHostHashValue.java index 28a3d1f..294d6a4 100644 --- a/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/KnownHostHashValue.java +++ b/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/KnownHostHashValue.java @@ -29,6 +29,7 @@ import org.apache.sshd.common.Factory; import org.apache.sshd.common.NamedFactory; import org.apache.sshd.common.NamedResource; import org.apache.sshd.common.RuntimeSshException; +import org.apache.sshd.common.config.ConfigFileReaderSupport; import org.apache.sshd.common.mac.Mac; import org.apache.sshd.common.util.GenericUtils; import org.apache.sshd.common.util.NumberUtils; @@ -82,17 +83,18 @@ public class KnownHostHashValue { * Checks if the host matches the hash * * @param host The host name/address - ignored if {@code null}/empty + * @param port The access port - ignored if non-positive or SSH default * @return {@code true} if host matches the hash * @throws RuntimeException If entry not properly initialized */ - public boolean isHostMatch(String host) { + public boolean isHostMatch(String host, int port) { if (GenericUtils.isEmpty(host)) { return false; } try { byte[] expected = getDigestValue(); - byte[] actual = calculateHashValue(host, getDigester(), getSaltValue()); + byte[] actual = calculateHashValue(host, port, getDigester(), getSaltValue()); return Arrays.equals(expected, actual); } catch (Throwable t) { if (t instanceof RuntimeException) { @@ -119,18 +121,42 @@ public class KnownHostHashValue { } // see http://nms.lcs.mit.edu/projects/ssh/README.hashed-hosts - public static byte[] calculateHashValue(String host, Factory<? extends Mac> factory, byte[] salt) throws Exception { - return calculateHashValue(host, factory.create(), salt); + public static byte[] calculateHashValue(String host, int port, Factory<? extends Mac> factory, byte[] salt) throws Exception { + return calculateHashValue(host, port, factory.create(), salt); } - public static byte[] calculateHashValue(String host, Mac mac, byte[] salt) throws Exception { + public static byte[] calculateHashValue(String host, int port, Mac mac, byte[] salt) throws Exception { mac.init(salt); - byte[] hostBytes = host.getBytes(StandardCharsets.UTF_8); + String hostPattern = createHostPattern(host, port); + byte[] hostBytes = hostPattern.getBytes(StandardCharsets.UTF_8); mac.update(hostBytes); return mac.doFinal(); } + public static String createHostPattern(String host, int port) { + try { + return appendHostPattern(new StringBuilder(host.length() + 8 /* port if necessary */), host, port).toString(); + } catch (IOException e) { + throw new RuntimeException("Unexpected (" + e.getClass().getSimpleName() + ") failure" + + " to generate host pattern of " + host + ":" + port + ": " + e.getMessage(), e); + } + } + + public static <A extends Appendable> A appendHostPattern(A sb, String host, int port) throws IOException { + boolean nonDefaultPort = (port > 0) && (port != ConfigFileReaderSupport.DEFAULT_PORT); + if (nonDefaultPort) { + sb.append(HostPatternsHolder.NON_STANDARD_PORT_PATTERN_ENCLOSURE_START_DELIM); + } + sb.append(host); + if (nonDefaultPort) { + sb.append(HostPatternsHolder.NON_STANDARD_PORT_PATTERN_ENCLOSURE_END_DELIM); + sb.append(HostPatternsHolder.PORT_VALUE_DELIMITER); + sb.append(Integer.toString(port)); + } + return sb; + } + public static <A extends Appendable> A append(A sb, KnownHostHashValue hashValue) throws IOException { return (hashValue == null) ? sb : append(sb, hashValue.getDigester(), hashValue.getSaltValue(), hashValue.getDigestValue()); } http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/422e7428/sshd-common/src/test/java/org/apache/sshd/client/config/hosts/KnownHostHashValueTest.java ---------------------------------------------------------------------- diff --git a/sshd-common/src/test/java/org/apache/sshd/client/config/hosts/KnownHostHashValueTest.java b/sshd-common/src/test/java/org/apache/sshd/client/config/hosts/KnownHostHashValueTest.java index 4f2bf0d..3ecea7f 100644 --- a/sshd-common/src/test/java/org/apache/sshd/client/config/hosts/KnownHostHashValueTest.java +++ b/sshd-common/src/test/java/org/apache/sshd/client/config/hosts/KnownHostHashValueTest.java @@ -22,6 +22,7 @@ package org.apache.sshd.client.config.hosts; import java.util.Arrays; import java.util.Collection; +import org.apache.sshd.common.config.ConfigFileReaderSupport; import org.apache.sshd.util.test.JUnit4ClassRunnerWithParametersFactory; import org.apache.sshd.util.test.JUnitTestSupport; import org.apache.sshd.util.test.NoIoTestCase; @@ -43,11 +44,13 @@ import org.junit.runners.Parameterized.UseParametersRunnerFactory; @Category({ NoIoTestCase.class }) public class KnownHostHashValueTest extends JUnitTestSupport { private final String hostName; + private final int port; private final String hashValue; private final KnownHostHashValue hash; - public KnownHostHashValueTest(String hostName, String hashValue) { + public KnownHostHashValueTest(String hostName, int port, String hashValue) { this.hostName = hostName; + this.port = port; this.hashValue = hashValue; this.hash = KnownHostHashValue.parse(hashValue); } @@ -56,10 +59,13 @@ public class KnownHostHashValueTest extends JUnitTestSupport { public static Collection<Object[]> parameters() { return Arrays.asList( // line generated `ssh xenon@localhost -p 10022 hostname` (SSH-2.0-OpenSSH_7.5) - new String[]{"[localhost]:10022", "|1|qhjoqX12EcnwZO3KNbpoFbxrdYE=|J+voEFzRbRL49TiHV+jbUfaS+kg="}, + new Object[]{"localhost", 10022, + "|1|qhjoqX12EcnwZO3KNbpoFbxrdYE=|J+voEFzRbRL49TiHV+jbUfaS+kg="}, // line generated `ssh xenon@localhost hostname` (SSH-2.0-OpenSSH_7.5) - new String[]{"localhost", "|1|vLQs+atPgodQmPes21ZaMSgLD0s=|A2K2Ym0ZPtQmD8kB3FVViQvQ7qQ="}, - new String[]{"192.168.1.61", "|1|F1E1KeoE/eEWhi10WpGv4OdiO6Y=|3988QV0VE8wmZL7suNrYQLITLCg="} + new Object[]{"localhost", ConfigFileReaderSupport.DEFAULT_PORT, + "|1|vLQs+atPgodQmPes21ZaMSgLD0s=|A2K2Ym0ZPtQmD8kB3FVViQvQ7qQ="}, + new Object[]{"192.168.1.61", ConfigFileReaderSupport.DEFAULT_PORT, + "|1|F1E1KeoE/eEWhi10WpGv4OdiO6Y=|3988QV0VE8wmZL7suNrYQLITLCg="} ); } @@ -71,19 +77,24 @@ public class KnownHostHashValueTest extends JUnitTestSupport { @Test public void testHostMatch() { - assertTrue("Specified host does not match", hash.isHostMatch(hostName)); - assertFalse("Unexpected host match", hash.isHostMatch(getCurrentTestName())); + assertTrue("Specified host does not match", hash.isHostMatch(hostName, port)); + assertFalse("Unexpected host match", hash.isHostMatch(getCurrentTestName(), port)); } @Test public void testCalculateHashValue() throws Exception { byte[] expected = hash.getDigestValue(); - byte[] actual = KnownHostHashValue.calculateHashValue(hostName, hash.getDigester(), hash.getSaltValue()); + byte[] actual = KnownHostHashValue.calculateHashValue( + hostName, port, hash.getDigester(), hash.getSaltValue()); assertArrayEquals("Mismatched hash value", expected, actual); } @Override public String toString() { - return getClass().getSimpleName() + "[host=" + hostName + ", hashValue=" + hashValue + "]"; + return getClass().getSimpleName() + + "[host=" + hostName + + ", port=" + port + + ", hashValue=" + hashValue + + "]"; } } http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/422e7428/sshd-core/src/main/java/org/apache/sshd/client/keyverifier/KnownHostsServerKeyVerifier.java ---------------------------------------------------------------------- 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 c4cb849..c06f270 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 @@ -39,7 +39,6 @@ import java.util.Objects; import java.util.TreeSet; import java.util.concurrent.atomic.AtomicReference; -import org.apache.sshd.client.config.hosts.HostPatternsHolder; import org.apache.sshd.client.config.hosts.KnownHostEntry; import org.apache.sshd.client.config.hosts.KnownHostHashValue; import org.apache.sshd.client.session.ClientSession; @@ -242,7 +241,8 @@ public class KnownHostsServerKeyVerifier return null; } - AuthorizedKeyEntry authEntry = ValidateUtils.checkNotNull(entry.getKeyEntry(), "No key extracted from %s", entry); + AuthorizedKeyEntry authEntry = + ValidateUtils.checkNotNull(entry.getKeyEntry(), "No key extracted from %s", entry); PublicKey key = authEntry.resolvePublicKey(resolver); if (log.isDebugEnabled()) { log.debug("resolveHostKey({}) loaded {}-{}", entry, KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key)); @@ -314,14 +314,15 @@ public class KnownHostsServerKeyVerifier protected void updateModifiedServerKey( ClientSession clientSession, SocketAddress remoteAddress, HostEntryPair match, PublicKey actual, Path file, Collection<HostEntryPair> knownHosts) - throws Exception { + throws Exception { KnownHostEntry entry = match.getHostEntry(); String matchLine = ValidateUtils.checkNotNullAndNotEmpty(entry.getConfigLine(), "No entry config line"); - String newLine = prepareModifiedServerKeyLine(clientSession, remoteAddress, entry, matchLine, match.getServerKey(), actual); + String newLine = prepareModifiedServerKeyLine( + clientSession, remoteAddress, entry, matchLine, match.getServerKey(), actual); if (GenericUtils.isEmpty(newLine)) { if (log.isDebugEnabled()) { log.debug("updateModifiedServerKey({})[{}] no replacement generated for {}", - clientSession, remoteAddress, matchLine); + clientSession, remoteAddress, matchLine); } return; } @@ -329,7 +330,7 @@ public class KnownHostsServerKeyVerifier if (matchLine.equals(newLine)) { if (log.isDebugEnabled()) { log.debug("updateModifiedServerKey({})[{}] unmodified updated line for {}", - clientSession, remoteAddress, matchLine); + clientSession, remoteAddress, matchLine); } return; } @@ -449,7 +450,7 @@ public class KnownHostsServerKeyVerifier PublicKey serverKey, Path file, Collection<HostEntryPair> knownHosts, Throwable reason) { // NOTE !!! this may mean the file is corrupted, but it can be recovered from the known hosts log.warn("acceptKnownHostEntries({})[{}] failed ({}) to update modified server key of {}: {}", - clientSession, remoteAddress, reason.getClass().getSimpleName(), match, reason.getMessage()); + clientSession, remoteAddress, reason.getClass().getSimpleName(), match, reason.getMessage()); if (log.isDebugEnabled()) { log.debug("acceptKnownHostEntries(" + clientSession + ")[" + remoteAddress + "]" + " modified key update failure details", reason); @@ -563,7 +564,7 @@ public class KnownHostsServerKeyVerifier protected boolean acceptUnknownHostKey(ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey) { if (log.isDebugEnabled()) { log.debug("acceptUnknownHostKey({}) host={}, key={}", - clientSession, remoteAddress, KeyUtils.getFingerPrint(serverKey)); + clientSession, remoteAddress, KeyUtils.getFingerPrint(serverKey)); } if (delegate.verifyServerKey(clientSession, remoteAddress, serverKey)) { @@ -597,9 +598,9 @@ public class KnownHostsServerKeyVerifier protected void handleKnownHostsFileUpdateFailure(ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey, Path file, Collection<HostEntryPair> knownHosts, Throwable reason) { log.warn("handleKnownHostsFileUpdateFailure({})[{}] failed ({}) to update key={}-{} in {}: {}", - clientSession, remoteAddress, reason.getClass().getSimpleName(), - KeyUtils.getKeyType(serverKey), KeyUtils.getFingerPrint(serverKey), - file, reason.getMessage()); + clientSession, remoteAddress, reason.getClass().getSimpleName(), + KeyUtils.getKeyType(serverKey), KeyUtils.getFingerPrint(serverKey), + file, reason.getMessage()); if (log.isDebugEnabled()) { log.debug("handleKnownHostsFileUpdateFailure(" + clientSession + ")[" + remoteAddress + "]" + " file update failure details", reason); @@ -624,13 +625,14 @@ public class KnownHostsServerKeyVerifier * @see #resetReloadAttributes() */ protected KnownHostEntry updateKnownHostsFile( - ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey, Path file, Collection<HostEntryPair> knownHosts) - throws Exception { + ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey, + Path file, Collection<HostEntryPair> knownHosts) + throws Exception { KnownHostEntry entry = prepareKnownHostEntry(clientSession, remoteAddress, serverKey); if (entry == null) { if (log.isDebugEnabled()) { log.debug("updateKnownHostsFile({})[{}] no entry generated for key={}", - clientSession, remoteAddress, KeyUtils.getFingerPrint(serverKey)); + clientSession, remoteAddress, KeyUtils.getFingerPrint(serverKey)); } return null; @@ -641,7 +643,9 @@ public class KnownHostsServerKeyVerifier boolean reuseExisting = Files.exists(file) && (Files.size(file) > 0); byte[] eolBytes = IoUtils.getEOLBytes(); synchronized (updateLock) { - try (OutputStream output = reuseExisting ? Files.newOutputStream(file, StandardOpenOption.APPEND) : Files.newOutputStream(file)) { + try (OutputStream output = reuseExisting + ? Files.newOutputStream(file, StandardOpenOption.APPEND) + : Files.newOutputStream(file)) { if (reuseExisting) { output.write(eolBytes); // separate from previous lines } @@ -671,8 +675,11 @@ public class KnownHostsServerKeyVerifier * @see #resolveHostNetworkIdentities(ClientSession, SocketAddress) * @see KnownHostEntry#getConfigLine() */ - protected KnownHostEntry prepareKnownHostEntry(ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey) throws Exception { - Collection<SshdSocketAddress> patterns = resolveHostNetworkIdentities(clientSession, remoteAddress); + protected KnownHostEntry prepareKnownHostEntry( + ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey) + throws Exception { + Collection<SshdSocketAddress> patterns = + resolveHostNetworkIdentities(clientSession, remoteAddress); if (GenericUtils.isEmpty(patterns)) { return null; } @@ -684,13 +691,14 @@ public class KnownHostsServerKeyVerifier sb.append(','); } - NamedFactory<Mac> digester = getHostValueDigester(clientSession, remoteAddress, hostIdentity); + NamedFactory<Mac> digester = + getHostValueDigester(clientSession, remoteAddress, hostIdentity); if (digester != null) { if (rnd == null) { FactoryManager manager = - Objects.requireNonNull(clientSession.getFactoryManager(), "No factory manager"); + Objects.requireNonNull(clientSession.getFactoryManager(), "No factory manager"); Factory<? extends Random> factory = - Objects.requireNonNull(manager.getRandomFactory(), "No random factory"); + Objects.requireNonNull(manager.getRandomFactory(), "No random factory"); rnd = Objects.requireNonNull(factory.create(), "No randomizer created"); } @@ -699,20 +707,12 @@ public class KnownHostsServerKeyVerifier byte[] salt = new byte[blockSize]; rnd.fill(salt); - byte[] digestValue = KnownHostHashValue.calculateHashValue(hostIdentity.getHostName(), mac, salt); + byte[] digestValue = + KnownHostHashValue.calculateHashValue( + hostIdentity.getHostName(), hostIdentity.getPort(), mac, salt); KnownHostHashValue.append(sb, digester, salt, digestValue); } else { - int portValue = hostIdentity.getPort(); - boolean nonDefaultPort = (portValue > 0) && (portValue != ConfigFileReaderSupport.DEFAULT_PORT); - if (nonDefaultPort) { - sb.append(HostPatternsHolder.NON_STANDARD_PORT_PATTERN_ENCLOSURE_START_DELIM); - } - sb.append(hostIdentity.getHostName()); - if (nonDefaultPort) { - sb.append(HostPatternsHolder.NON_STANDARD_PORT_PATTERN_ENCLOSURE_END_DELIM); - sb.append(HostPatternsHolder.PORT_VALUE_DELIMITER); - sb.append(portValue); - } + KnownHostHashValue.appendHostPattern(sb, hostIdentity.getHostName(), hostIdentity.getPort()); } } @@ -730,7 +730,8 @@ public class KnownHostsServerKeyVerifier * @param hostIdentity The entry's host name/address * @return The digester {@link NamedFactory} - {@code null} if no hashing is to be made */ - protected NamedFactory<Mac> getHostValueDigester(ClientSession clientSession, SocketAddress remoteAddress, SshdSocketAddress hostIdentity) { + protected NamedFactory<Mac> getHostValueDigester( + ClientSession clientSession, SocketAddress remoteAddress, SshdSocketAddress hostIdentity) { return null; }
