SSHD-1269: Fix TCP/IP remote port forwarding with wildcard addresses

Remote port forwarding using an OpenSSH client and an Apache MINA sshd
server didn't work with wildcard addresses or with "localhost". For
instance,

ssh ... -R 0.0.0.0:0:somewhere:1234

would fail: the Apache MINA sshd server would send a forwarded-tcpip
request with 127.0.0.1:55555 (if port 55555 was chosen) to OpenSSH,
but OpenSSH expected 0.0.0.0:55555.

ssh ... -R 0:somewhere:1234

also failed in the same way.

Fix this by sending back in the forwarded-tcpip request the original
address with the bound port.

(Note: this was already fixed by SSHD-792 as of Apache MINA sshd 2.2.0,
but was broken again in 2.6.0. Unit tests using an Apache MINA sshd
client to initiate the remote port forwarding didn't detect the problem
because Apache MINA sshd only checks the port but not the hostname.)

Author: tomaswolf

CHANGES.md

@@ -107,3 +107,4 @@ Was originally in *HostConfigEntry*.
 * [SSHD-1262](https://issues.apache.org/jira/browse/SSHD-1262) TCP/IP port forwarding: don't buffer, and don't read from port before channel is open
 * [SSHD-1264](https://issues.apache.org/jira/browse/SSHD-1264) Create KEX negotiation proposal only once per session, not on every re-KEX
 * [SSHD-1266](https://issues.apache.org/jira/browse/SSHD-1266) Fix encoding/decoding critical options in OpenSSH certificates
+* [SSHD-1269](https://issues.apache.org/jira/browse/SSHD-1269) Fix TCP/IP remote port forwarding with wildcard addresses

sshd-core/src/main/java/org/apache/sshd/common/forward/TcpipClientChannel.java

@@ -93,7 +93,13 @@ public Type getTcpipChannelType() {
 
     public void updateLocalForwardingEntry(LocalForwardingEntry entry) {
         Objects.requireNonNull(entry, "No local forwarding entry provided");
-        localEntry = entry.getBoundAddress();
+        // OpenSSH requires the host string it passed in the tcpip-forward global request: it compares both host and
+        // port and refuses the forwarding request when it doesn't match in the forwarded-tcpip request.
+        //
+        // Note: Apache MINA sshd is currently more lenient in that respect and compares only the port. RFC 4254
+        // states that "Implementations MUST reject these messages unless they have previously requested a remote TCP/IP
+        // port forwarding with the given port number"; it does not require the host name to be checked.
+        localEntry = new SshdSocketAddress(entry.getLocalAddress().getHostName(), entry.getBoundAddress().getPort());
     }
 
     @Override

…he/sshd/common/forward/Sshd1055Test.java …rward/PortForwardingWithOpenSshTest.javasshd-core/src/test/java/org/apache/sshd/common/forward/Sshd1055Test.java renamed to sshd-core/src/test/java/org/apache/sshd/common/forward/PortForwardingWithOpenSshTest.java sshd-core/src/test/java/org/apache/sshd/common/forward/Sshd1055Test.java renamed to sshd-core/src/test/java/org/apache/sshd/common/forward/PortForwardingWithOpenSshTest.java

@@ -48,6 +48,8 @@
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testcontainers.Testcontainers;
@@ -68,11 +70,20 @@
  * half-closing the socket (shutting down its output), otherwise the client connected to the forwarded port on localhost
  * will hang.
  * </p>
+ * <p>
+ * The test is re-run with different ways to specify the remote port forwarding, including wildcard addresses. The port
+ * is always zero, letting the server choose any unused port. With a fixed port number, the test might fail if that
+ * fixed port was already used in the CI environment.
+ * </p>
+ *
+ * @see <a href="https://issues.apache.org/jira/browse/SSHD-1055">SSHD-1055</a>
+ * @see <a href="https://issues.apache.org/jira/browse/SSHD-1269">SSHD-1269</a>
  */
+@RunWith(Parameterized.class)
 @Category(ContainerTestCase.class)
-public class Sshd1055Test extends BaseTestSupport {
+public class PortForwardingWithOpenSshTest extends BaseTestSupport {
 
-    private static final Logger LOG = LoggerFactory.getLogger(Sshd1055Test.class);
+    private static final Logger LOG = LoggerFactory.getLogger(PortForwardingWithOpenSshTest.class);
 
     // We re-use a key from the ClientOpenSSHCertificatesTest.
     private static final String TEST_KEYS = "org/apache/sshd/client/opensshcerts/user";
@@ -89,8 +100,21 @@ public class Sshd1055Test extends BaseTestSupport {
     private CountDownLatch forwardingSetup;
     private int forwardedPort;
 
-    public Sshd1055Test() {
-        super();
+    private final String portToForward;
+
+    public PortForwardingWithOpenSshTest(String portToForward) {
+        this.portToForward = portToForward;
+    }
+
+    /**
+     * Uses different ways to specify the remote port forwarding.
+     *
+     * @return the remote port specifications to use
+     * @see    <a href="https://issues.apache.org/jira/browse/SSHD-1269">SSHD-1269</a>
+     */
+    @Parameterized.Parameters(name = "{0}")
+    public static String[] portSpecifications() {
+        return new String[] { "127.0.0.1:0", "0.0.0.0:0", "0", "localhost:0" };
     }
 
     @Before
@@ -113,7 +137,7 @@ public void startServers() throws Exception {
         LOG.info("gRPC running on port {}", gRpcPort);
         // sshd server
         forwardingSetup = new CountDownLatch(1);
-        sshd = CoreTestSupportUtils.setupTestServer(Sshd1055Test.class);
+        sshd = CoreTestSupportUtils.setupTestServer(PortForwardingWithOpenSshTest.class);
         sshd.setForwardingFilter(AcceptAllForwardingFilter.INSTANCE);
         sshd.setForwarderFactory(new DefaultForwarderFactory() {
             @Override
@@ -148,13 +172,14 @@ public void teardownServers() throws Exception {
 
     @Test
     public void forwardingWithConnectionClose() throws Exception {
-        // Write the entrypoint file
+        // Write the entrypoint file. From within the test container, the host running the container and our two servers
+        // is accessible as "host.testcontainers.internal".
         File entryPoint = tmp.newFile();
         String lines = "#!/bin/sh\n" //
                        + "\n" //
                        + "chmod 0600 /root/.ssh/*\n" //
                        + "/usr/bin/ssh -o 'ExitOnForwardFailure yes' -o 'StrictHostKeyChecking off' -vvv -p " + sshPort //
-                       + " -x -N -T -R 127.0.0.1:0:host.testcontainers.internal:" + gRpcPort //
+                       + " -x -N -T -R " + portToForward + ":host.testcontainers.internal:" + gRpcPort //
                        + " bob@host.testcontainers.internal\n";
         Files.write(entryPoint.toPath(), lines.getBytes(StandardCharsets.US_ASCII));
         // Create the container

sshd-mina/pom.xml

@@ -135,7 +135,7 @@
                         <exclude>**/ClientOpenSSHCertificatesTest.java</exclude>
                         <exclude>**/SessionReKeyHostKeyExchangeTest.java</exclude>
                         <exclude>**/HostBoundPubKeyAuthTest.java</exclude>
-                        <exclude>**/Sshd1055Test.java</exclude>
+                        <exclude>**/PortForwardingWithOpenSshTest.java</exclude>
                         <!-- reading files from classpath doesn't work correctly w/ reusable test jar -->
                         <exclude>**/OpenSSHCertificateTest.java</exclude>
                     </excludes>

sshd-netty/pom.xml

@@ -161,7 +161,7 @@
                         <exclude>**/ClientOpenSSHCertificatesTest.java</exclude>
                         <exclude>**/SessionReKeyHostKeyExchangeTest.java</exclude>
                         <exclude>**/HostBoundPubKeyAuthTest.java</exclude>
-                        <exclude>**/Sshd1055Test.java</exclude>
+                        <exclude>**/PortForwardingWithOpenSshTest.java</exclude>
                         <!-- reading files from classpath doesn't work correctly w/ reusable test jar -->
                         <exclude>**/OpenSSHCertificateTest.java</exclude>
                     </excludes>