Discussion:
mina-sshd git commit: [SSHD-852] Make sure non-standard port and hash are correctly written to known-hosts file
l***@apache.org
2018-10-19 17:29:04 UTC
Permalink
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 <***@apache.org>
Authored: Fri Oct 19 20:33:29 2018 +0300
Committer: Lyor Goldstein <***@apache.org>
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 ***@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 ***@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;
}

Loading...