From ea3af064ede443ac9277fbdb15a894a9fd032de2 Mon Sep 17 00:00:00 2001 From: exceptionfactory Date: Tue, 13 Sep 2022 23:08:44 -0500 Subject: [PATCH 1/2] Added Transport.isKeyExchangeRequired() to avoid unnecessary KEXINIT - Updated SSHClient.onConnect() to check isKeyExchangeRequired() before calling doKex() - Added started timestamp in ThreadNameProvider for improved tracking --- .../com/hierynomus/sshj/common/ThreadNameProvider.java | 3 ++- src/main/java/net/schmizz/sshj/SSHClient.java | 7 ++++++- .../java/net/schmizz/sshj/transport/Transport.java | 7 +++++++ .../java/net/schmizz/sshj/transport/TransportImpl.java | 10 ++++++++++ 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/hierynomus/sshj/common/ThreadNameProvider.java b/src/main/java/com/hierynomus/sshj/common/ThreadNameProvider.java index be083716..c0070583 100644 --- a/src/main/java/com/hierynomus/sshj/common/ThreadNameProvider.java +++ b/src/main/java/com/hierynomus/sshj/common/ThreadNameProvider.java @@ -29,7 +29,8 @@ public class ThreadNameProvider { public static void setThreadName(final Thread thread, final RemoteAddressProvider remoteAddressProvider) { final InetSocketAddress remoteSocketAddress = remoteAddressProvider.getRemoteSocketAddress(); final String address = remoteSocketAddress == null ? DISCONNECTED : remoteSocketAddress.toString(); - final String threadName = String.format("sshj-%s-%s", thread.getClass().getSimpleName(), address); + final long started = System.currentTimeMillis(); + final String threadName = String.format("sshj-%s-%s-%d", thread.getClass().getSimpleName(), address, started); thread.setName(threadName); } } diff --git a/src/main/java/net/schmizz/sshj/SSHClient.java b/src/main/java/net/schmizz/sshj/SSHClient.java index 0829f3fd..70b71589 100644 --- a/src/main/java/net/schmizz/sshj/SSHClient.java +++ b/src/main/java/net/schmizz/sshj/SSHClient.java @@ -810,7 +810,12 @@ protected void onConnect() ThreadNameProvider.setThreadName(conn.getKeepAlive(), trans); keepAliveThread.start(); } - doKex(); + if (trans.isKeyExchangeRequired()) { + log.debug("Initiating Key Exchange for new connection"); + doKex(); + } else { + log.debug("Key Exchange already completed for new connection"); + } } /** diff --git a/src/main/java/net/schmizz/sshj/transport/Transport.java b/src/main/java/net/schmizz/sshj/transport/Transport.java index 5ae55968..d8175698 100644 --- a/src/main/java/net/schmizz/sshj/transport/Transport.java +++ b/src/main/java/net/schmizz/sshj/transport/Transport.java @@ -71,6 +71,13 @@ void init(String host, int port, InputStream in, OutputStream out) void doKex() throws TransportException; + /** + * Is Key Exchange required based on current transport status + * + * @return Key Exchange required status + */ + boolean isKeyExchangeRequired(); + /** @return the version string used by this client to identify itself to an SSH server, e.g. "SSHJ_3_0" */ String getClientVersion(); diff --git a/src/main/java/net/schmizz/sshj/transport/TransportImpl.java b/src/main/java/net/schmizz/sshj/transport/TransportImpl.java index 58107c5b..edff191c 100644 --- a/src/main/java/net/schmizz/sshj/transport/TransportImpl.java +++ b/src/main/java/net/schmizz/sshj/transport/TransportImpl.java @@ -254,6 +254,16 @@ public void doKex() kexer.startKex(true); } + /** + * Is Key Exchange required returns true when Key Exchange is not done and when Key Exchange is not ongoing + * + * @return Key Exchange required status + */ + @Override + public boolean isKeyExchangeRequired() { + return !kexer.isKexDone() && !kexer.isKexOngoing(); + } + public boolean isKexDone() { return kexer.isKexDone(); } From 744630728dfae657eda38afdb9024b6584edc6fa Mon Sep 17 00:00:00 2001 From: exceptionfactory Date: Fri, 16 Sep 2022 07:53:10 -0500 Subject: [PATCH 2/2] Moved KeepAliveThread State check after authentication to avoid test timing issues --- .../sshj/keepalive/KeepAliveThreadTerminationTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/hierynomus/sshj/keepalive/KeepAliveThreadTerminationTest.java b/src/test/java/com/hierynomus/sshj/keepalive/KeepAliveThreadTerminationTest.java index 6bdc2423..9e743f6b 100644 --- a/src/test/java/com/hierynomus/sshj/keepalive/KeepAliveThreadTerminationTest.java +++ b/src/test/java/com/hierynomus/sshj/keepalive/KeepAliveThreadTerminationTest.java @@ -59,10 +59,11 @@ public void shouldStartThreadOnConnectAndInterruptOnDisconnect() throws IOExcept assertEquals(Thread.State.NEW, keepAlive.getState()); fixture.connectClient(sshClient); - assertEquals(Thread.State.TIMED_WAITING, keepAlive.getState()); assertThrows(UserAuthException.class, () -> sshClient.authPassword("bad", "credentials")); + assertEquals(Thread.State.TIMED_WAITING, keepAlive.getState()); + fixture.stopClient(); Thread.sleep(STOP_SLEEP);