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 {