You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2019/07/18 11:22:57 UTC
[mina-sshd] branch master updated (42b00eb -> 1419074)
This is an automated email from the ASF dual-hosted git repository.
lgoldstein pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git.
from 42b00eb [maven-release-plugin] prepare for next development iteration
new 9c194db Split changes list for 2.3.0
new e19bcfa [SSHD-930] Added configuration to enable client to delay sending its identification until after the server's was received
new 57ba26d [SSHD-930] Added SessionListener#sessionPeerIdentificationReceived callback
new f68a8f5 Fix possible confusion about commands having a high numerical value received while KEX not yet completed
new 1419074 Fix possible NPE when handling immediate KEX packet follow-up in case a peer proposal is not yet set
The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
CHANGES.md | 146 +--------------------
CHANGES.md => docs/changes/2.3.0.md | 4 +-
docs/client-setup.md | 14 ++
.../apache/sshd/client/ClientFactoryManager.java | 13 ++
.../sshd/client/session/AbstractClientSession.java | 144 +++++++++++++-------
.../sshd/client/session/ClientProxyConnector.java | 12 +-
.../sshd/client/session/ClientSessionImpl.java | 21 ++-
.../org/apache/sshd/common/FactoryManager.java | 10 +-
.../sshd/common/session/SessionListener.java | 14 ++
.../common/session/helpers/AbstractSession.java | 21 +--
.../sshd/common/session/helpers/SessionHelper.java | 77 +++++++----
.../sshd/server/session/AbstractServerSession.java | 36 ++---
.../session/helpers/AbstractSessionTest.java | 30 +++--
.../java/org/apache/sshd/server/ServerTest.java | 43 ++++--
14 files changed, 301 insertions(+), 284 deletions(-)
copy CHANGES.md => docs/changes/2.3.0.md (99%)
[mina-sshd] 01/05: Split changes list for 2.3.0
Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit 9c194dbe81afd3f152b773c7805db2efc4b905cc
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Thu Jul 18 14:19:31 2019 +0300
Split changes list for 2.3.0
---
CHANGES.md | 143 +-----------------------------------
CHANGES.md => docs/changes/2.3.0.md | 4 +-
2 files changed, 3 insertions(+), 144 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 96fd0ac..24de6f1 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,152 +1,13 @@
# [Version 2.1.0 to 2.2.0](./docs/changes/2.2.0.md)
+# [Version 2.2.0 to 2.3.0](./docs/changes/2.3.0.md)
+
# Planned for next version
## Major code re-factoring
-* The `ChannelSession` provides a mechanism for supporting non-standard extended data (a.k.a. STDERR data)
-in a similar manner as the "regular" data. Please read the relevant section in the main documentation page.
-
-* The user can use a registered `SessionDisconnectHandler` in order be informed and also intervene in cases
-where the code decides to disconnect the session due to various protocol or configuration parameters violations.
-
-* `ScpFileOpener#getMatchingFilesToSend` has been modified to accept a `Path` as the base directory
-and also return an `Iterable<Path>`.
-
-* The SFTP command line client provides a `kex` command that displays the KEX parameters of the
-current sesssion - client/server proposals and what has been negotiated.
-
-* The `Session` object provides a `KexExtensionHandler` for usage with [KEX extension negotiation](https://tools.wordtothewise.com/rfc/rfc8308)
-
-* The `SignalListener` accepts a `Channel` argument indicating the channel instance through which the signal was received.
-
-* When creating a client shell or command channel one can provide optional PTY and/or environment values in order
-to override the internal default ones.
-
- * In this context, the `PtyCapableChannelSession#setEnv` method has been modified to accept ANY object.
- When the environment values are sent to the server, the object's `toString()` will be used. Furthermore,
- if one provides a `null` value, the previous registered value (if any) is **removed**.
-
-* The `SftpFileSystemAccessor` callbacks are now provided with the relevant `Handle` they are servicing
-(*Note:* in special cases a `null` value is provided to indicate invocation outside the scope of such a handle).
-
- * Closing of file channel/directory streams created by the accessor are also closed
- via callbacks to the same accessor
-
- * When closing a file channel that may have been potentially modified, the default implementation
- forces a synchronization of the data with the file-system. This behavior can be modified
- by setting the `sftp-auto-fsync-on-close` property to *false*.
-
-* The `ScpFileOpener` methods are also invoked in order to close input/output streams created through it
-when they are no longer needed once data has been successfully copied.
-
-* The `CommandFactory` and `ShellFactory` have been modified to accept the server's `ChannelSession` instance through
-which they are being invoked.
-
-* The various implementations of public/private keys/pairs decoders/loaders are provided with a `Map` of any headers that
-may be available in the relevant data file.
-
-* `org.apache.sshd.agent.unix.AgentClient` constructor expects a non-*null* `FactoryManager` instance which
-it then exposes via its `getFactoryManager`.
-
-* `SftpEventListener#removing/removed` callbacks accept an `isDirectory` flag indicating the type of `Path` being
-removed - file or directory.
-
## Minor code helpers
-* The `Session` object provides a `isServerSession` method that can be used to distinguish between
-client/server instances without having to resort to `instanceof`.
-
-* When creating a CLI SSH client one can specify `-o KexExtensionHandler=XXX` option to initialize
-a client-side `KexExtensionHandler` using an FQCN. If `default` is specified as the option value,
-then the internal `DefaultClientKexExtensionHandler` is used.
-
## Behavioral changes and enhancements
-* [SSHD-782](https://issues.apache.org/jira/browse/SSHD-882) - Added session level heartbeat mechanism via `SSH_MSG_IGNORE`
-or customized user provided code.
-
-In order to support customized user code for this feature, the `ReservedSessionMessagesHandler` can be used to
-implement any kind of user-defined heartbeat. *Note:* if the user configured such a mechanism, then the
-`sendReservedHeartbeat` method **must** be implemented since the default throws `UnsupportedOperationException`
-which will cause the session to be terminated the 1st time the method is invoked.
-
-* [SSHD-882](https://issues.apache.org/jira/browse/SSHD-882) - Provide hooks to allow users to register a consumer
-for STDERR data sent via the `ChannelSession` - especially for the SFTP subsystem.
-
-* [SSHD-892](https://issues.apache.org/jira/browse/SSHD-882) - Inform user about possible session disconnect prior
-to disconnecting and allow intervention via `SessionDisconnectHandler`.
-
-* [SSHD-893](https://issues.apache.org/jira/browse/SSHD-893) - Using Path(s) instead of String(s) as DirectoryScanner results
-
-* [SSHD-895](https://issues.apache.org/jira/browse/SSHD-895) - Add support for RSA + SHA-256/512 signatures. **Note:** according
-to [RFC - 8332 - section 3.3](https://tools.ietf.org/html/rfc8332#section-3.3):
-
->> Implementation experience has shown that there are servers that apply
->> authentication penalties to clients attempting public key algorithms
->> that the SSH server does not support.
-
->> When authenticating with an RSA key against a server that does not
->> implement the "server-sig-algs" extension, clients MAY default to an
->> "ssh-rsa" signature to avoid authentication penalties. When the new
->> rsa-sha2-* algorithms have been sufficiently widely adopted to
->> warrant disabling "ssh-rsa", clients MAY default to one of the new
->> algorithms.
-
-Therefore we do not include by default the "rsa-sha-*" signature factories in the `SshClient`. They can
-be easily added by using the relevant `BuiltinSignatures`:
-
-```java
-SshClient client = SshClient.setupDefaultClient();
-client.setSignatureFactories(
- Arrays.asList(
- /* This is the full list in the recommended preference order,
- * but the initialization code can choose and/or re-order
- */
- BuiltinSignatures.nistp256,
- BuiltinSignatures.nistp384,
- BuiltinSignatures.nistp521,
- BuiltinSignatures.ed25519,
- BuiltinSignatures.rsaSHA512,
- BuiltinSignatures.rsaSHA256, // should check if isSupported since not required by default for Java 8
- BuiltinSignatures.rsa,
- BuiltinSignatures.dsa));
-```
-
-* [SSHD-896](https://issues.apache.org/jira/browse/SSHD-896) - Added support for [KEX extension negotiation](https://tools.ietf.org/html/rfc8308)
-
-* [SSHD-870](https://issues.apache.org/jira/browse/SSHD-896) - Added support for GPGv2 public keyring (Note: requires upgraded
-[Bouncycastle](https://mvnrepository.com/artifact/org.bouncycastle/bcpg-jdk15on/1.61) and [jpgpj](https://mvnrepository.com/artifact/org.c02e.jpgpj/jpgpj/0.6.1) versions).
-
-* [SSHD-897](https://issues.apache.org/jira/browse/SSHD-897) - The default CLI code automatically tries to detect the PTY settings to use
-if opening a shell or command channel.
-
-* [SSHD-901](https://issues.apache.org/jira/browse/SSHD-901) - Added capability to request a reply for the `keepalive@...` heartbeat request
-in order to avoid client-side session timeout due to no traffic from server.
-
-* [SSHD-902](https://issues.apache.org/jira/browse/SSHD-902) - Shutdown output when receiving `SSH_MSG_CHANNEL_EOF` message via port forwarding channel.
-
-* [SSHD-903](https://issues.apache.org/jira/browse/SSHD-903) - Fixed the SFTP version negotiation behavior in case client proposed version is higher than server supported one.
-
-* [SSHD-904](https://issues.apache.org/jira/browse/SSHD-904) - Add option to enable/disable 'fsync' on modified file contents via SFTP (default=enabled).
-
-* [SSHD-905](https://issues.apache.org/jira/browse/SSHD-905) - Add option to enable/disable 'fsync' on modified file contents via SCP (default=enabled).
-
-* [SSHD-907](https://issues.apache.org/jira/browse/SSHD-907) - `StpEventListener` invokes (new) `exiting` method to inform about SFTP subsystem exiting
-and therefore closing all currently tracked file/directory handles.
-
-* [SSHD-909](https://issues.apache.org/jira/browse/SSHD-909) - SFTP versions extension handler ignores non-numerical versions when resolving the available ones.
-
-* [SSHD-913](https://issues.apache.org/jira/browse/SSHD-913) - Provide channel session instance to command and/or shell factories creators
-
-* [SSHD-912](https://issues.apache.org/jira/browse/SSHD-912) - Use separate locks for Future(s) and Session/Channel instances.
-
-* [SSHD-916](https://issues.apache.org/jira/browse/SSHD-916) - Avoid locking the session lock when signalling client session authentication failure.
-
-* [SSHD-917](https://issues.apache.org/jira/browse/SSHD-917) - Add support for SSH2 public key file format.
-
-* [SSHD-921](https://issues.apache.org/jira/browse/SSHD-921) - Do not send session disconnect message due to timeout expiration if already done so.
-
-* [SSHD-923](https://issues.apache.org/jira/browse/SSHD-923) - Added agent close detection mechanisms to avoid infinite waits on incoming messages.
-* [SSHD-929](https://issues.apache.org/jira/browse/SSHD-929) - Provide file/directory flag indicator to SFTP event listener callback for removal.
diff --git a/CHANGES.md b/docs/changes/2.3.0.md
similarity index 99%
copy from CHANGES.md
copy to docs/changes/2.3.0.md
index 96fd0ac..4c3fa4b 100644
--- a/CHANGES.md
+++ b/docs/changes/2.3.0.md
@@ -1,6 +1,4 @@
-# [Version 2.1.0 to 2.2.0](./docs/changes/2.2.0.md)
-
-# Planned for next version
+# Introduced in version 2.3.0
## Major code re-factoring
[mina-sshd] 04/05: Fix possible confusion about commands having a
high numerical value received while KEX not yet completed
Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit f68a8f513a6c3be6cde0af7d8ac46d18b251ab3a
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Tue Jul 9 09:07:51 2019 +0300
Fix possible confusion about commands having a high numerical value received while KEX not yet completed
---
.../java/org/apache/sshd/common/session/helpers/AbstractSession.java | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
index 55f3fc7..9c6dc7e 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
@@ -799,9 +799,9 @@ public abstract class AbstractSession extends SessionHelper {
// While exchanging key, queue high level packets
if (!KexState.DONE.equals(kexState.get())) {
byte[] bufData = buffer.array();
- byte cmd = bufData[buffer.rpos()];
+ int cmd = bufData[buffer.rpos()] & 0xFF;
if (cmd > SshConstants.SSH_MSG_KEX_LAST) {
- String cmdName = SshConstants.getCommandMessageName(cmd & 0xFF);
+ String cmdName = SshConstants.getCommandMessageName(cmd);
synchronized (pendingPackets) {
if (!KexState.DONE.equals(kexState.get())) {
if (pendingPackets.isEmpty()) {
[mina-sshd] 03/05: [SSHD-930] Added
SessionListener#sessionPeerIdentificationReceived callback
Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit 57ba26d7104db555b8efa0cf23fe59f873eba38a
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Mon Jul 8 13:33:51 2019 +0300
[SSHD-930] Added SessionListener#sessionPeerIdentificationReceived callback
---
CHANGES.md | 3 ++
.../sshd/client/session/AbstractClientSession.java | 8 ++--
.../sshd/common/session/SessionListener.java | 14 +++++++
.../sshd/common/session/helpers/SessionHelper.java | 32 ++++++++++++++++
.../sshd/server/session/AbstractServerSession.java | 4 +-
.../java/org/apache/sshd/server/ServerTest.java | 43 +++++++++++++++++-----
6 files changed, 90 insertions(+), 14 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 2c7a43c..60f3b07 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -8,6 +8,9 @@
## Minor code helpers
+* `SessionListener` supports `sessionPeerIdentificationReceived` that is invoked once successful
+peer version data is received.
+
## Behavioral changes and enhancements
* [SSHD-930](https://issues.apache.org/jira/browse/SSHD-930) - Added configuration allowing the user to specify whether client should wait
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java b/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java
index 5f44b19..1d865aa 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java
@@ -472,7 +472,9 @@ public abstract class AbstractClientSession extends AbstractSession implements C
return true;
}
- protected void signalExtraServerVersionInfo(String version, List<String> lines) throws IOException {
+ protected void signalExtraServerVersionInfo(String version, List<String> lines) throws Exception {
+ signalPeerIdentificationReceived(version, lines);
+
if (GenericUtils.isEmpty(lines)) {
return;
}
@@ -483,8 +485,8 @@ public abstract class AbstractClientSession extends AbstractSession implements C
ui.serverVersionInfo(this, lines);
}
} catch (Error e) {
- log.warn("signalExtraServerVersionInfo({})[{}] failed ({}) to consult interaction: {}", this, version,
- e.getClass().getSimpleName(), e.getMessage());
+ log.warn("signalExtraServerVersionInfo({})[{}] failed ({}) to consult interaction: {}",
+ this, version, e.getClass().getSimpleName(), e.getMessage());
if (log.isDebugEnabled()) {
log.debug("signalExtraServerVersionInfo(" + this + ")[" + version
+ "] interaction consultation failure details", e);
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/SessionListener.java b/sshd-core/src/main/java/org/apache/sshd/common/session/SessionListener.java
index 544f89c..d322f9d 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/SessionListener.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/SessionListener.java
@@ -18,6 +18,7 @@
*/
package org.apache.sshd.common.session;
+import java.util.List;
import java.util.Map;
import org.apache.sshd.common.kex.KexProposalOption;
@@ -43,6 +44,19 @@ public interface SessionListener extends SshdEventListener {
}
/**
+ * The peer's identification version was received
+ *
+ * @param session The {@link Session} instance
+ * @param version The retrieved identification version
+ * @param extraLines Extra data preceding the identification
+ * @see <A HREF="https://tools.ietf.org/html/rfc4253#section-4.2">RFC 4253 - section 4.2 - Protocol Version Exchange</A>
+ */
+ default void sessionPeerIdentificationReceived(
+ Session session, String version, List<String> extraLines) {
+ // ignored
+ }
+
+ /**
* Signals the start of the negotiation options handling
*
* @param session The referenced {@link Session}
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
index 914d5d6..f58ed6e 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
@@ -550,6 +550,38 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements
listener.sessionCreated(this);
}
+ protected void signalPeerIdentificationReceived(String version, List<String> extraLines) throws Exception {
+ try {
+ invokeSessionSignaller(l -> {
+ signalPeerIdentificationReceived(l, version, extraLines);
+ return null;
+ });
+ } catch (Throwable err) {
+ Throwable e = GenericUtils.peelException(err);
+ if (log.isDebugEnabled()) {
+ log.debug("signalPeerIdentificationReceived({}) Failed ({}) to announce peer={}: {}",
+ this, e.getClass().getSimpleName(), version, e.getMessage());
+ }
+ if (log.isTraceEnabled()) {
+ log.trace("signalPeerIdentificationReceived(" + this + ")[" + version + "] failure details", e);
+ }
+ if (e instanceof Exception) {
+ throw (Exception) e;
+ } else {
+ throw new RuntimeSshException(e);
+ }
+ }
+
+ }
+
+ protected void signalPeerIdentificationReceived(SessionListener listener, String version, List<String> extraLines) {
+ if (listener == null) {
+ return;
+ }
+
+ listener.sessionPeerIdentificationReceived(this, version, extraLines);
+ }
+
/**
* Sends a session event to all currently registered session listeners
*
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java b/sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java
index 529b5f8..1d5eceb 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java
@@ -406,7 +406,7 @@ public abstract class AbstractServerSession extends AbstractSession implements S
}
@Override
- protected boolean readIdentification(Buffer buffer) throws IOException, GeneralSecurityException {
+ protected boolean readIdentification(Buffer buffer) throws Exception {
ServerProxyAcceptor acceptor = getServerProxyAcceptor();
int rpos = buffer.rpos();
boolean debugEnabled = log.isDebugEnabled();
@@ -467,6 +467,8 @@ public abstract class AbstractServerSession extends AbstractSession implements S
throw err;
}
+ signalPeerIdentificationReceived(clientVersion, ident);
+
kexState.set(KexState.INIT);
sendKexInit();
return true;
diff --git a/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java b/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
index ae3ce35..faebaf1 100644
--- a/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
@@ -931,11 +931,7 @@ public class ServerTest extends BaseTestSupport {
client.addSessionListener(listener);
client.start();
- try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, sshd.getPort())
- .verify(7L, TimeUnit.SECONDS)
- .getSession()) {
- session.addPasswordIdentity(getCurrentTestName());
- session.auth().verify(9L, TimeUnit.SECONDS);
+ try (ClientSession session = createTestClientSession(sshd)) {
assertEquals("Mismatched client identification", expClientIdent, session.getClientVersion());
assertEquals("Mismatched server identification", expServerIdent, session.getServerVersion());
} finally {
@@ -980,11 +976,7 @@ public class ServerTest extends BaseTestSupport {
});
client.start();
- try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, sshd.getPort())
- .verify(7L, TimeUnit.SECONDS)
- .getSession()) {
- session.addPasswordIdentity(getCurrentTestName());
- session.auth().verify(9L, TimeUnit.SECONDS);
+ try (ClientSession session = createTestClientSession(sshd)) {
assertTrue("No signal received in time", signal.tryAcquire(11L, TimeUnit.SECONDS));
} finally {
client.stop();
@@ -995,6 +987,37 @@ public class ServerTest extends BaseTestSupport {
assertListEquals("Server information", expected, actual);
}
+ @Test // see SSHD-930
+ public void testDelayClientIdentification() throws Exception {
+ sshd.start();
+
+ PropertyResolverUtils.updateProperty(
+ client, ClientFactoryManager.SEND_IMMEDIATE_IDENTIFICATION, false);
+ AtomicReference<String> peerVersion = new AtomicReference<>();
+ client.addSessionListener(new SessionListener() {
+ @Override
+ public void sessionPeerIdentificationReceived(Session session, String version, List<String> extraLines) {
+ String clientVersion = session.getClientVersion();
+ if (GenericUtils.isNotEmpty(clientVersion)) {
+ throw new IllegalStateException("Client version already established");
+ }
+
+ String prev = peerVersion.getAndSet(version);
+ if (GenericUtils.isNotEmpty(prev)) {
+ throw new IllegalStateException("Peer version already signalled: " + prev);
+ }
+ }
+ });
+ client.start();
+
+ try (ClientSession session = createTestClientSession(sshd)) {
+ String version = peerVersion.getAndSet(null);
+ assertTrue("Peer version not signalled", GenericUtils.isNotEmpty(version));
+ } finally {
+ client.stop();
+ }
+ }
+
private ClientSession createTestClientSession(SshServer server) throws Exception {
ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, server.getPort())
.verify(7L, TimeUnit.SECONDS)
[mina-sshd] 02/05: [SSHD-930] Added configuration to enable client
to delay sending its identification until after the server's was received
Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit e19bcfa97e356cf11c4981da8a277cd2a0cc293d
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Mon Jul 8 12:47:10 2019 +0300
[SSHD-930] Added configuration to enable client to delay sending its identification until after the server's was received
---
CHANGES.md | 2 +
docs/client-setup.md | 14 ++
.../apache/sshd/client/ClientFactoryManager.java | 13 ++
.../sshd/client/session/AbstractClientSession.java | 142 ++++++++++++++-------
.../sshd/client/session/ClientProxyConnector.java | 12 +-
.../sshd/client/session/ClientSessionImpl.java | 21 ++-
.../org/apache/sshd/common/FactoryManager.java | 10 +-
.../common/session/helpers/AbstractSession.java | 6 +-
.../sshd/common/session/helpers/SessionHelper.java | 45 +++----
.../sshd/server/session/AbstractServerSession.java | 32 ++---
.../session/helpers/AbstractSessionTest.java | 30 +++--
11 files changed, 202 insertions(+), 125 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 24de6f1..2c7a43c 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -10,4 +10,6 @@
## Behavioral changes and enhancements
+* [SSHD-930](https://issues.apache.org/jira/browse/SSHD-930) - Added configuration allowing the user to specify whether client should wait
+for the server's identification before sending its own.
diff --git a/docs/client-setup.md b/docs/client-setup.md
index 05283fa..ce6453e 100644
--- a/docs/client-setup.md
+++ b/docs/client-setup.md
@@ -150,6 +150,20 @@ sessions depends on the actual changed configuration. Here is how a typical usag
```
+## Configuring the protocol exchange phase
+
+[RFC 4253 section 4.2](https://tools.ietf.org/html/rfc4253#section-4.2) does not specify when the client/server should send
+their respective identification strings. All it states is that these strings must be available before KEX stage since they
+participate in it. By default, the client sends its identification string immediately upon session being established. However,
+this can be modified so that the client waits for the server's identification before sending its own.
+
+```java
+ SshClient client = ...setup client...
+ PropertyResolverUtils.updateProperty(
+ client, ClientFactoryManager.SEND_IMMEDIATE_IDENTIFICATION, false);
+ client.start();
+```
+
## Keeping the session alive while no traffic
The client-side implementation has a 2 builtin mechanisms for maintaining the session alive as far as the **server** is concerned
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/ClientFactoryManager.java b/sshd-core/src/main/java/org/apache/sshd/client/ClientFactoryManager.java
index 026467b..42197f7 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/ClientFactoryManager.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/ClientFactoryManager.java
@@ -48,6 +48,19 @@ public interface ClientFactoryManager
String CLIENT_IDENTIFICATION = "client-identification";
/**
+ * Whether to send the identification string immediately upon session connection
+ * being established or wait for the peer's identification before sending our own.
+ *
+ * @see <A HREF="https://tools.ietf.org/html/rfc4253#section-4.2">RFC 4253 - section 4.2 - Protocol Version Exchange</A>
+ */
+ String SEND_IMMEDIATE_IDENTIFICATION = "send-immediate-identification";
+
+ /**
+ * Value of {@value #SEND_IMMEDIATE_IDENTIFICATION} if none configured
+ */
+ boolean DEFAULT_SEND_IMMEDIATE_IDENTIFICATION = true;
+
+ /**
* Key used to set the heartbeat interval in milliseconds (0 to disable = default)
*/
String HEARTBEAT_INTERVAL = "heartbeat-interval";
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java b/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java
index 20610c4..5f44b19 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java
@@ -44,6 +44,7 @@ import org.apache.sshd.common.AttributeRepository;
import org.apache.sshd.common.FactoryManager;
import org.apache.sshd.common.NamedFactory;
import org.apache.sshd.common.NamedResource;
+import org.apache.sshd.common.PropertyResolverUtils;
import org.apache.sshd.common.RuntimeSshException;
import org.apache.sshd.common.SshConstants;
import org.apache.sshd.common.SshException;
@@ -59,6 +60,8 @@ import org.apache.sshd.common.io.IoSession;
import org.apache.sshd.common.io.IoWriteFuture;
import org.apache.sshd.common.kex.KexProposalOption;
import org.apache.sshd.common.kex.KexState;
+import org.apache.sshd.common.kex.extension.KexExtensionHandler;
+import org.apache.sshd.common.kex.extension.KexExtensionHandler.AvailabilityPhase;
import org.apache.sshd.common.keyprovider.KeyIdentityProvider;
import org.apache.sshd.common.session.ConnectionService;
import org.apache.sshd.common.session.SessionContext;
@@ -76,6 +79,8 @@ import org.apache.sshd.common.util.net.SshdSocketAddress;
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public abstract class AbstractClientSession extends AbstractSession implements ClientSession {
+ protected final boolean sendImmediateIdentification;
+
private final List<Object> identities = new CopyOnWriteArrayList<>();
private final AuthenticationIdentitiesProvider identitiesProvider;
private final AttributeRepository connectionContext;
@@ -90,9 +95,12 @@ public abstract class AbstractClientSession extends AbstractSession implements C
protected AbstractClientSession(ClientFactoryManager factoryManager, IoSession ioSession) {
super(false, factoryManager, ioSession);
+ this.sendImmediateIdentification = PropertyResolverUtils.getBooleanProperty(
+ factoryManager, ClientFactoryManager.SEND_IMMEDIATE_IDENTIFICATION,
+ ClientFactoryManager.DEFAULT_SEND_IMMEDIATE_IDENTIFICATION);
+
identitiesProvider = AuthenticationIdentitiesProvider.wrapIdentities(identities);
- this.connectionContext =
- (AttributeRepository) ioSession.getAttribute(AttributeRepository.class);
+ this.connectionContext = (AttributeRepository) ioSession.getAttribute(AttributeRepository.class);
}
@Override
@@ -155,7 +163,8 @@ public abstract class AbstractClientSession extends AbstractSession implements C
@Override
public PasswordIdentityProvider getPasswordIdentityProvider() {
ClientFactoryManager manager = getFactoryManager();
- return resolveEffectiveProvider(PasswordIdentityProvider.class, passwordIdentityProvider, manager.getPasswordIdentityProvider());
+ return resolveEffectiveProvider(PasswordIdentityProvider.class, passwordIdentityProvider,
+ manager.getPasswordIdentityProvider());
}
@Override
@@ -166,7 +175,8 @@ public abstract class AbstractClientSession extends AbstractSession implements C
@Override
public KeyIdentityProvider getKeyIdentityProvider() {
ClientFactoryManager manager = getFactoryManager();
- return resolveEffectiveProvider(KeyIdentityProvider.class, keyIdentityProvider, manager.getKeyIdentityProvider());
+ return resolveEffectiveProvider(KeyIdentityProvider.class, keyIdentityProvider,
+ manager.getKeyIdentityProvider());
}
@Override
@@ -201,8 +211,8 @@ public abstract class AbstractClientSession extends AbstractSession implements C
return null;
}
- int index = AuthenticationIdentitiesProvider.findIdentityIndex(
- identities, AuthenticationIdentitiesProvider.PASSWORD_IDENTITY_COMPARATOR, password);
+ int index = AuthenticationIdentitiesProvider.findIdentityIndex(identities,
+ AuthenticationIdentitiesProvider.PASSWORD_IDENTITY_COMPARATOR, password);
if (index >= 0) {
return (String) identities.remove(index);
} else {
@@ -220,8 +230,7 @@ public abstract class AbstractClientSession extends AbstractSession implements C
if (log.isDebugEnabled()) {
PublicKey key = kp.getPublic();
- log.debug("addPublicKeyIdentity({}) {}-{}",
- this, KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key));
+ log.debug("addPublicKeyIdentity({}) {}-{}", this, KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key));
}
}
@@ -231,8 +240,8 @@ public abstract class AbstractClientSession extends AbstractSession implements C
return null;
}
- int index = AuthenticationIdentitiesProvider.findIdentityIndex(
- identities, AuthenticationIdentitiesProvider.KEYPAIR_IDENTITY_COMPARATOR, kp);
+ int index = AuthenticationIdentitiesProvider.findIdentityIndex(identities,
+ AuthenticationIdentitiesProvider.KEYPAIR_IDENTITY_COMPARATOR, kp);
if (index >= 0) {
return (KeyPair) identities.remove(index);
} else {
@@ -240,28 +249,57 @@ public abstract class AbstractClientSession extends AbstractSession implements C
}
}
- protected IoWriteFuture sendClientIdentification() throws Exception {
- clientVersion = resolveIdentificationString(ClientFactoryManager.CLIENT_IDENTIFICATION);
+ protected void initializeKexPhase() throws Exception {
+ sendClientIdentification();
+ KexExtensionHandler extHandler = getKexExtensionHandler();
+ if ((extHandler == null) || (!extHandler.isKexExtensionsAvailable(this, AvailabilityPhase.PREKEX))) {
+ kexState.set(KexState.INIT);
+ sendKexInit();
+ } else {
+ if (log.isDebugEnabled()) {
+ log.debug("initializeKexPhase({}) delay KEX-INIT until server-side one received", this);
+ }
+ }
+ }
+
+ protected void initializeProxyConnector() throws Exception {
ClientProxyConnector proxyConnector = getClientProxyConnector();
- if (proxyConnector != null) {
- try {
- proxyConnector.sendClientProxyMetadata(this);
- } catch (Throwable t) {
- log.warn("sendClientIdentification({}) failed ({}) to send proxy metadata: {}",
- this, t.getClass().getSimpleName(), t.getMessage());
- if (log.isDebugEnabled()) {
- log.debug("sendClientIdentification(" + this + ") proxy metadata send failure details", t);
- }
+ boolean debugEnabled = log.isDebugEnabled();
+ if (proxyConnector == null) {
+ if (debugEnabled) {
+ log.debug("initializeProxyConnector({}) no proxy to initialize", this);
+ }
+ return;
+ }
- if (t instanceof Exception) {
- throw (Exception) t;
- } else {
- throw new RuntimeSshException(t);
- }
+ try {
+ if (debugEnabled) {
+ log.debug("initializeProxyConnector({}) initialize proxy={}", this, proxyConnector);
+ }
+
+ proxyConnector.sendClientProxyMetadata(this);
+
+ if (debugEnabled) {
+ log.debug("initializeProxyConnector({}) proxy={} initialized", this, proxyConnector);
+ }
+ } catch (Throwable t) {
+ log.warn("initializeProxyConnector({}) failed ({}) to send proxy metadata: {}",
+ this, t.getClass().getSimpleName(), t.getMessage());
+ if (debugEnabled) {
+ log.debug("initializeProxyConnector(" + this + ") proxy metadata send failure details", t);
+ }
+
+ if (t instanceof Exception) {
+ throw (Exception) t;
+ } else {
+ throw new RuntimeSshException(t);
}
}
+ }
+ protected IoWriteFuture sendClientIdentification() throws Exception {
+ clientVersion = resolveIdentificationString(ClientFactoryManager.CLIENT_IDENTIFICATION);
return sendIdentification(clientVersion);
}
@@ -284,9 +322,8 @@ public abstract class AbstractClientSession extends AbstractSession implements C
}
@Override
- public ChannelExec createExecChannel(
- String command, PtyChannelConfigurationHolder ptyConfig, Map<String, ?> env)
- throws IOException {
+ public ChannelExec createExecChannel(String command, PtyChannelConfigurationHolder ptyConfig, Map<String, ?> env)
+ throws IOException {
ChannelExec channel = new ChannelExec(command, ptyConfig, env);
ConnectionService service = getConnectionService();
int id = service.registerChannel(channel);
@@ -308,7 +345,8 @@ public abstract class AbstractClientSession extends AbstractSession implements C
}
@Override
- public ChannelDirectTcpip createDirectTcpipChannel(SshdSocketAddress local, SshdSocketAddress remote) throws IOException {
+ public ChannelDirectTcpip createDirectTcpipChannel(SshdSocketAddress local, SshdSocketAddress remote)
+ throws IOException {
ChannelDirectTcpip channel = new ChannelDirectTcpip(local, remote);
ConnectionService service = getConnectionService();
int id = service.registerChannel(channel);
@@ -328,7 +366,8 @@ public abstract class AbstractClientSession extends AbstractSession implements C
}
@Override
- public SshdSocketAddress startLocalPortForwarding(SshdSocketAddress local, SshdSocketAddress remote) throws IOException {
+ public SshdSocketAddress startLocalPortForwarding(SshdSocketAddress local, SshdSocketAddress remote)
+ throws IOException {
ForwardingFilter filter = getForwardingFilter();
return filter.startLocalPortForwarding(local, remote);
}
@@ -340,7 +379,8 @@ public abstract class AbstractClientSession extends AbstractSession implements C
}
@Override
- public SshdSocketAddress startRemotePortForwarding(SshdSocketAddress remote, SshdSocketAddress local) throws IOException {
+ public SshdSocketAddress startRemotePortForwarding(SshdSocketAddress remote, SshdSocketAddress local)
+ throws IOException {
ForwardingFilter filter = getForwardingFilter();
return filter.startRemotePortForwarding(remote, local);
}
@@ -379,8 +419,8 @@ public abstract class AbstractClientSession extends AbstractSession implements C
@Override
public void startService(String name, Buffer buffer) throws Exception {
SessionDisconnectHandler handler = getSessionDisconnectHandler();
- if ((handler != null)
- && handler.handleUnsupportedServiceDisconnectReason(this, SshConstants.SSH_MSG_SERVICE_REQUEST, name, buffer)) {
+ if ((handler != null) && handler.handleUnsupportedServiceDisconnectReason(this,
+ SshConstants.SSH_MSG_SERVICE_REQUEST, name, buffer)) {
if (log.isDebugEnabled()) {
log.debug("startService({}) ignore unknown service={} by handler", this, name);
}
@@ -391,7 +431,8 @@ public abstract class AbstractClientSession extends AbstractSession implements C
}
@Override
- public ChannelShell createShellChannel(PtyChannelConfigurationHolder ptyConfig, Map<String, ?> env) throws IOException {
+ public ChannelShell createShellChannel(PtyChannelConfigurationHolder ptyConfig, Map<String, ?> env)
+ throws IOException {
if ((inCipher instanceof CipherNone) || (outCipher instanceof CipherNone)) {
throw new IllegalStateException("Interactive channels are not supported with none cipher");
}
@@ -406,7 +447,7 @@ public abstract class AbstractClientSession extends AbstractSession implements C
}
@Override
- protected boolean readIdentification(Buffer buffer) throws IOException {
+ protected boolean readIdentification(Buffer buffer) throws Exception {
List<String> ident = doReadIdentification(buffer, false);
int numLines = GenericUtils.size(ident);
serverVersion = (numLines <= 0) ? null : ident.remove(numLines - 1);
@@ -420,14 +461,18 @@ public abstract class AbstractClientSession extends AbstractSession implements C
if (!SessionContext.isValidVersionPrefix(serverVersion)) {
throw new SshException(SshConstants.SSH2_DISCONNECT_PROTOCOL_VERSION_NOT_SUPPORTED,
- "Unsupported protocol version: " + serverVersion);
+ "Unsupported protocol version: " + serverVersion);
+ }
+
+ signalExtraServerVersionInfo(serverVersion, ident);
+ if (!sendImmediateIdentification) {
+ initializeKexPhase();
}
- signalExtraServerVersionInfo(ident);
return true;
}
- protected void signalExtraServerVersionInfo(List<String> lines) throws IOException {
+ protected void signalExtraServerVersionInfo(String version, List<String> lines) throws IOException {
if (GenericUtils.isEmpty(lines)) {
return;
}
@@ -438,10 +483,11 @@ public abstract class AbstractClientSession extends AbstractSession implements C
ui.serverVersionInfo(this, lines);
}
} catch (Error e) {
- log.warn("signalExtraServerVersionInfo({}) failed ({}) to consult interaction: {}",
- this, e.getClass().getSimpleName(), e.getMessage());
+ log.warn("signalExtraServerVersionInfo({})[{}] failed ({}) to consult interaction: {}", this, version,
+ e.getClass().getSimpleName(), e.getMessage());
if (log.isDebugEnabled()) {
- log.debug("signalExtraServerVersionInfo(" + this + ") interaction consultation failure details", e);
+ log.debug("signalExtraServerVersionInfo(" + this + ")[" + version
+ + "] interaction consultation failure details", e);
}
throw new RuntimeSshException(e);
@@ -463,9 +509,8 @@ public abstract class AbstractClientSession extends AbstractSession implements C
protected byte[] receiveKexInit(Buffer buffer) throws Exception {
byte[] seed = super.receiveKexInit(buffer);
/*
- * Check if the session has delayed its KEX-INIT until the
- * server's one was received in order to support KEX
- * extension negotiation (RFC 8308).
+ * Check if the session has delayed its KEX-INIT until the server's one was
+ * received in order to support KEX extension negotiation (RFC 8308).
*/
if (kexState.compareAndSet(KexState.UNKNOWN, KexState.RUN)) {
if (log.isDebugEnabled()) {
@@ -492,8 +537,8 @@ public abstract class AbstractClientSession extends AbstractSession implements C
PublicKey serverKey = kex.getServerKey();
boolean verified = serverKeyVerifier.verifyServerKey(this, remoteAddress, serverKey);
if (log.isDebugEnabled()) {
- log.debug("checkKeys({}) key={}-{}, verified={}",
- this, KeyUtils.getKeyType(serverKey), KeyUtils.getFingerPrint(serverKey), verified);
+ log.debug("checkKeys({}) key={}-{}, verified={}", this, KeyUtils.getKeyType(serverKey),
+ KeyUtils.getFingerPrint(serverKey), verified);
}
if (!verified) {
@@ -505,7 +550,8 @@ public abstract class AbstractClientSession extends AbstractSession implements C
public KeyExchangeFuture switchToNoneCipher() throws IOException {
if (!(currentService instanceof AbstractConnectionService)
|| !GenericUtils.isEmpty(((AbstractConnectionService) currentService).getChannels())) {
- throw new IllegalStateException("The switch to the none cipher must be done immediately after authentication");
+ throw new IllegalStateException(
+ "The switch to the none cipher must be done immediately after authentication");
}
if (kexState.compareAndSet(KexState.DONE, KexState.INIT)) {
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientProxyConnector.java b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientProxyConnector.java
index 90999da..b0e4f5c 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientProxyConnector.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientProxyConnector.java
@@ -31,13 +31,15 @@ package org.apache.sshd.client.session;
@FunctionalInterface
public interface ClientProxyConnector {
/**
- * Invoked just before the client identification is sent so that the
- * proxy can send the meta-data to its peer. Upon successful return
- * the SSH identification line is sent and the protocol proceeds as usual.
+ * Invoked once initial connection has been established so that the proxy can open
+ * its channel and send the meta-data to its peer. Upon successful return the SSH
+ * identification line is eventually sent and the protocol proceeds as usual.
*
- * @param session The {@link ClientSession} instance
- * @throws Exception If failed to send the data - which will also
+ * @param session The {@link ClientSession} instance - <B>Note:</B> at this stage
+ * the client's identification line is not set yet.
+ * @throws Exception If failed to initialize the proxy - which will also
* terminate the session
+ * @see org.apache.sshd.client.ClientFactoryManager#SEND_IMMEDIATE_IDENTIFICATION SEND_IMMEDIATE_IDENTIFICATION
*/
void sendClientProxyMetadata(ClientSession session) throws Exception;
}
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
index 200e3c3..7abc2ea 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
@@ -39,8 +39,6 @@ import org.apache.sshd.common.SshConstants;
import org.apache.sshd.common.SshException;
import org.apache.sshd.common.io.IoSession;
import org.apache.sshd.common.kex.KexState;
-import org.apache.sshd.common.kex.extension.KexExtensionHandler;
-import org.apache.sshd.common.kex.extension.KexExtensionHandler.AvailabilityPhase;
import org.apache.sshd.common.session.SessionListener;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.ValidateUtils;
@@ -90,16 +88,17 @@ public class ClientSessionImpl extends AbstractClientSession {
authFuture.setAuthed(false);
signalSessionCreated(ioSession);
- sendClientIdentification();
- KexExtensionHandler extHandler = getKexExtensionHandler();
- if ((extHandler == null) || (!extHandler.isKexExtensionsAvailable(this, AvailabilityPhase.PREKEX))) {
- kexState.set(KexState.INIT);
- sendKexInit();
- } else {
- if (log.isDebugEnabled()) {
- log.debug("<init>({}) delay KEX-INIT until server-side one received", this);
- }
+ /*
+ * Must be called regardless of whether the client identification
+ * is sent or not immediately in order to allow opening any underlying
+ * proxy protocol - e.g., SOCKS or HTTP CONNECT - otherwise the server's
+ * identification will never arrive
+ */
+ initializeProxyConnector();
+
+ if (sendImmediateIdentification) {
+ initializeKexPhase();
}
}
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/FactoryManager.java b/sshd-core/src/main/java/org/apache/sshd/common/FactoryManager.java
index 02ea05a..aec9f5d 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/FactoryManager.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/FactoryManager.java
@@ -362,6 +362,11 @@ public interface FactoryManager
String IGNORE_MESSAGE_SIZE = "ignore-message-size";
/**
+ * Value of {@value #IGNORE_MESSAGE_SIZE} if none configured
+ */
+ int DEFAULT_IGNORE_MESSAGE_SIZE = 16;
+
+ /**
* The request type of agent forwarding. The value may be {@value #AGENT_FORWARDING_TYPE_IETF} or
* {@value #AGENT_FORWARDING_TYPE_OPENSSH}.
*/
@@ -378,11 +383,6 @@ public interface FactoryManager
String AGENT_FORWARDING_TYPE_OPENSSH = "auth-agent-req@openssh.com";
/**
- * Value of {@value #IGNORE_MESSAGE_SIZE} if none configured
- */
- int DEFAULT_IGNORE_MESSAGE_SIZE = 16;
-
- /**
* An upper case string identifying the version of the software used on client or server side.
* This version includes the name and version of the software and usually looks like this:
* <code>SSHD-CORE-1.0</code>
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
index 074488e..55f3fc7 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
@@ -1227,10 +1227,10 @@ public abstract class AbstractSession extends SessionHelper {
* @param buffer The {@link Buffer} containing the remote identification
* @return <code>true</code> if the identification has been fully read or
* <code>false</code> if more data is needed
- * @throws IOException if an error occurs such as a bad protocol version
- * @throws GeneralSecurityException If unsuccessful KEX was involved
+ * @throws Exception if an error occurs such as a bad protocol version or unsuccessful
+ * KEX was involved
*/
- protected abstract boolean readIdentification(Buffer buffer) throws IOException, GeneralSecurityException;
+ protected abstract boolean readIdentification(Buffer buffer) throws Exception;
/**
* Send the key exchange initialization packet.
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
index 95ea0ae..914d5d6 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
@@ -19,6 +19,7 @@
package org.apache.sshd.common.session.helpers;
import java.io.IOException;
+import java.io.StreamCorruptedException;
import java.net.SocketAddress;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
@@ -79,22 +80,16 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements
/** Session level lock for regulating access to sensitive data */
protected final Object sessionLock = new Object();
- /**
- * Client or server side
- */
+ /** Client or server side */
private final boolean serverSession;
- /**
- * The underlying network session
- */
+
+ /** The underlying network session */
private final IoSession ioSession;
- /**
- * The session specific properties
- */
+ /** The session specific properties */
private final Map<String, Object> properties = new ConcurrentHashMap<>();
- /**
- * Session specific attributes
- */
+
+ /** Session specific attributes */
private final Map<AttributeRepository.AttributeKey<?>, Object> attributes = new ConcurrentHashMap<>();
// Session timeout measurements
@@ -729,12 +724,14 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements
* @return A {@link List} of all received remote identification lines until
* the version line was read or {@code null} if more data is needed.
* The identification line is the <U>last</U> one in the list
+ * @throws IOException if malformed identification found
*/
- protected List<String> doReadIdentification(Buffer buffer, boolean server) {
+ protected List<String> doReadIdentification(Buffer buffer, boolean server) throws IOException {
int maxIdentSize = PropertyResolverUtils.getIntProperty(this,
FactoryManager.MAX_IDENTIFICATION_SIZE, FactoryManager.DEFAULT_MAX_IDENTIFICATION_SIZE);
List<String> ident = null;
int rpos = buffer.rpos();
+ boolean debugEnabled = log.isDebugEnabled();
for (byte[] data = new byte[SessionContext.MAX_VERSION_LINE_LENGTH];;) {
int pos = 0; // start accumulating line from scratch
for (boolean needLf = false;;) {
@@ -751,9 +748,9 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements
* "The null character MUST NOT be sent"
*/
if (b == 0) {
- throw new IllegalStateException("Incorrect identification (null characters not allowed) - "
- + " at line " + (GenericUtils.size(ident) + 1) + " character #" + (pos + 1)
- + " after '" + new String(data, 0, pos, StandardCharsets.UTF_8) + "'");
+ throw new StreamCorruptedException("Incorrect identification (null characters not allowed) - "
+ + " at line " + (GenericUtils.size(ident) + 1) + " character #" + (pos + 1)
+ + " after '" + new String(data, 0, pos, StandardCharsets.UTF_8) + "'");
}
if (b == '\r') {
needLf = true;
@@ -765,22 +762,22 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements
}
if (needLf) {
- throw new IllegalStateException("Incorrect identification (bad line ending) "
- + " at line " + (GenericUtils.size(ident) + 1)
- + ": " + new String(data, 0, pos, StandardCharsets.UTF_8));
+ throw new StreamCorruptedException("Incorrect identification (bad line ending) "
+ + " at line " + (GenericUtils.size(ident) + 1)
+ + ": " + new String(data, 0, pos, StandardCharsets.UTF_8));
}
if (pos >= data.length) {
- throw new IllegalStateException("Incorrect identification (line too long): "
- + " at line " + (GenericUtils.size(ident) + 1)
- + ": " + new String(data, 0, pos, StandardCharsets.UTF_8));
+ throw new StreamCorruptedException("Incorrect identification (line too long): "
+ + " at line " + (GenericUtils.size(ident) + 1)
+ + ": " + new String(data, 0, pos, StandardCharsets.UTF_8));
}
data[pos++] = b;
}
String str = new String(data, 0, pos, StandardCharsets.UTF_8);
- if (log.isDebugEnabled()) {
+ if (debugEnabled) {
log.debug("doReadIdentification({}) line='{}'", this, str);
}
@@ -795,7 +792,7 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements
}
if (buffer.rpos() > maxIdentSize) {
- throw new IllegalStateException("Incorrect identification (too many header lines): size > " + maxIdentSize);
+ throw new StreamCorruptedException("Incorrect identification (too many header lines): size > " + maxIdentSize);
}
}
}
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java b/sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java
index 5c6dfd3..529b5f8 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java
@@ -444,25 +444,27 @@ public abstract class AbstractServerSession extends AbstractSession implements S
log.debug("readIdentification({}) client version string: {}", this, clientVersion);
}
- String errorMessage = null;
- if (!SessionContext.isValidVersionPrefix(clientVersion)) {
- errorMessage = "Unsupported protocol version: " + clientVersion;
- }
-
- /*
- * NOTE: because of the way that "doReadIdentification" works we are
- * assured that there are no extra lines beyond the version one, but
- * we check this nevertheless
- */
- if ((errorMessage == null) && (numLines > 1)) {
- errorMessage = "Unexpected extra " + (numLines - 1) + " lines from client=" + clientVersion;
+ IOException err;
+ if (SessionContext.isValidVersionPrefix(clientVersion)) {
+ /*
+ * NOTE: because of the way that "doReadIdentification" works we are
+ * assured that there are no extra lines beyond the version one, but
+ * we check this nevertheless
+ */
+ err = (numLines > 1)
+ ? new SshException(SshConstants.SSH2_DISCONNECT_PROTOCOL_ERROR,
+ "Unexpected extra " + (numLines - 1) + " lines from client=" + clientVersion)
+ : null;
+ } else {
+ err = new SshException(SshConstants.SSH2_DISCONNECT_PROTOCOL_VERSION_NOT_SUPPORTED,
+ "Unsupported protocol version: " + clientVersion);
}
- if (GenericUtils.length(errorMessage) > 0) {
+ if (err != null) {
IoSession networkSession = getIoSession();
- networkSession.writePacket(new ByteArrayBuffer((errorMessage + "\n").getBytes(StandardCharsets.UTF_8)))
+ networkSession.writePacket(new ByteArrayBuffer((err.getMessage() + "\n").getBytes(StandardCharsets.UTF_8)))
.addListener(future -> close(true));
- throw new SshException(errorMessage);
+ throw err;
}
kexState.set(KexState.INIT);
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java b/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java
index c3351a8..2e06fdc 100644
--- a/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java
@@ -20,6 +20,7 @@ package org.apache.sshd.common.session.helpers;
import java.io.EOFException;
import java.io.IOException;
+import java.io.StreamCorruptedException;
import java.io.WriteAbortedException;
import java.net.SocketAddress;
import java.nio.charset.StandardCharsets;
@@ -84,45 +85,46 @@ public class AbstractSessionTest extends BaseTestSupport {
}
@Test
- public void testReadIdentSimple() {
+ public void testReadIdentSimple() throws IOException {
Buffer buf = new ByteArrayBuffer("SSH-2.0-software\r\n".getBytes(StandardCharsets.UTF_8));
String ident = readIdentification(session, buf);
assertEquals("SSH-2.0-software", ident);
}
@Test
- public void testReadIdentWithoutCR() {
+ public void testReadIdentWithoutCR() throws IOException {
Buffer buf = new ByteArrayBuffer("SSH-2.0-software\n".getBytes(StandardCharsets.UTF_8));
String ident = readIdentification(session, buf);
assertEquals("SSH-2.0-software", ident);
}
@Test
- public void testReadIdentWithHeaders() {
+ public void testReadIdentWithHeaders() throws IOException {
Buffer buf = new ByteArrayBuffer("a header line\r\nSSH-2.0-software\r\n".getBytes(StandardCharsets.UTF_8));
String ident = readIdentification(session, buf);
assertEquals("SSH-2.0-software", ident);
}
@Test
- public void testReadIdentWithSplitPackets() {
+ public void testReadIdentWithSplitPackets() throws IOException {
Buffer buf = new ByteArrayBuffer("header line\r\nSSH".getBytes(StandardCharsets.UTF_8));
String ident = readIdentification(session, buf);
assertNull("Unexpected identification for header only", ident);
+
buf.putRawBytes("-2.0-software\r\n".getBytes(StandardCharsets.UTF_8));
ident = readIdentification(session, buf);
assertEquals("SSH-2.0-software", ident);
}
- @Test(expected = IllegalStateException.class)
- public void testReadIdentBadLineEnding() {
+ @Test(expected = StreamCorruptedException.class)
+ public void testReadIdentBadLineEnding() throws IOException {
Buffer buf = new ByteArrayBuffer("SSH-2.0-software\ra".getBytes(StandardCharsets.UTF_8));
String ident = readIdentification(session, buf);
fail("Unexpected success: " + ident);
}
- @Test(expected = IllegalStateException.class)
- public void testReadIdentLongLine() {
+ @Test(expected = StreamCorruptedException.class)
+ public void testReadIdentLongLine() throws IOException {
StringBuilder sb = new StringBuilder(SessionContext.MAX_VERSION_LINE_LENGTH + Integer.SIZE);
sb.append("SSH-2.0-software");
do {
@@ -134,16 +136,16 @@ public class AbstractSessionTest extends BaseTestSupport {
fail("Unexpected success: " + ident);
}
- @Test(expected = IllegalStateException.class)
- public void testReadIdentWithNullChar() {
+ @Test(expected = StreamCorruptedException.class)
+ public void testReadIdentWithNullChar() throws IOException {
String id = "SSH-2.0" + '\0' + "-software\r\n";
Buffer buf = new ByteArrayBuffer(id.getBytes(StandardCharsets.UTF_8));
String ident = readIdentification(session, buf);
fail("Unexpected success: " + ident);
}
- @Test(expected = IllegalStateException.class)
- public void testReadIdentLongHeader() {
+ @Test(expected = StreamCorruptedException.class)
+ public void testReadIdentLongHeader() throws IOException {
StringBuilder sb = new StringBuilder(FactoryManager.DEFAULT_MAX_IDENTIFICATION_SIZE + Integer.SIZE);
do {
sb.append("01234567890123456789012345678901234567890123456789\r\n");
@@ -294,7 +296,7 @@ public class AbstractSessionTest extends BaseTestSupport {
}
}
- private static String readIdentification(MySession session, Buffer buf) {
+ private static String readIdentification(MySession session, Buffer buf) throws IOException {
List<String> lines = session.doReadIdentification(buf);
return GenericUtils.isEmpty(lines) ? null : lines.get(lines.size() - 1);
}
@@ -432,7 +434,7 @@ public class AbstractSessionTest extends BaseTestSupport {
return false;
}
- public List<String> doReadIdentification(Buffer buffer) {
+ public List<String> doReadIdentification(Buffer buffer) throws IOException {
return super.doReadIdentification(buffer, false);
}
[mina-sshd] 05/05: Fix possible NPE when handling immediate KEX
packet follow-up in case a peer proposal is not yet set
Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit 141907415b3d0bb38c3b177d30bdcdef90983667
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Tue Jul 9 09:14:54 2019 +0300
Fix possible NPE when handling immediate KEX packet follow-up in case a peer proposal is not yet set
---
.../apache/sshd/common/session/helpers/AbstractSession.java | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
index 9c6dc7e..cc5c926 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
@@ -488,10 +488,15 @@ public abstract class AbstractSession extends SessionHelper {
*/
protected SimpleImmutableEntry<String, String> comparePreferredKexProposalOption(KexProposalOption option) {
String[] clientPreferences = GenericUtils.split(clientProposal.get(option), ',');
- String clientValue = clientPreferences[0];
+ String clientValue = GenericUtils.isEmpty(clientPreferences) ? null : clientPreferences[0];
String[] serverPreferences = GenericUtils.split(serverProposal.get(option), ',');
- String serverValue = serverPreferences[0];
- return Objects.equals(clientValue, serverValue) ? null : new SimpleImmutableEntry<>(clientValue, serverValue);
+ String serverValue = GenericUtils.isEmpty(serverPreferences) ? null : serverPreferences[0];
+ if (GenericUtils.isEmpty(clientValue) || GenericUtils.isEmpty(serverValue)
+ || (!Objects.equals(clientValue, serverValue))) {
+ return new SimpleImmutableEntry<>(clientValue, serverValue);
+ }
+
+ return null;
}
/**