apacheGH-318: fix proxyJumps

Previous code only parsed the proxy jumps of the initial
HostConfigEntry. However, if the last entry in that list has a
HostConfigEntry that again has proxy jumps, these additional
proxies must be added to the list. And so on.

To guard against proxy cascades with loops we limit the total
number of proxies to at most 10. The limit is configurable through
property CoreModuleProperties.MAX_PROXY_JUMPS.

Bug: apache#318

CHANGES.md

@@ -32,6 +32,7 @@
 
 ## Bug Fixes
 
+* [GH-318](https://github.com/apache/mina-sshd/issues/318) Handle cascaded proxy jumps
 * [GH-427](https://github.com/apache/mina-sshd/issues/427) SCP client: fix `DefaultScpClient.upload(InputStream, ...)`
 * [GH-455](https://github.com/apache/mina-sshd/issues/455) Fix `BaseCipher`: make sure all bytes are processed
 * [GH-461](https://github.com/apache/mina-sshd/issues/461) Fix heartbeats with `wantReply=true`
@@ -62,6 +63,63 @@ More information can be found in IETF Memo [Secure Shell (SSH) Key Exchange Meth
 
 ## Behavioral changes and enhancements
 
+### [GH-318](https://github.com/apache/mina-sshd/issues/318) Handle cascaded proxy jumps
+
+Proxy jumps can be configured via host configuration entries in two ways. First, proxies can be _chained_
+directly by specifiying several proxies in one `ProxyJump` directive:
+
+```
+Host target
+Hostname somewhere.example.org
+User some_user
+IdentityFile ~/.ssh/some_id
+ProxyJump jumphost2, jumphost1
+
+Host jumphost1
+Hostname [email protected]
+User jumphost1_user
+IdentityFile ~/.ssh/id_jumphost1
+
+Host jumphost2
+Hostname [email protected]
+User jumphost2_user
+IdentityFile ~/.ssh/id_jumphost2
+```
+
+Connecting to server `target` will first connect to `jumphost1`, then tunnel through to `jumphost2`, and finally
+tunnel to `target`. So the full connection will be `client`→`jumphost1`→`jumphost2`→`target`.
+
+Such proxy jump chains were already supported in Apache MINA SSHD.
+
+Newly, Apache MINA SSHD also supports _cascading_ proxy jumps, so a configuration like
+
+```
+Host target
+Hostname somewhere.example.org
+User some_user
+IdentityFile ~/.ssh/some_id
+ProxyJump jumphost2
+
+Host jumphost1
+Hostname [email protected]
+User jumphost1_user
+IdentityFile ~/.ssh/id_jumphost1
+
+Host jumphost2
+Hostname [email protected]
+ProxyJump jumphost1
+User jumphost2_user
+IdentityFile ~/.ssh/id_jumphost2
+```
+
+also works now, and produces the same connection `client`→`jumphost1`→`jumphost2`→`target`.
+
+It is possible to mis-configure such proxy jump cascades to have loops. (For instance, if host `jumphost1` in
+the above example had a `ProxyJump jumphost2` directive.) To catch such misconfigurations, Apache MINA SSHD
+imposes an upper limit on the total number of proxy jumps in a connection. An exception is thrown if there
+are more than `CoreModuleProperties.MAX_PROXY_JUMPS` proxy jumps in a connection. The default value of this
+property is 10. Most real uses of proxy jumps will have one or maybe two proxy jumps only.
+
 ### [GH-461](https://github.com/apache/mina-sshd/issues/461) Fix heartbeats with `wantReply=true`
 
 The client-side heartbeat mechanism has been updated. Such heartbeats are configured via the

sshd-core/src/main/java/org/apache/sshd/client/SshClient.java

@@ -548,7 +548,7 @@ public ConnectFuture connect(
     public ConnectFuture connect(
             HostConfigEntry hostConfig, AttributeRepository context, SocketAddress localAddress)
             throws IOException {
-        List<HostConfigEntry> jumps = parseProxyJumps(hostConfig.getProxyJump(), context);
+        List<HostConfigEntry> jumps = parseProxyJumps(hostConfig, context);
         return doConnect(hostConfig, jumps, context, localAddress);
     }
 
@@ -672,13 +672,52 @@ protected ConnectFuture doConnect(
         return connectFuture;
     }
 
+    protected List<HostConfigEntry> parseProxyJumps(HostConfigEntry entry, AttributeRepository context)
+            throws IOException {
+        List<HostConfigEntry> hops;
+        try {
+            hops = parseProxyJumps(entry.getProxyJump(), context);
+            if (GenericUtils.isEmpty(hops)) {
+                return hops;
+            }
+            // If the last entry itself has proxy jumps, we need to append them. Guard against possible proxy jump
+            // loops by imposing an upper limit on the total number of jumps.
+            for (;;) {
+                HostConfigEntry last = hops.get(hops.size() - 1);
+                try {
+                    List<HostConfigEntry> additionalHops = parseProxyJumps(last.getProxyJump(), context);
+                    if (additionalHops.isEmpty()) {
+                        break;
+                    } else {
+                        hops.addAll(additionalHops);
+                        if (hops.size() > CoreModuleProperties.MAX_PROXY_JUMPS.getRequired(this)) {
+                            throw new IllegalArgumentException("Too many proxy jumps for host " + entry.getHost());
+                        }
+                    }
+                } catch (IOException | RuntimeException e) {
+                    throw new IllegalArgumentException("Problem parsing proxyJump from host config " + last.getHost(), e);
+                }
+            }
+        } catch (IOException | RuntimeException e) {
+            throw new IllegalArgumentException("Problem parsing proxyJump from host config " + entry.getHost(), e);
+        }
+        return hops;
+    }
+
     protected List<HostConfigEntry> parseProxyJumps(String proxyJump, AttributeRepository context) throws IOException {
         List<HostConfigEntry> jumps = new ArrayList<>();
-        for (String jump : GenericUtils.split(proxyJump, ',')) {
-            String j = jump.trim();
-            URI uri = URI.create(j.contains("//") ? j : "ssh://" + j);
+        if (GenericUtils.isEmpty(proxyJump)) {
+            return jumps;
+        }
+        String[] hops = GenericUtils.split(proxyJump, ',');
+        for (String hop : hops) {
+            String h = hop.trim();
+            if (h.isEmpty()) {
+                throw new IllegalArgumentException("Empty proxy jump in list: " + proxyJump);
+            }
+            URI uri = URI.create(h.contains("://") ? h : "ssh://" + h);
             if (GenericUtils.isNotEmpty(uri.getScheme()) && !"ssh".equals(uri.getScheme())) {
-                throw new IllegalArgumentException("Unsupported scheme for proxy jump: " + jump);
+                throw new IllegalArgumentException("Unsupported scheme for proxy jump: " + hop);
             }
             String host = uri.getHost();
             int port = uri.getPort();

sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java

@@ -789,6 +789,17 @@ public final class CoreModuleProperties {
     public static final Property<String> X11_BIND_HOST
             = Property.string("x11-fwd-bind-host", SshdSocketAddress.LOCALHOST_IPV4);
 
+    /**
+     * Configuration value for the maximum number of proxy jumps to allow in an SSH connection; by default 10. If there
+     * are more proxy jumps for an SSH connection, chances are that the proxy chain has a loop.
+     */
+    public static final Property<Integer> MAX_PROXY_JUMPS = Property.validating(Property.integer("max-proxy-jumps", 10), p -> {
+        if (p != null) {
+            ValidateUtils.checkTrue(p.intValue() > 0, "Maximum number of proxy jumps allowed must be greater than zero, is %d",
+                    p);
+        }
+    });
+
     private CoreModuleProperties() {
         throw new UnsupportedOperationException("No instance");
     }