This is an automated email from the ASF dual-hosted git repository.
lgoldstein 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 a52b1b3 [SSHD-1063] Fixed known-hosts file server key verifier
matching of same host with different ports
a52b1b3 is described below
commit a52b1b3cb46a601d34e59c43079918c0b09e0622
Author: Lyor Goldstein <[email protected]>
AuthorDate: Tue Aug 25 20:01:36 2020 +0300
[SSHD-1063] Fixed known-hosts file server key verifier matching of same
host with different ports
---
CHANGES.md | 2 +
.../sshd/cli/client/SshClientCliSupport.java | 4 +-
.../client/config/hosts/HostPatternsHolder.java | 19 +++++--
.../client/config/hosts/KnownHostHashValue.java | 4 +-
.../java/org/apache/sshd/common/SshConstants.java | 4 ++
.../KnownHostsServerKeyVerifierTest.java | 65 ++++++++++++++++++++--
.../org/apache/sshd/scp/common/ScpLocation.java | 3 +-
.../sftp/client/fs/SftpFileSystemProvider.java | 2 +-
8 files changed, 86 insertions(+), 17 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index bf19bdd..1893207 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -47,3 +47,5 @@ or `-key-file` command line option.
* [SSHD-1057](https://issues.apache.org/jira/browse/SSHD-1057) Added
capability to select a ShellFactory based on the current session + use it for
"WinSCP"
* [SSHD-1058](https://issues.apache.org/jira/browse/SSHD-1058) Improve
exception logging strategy.
* [SSHD-1059](https://issues.apache.org/jira/browse/SSHD-1059) Do not send
heartbeat if KEX state not DONE
+* [SSHD-1063](https://issues.apache.org/jira/browse/SSHD-1063) Fixed
known-hosts file server key verifier matching of same host with different ports
+
diff --git
a/sshd-cli/src/main/java/org/apache/sshd/cli/client/SshClientCliSupport.java
b/sshd-cli/src/main/java/org/apache/sshd/cli/client/SshClientCliSupport.java
index 685c812..db26179 100644
--- a/sshd-cli/src/main/java/org/apache/sshd/cli/client/SshClientCliSupport.java
+++ b/sshd-cli/src/main/java/org/apache/sshd/cli/client/SshClientCliSupport.java
@@ -251,9 +251,7 @@ public abstract class SshClientCliSupport extends
CliSupport {
login = OsUtils.getCurrentUser();
}
- if (port <= 0) {
- port = SshConstants.DEFAULT_PORT;
- }
+ port = SshConstants.TO_EFFECTIVE_PORT.applyAsInt(port);
HostConfigEntry entry = resolveHost(client, login, host, port,
proxyJump);
// TODO use a configurable wait time
diff --git
a/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/HostPatternsHolder.java
b/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/HostPatternsHolder.java
index ecdd6dd..7a7a88c 100644
---
a/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/HostPatternsHolder.java
+++
b/sshd-common/src/main/java/org/apache/sshd/client/config/hosts/HostPatternsHolder.java
@@ -29,6 +29,7 @@ import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import org.apache.sshd.common.SshConstants;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.ValidateUtils;
@@ -186,6 +187,10 @@ public abstract class HostPatternsHolder {
continue;
}
+ if (!isPortMatch(port, pv.getPort())) {
+ continue;
+ }
+
/*
* According to
https://www.freebsd.org/cgi/man.cgi?query=ssh_config&sektion=5:
*
@@ -196,11 +201,6 @@ public abstract class HostPatternsHolder {
return false;
}
- int pvPort = pv.getPort();
- if ((pvPort != 0) && (port != 0) && (pvPort != port)) {
- continue;
- }
-
matchFound = true;
}
@@ -208,6 +208,15 @@ public abstract class HostPatternsHolder {
}
/**
+ * @param port1 1st port value - if non-positive the assumed to be {@link
SshConstants#DEFAULT_PORT DEFAULT_PORT}
+ * @param port2 2nd port value - if non-positive the assumed to be {@link
SshConstants#DEFAULT_PORT DEFAULT_PORT}
+ * @return {@code true} if ports are effectively equal
+ */
+ public static boolean isPortMatch(int port1, int port2) {
+ return SshConstants.TO_EFFECTIVE_PORT.applyAsInt(port1) ==
SshConstants.TO_EFFECTIVE_PORT.applyAsInt(port2);
+ }
+
+ /**
* Checks if a given host name / address matches a host pattern
*
* @param host The host name / address - ignored if {@code null}/empty
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 186b86b..bb7bd72 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
@@ -136,7 +136,7 @@ public class KnownHostHashValue {
}
public static String createHostPattern(String host, int port) {
- if ((port <= 0) || (port == SshConstants.DEFAULT_PORT)) {
+ if (SshConstants.TO_EFFECTIVE_PORT.applyAsInt(port) ==
SshConstants.DEFAULT_PORT) {
return host;
}
@@ -151,7 +151,7 @@ public class KnownHostHashValue {
}
public static <A extends Appendable> A appendHostPattern(A sb, String
host, int port) throws IOException {
- boolean nonDefaultPort = (port > 0) && (port !=
SshConstants.DEFAULT_PORT);
+ boolean nonDefaultPort =
SshConstants.TO_EFFECTIVE_PORT.applyAsInt(port) != SshConstants.DEFAULT_PORT;
if (nonDefaultPort) {
sb.append(HostPatternsHolder.NON_STANDARD_PORT_PATTERN_ENCLOSURE_START_DELIM);
}
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/SshConstants.java
b/sshd-common/src/main/java/org/apache/sshd/common/SshConstants.java
index 900a865..05d2019 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/SshConstants.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/SshConstants.java
@@ -23,6 +23,7 @@ import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
+import java.util.function.IntUnaryOperator;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.logging.LoggingUtils;
@@ -35,6 +36,9 @@ import org.apache.sshd.common.util.logging.LoggingUtils;
public final class SshConstants {
public static final int DEFAULT_PORT = 22;
+ /** Converts non-positive port value to {@value #DEFAULT_PORT} */
+ public static final IntUnaryOperator TO_EFFECTIVE_PORT = port -> (port >
0) ? port : DEFAULT_PORT;
+
//
// SSH message identifiers
//
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 9482ffd..d23521c 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
@@ -41,6 +41,7 @@ import org.apache.sshd.client.config.hosts.KnownHostHashValue;
import
org.apache.sshd.client.keyverifier.KnownHostsServerKeyVerifier.HostEntryPair;
import org.apache.sshd.client.session.ClientSession;
import org.apache.sshd.common.NamedFactory;
+import org.apache.sshd.common.SshConstants;
import org.apache.sshd.common.config.keys.AuthorizedKeyEntry;
import org.apache.sshd.common.config.keys.KeyUtils;
import org.apache.sshd.common.config.keys.PublicKeyEntry;
@@ -266,8 +267,8 @@ public class KnownHostsServerKeyVerifierTest extends
BaseTestSupport {
@Test
public void testRejectModifiedServerKey() throws Exception {
KeyPair kp =
CommonTestSupportUtils.generateKeyPair(KeyUtils.RSA_ALGORITHM, 1024);
- final PublicKey modifiedKey = kp.getPublic();
- final AtomicInteger acceptCount = new AtomicInteger(0);
+ PublicKey modifiedKey = kp.getPublic();
+ AtomicInteger acceptCount = new AtomicInteger(0);
ServerKeyVerifier verifier
= new
KnownHostsServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE,
createKnownHostsCopy()) {
@Override
@@ -296,7 +297,7 @@ public class KnownHostsServerKeyVerifierTest extends
BaseTestSupport {
@Test
public void testAcceptModifiedServerKeyUpdatesFile() throws Exception {
KeyPair kp =
CommonTestSupportUtils.generateKeyPair(KeyUtils.RSA_ALGORITHM, 1024);
- final PublicKey modifiedKey = kp.getPublic();
+ PublicKey modifiedKey = kp.getPublic();
Path path = createKnownHostsCopy();
ServerKeyVerifier verifier = new
KnownHostsServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE, path) {
@Override
@@ -338,6 +339,62 @@ public class KnownHostsServerKeyVerifierTest extends
BaseTestSupport {
assertTrue("Unexpected extra updated entries: " + updatedKeys,
updatedKeys.isEmpty());
}
+ @Test // SSHD-1063
+ public void testUpdateSameHost2PortsStdFirstSameKey() throws Exception {
+ testUpdateSameHostWithDifferentPorts(SshConstants.DEFAULT_PORT, 2020,
true);
+ }
+
+ @Test // SSHD-1063
+ public void testUpdateSameHost2PortsStdLastSameKey() throws Exception {
+ testUpdateSameHostWithDifferentPorts(2020, SshConstants.DEFAULT_PORT,
true);
+ }
+
+ @Test // SSHD-1063
+ public void testUpdateSameHost2NonStdPortsSameKey() throws Exception {
+ testUpdateSameHostWithDifferentPorts(2020, 2222, true);
+ }
+
+ @Test // SSHD-1063
+ public void testUpdateSameHost2PortsStdFirstDiffKeys() throws Exception {
+ testUpdateSameHostWithDifferentPorts(SshConstants.DEFAULT_PORT, 2020,
false);
+ }
+
+ @Test // SSHD-1063
+ public void testUpdateSameHost2PortsStdLastDiffKeys() throws Exception {
+ testUpdateSameHostWithDifferentPorts(2020, SshConstants.DEFAULT_PORT,
false);
+ }
+
+ @Test // SSHD-1063
+ public void testUpdateSameHost2NonStdPortsDiffKeys() throws Exception {
+ testUpdateSameHostWithDifferentPorts(2020, 2222, false);
+ }
+
+ private void testUpdateSameHostWithDifferentPorts(int port1, int port2,
boolean useSameKey) throws Exception {
+ Path path = getKnownHostCopyPath();
+ Files.write(path, Collections.singletonList("")); // start empty
+ // accept all unknown entries
+ KnownHostsServerKeyVerifier verifier = new
KnownHostsServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE, path);
+ // Reject modified entries
+ verifier.setModifiedServerKeyAcceptor((clientSession, remoteAddress,
entry, expected, actual) -> false);
+
+ KeyPair kp1 =
CommonTestSupportUtils.generateKeyPair(KeyUtils.RSA_ALGORITHM, 1024);
+ PublicKey serverKey1 = kp1.getPublic();
+
+ SocketAddress address1 = new SshdSocketAddress(HASHED_HOST, port1);
+ boolean accepted1 = invokeVerifier(verifier, address1, serverKey1);
+ assertTrue("Accepted on port=" + port1 + " ?", accepted1);
+
+ KeyPair kp2 = useSameKey ? kp1 :
CommonTestSupportUtils.generateKeyPair(KeyUtils.RSA_ALGORITHM, 1024);
+ PublicKey serverKey2 = kp2.getPublic();
+
+ SocketAddress address2 = new SshdSocketAddress(HASHED_HOST, port2);
+ boolean accepted2 = invokeVerifier(verifier, address2, serverKey2);
+ assertTrue("Accepted on port=" + port2 + " ?", accepted2);
+
+ Map<SshdSocketAddress, KnownHostEntry> updatedKeys = loadEntries(path);
+ assertEquals("Mismatched total entries count", 2,
GenericUtils.size(updatedKeys));
+ }
+
private Path createKnownHostsCopy() throws IOException {
Path file = getKnownHostCopyPath();
Files.copy(entriesFile, file, StandardCopyOption.REPLACE_EXISTING);
@@ -350,7 +407,7 @@ public class KnownHostsServerKeyVerifierTest extends
BaseTestSupport {
return file;
}
- private boolean invokeVerifier(ServerKeyVerifier verifier,
SshdSocketAddress hostIdentity, PublicKey serverKey) {
+ 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 + "]");
diff --git a/sshd-scp/src/main/java/org/apache/sshd/scp/common/ScpLocation.java
b/sshd-scp/src/main/java/org/apache/sshd/scp/common/ScpLocation.java
index 4605bbc..c5ed07b 100644
--- a/sshd-scp/src/main/java/org/apache/sshd/scp/common/ScpLocation.java
+++ b/sshd-scp/src/main/java/org/apache/sshd/scp/common/ScpLocation.java
@@ -92,8 +92,7 @@ public class ScpLocation implements MutableUserHolder,
Serializable, Cloneable {
}
public int resolvePort() {
- int portValue = getPort();
- return (portValue <= 0) ? SshConstants.DEFAULT_PORT : portValue;
+ return SshConstants.TO_EFFECTIVE_PORT.applyAsInt(getPort());
}
@Override
diff --git
a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java
b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java
index 3664697..e1645c2 100644
---
a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java
+++
b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java
@@ -1328,7 +1328,7 @@ public class SftpFileSystemProvider extends
FileSystemProvider {
public static String getFileSystemIdentifier(String host, int port, String
username) {
return GenericUtils.trimToEmpty(host) + ':'
- + ((port <= 0) ? SshConstants.DEFAULT_PORT : port) + ':'
+ + SshConstants.TO_EFFECTIVE_PORT.applyAsInt(port) + ':'
+ GenericUtils.trimToEmpty(username);
}
