GH-318: fix proxyJumps

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 jumphost1@example.org
+User jumphost1_user
+IdentityFile ~/.ssh/id_jumphost1
+
+Host jumphost2
+Hostname jumphost2@example.org
+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 jumphost1@example.org
+User jumphost1_user
+IdentityFile ~/.ssh/id_jumphost1
+
+Host jumphost2
+Hostname jumphost2@example.org
+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");
  }