diff --git a/CHANGES.md b/CHANGES.md
index 800246c46..364d67add 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -33,6 +33,7 @@
## New Features
* [GH-429](https://github.com/apache/mina-sshd/issues/429) Support GIT protocol-v2
+* [GH-445](https://github.com/apache/mina-sshd/issues/445) OpenSSH "strict key exchange" protocol extension
## Behavioral changes and enhancements
@@ -43,6 +44,15 @@ acknowledgements of a `receive` related command. The user is free to inspect the
to handle it - including even throwing an exception if OK status (if this makes sense for whatever reason). The default implementation checks for ERROR code and throws
an exception if so.
+### OpenSSH protocol extension: strict key exchange
+
+[GH-445](https://github.com/apache/mina-sshd/issues/445) implements an extension to the SSH protocol introduced
+in OpenSSH 9.6. This ["strict key exchange" extension](https://github.com/openssh/openssh-portable/blob/master/PROTOCOL)
+hardens the SSH key exchange against the ["Terrapin attack"](https://www.terrapin-attack.com/). The extension
+is active if both parties announce their support for it at the start of the initial key exchange. If only one
+party announces support, it is not activated to ensure compatibility with SSH implementations that do not
+implement it. Apache MINA sshd clients and servers always announce their support for strict key exchange.
+
## Potential compatibility issues
## Major Code Re-factoring
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/kex/extension/KexExtensions.java b/sshd-common/src/main/java/org/apache/sshd/common/kex/extension/KexExtensions.java
index 9fac45c13..0f0746425 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/kex/extension/KexExtensions.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/kex/extension/KexExtensions.java
@@ -23,6 +23,7 @@
import java.nio.ByteBuffer;
import java.util.AbstractMap.SimpleImmutableEntry;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
@@ -59,6 +60,24 @@ public final class KexExtensions {
public static final String CLIENT_KEX_EXTENSION = "ext-info-c";
public static final String SERVER_KEX_EXTENSION = "ext-info-s";
+ /**
+ * Reminder:
+ *
+ * These pseudo-algorithms are only valid in the initial SSH2_MSG_KEXINIT and MUST be ignored if they are present in
+ * subsequent SSH2_MSG_KEXINIT packets.
+ *
+ * Note: these values are appended to the initial proposals and removed if received before proceeding
+ * with the standard KEX proposals negotiation.
+ *
+ * @see OpenSSH PROTOCOL - 1.9 transport:
+ * strict key exchange extension
+ */
+ public static final String STRICT_KEX_CLIENT_EXTENSION = "kex-strict-c-v00@openssh.com";
+ public static final String STRICT_KEX_SERVER_EXTENSION = "kex-strict-s-v00@openssh.com";
+ public static final List STRICT_KEX_EXTENSIONS = Collections.unmodifiableList(
+ Arrays.asList(
+ STRICT_KEX_CLIENT_EXTENSION, STRICT_KEX_SERVER_EXTENSION));
+
@SuppressWarnings("checkstyle:Indentation")
public static final Predicate IS_KEX_EXTENSION_SIGNAL
= n -> CLIENT_KEX_EXTENSION.equalsIgnoreCase(n) || SERVER_KEX_EXTENSION.equalsIgnoreCase(n);
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 4bdb39c4c..7ac065677 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
@@ -27,13 +27,17 @@
import java.time.Duration;
import java.time.Instant;
import java.util.AbstractMap.SimpleImmutableEntry;
+import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Deque;
import java.util.EnumMap;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentLinkedDeque;
import java.util.concurrent.CopyOnWriteArraySet;
@@ -45,6 +49,7 @@
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.LongConsumer;
import java.util.logging.Level;
+import java.util.stream.Collectors;
import org.apache.sshd.common.Closeable;
import org.apache.sshd.common.Factory;
@@ -109,6 +114,7 @@
*
* @author Apache MINA SSHD Project
*/
+@SuppressWarnings("checkstyle:MethodCount")
public abstract class AbstractSession extends SessionHelper {
/**
* Name of the property where this session is stored in the attributes of the underlying MINA session. See
@@ -192,6 +198,22 @@ public abstract class AbstractSession extends SessionHelper {
protected final Object decodeLock = new Object();
protected final Object requestLock = new Object();
+ /**
+ * "Strict KEX" is a mitigation for the "Terrapin attack". The KEX protocol is modified as follows:
+ *
+ * - During the initial (unencrypted) KEX, no extra messages not strictly necessary for KEX are allowed. The
+ * KEX_INIT message must be the first one after the version identification, and no IGNORE or DEBUG messages are
+ * allowed until the KEX is completed. If a party receives such a message, it terminates the connection.
+ * - Message sequence numbers are reset to zero after a key exchange (initial or later). When the NEW_KEYS message
+ * has been sent, the outgoing message number is reset; after a NEW_KEYS message has been received, the incoming
+ * message number is reset.
+ *
+ * Strict KEX is negotiated in the original KEX proposal; it is active if and only if both parties indicate that
+ * they support strict KEX.
+ */
+ protected boolean strictKex;
+ protected long initialKexInitSequenceNumber = -1;
+
/**
* The {@link KeyExchangeMessageHandler} instance also serves as lock protecting {@link #kexState} changes from DONE
* to INIT or RUN, and from KEYS to DONE.
@@ -550,18 +572,24 @@ protected void doHandleMessage(Buffer buffer) throws Exception {
handleDisconnect(buffer);
break;
case SshConstants.SSH_MSG_IGNORE:
+ failStrictKex(cmd);
handleIgnore(buffer);
break;
case SshConstants.SSH_MSG_UNIMPLEMENTED:
+ failStrictKex(cmd);
handleUnimplemented(buffer);
break;
case SshConstants.SSH_MSG_DEBUG:
+ // Fail after handling -- by default a message will be logged, which might be helpful.
handleDebug(buffer);
+ failStrictKex(cmd);
break;
case SshConstants.SSH_MSG_SERVICE_REQUEST:
+ failStrictKex(cmd);
handleServiceRequest(buffer);
break;
case SshConstants.SSH_MSG_SERVICE_ACCEPT:
+ failStrictKex(cmd);
handleServiceAccept(buffer);
break;
case SshConstants.SSH_MSG_KEXINIT:
@@ -571,9 +599,11 @@ protected void doHandleMessage(Buffer buffer) throws Exception {
handleNewKeys(cmd, buffer);
break;
case KexExtensions.SSH_MSG_EXT_INFO:
+ failStrictKex(cmd);
handleKexExtension(cmd, buffer);
break;
case KexExtensions.SSH_MSG_NEWCOMPRESS:
+ failStrictKex(cmd);
handleNewCompression(cmd, buffer);
break;
default:
@@ -589,26 +619,35 @@ protected void doHandleMessage(Buffer buffer) throws Exception {
}
handleKexMessage(cmd, buffer);
- } else if (currentService.process(cmd, buffer)) {
- resetIdleTimeout();
} else {
- /*
- * According to https://tools.ietf.org/html/rfc4253#section-11.4
- *
- * An implementation MUST respond to all unrecognized messages with an SSH_MSG_UNIMPLEMENTED message
- * in the order in which the messages were received.
- */
- if (log.isDebugEnabled()) {
- log.debug("process({}) Unsupported command: {}",
- this, SshConstants.getCommandMessageName(cmd));
+ failStrictKex(cmd);
+ if (currentService.process(cmd, buffer)) {
+ resetIdleTimeout();
+ } else {
+ /*
+ * According to https://tools.ietf.org/html/rfc4253#section-11.4
+ *
+ * An implementation MUST respond to all unrecognized messages with an SSH_MSG_UNIMPLEMENTED
+ * message in the order in which the messages were received.
+ */
+ if (log.isDebugEnabled()) {
+ log.debug("process({}) Unsupported command: {}", this, SshConstants.getCommandMessageName(cmd));
+ }
+ notImplemented(cmd, buffer);
}
- notImplemented(cmd, buffer);
}
break;
}
checkRekey();
}
+ protected void failStrictKex(int cmd) throws SshException {
+ if (!initialKexDone && strictKex) {
+ throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
+ SshConstants.getCommandMessageName(cmd) + " not allowed during initial key exchange in strict KEX");
+ }
+ }
+
protected boolean handleFirstKexPacketFollows(int cmd, Buffer buffer, boolean followFlag) {
if (!followFlag) {
return true; // if 1st KEX packet does not follow then process the command
@@ -1118,7 +1157,7 @@ protected IoWriteFuture doWritePacket(Buffer buffer) throws IOException {
}
protected int resolveIgnoreBufferDataLength() {
- if ((ignorePacketDataLength <= 0)
+ if (!initialKexDone || (ignorePacketDataLength <= 0)
|| (ignorePacketsFrequency <= 0L)
|| (ignorePacketsVariance < 0)) {
return 0;
@@ -1931,6 +1970,13 @@ protected void prepareNewKeys() throws Exception {
* @throws Exception on errors
*/
protected void setOutputEncoding() throws Exception {
+ if (strictKex) {
+ if (log.isDebugEnabled()) {
+ log.debug("setOutputEncoding({}): strict KEX resets output message sequence number from {} to 0", this, seqo);
+ }
+ seqo = 0;
+ }
+
outCipher = outSettings.getCipher(seqo);
outMac = outSettings.getMac();
outCompression = outSettings.getCompression();
@@ -1962,6 +2008,13 @@ protected void setOutputEncoding() throws Exception {
* @throws Exception on errors
*/
protected void setInputEncoding() throws Exception {
+ if (strictKex) {
+ if (log.isDebugEnabled()) {
+ log.debug("setInputEncoding({}): strict KEX resets input message sequence number from {} to 0", this, seqi);
+ }
+ seqi = 0;
+ }
+
inCipher = inSettings.getCipher(seqi);
inMac = inSettings.getMac();
inCompression = inSettings.getCompression();
@@ -2044,6 +2097,25 @@ protected IoWriteFuture notImplemented(int cmd, Buffer buffer) throws Exception
return sendNotImplemented(seqi - 1L);
}
+ /**
+ * Given a KEX proposal and a {@link KexProposalOption}, removes all occurrences of a value from a comma-separated
+ * value list.
+ *
+ * @param options {@link Map} holding the Kex proposal
+ * @param option {@link KexProposalOption} to modify
+ * @param toRemove value to remove
+ * @return {@code true} if the option contained the value (and it was removed); {@code false} otherwise
+ */
+ protected boolean removeValue(Map options, KexProposalOption option, String toRemove) {
+ String val = options.get(option);
+ Set algorithms = new LinkedHashSet<>(Arrays.asList(val.split(",")));
+ boolean result = algorithms.remove(toRemove);
+ if (result) {
+ options.put(option, algorithms.stream().collect(Collectors.joining(",")));
+ }
+ return result;
+ }
+
/**
* Compute the negotiated proposals by merging the client and server proposal. The negotiated proposal will also be
* stored in the {@link #negotiationResult} property.
@@ -2056,11 +2128,33 @@ protected Map negotiate() throws Exception {
Map s2cOptions = getServerKexProposals();
signalNegotiationStart(c2sOptions, s2cOptions);
+ // Make modifiable. Strict KEX flags are to be heeded only in initial KEX, and to be ignored afterwards.
+ c2sOptions = new EnumMap<>(c2sOptions);
+ s2cOptions = new EnumMap<>(s2cOptions);
+ boolean strictKexClient = removeValue(c2sOptions, KexProposalOption.ALGORITHMS,
+ KexExtensions.STRICT_KEX_CLIENT_EXTENSION);
+ boolean strictKexServer = removeValue(s2cOptions, KexProposalOption.ALGORITHMS,
+ KexExtensions.STRICT_KEX_SERVER_EXTENSION);
+ // Make unmodifiable again
+ c2sOptions = Collections.unmodifiableMap(c2sOptions);
+ s2cOptions = Collections.unmodifiableMap(s2cOptions);
Map guess = new EnumMap<>(KexProposalOption.class);
Map negotiatedGuess = Collections.unmodifiableMap(guess);
try {
boolean debugEnabled = log.isDebugEnabled();
boolean traceEnabled = log.isTraceEnabled();
+ if (!initialKexDone) {
+ strictKex = strictKexClient && strictKexServer;
+ if (debugEnabled) {
+ log.debug("negotiate({}) strict KEX={} client={} server={}", this, strictKex, strictKexClient,
+ strictKexServer);
+ }
+ if (strictKex && initialKexInitSequenceNumber != 1) {
+ throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
+ "Strict KEX negotiated but sequence number of first KEX_INIT received is not 1: "
+ + initialKexInitSequenceNumber);
+ }
+ }
SessionDisconnectHandler discHandler = getSessionDisconnectHandler();
KexExtensionHandler extHandler = getKexExtensionHandler();
for (KexProposalOption paramType : KexProposalOption.VALUES) {
@@ -2520,8 +2614,34 @@ protected String resolveSessionKexProposal(String hostKeyTypes) throws IOExcepti
}
}
+ protected Map doStrictKexProposal(Map proposal) {
+ String value = proposal.get(KexProposalOption.ALGORITHMS);
+ String askForStrictKex = isServerSession()
+ ? KexExtensions.STRICT_KEX_SERVER_EXTENSION
+ : KexExtensions.STRICT_KEX_CLIENT_EXTENSION;
+ if (!initialKexDone) {
+ // On the initial KEX, include the strict KEX flag
+ if (GenericUtils.isEmpty(value)) {
+ value = askForStrictKex;
+ } else {
+ value += "," + askForStrictKex;
+ }
+ } else if (!GenericUtils.isEmpty(value)) {
+ // On subsequent KEXes, do not include ext-info-c/ext-info-s or the strict KEX flag in the proposal.
+ List algorithms = new ArrayList<>(Arrays.asList(value.split(",")));
+ String extType = isServerSession() ? KexExtensions.SERVER_KEX_EXTENSION : KexExtensions.CLIENT_KEX_EXTENSION;
+ boolean changed = algorithms.remove(extType);
+ changed |= algorithms.remove(askForStrictKex);
+ if (changed) {
+ value = algorithms.stream().collect(Collectors.joining(","));
+ }
+ }
+ proposal.put(KexProposalOption.ALGORITHMS, value);
+ return proposal;
+ }
+
protected byte[] sendKexInit() throws Exception {
- Map proposal = getKexProposal();
+ Map proposal = doStrictKexProposal(getKexProposal());
byte[] seed;
synchronized (kexState) {
@@ -2588,6 +2708,9 @@ protected void setServerKexData(byte[] data) {
protected byte[] receiveKexInit(Buffer buffer) throws Exception {
Map proposal = new EnumMap<>(KexProposalOption.class);
+ if (!initialKexDone) {
+ initialKexInitSequenceNumber = seqi;
+ }
byte[] seed;
synchronized (kexState) {
seed = receiveKexInit(buffer, proposal);
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 6fe6e8104..8d74d51b7 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
@@ -437,6 +437,7 @@ public static class MySession extends AbstractSession {
public MySession() {
super(true, org.apache.sshd.util.test.CoreTestSupportUtils.setupTestServer(AbstractSessionTest.class),
new MyIoSession());
+ initialKexDone = true;
}
@Override