From 12e5283710a95a48d17ba70a7082fefe936b4964 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sat, 1 Jun 2024 21:56:15 +0200 Subject: [PATCH] GH-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: https://github.com/apache/mina-sshd/issues/318 --- CHANGES.md | 58 ++++ .../org/apache/sshd/client/SshClient.java | 49 +++- .../sshd/core/CoreModuleProperties.java | 11 + .../org/apache/sshd/client/ProxyTest.java | 254 ++++++++++++++++++ 4 files changed, 367 insertions(+), 5 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 55f79a187..405eb4225 100644 --- a/CHANGES.md +++ b/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 diff --git a/sshd-core/src/main/java/org/apache/sshd/client/SshClient.java b/sshd-core/src/main/java/org/apache/sshd/client/SshClient.java index 7555b309f..bbb517cb9 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/SshClient.java +++ b/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 jumps = parseProxyJumps(hostConfig.getProxyJump(), context); + List jumps = parseProxyJumps(hostConfig, context); return doConnect(hostConfig, jumps, context, localAddress); } @@ -672,13 +672,52 @@ protected ConnectFuture doConnect( return connectFuture; } + protected List parseProxyJumps(HostConfigEntry entry, AttributeRepository context) + throws IOException { + List 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 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 parseProxyJumps(String proxyJump, AttributeRepository context) throws IOException { List 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(); diff --git a/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java b/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java index 641c0923b..d8c8d35c1 100644 --- a/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java +++ b/sshd-core/src/main/java/org/apache/sshd/core/CoreModuleProperties.java @@ -789,6 +789,17 @@ public final class CoreModuleProperties { public static final Property 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 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"); } diff --git a/sshd-core/src/test/java/org/apache/sshd/client/ProxyTest.java b/sshd-core/src/test/java/org/apache/sshd/client/ProxyTest.java index 2a8297bee..cb4af069f 100644 --- a/sshd-core/src/test/java/org/apache/sshd/client/ProxyTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/client/ProxyTest.java @@ -28,18 +28,25 @@ import java.nio.file.StandardOpenOption; import java.rmi.RemoteException; import java.security.KeyPair; +import java.security.KeyPairGenerator; import java.util.Arrays; +import java.util.Collections; import java.util.concurrent.TimeUnit; import org.apache.sshd.client.config.hosts.HostConfigEntry; import org.apache.sshd.client.config.hosts.KnownHostHashValue; +import org.apache.sshd.client.keyverifier.AcceptAllServerKeyVerifier; import org.apache.sshd.client.keyverifier.KnownHostsServerKeyVerifier; import org.apache.sshd.client.keyverifier.RejectAllServerKeyVerifier; import org.apache.sshd.client.session.ClientSession; +import org.apache.sshd.common.config.keys.KeyUtils; import org.apache.sshd.common.config.keys.PublicKeyEntry; +import org.apache.sshd.common.session.Session; import org.apache.sshd.common.util.GenericUtils; +import org.apache.sshd.common.util.net.SshdSocketAddress; import org.apache.sshd.server.SshServer; import org.apache.sshd.server.forward.AcceptAllForwardingFilter; +import org.apache.sshd.server.forward.StaticDecisionForwardingFilter; import org.apache.sshd.util.test.BaseTestSupport; import org.apache.sshd.util.test.CommandExecutionHelper; import org.junit.FixMethodOrder; @@ -200,6 +207,253 @@ public void testProxyWithHostKeyVerificationAndCustomConfig() throws Exception { } } + @Test + public void testProxyChain() throws Exception { + try (SshServer target = setupTestServer(); + SshServer proxy1 = setupTestServer(); + SshServer proxy2 = setupTestServer(); + SshClient client = setupTestClient()) { + target.setCommandFactory((session, command) -> new CommandExecutionHelper(command) { + @Override + protected boolean handleCommandLine(String command) throws Exception { + OutputStream stdout = getOutputStream(); + stdout.write(command.getBytes(StandardCharsets.US_ASCII)); + stdout.flush(); + return false; + } + }); + + client.setServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE); + KeyPair kp = KeyPairGenerator.getInstance("RSA").generateKeyPair(); + client.setKeyIdentityProvider(s -> { + return Collections.singletonList(kp); + }); + target.setPublickeyAuthenticator((u, k, s) -> "userT".equals(u) && KeyUtils.compareKeys(k, kp.getPublic())); + proxy1.setPublickeyAuthenticator((u, k, s) -> "user1".equals(u) && KeyUtils.compareKeys(k, kp.getPublic())); + proxy2.setPublickeyAuthenticator((u, k, s) -> "user2".equals(u) && KeyUtils.compareKeys(k, kp.getPublic())); + int[] forwarded = new int[2]; + proxy1.setForwardingFilter(new StaticDecisionForwardingFilter(true) { + + @Override + protected boolean checkAcceptance(String request, Session session, SshdSocketAddress target) { + forwarded[0] = target.getPort(); + return super.checkAcceptance(request, session, target); + } + }); + proxy2.setForwardingFilter(new StaticDecisionForwardingFilter(true) { + + @Override + protected boolean checkAcceptance(String request, Session session, SshdSocketAddress target) { + forwarded[1] = target.getPort(); + return super.checkAcceptance(request, session, target); + } + }); + target.start(); + proxy1.start(); + proxy2.start(); + client.setHostConfigEntryResolver(HostConfigEntry.toHostConfigEntryResolver( + Arrays.asList(new HostConfigEntry("target", "localhost", target.getPort(), "userT", "proxy2, proxy1"), + new HostConfigEntry("proxy1", "localhost", proxy1.getPort(), "user1"), + new HostConfigEntry("proxy2", "localhost", proxy2.getPort(), "user2")))); + client.start(); + try (ClientSession session = client.connect("target").verify(CONNECT_TIMEOUT).getSession()) { + session.auth().verify(AUTH_TIMEOUT); + + assertTrue(session.isOpen()); + doTestCommand(session, "ls -la"); + } + assertEquals(proxy2.getPort(), forwarded[0]); + assertEquals(target.getPort(), forwarded[1]); + } + } + + @Test + public void testProxyCascade() throws Exception { + try (SshServer target = setupTestServer(); + SshServer proxy1 = setupTestServer(); + SshServer proxy2 = setupTestServer(); + SshClient client = setupTestClient()) { + target.setCommandFactory((session, command) -> new CommandExecutionHelper(command) { + @Override + protected boolean handleCommandLine(String command) throws Exception { + OutputStream stdout = getOutputStream(); + stdout.write(command.getBytes(StandardCharsets.US_ASCII)); + stdout.flush(); + return false; + } + }); + + client.setServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE); + KeyPair kp = KeyPairGenerator.getInstance("RSA").generateKeyPair(); + client.setKeyIdentityProvider(s -> { + return Collections.singletonList(kp); + }); + target.setPublickeyAuthenticator((u, k, s) -> "userT".equals(u) && KeyUtils.compareKeys(k, kp.getPublic())); + proxy1.setPublickeyAuthenticator((u, k, s) -> "user1".equals(u) && KeyUtils.compareKeys(k, kp.getPublic())); + proxy2.setPublickeyAuthenticator((u, k, s) -> "user2".equals(u) && KeyUtils.compareKeys(k, kp.getPublic())); + int[] forwarded = new int[2]; + proxy1.setForwardingFilter(new StaticDecisionForwardingFilter(true) { + + @Override + protected boolean checkAcceptance(String request, Session session, SshdSocketAddress target) { + forwarded[0] = target.getPort(); + return super.checkAcceptance(request, session, target); + } + }); + proxy2.setForwardingFilter(new StaticDecisionForwardingFilter(true) { + + @Override + protected boolean checkAcceptance(String request, Session session, SshdSocketAddress target) { + forwarded[1] = target.getPort(); + return super.checkAcceptance(request, session, target); + } + }); + target.start(); + proxy1.start(); + proxy2.start(); + client.setHostConfigEntryResolver(HostConfigEntry.toHostConfigEntryResolver( + Arrays.asList(new HostConfigEntry("target", "localhost", target.getPort(), "userT", "proxy2"), + new HostConfigEntry("proxy1", "localhost", proxy1.getPort(), "user1"), + new HostConfigEntry("proxy2", "localhost", proxy2.getPort(), "user2", "proxy1")))); + client.start(); + try (ClientSession session = client.connect("target").verify(CONNECT_TIMEOUT).getSession()) { + session.auth().verify(AUTH_TIMEOUT); + + assertTrue(session.isOpen()); + doTestCommand(session, "ls -la"); + } + assertEquals(proxy2.getPort(), forwarded[0]); + assertEquals(target.getPort(), forwarded[1]); + } + } + + @Test + public void testProxyInfinite() throws Exception { + try (SshServer target = setupTestServer(); + SshServer proxy1 = setupTestServer(); + SshServer proxy2 = setupTestServer(); + SshClient client = setupTestClient()) { + target.setCommandFactory((session, command) -> new CommandExecutionHelper(command) { + @Override + protected boolean handleCommandLine(String command) throws Exception { + OutputStream stdout = getOutputStream(); + stdout.write(command.getBytes(StandardCharsets.US_ASCII)); + stdout.flush(); + return false; + } + }); + + client.setServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE); + KeyPair kp = KeyPairGenerator.getInstance("RSA").generateKeyPair(); + client.setKeyIdentityProvider(s -> { + return Collections.singletonList(kp); + }); + target.setPublickeyAuthenticator((u, k, s) -> "userT".equals(u) && KeyUtils.compareKeys(k, kp.getPublic())); + proxy1.setPublickeyAuthenticator((u, k, s) -> "user1".equals(u) && KeyUtils.compareKeys(k, kp.getPublic())); + proxy2.setPublickeyAuthenticator((u, k, s) -> "user2".equals(u) && KeyUtils.compareKeys(k, kp.getPublic())); + int[] forwarded = new int[2]; + proxy1.setForwardingFilter(new StaticDecisionForwardingFilter(true) { + + @Override + protected boolean checkAcceptance(String request, Session session, SshdSocketAddress target) { + forwarded[0] = target.getPort(); + return super.checkAcceptance(request, session, target); + } + }); + proxy2.setForwardingFilter(new StaticDecisionForwardingFilter(true) { + + @Override + protected boolean checkAcceptance(String request, Session session, SshdSocketAddress target) { + forwarded[1] = target.getPort(); + return super.checkAcceptance(request, session, target); + } + }); + target.start(); + proxy1.start(); + proxy2.start(); + client.setHostConfigEntryResolver(HostConfigEntry.toHostConfigEntryResolver( + Arrays.asList(new HostConfigEntry("target", "localhost", target.getPort(), "userT", "proxy2"), + new HostConfigEntry("proxy1", "localhost", proxy1.getPort(), "user1", "proxy2"), + new HostConfigEntry("proxy2", "localhost", proxy2.getPort(), "user2", "proxy1")))); + client.start(); + Exception e = assertThrows(Exception.class, () -> { + try (ClientSession session = client.connect("target").verify(CONNECT_TIMEOUT).getSession()) { + // Nothing + } + }); + // One exception should have a message "Too many proxy jumps" + Throwable t = e; + while (t != null) { + if (t.getMessage().contains("Too many proxy jumps")) { + break; + } + t = t.getCause(); + } + assertNotNull(t); + } + } + + @Test + public void testProxyOverride() throws Exception { + try (SshServer target = setupTestServer(); + SshServer proxy1 = setupTestServer(); + SshServer proxy2 = setupTestServer(); + SshClient client = setupTestClient()) { + target.setCommandFactory((session, command) -> new CommandExecutionHelper(command) { + @Override + protected boolean handleCommandLine(String command) throws Exception { + OutputStream stdout = getOutputStream(); + stdout.write(command.getBytes(StandardCharsets.US_ASCII)); + stdout.flush(); + return false; + } + }); + + client.setServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE); + KeyPair kp = KeyPairGenerator.getInstance("RSA").generateKeyPair(); + client.setKeyIdentityProvider(s -> { + return Collections.singletonList(kp); + }); + target.setPublickeyAuthenticator((u, k, s) -> "userT".equals(u) && KeyUtils.compareKeys(k, kp.getPublic())); + proxy1.setPublickeyAuthenticator((u, k, s) -> "user1".equals(u) && KeyUtils.compareKeys(k, kp.getPublic())); + proxy2.setPublickeyAuthenticator((u, k, s) -> "user2".equals(u) && KeyUtils.compareKeys(k, kp.getPublic())); + int[] forwarded = new int[2]; + proxy1.setForwardingFilter(new StaticDecisionForwardingFilter(true) { + + @Override + protected boolean checkAcceptance(String request, Session session, SshdSocketAddress target) { + forwarded[0] = target.getPort(); + return super.checkAcceptance(request, session, target); + } + }); + proxy2.setForwardingFilter(new StaticDecisionForwardingFilter(true) { + + @Override + protected boolean checkAcceptance(String request, Session session, SshdSocketAddress target) { + forwarded[1] = target.getPort(); + return super.checkAcceptance(request, session, target); + } + }); + target.start(); + proxy1.start(); + proxy2.start(); + // "Proxy3" should be ignored. + client.setHostConfigEntryResolver(HostConfigEntry.toHostConfigEntryResolver( + Arrays.asList(new HostConfigEntry("target", "localhost", target.getPort(), "userT", "proxy2, proxy1"), + new HostConfigEntry("proxy1", "localhost", proxy1.getPort(), "user1"), + new HostConfigEntry("proxy2", "localhost", proxy2.getPort(), "user2", "proxy3")))); + client.start(); + try (ClientSession session = client.connect("target").verify(CONNECT_TIMEOUT).getSession()) { + session.auth().verify(AUTH_TIMEOUT); + + assertTrue(session.isOpen()); + doTestCommand(session, "ls -la"); + } + assertEquals(proxy2.getPort(), forwarded[0]); + assertEquals(target.getPort(), forwarded[1]); + } + } + @Test @Ignore public void testExternal() throws Exception {