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/02/21 17:59:09 UTC
[mina-sshd] 01/02: [SSHD-898] Allow client session to delay sending
KEX_INIT until server KEX_INIT received
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 6c677d60f2b27d2f63098fec463f9936c9ed6097
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Wed Feb 20 07:59:23 2019 +0200
[SSHD-898] Allow client session to delay sending KEX_INIT until server KEX_INIT received
---
README.md | 8 +-
docs/event-listeners.md | 6 +-
.../sshd/client/session/AbstractClientSession.java | 19 +++++
.../sshd/client/session/ClientSessionImpl.java | 14 +++-
.../DefaultClientKexExtensionHandler.java | 97 ++++++++++++----------
.../common/kex/extension/KexExtensionHandler.java | 46 ++++++++--
.../common/session/helpers/AbstractSession.java | 20 +++--
.../sshd/server/session/AbstractServerSession.java | 3 +-
8 files changed, 149 insertions(+), 64 deletions(-)
diff --git a/README.md b/README.md
index fc22ec0..d40a88a 100644
--- a/README.md
+++ b/README.md
@@ -15,11 +15,13 @@ based applications requiring SSH support.
* [RFC 4256 - Generic Message Exchange Authentication for the Secure Shell Protocol (SSH)](https://tools.ietf.org/html/rfc4256)
* [RFC 5480 - Elliptic Curve Cryptography Subject Public Key Information](https://tools.ietf.org/html/rfc5480)
* [RFC 8308 - Extension Negotiation in the Secure Shell (SSH) Protocol](https://tools.ietf.org/html/rfc8308)
- * **Note:** - the code contains **hooks** for implementing the RFC but beyond allowing convenient
- access to the required protocol details, it does not implement any logic that handles the messages.
+ * **Note:** - the code contains [**hooks**](./docs/event-listeners.md#kexextensionhandler) for implementing the RFC but
+ beyond allowing convenient support for the required protocol details, it does not implement any default logic that handles
+ the messages or manages the actual extension negotiation (though some **experimental** code is available).
* [RFC 8332 - Use of RSA Keys with SHA-256 and SHA-512 in the Secure Shell (SSH) Protocol](https://tools.ietf.org/html/rfc8332)
* **Note:** - the server side supports these signatures by default. The client side requires specific
- initialization - see [section 3.3](https://tools.ietf.org/html/rfc8332#section-3.3).
+ initialization - see [section 3.3](https://tools.ietf.org/html/rfc8332#section-3.3) and also the
+ above mentioned hooks for [RFC 8308]((https://tools.ietf.org/html/rfc8308).
* SFTP version 3-6 + extensions
* `supported` - [DRAFT 05 - section 4.4](http://tools.ietf.org/wg/secsh/draft-ietf-secsh-filexfer/draft-ietf-secsh-filexfer-05.tx)
* `supported2` - [DRAFT 13 section 5.4](https://tools.ietf.org/html/draft-ietf-secsh-filexfer-13#page-10)
diff --git a/docs/event-listeners.md b/docs/event-listeners.md
index dd6230e..2557a69 100644
--- a/docs/event-listeners.md
+++ b/docs/event-listeners.md
@@ -81,7 +81,11 @@ and/or connection service - the one registered "closest" to connection service w
### `KexExtensionHandler`
-Provides hook for implementing [KEX extension negotiation](https://tools.wordtothewise.com/rfc/rfc8308)
+Provides hooks for implementing [KEX extension negotiation](https://tools.ietf.org/html/rfc8308).
+
+**Note:** it can be used for monitoring the KEX mechanism and intervene in a more general case for other purposes as well. In any case, it is
+highly recommended though to read the interface documentation and also review the code that invokes it before attempting to use it.
+An **experimental** implementation example is available for the client side - see `DefaultClientKexExtensionHandler`.
### `ReservedSessionMessagesHandler`
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 aebdd2c..e197077 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
@@ -457,6 +457,25 @@ public abstract class AbstractClientSession extends AbstractSession implements C
}
@Override
+ 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).
+ */
+ if (kexState.compareAndSet(KexState.UNKNOWN, KexState.RUN)) {
+ if (log.isDebugEnabled()) {
+ log.debug("receiveKexInit({}) sending client proposal", this);
+ }
+ kexState.set(KexState.INIT);
+ sendKexInit();
+ }
+
+ return seed;
+ }
+
+ @Override
protected void receiveKexInit(Map<KexProposalOption, String> proposal, byte[] seed) throws IOException {
mergeProposals(serverProposal, proposal);
setServerKexData(seed);
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 8e4408e..bc9c6a2 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
@@ -38,6 +38,8 @@ 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;
@@ -88,8 +90,16 @@ public class ClientSessionImpl extends AbstractClientSession {
signalSessionCreated(ioSession);
sendClientIdentification();
- kexState.set(KexState.INIT);
- sendKexInit();
+
+ 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);
+ }
+ }
}
@Override
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java b/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
index 8654a84..81a8d76 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
@@ -56,7 +56,12 @@ public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implem
/**
* Session {@link AttributeKey} used to store the client's proposal
*/
- public static final AttributeKey<Map<KexProposalOption, String>> PROPOSAL_KEY = new AttributeKey<>();
+ public static final AttributeKey<Map<KexProposalOption, String>> CLIENT_PROPOSAL_KEY = new AttributeKey<>();
+
+ /**
+ * Session {@link AttributeKey} used to store the server's proposal
+ */
+ public static final AttributeKey<Map<KexProposalOption, String>> SERVER_PROPOSAL_KEY = new AttributeKey<>();
public static final NavigableSet<String> DEFAULT_EXTRA_SIGNATURES =
Collections.unmodifiableNavigableSet(
@@ -71,73 +76,75 @@ public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implem
}
@Override
- public boolean isKexExtensionsAvailable(Session session) throws IOException {
- return (session != null) && (!session.isServerSession());
- }
-
- @Override
- public void handleKexInitProposal(
- Session session, boolean initiator, Map<KexProposalOption, String> proposal)
- throws IOException {
- if (session.isServerSession()) {
- return; // just in case
+ public boolean isKexExtensionsAvailable(Session session, AvailabilityPhase phase) throws IOException {
+ if ((session == null) || session.isServerSession()) {
+ return false;
}
- boolean debugEnabled = log.isDebugEnabled();
- Collection<String> extraAlgos = getExtraSignatureAlgorithms(session);
- if (GenericUtils.isEmpty(extraAlgos)) {
- if (debugEnabled) {
- log.debug("handleKexInitProposal({}) no extra signatures to add to {}", session, proposal);
- }
- return;
+ // We only need to take special care during the proposal build phase
+ if (phase != AvailabilityPhase.PROPOSAL) {
+ return true;
}
- Collection<? extends NamedResource> sigList = session.getSignatureFactories();
- long existCount = sigList.stream().filter(f -> extraAlgos.contains(f.getName())).count();
- if (existCount == extraAlgos.size()) {
- if (debugEnabled) {
- log.debug("handleKexInitProposal({}) required extra signatures ({}) already supported for {}",
- session, extraAlgos, proposal);
- }
- return;
- }
-
- if (initiator) {
- session.setAttribute(PROPOSAL_KEY, new EnumMap<>(proposal));
+ boolean debugEnabled = log.isDebugEnabled();
+ // Check if client already sent its proposal - if not, we can still influence it
+ Map<KexProposalOption, String> clientProposal = session.getAttribute(CLIENT_PROPOSAL_KEY);
+ Map<KexProposalOption, String> serverProposal = session.getAttribute(SERVER_PROPOSAL_KEY);
+ if (GenericUtils.isNotEmpty(clientProposal)) {
if (debugEnabled) {
- log.debug("handleKexInitProposal({}) initial proposal={}", session, proposal);
+ log.debug("isKexExtensionsAvailable({})[{}] already sent proposal={} (server={})",
+ session, phase, clientProposal, serverProposal);
}
- return;
+ return false;
}
- // Check if client already sent its proposal - if not, we can still influence it
- Map<KexProposalOption, String> sentProposal = session.getAttribute(PROPOSAL_KEY);
- if (GenericUtils.isNotEmpty(sentProposal)) {
+ /*
+ * According to https://tools.ietf.org/html/rfc8308#section-3.1:
+ *
+ *
+ * Note that implementations are known to exist that apply
+ * authentication penalties if the client attempts to use an
+ * unexpected public key algorithm.
+ *
+ * Therefore we want to be sure the server declared its support
+ * for extensions before we declare ours.
+ */
+ if (GenericUtils.isEmpty(serverProposal)) {
if (debugEnabled) {
- log.debug("handleKexInitProposal({}) already sent proposal={} (server={})",
- session, sentProposal, proposal);
+ log.debug("isKexExtensionsAvailable({})[{}] no server proposal", session, phase);
}
- return;
+ return false;
}
- String algos = proposal.get(KexProposalOption.ALGORITHMS);
+ String algos = serverProposal.get(KexProposalOption.ALGORITHMS);
String extDeclared = Stream.of(GenericUtils.split(algos, ','))
.filter(s -> KexExtensions.SERVER_KEX_EXTENSION.equalsIgnoreCase(s))
.findFirst()
.orElse(null);
if (GenericUtils.isEmpty(extDeclared)) {
if (debugEnabled) {
- log.debug("handleKexInitProposal({}) server proposal={} does not include extension indicator",
- session, proposal);
+ log.debug("isKexExtensionsAvailable({})[{}] server proposal does not include extension indicator: {}",
+ session, phase, algos);
}
- return;
+ return false;
}
- updateAvailableSignatureFactories(session, extraAlgos);
+ return true;
}
- protected Collection<String> getExtraSignatureAlgorithms(Session session) throws IOException {
- return DEFAULT_EXTRA_SIGNATURES;
+ @Override
+ public void handleKexInitProposal(
+ Session session, boolean initiator, Map<KexProposalOption, String> proposal)
+ throws IOException {
+ if (session.isServerSession()) {
+ return; // just in case
+ }
+
+ session.setAttribute(initiator ? CLIENT_PROPOSAL_KEY : SERVER_PROPOSAL_KEY, new EnumMap<>(proposal));
+ if (log.isDebugEnabled()) {
+ log.debug("handleKexInitProposal({})[initiator={}] proposal={}", session, initiator, proposal);
+ }
+ return;
}
@Override
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/KexExtensionHandler.java b/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/KexExtensionHandler.java
index a34c0f0..d3d9326 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/KexExtensionHandler.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/KexExtensionHandler.java
@@ -35,20 +35,44 @@ import org.apache.sshd.common.util.buffer.Buffer;
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public interface KexExtensionHandler {
- enum KexPhase {
+ /**
+ * Provides a hint as to the context in which {@code isKexExtensionsAvailable} is invoked
+ *
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+ enum AvailabilityPhase {
+ /**
+ * Decide whether to delay sending the KEX-INIT message until
+ * the peer one has been received. <B>Note:</B> currently invoked only
+ * by client sessions, but code should not rely on this implicit assumption.
+ */
+ PREKEX,
+
+ /**
+ * About to create the KEX-INIT proposal - should this session declare
+ * it support the KEX negotiation extension mechanism or not.
+ */
+ PROPOSAL,
+
+ /**
+ * About to send the {@code SSH_MSG_NEWKEYS} message
+ */
NEWKEYS,
- AUTHOK;
- public static final Set<KexPhase> VALUES =
- Collections.unmodifiableSet(EnumSet.allOf(KexPhase.class));
+ /**
+ * About to send {@code SSH_MSG_USERAUTH_SUCCESS} message. <B>Note:</B> currently
+ * invoked only by server sessions, but code should not rely on this implicit assumption.
+ */
+ AUTHOK;
}
/**
* @param session The {@link Session} about to execute KEX
+ * @param phase The {@link AvailabilityPhase} hint as to why the query is being made
* @return {@code true} whether to KEX extensions are supported/allowed for the session
* @throws IOException If failed to process the request
*/
- default boolean isKexExtensionsAvailable(Session session) throws IOException {
+ default boolean isKexExtensionsAvailable(Session session, AvailabilityPhase phase) throws IOException {
return true;
}
@@ -95,6 +119,18 @@ public interface KexExtensionHandler {
}
/**
+ * The phase at which {@code sendKexExtensions} is invoked
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+ enum KexPhase {
+ NEWKEYS,
+ AUTHOK;
+
+ public static final Set<KexPhase> VALUES =
+ Collections.unmodifiableSet(EnumSet.allOf(KexPhase.class));
+ }
+
+ /**
* Invoked in order to allow the handler to send an {@code SSH_MSG_EXT_INFO} message.
* <B>Note:</B> this method is called only if {@link #isKexExtensionsAvailable(Session)}
* returns {@code true} for the session.
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 c452293..7f41db3 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
@@ -64,6 +64,7 @@ import org.apache.sshd.common.kex.KexProposalOption;
import org.apache.sshd.common.kex.KexState;
import org.apache.sshd.common.kex.KeyExchange;
import org.apache.sshd.common.kex.extension.KexExtensionHandler;
+import org.apache.sshd.common.kex.extension.KexExtensionHandler.AvailabilityPhase;
import org.apache.sshd.common.kex.extension.KexExtensionHandler.KexPhase;
import org.apache.sshd.common.kex.extension.KexExtensions;
import org.apache.sshd.common.mac.Mac;
@@ -520,7 +521,7 @@ public abstract class AbstractSession extends SessionHelper {
* + As the next packet following the server's first SSH_MSG_NEWKEYS.
*/
KexExtensionHandler extHandler = getKexExtensionHandler();
- if ((extHandler == null) || (!extHandler.isKexExtensionsAvailable(this))) {
+ if ((extHandler == null) || (!extHandler.isKexExtensionsAvailable(this, AvailabilityPhase.NEWKEYS))) {
return future;
}
@@ -621,7 +622,10 @@ public abstract class AbstractSession extends SessionHelper {
log.debug("handleKexInit({}) SSH_MSG_KEXINIT", this);
}
receiveKexInit(buffer);
+ doKexNegotiation();
+ }
+ protected void doKexNegotiation() throws Exception {
if (kexState.compareAndSet(KexState.DONE, KexState.RUN)) {
sendKexInit();
} else if (!kexState.compareAndSet(KexState.INIT, KexState.RUN)) {
@@ -632,9 +636,10 @@ public abstract class AbstractSession extends SessionHelper {
String kexAlgorithm = result.get(KexProposalOption.ALGORITHMS);
Collection<? extends NamedFactory<KeyExchange>> kexFactories = getKeyExchangeFactories();
synchronized (pendingPackets) {
- kex = ValidateUtils.checkNotNull(NamedFactory.create(kexFactories, kexAlgorithm),
- "Unknown negotiated KEX algorithm: %s",
- kexAlgorithm);
+ kex = ValidateUtils.checkNotNull(
+ NamedFactory.create(kexFactories, kexAlgorithm),
+ "Unknown negotiated KEX algorithm: %s",
+ kexAlgorithm);
}
byte[] v_s = serverVersion.getBytes(StandardCharsets.UTF_8);
@@ -1941,7 +1946,7 @@ public abstract class AbstractSession extends SessionHelper {
String proposal = super.resolveSessionKexProposal(hostKeyTypes);
// see https://tools.ietf.org/html/rfc8308
KexExtensionHandler extHandler = getKexExtensionHandler();
- if ((extHandler == null) || (!extHandler.isKexExtensionsAvailable(this))) {
+ if ((extHandler == null) || (!extHandler.isKexExtensionsAvailable(this, AvailabilityPhase.PROPOSAL))) {
return proposal;
}
@@ -2048,7 +2053,7 @@ public abstract class AbstractSession extends SessionHelper {
*/
protected abstract void checkKeys() throws IOException;
- protected byte[] receiveKexInit(Buffer buffer) throws IOException {
+ protected byte[] receiveKexInit(Buffer buffer) throws Exception {
Map<KexProposalOption, String> proposal = new EnumMap<>(KexProposalOption.class);
byte[] seed;
@@ -2058,7 +2063,8 @@ public abstract class AbstractSession extends SessionHelper {
}
if (log.isTraceEnabled()) {
- log.trace("receiveKexInit({}) proposal={} seed: {}", this, proposal, BufferUtils.toHex(':', seed));
+ log.trace("receiveKexInit({}) proposal={} seed: {}",
+ this, proposal, BufferUtils.toHex(':', seed));
}
return seed;
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 fb9c3aa..a882f78 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
@@ -45,6 +45,7 @@ import org.apache.sshd.common.kex.KexFactoryManager;
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.kex.extension.KexExtensionHandler.KexPhase;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
import org.apache.sshd.common.session.ConnectionService;
@@ -280,7 +281,7 @@ public abstract class AbstractServerSession extends AbstractSession implements S
* + Immediately preceding the server's SSH_MSG_USERAUTH_SUCCESS
*/
KexExtensionHandler extHandler = getKexExtensionHandler();
- if ((extHandler != null) && extHandler.isKexExtensionsAvailable(this)) {
+ if ((extHandler != null) && extHandler.isKexExtensionsAvailable(this, AvailabilityPhase.AUTHOK)) {
extHandler.sendKexExtensions(this, KexPhase.AUTHOK);
}