From 485d1899616e361037a0c4b83bb28707905f271a Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 22 Sep 2026 10:00:14 -0700 Subject: [PATCH 1/8] Add strict key exchange, the Terrapin mitigation wolfSSH negotiates strict key exchange, the mitigation from draft-miller-sshm-strict-kex for CVE-2023-48795, and turns it on when the peer asks for it too. Nothing in the initial KEX is authenticated, so a packet spliced into it shifts the receiver's sequence number, and that shift is the attack. - SendKexInit() carries the marker on the initial KEXINIT only, sending both the -v00@openssh.com name deployed OpenSSH uses and the draft's unprefixed one - DoKexInit() matches either spelling and enables the mitigation when both sides asked for it; a marker in a rekey KEXINIT is ignored - DoPacket() and SendNewKeys() zero peerSeq and seq at every NEWKEYS, the outbound reset landing before SendPendingChannelWindowAdjust() can bundle a packet at the old number - IsMessageAllowed() takes an allow list until the peer's NEWKEYS arrives: DISCONNECT, KEXINIT, NEWKEYS and the key exchange range - a refused message ends the connection with SSH_MSG_DISCONNECT and KEY_EXCHANGE_FAILED rather than being dropped - wolfSSH_CTX_SetStrictKex() and wolfSSH_SetStrictKex() opt out per context or per session, the context seeding each session it makes - the handshake freezes the setting when it starts, so advertising the marker and enforcing it cannot read different values - ID_EXTINFO_{S,C} become ID_EXT_INFO_{S,C}, beside four new pseudo-KEX ids for the strict KEX names - reword WS_MSGID_NOT_ALLOWED_E, which no longer only covers userauth - cover the negotiation, the gate, the injections and the resets in regress.c, and the new calls in api.c - describe strict KEX and its opt-outs in README.md --- README.md | 34 ++ src/internal.c | 168 +++++++- src/ssh.c | 54 ++- tests/api.c | 64 +++ tests/regress.c | 1029 +++++++++++++++++++++++++++++++++++++++++++- wolfssh/internal.h | 15 +- wolfssh/ssh.h | 15 + 7 files changed, 1355 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 15b3f2935..21286d7a2 100644 --- a/README.md +++ b/README.md @@ -766,6 +766,40 @@ Notes: `-DWC_SIG_MIN_HASH_TYPE=WC_HASH_TYPE_SHA`. This re-enables a deprecated hash; prefer ECDSA unless RSA is mandated. +STRICT KEY EXCHANGE +=================== + +wolfSSH implements strict key exchange, the mitigation for the Terrapin attack +(CVE-2023-48795) described in `draft-miller-sshm-strict-kex`. It is negotiated +in the initial KEXINIT and enabled whenever the peer asks for it too, so no +configuration is needed for the usual case. + +With strict KEX in force, wolfSSH accepts nothing but the key exchange itself +and SSH_MSG_DISCONNECT until the peer's SSH_MSG_NEWKEYS arrives, and it zeroes +the packet sequence numbers at every SSH_MSG_NEWKEYS. Together those stop an +attacker splicing packets into the unauthenticated initial exchange to shift +the sequence numbers. A message that arrives out of turn ends the connection +with SSH_MSG_DISCONNECT rather than being ignored. + +Note that wolfSSH offers neither `chacha20-poly1305@openssh.com` nor the +`*-etm@openssh.com` MACs, the modes whose nonce comes from the sequence +number. The full silent-truncation form of Terrapin needs one of those, so it +was never reachable here; strict KEX closes the sequence-number shift the +attack is built on. + +A caller that has to interoperate with a peer that mishandles the marker can +turn it off, per context or per session: + + wolfSSH_CTX_SetStrictKex(ctx, 0); /* seeds every session from ctx */ + wolfSSH_SetStrictKex(ssh, 0); /* this session only */ + +Only the initial KEXINIT carries the marker, and only that exchange decides +whether the mitigation is on, so the session call has to be made before the +connection starts. A rekey neither re-advertises the marker nor revisits the +decision, and a change made to a session already keying is ignored for the +life of that session. The context call is the one that affects anything +later: it seeds the sessions made after it. + WOLFSSH APPLICATIONS ==================== diff --git a/src/internal.c b/src/internal.c index c721b7156..85dbed456 100644 --- a/src/internal.c +++ b/src/internal.c @@ -585,7 +585,7 @@ const char* GetErrorString(int err) return "not a regular file"; case WS_MSGID_NOT_ALLOWED_E: - return "message not allowed before user authentication"; + return "message ID not allowed at this point in the connection"; case WS_ED25519_E: return "Ed25519 buffer error"; @@ -671,8 +671,9 @@ static INLINE int HighwaterCheck(WOLFSSH* ssh, byte side) /* RFC 4344 Sec 3.1: rekey before the 32-bit SSH sequence number wraps * to prevent MAC/nonce reuse. Counter is per-key (resets on rekey), - * not the absolute ssh->seq (which does not reset); default 2^31 - * keeps each epoch comfortably under the 2^32 wrap. */ + * not the absolute ssh->seq (which resets only at a strict KEX + * NEWKEYS); default 2^31 keeps each epoch comfortably under the + * 2^32 wrap. */ if (!ssh->msgHighwaterFlag && ssh->msgHighwaterMark && (ssh->txMsgCount >= ssh->msgHighwaterMark || ssh->rxMsgCount >= ssh->msgHighwaterMark)) { @@ -725,6 +726,27 @@ static HandshakeInfo* HandshakeInfoNew(void* heap) } +/* The strict KEX setting this handshake runs on. Frozen the first time it + * is read -- at our KEXINIT or at the peer's, whichever comes first -- so + * that a caller toggling the setting mid-handshake cannot leave one side + * advertising the marker and the other refusing to enforce it. A later + * toggle takes effect on the next handshake, which allocates its own + * HandshakeInfo. */ +static byte HandshakeStrictKex(WOLFSSH* ssh) +{ + if (ssh->handshake == NULL) { + return ssh->sendStrictKex; + } + + if (!ssh->handshake->strictKexSet) { + ssh->handshake->strictKex = ssh->sendStrictKex ? 1 : 0; + ssh->handshake->strictKexSet = 1; + } + + return ssh->handshake->strictKex; +} + + static void HandshakeInfoFree(HandshakeInfo* hs, void* heap) { WOLFSSH_UNUSED(heap); @@ -1019,6 +1041,27 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg) * send some of those ids the other way. */ INLINE static int IsMessageAllowed(WOLFSSH *ssh, byte msg, byte state) { + /* Strict KEX (Terrapin mitigation). Nothing in the initial KEX is + * authenticated, so any packet spliced into it shifts the receiver's + * sequence number, and that shift is the attack. Take an allow list + * rather than name the messages to refuse: until the peer's NEWKEYS + * lands, the only things it can legitimately send are DISCONNECT and + * the key exchange itself. IGNORE, DEBUG, UNIMPLEMENTED, EXT_INFO and + * the unassigned transport IDs are all otherwise accepted here, and + * every one of them counts against peerSeq. DISCONNECT stays allowed + * so a peer can still tear the connection down. */ + if (state == WS_MSG_RECV && ssh->strictKexEnabled && + !ssh->initialKexDone) { + if (msg != MSGID_DISCONNECT && msg != MSGID_KEXINIT && + msg != MSGID_NEWKEYS && !MSGIDLIMIT_TRANS_KEX(msg)) { + WLOG(WS_LOG_DEBUG, + "Message ID %u not allowed during the initial strict KEX", + msg); + ssh->error = WS_MSGID_NOT_ALLOWED_E; + return 0; + } + } + #ifndef NO_WOLFSSH_SERVER if (ssh->ctx->side == WOLFSSH_ENDPOINT_SERVER) { return IsMessageAllowedServer(ssh, msg); @@ -1399,6 +1442,7 @@ WOLFSSH_CTX* CtxInit(WOLFSSH_CTX* ctx, byte side, void* heap) ctx->windowSz = DEFAULT_WINDOW_SZ; ctx->maxPacketSz = DEFAULT_MAX_PACKET_SZ; ctx->maxAuthAttempts = DEFAULT_MAX_AUTH_ATTEMPTS; + ctx->sendStrictKex = 1; /* default-enabled, callers can opt out */ ctx->sshProtoIdStr = sshProtoIdStr; ctx->sshProtoIdStrSz = (word32)(sizeof(sshProtoIdStr) - 1); ctx->algoListKex = cannedKexAlgoNames; @@ -1795,6 +1839,7 @@ WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx) ssh->acceptState = ACCEPT_BEGIN; ssh->clientState = CLIENT_BEGIN; ssh->isKeying = 0; /* initial state of not keying yet */ + ssh->sendStrictKex = ctx->sendStrictKex; ssh->authId = ID_USERAUTH_PUBLICKEY; ssh->supportedAuth[0] = ID_USERAUTH_PUBLICKEY; ssh->supportedAuth[1] = ID_USERAUTH_PASSWORD; @@ -3787,8 +3832,16 @@ static const NameIdPair NameIdMap[] = { { ID_CURVE25519_SHA256, TYPE_KEX, "curve25519-sha256" }, { ID_CURVE25519_SHA256_LIBSSH, TYPE_KEX, "curve25519-sha256@libssh.org" }, #endif - { ID_EXTINFO_S, TYPE_OTHER, "ext-info-s" }, - { ID_EXTINFO_C, TYPE_OTHER, "ext-info-c" }, + { ID_EXT_INFO_S, TYPE_OTHER, "ext-info-s" }, + { ID_EXT_INFO_C, TYPE_OTHER, "ext-info-c" }, + /* Strict KEX marker. draft-miller-sshm-strict-kex defines the + * unprefixed names for eventual IETF standardization; deployed + * OpenSSH only sends the -v00@openssh.com ones. Advertise and accept + * both spellings. */ + { ID_EXT_STRICT_KEX_S, TYPE_OTHER, "kex-strict-s" }, + { ID_EXT_STRICT_KEX_C, TYPE_OTHER, "kex-strict-c" }, + { ID_EXT_PRE_STRICT_KEX_S, TYPE_OTHER, "kex-strict-s-v00@openssh.com" }, + { ID_EXT_PRE_STRICT_KEX_C, TYPE_OTHER, "kex-strict-c-v00@openssh.com" }, /* Public Key IDs */ #ifndef WOLFSSH_NO_RSA @@ -6651,12 +6704,45 @@ static int DoKexInit(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) /* Match the peer accepts extInfo. */ algoId = (side == WOLFSSH_ENDPOINT_SERVER) - ? ID_EXTINFO_C : ID_EXTINFO_S; + ? ID_EXT_INFO_C : ID_EXT_INFO_S; extInfo = MatchIdLists(side, list, listSz, &algoId, 1); ssh->sendExtInfo = extInfo == algoId; } } + /* Strict KEX marker (Terrapin mitigation). Only valid in the initial + * KEXINIT (sessionIdSz == 0); if the peer offers it during a rekey, + * ignore the marker per draft-miller-sshm-strict-kex. */ + if (ret == WS_SUCCESS) { + if (ssh->sessionIdSz == 0) { + /* OpenSSH only ever sends the -v00@openssh.com name, so both + * spellings have to be accepted for the marker to negotiate + * against a real peer. */ + byte expectedStrict[2]; + byte matched; + + if (side == WOLFSSH_ENDPOINT_SERVER) { + expectedStrict[0] = ID_EXT_STRICT_KEX_C; + expectedStrict[1] = ID_EXT_PRE_STRICT_KEX_C; + } + else { + expectedStrict[0] = ID_EXT_STRICT_KEX_S; + expectedStrict[1] = ID_EXT_PRE_STRICT_KEX_S; + } + matched = MatchIdLists(side, list, listSz, expectedStrict, 2); + ssh->peerStrictKex = (matched != ID_UNKNOWN); + ssh->strictKexEnabled = + ssh->peerStrictKex && HandshakeStrictKex(ssh); + if (ssh->strictKexEnabled) { + WLOG(WS_LOG_DEBUG, "DKI: strict KEX negotiated"); + } + } + else { + WLOG(WS_LOG_DEBUG, + "DKI: rekey, ignoring any peer strict KEX marker"); + } + } + /* Server Host Key Algorithms */ if (ret == WS_SUCCESS) { WLOG(WS_LOG_DEBUG, "DKI: Server Host Key Algorithms"); @@ -13917,6 +14003,20 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed) msgAllowed = IsMessageAllowed(ssh, msg, WS_MSG_RECV); + if (!msgAllowed && ssh->strictKexEnabled && !ssh->initialKexDone) { + /* Strict KEX calls for terminating the connection, not just + * dropping the packet, so tell the peer why on the way out. Every + * refused id ends it here, the unassigned ones an UNIMPLEMENTED + * would otherwise answer included. The error is relatched because + * the send path can overwrite it. */ + if (!ssh->disconnected) { + (void)SendDisconnect(ssh, + WOLFSSH_DISCONNECT_KEY_EXCHANGE_FAILED); + ssh->error = WS_MSGID_NOT_ALLOWED_E; + } + return WS_MSGID_NOT_ALLOWED_E; + } + if (!msgAllowed && (MsgIdKnown(msg) || MSGIDLIMIT_POST_USERAUTH(msg))) { /* RFC 4252 section 6: disconnect on a known id at the wrong time, * and on any id of 80 or higher, which IsMessageAllowed() refuses @@ -14168,7 +14268,23 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed) idx = len; } ssh->inputBuffer.idx = idx; - ssh->peerSeq++; + if (msg == MSGID_NEWKEYS) { + /* Strict KEX (Terrapin mitigation): once negotiated, every + * SSH_MSG_NEWKEYS resets the incoming sequence number so the + * next inbound packet starts at zero under the new keys. */ + if (ssh->strictKexEnabled) { + ssh->peerSeq = 0; + } + else { + ssh->peerSeq++; + } + /* The peer's initial KEX is over, so IGNORE, DEBUG, and + * UNIMPLEMENTED are legal from it again. */ + ssh->initialKexDone = 1; + } + else { + ssh->peerSeq++; + } ssh->rxMsgCount++; *bufferConsumed = 1; @@ -15260,12 +15376,36 @@ int SendKexInit(WOLFSSH* ssh) } if (ret == WS_SUCCESS) { + /* The strict-kex marker is only advertised during the initial KEX + * (RFC draft-miller-sshm-strict-kex), distinguished here by an + * empty session id. The sendStrictKex flag lets callers opt out + * at runtime; the handshake freezes it so this and the enforcement + * decision in DoKexInit() cannot read different values. */ + int includeStrictKex = + (ssh->sessionIdSz == 0) && HandshakeStrictKex(ssh); + if (ssh->ctx->side == WOLFSSH_ENDPOINT_CLIENT) { - kexAlgoNamesPlus = ",ext-info-c"; + if (includeStrictKex) { + kexAlgoNamesPlus = + ",ext-info-c" + ",kex-strict-c-v00@openssh.com" + ",kex-strict-c"; + } + else { + kexAlgoNamesPlus = ",ext-info-c"; + } kexAlgoNamesPlusSz = (word32)WSTRLEN(kexAlgoNamesPlus); } else { - kexAlgoNamesPlus = ",ext-info-s"; + if (includeStrictKex) { + kexAlgoNamesPlus = + ",ext-info-s" + ",kex-strict-s-v00@openssh.com" + ",kex-strict-s"; + } + else { + kexAlgoNamesPlus = ",ext-info-s"; + } kexAlgoNamesPlusSz = (word32)WSTRLEN(kexAlgoNamesPlus); } @@ -18717,6 +18857,16 @@ int SendNewKeys(WOLFSSH* ssh) ssh->outputBuffer.length = idx; ret = BundlePacket(ssh); + + /* Strict KEX (Terrapin mitigation): once negotiated, every + * SSH_MSG_NEWKEYS resets the outgoing sequence number so the next + * outbound packet starts at zero under the new keys. This has to + * happen here, before SendPendingChannelWindowAdjust() below can + * bundle a packet, or that packet goes out at the old sequence + * number while the peer MACs it at zero. */ + if (ret == WS_SUCCESS && ssh->strictKexEnabled) { + ssh->seq = 0; + } } if (ret == WS_SUCCESS) { diff --git a/src/ssh.c b/src/ssh.c index fa75c4177..6e729eeaa 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -3465,6 +3465,56 @@ int wolfSSH_CheckAlgoName(const char* name) } +int wolfSSH_CTX_SetStrictKex(WOLFSSH_CTX* ctx, byte enable) +{ + int ret = WS_BAD_ARGUMENT; + + if (ctx) { + ctx->sendStrictKex = enable ? 1 : 0; + ret = WS_SUCCESS; + } + + return ret; +} + + +int wolfSSH_CTX_GetStrictKex(WOLFSSH_CTX* ctx) +{ + int ret = WS_BAD_ARGUMENT; + + if (ctx) { + ret = ctx->sendStrictKex ? 1 : 0; + } + + return ret; +} + + +int wolfSSH_SetStrictKex(WOLFSSH* ssh, byte enable) +{ + int ret = WS_SSH_NULL_E; + + if (ssh) { + ssh->sendStrictKex = enable ? 1 : 0; + ret = WS_SUCCESS; + } + + return ret; +} + + +int wolfSSH_GetStrictKex(WOLFSSH* ssh) +{ + int ret = WS_SSH_NULL_E; + + if (ssh) { + ret = ssh->sendStrictKex ? 1 : 0; + } + + return ret; +} + + const char* wolfSSH_QueryKex(word32* idx) { return NameByIndexType(TYPE_KEX, idx); @@ -5699,7 +5749,7 @@ size_t wolfSSH_GetText(WOLFSSH *ssh, WS_Text id, char *str, size_t strSz) break; #endif /* !WOLFSSH_NO_DH */ - case ID_EXTINFO_S: + case ID_EXT_INFO_S: #if defined(__CCRX__) ret = WSNPRINTF0(str, strSz, "Server extensions KEX"); #else @@ -5707,7 +5757,7 @@ size_t wolfSSH_GetText(WOLFSSH *ssh, WS_Text id, char *str, size_t strSz) #endif break; - case ID_EXTINFO_C: + case ID_EXT_INFO_C: #if defined(__CCRX__) ret = WSNPRINTF0(str, strSz, "Client extensions KEX"); #else diff --git a/tests/api.c b/tests/api.c index 916180037..42e27bf05 100644 --- a/tests/api.c +++ b/tests/api.c @@ -7320,6 +7320,69 @@ static void test_wolfSSH_RealPath(void) { ; } #endif +/* Strict KEX (Terrapin mitigation, CVE-2023-48795) is on by default; the + * runtime controls exist so a caller stuck with a broken peer can turn the + * marker off. */ +static void test_wolfSSH_StrictKex(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + + AssertIntEQ(wolfSSH_SetStrictKex(NULL, 1), WS_SSH_NULL_E); + AssertIntEQ(wolfSSH_GetStrictKex(NULL), WS_SSH_NULL_E); + AssertIntEQ(wolfSSH_CTX_SetStrictKex(NULL, 1), WS_BAD_ARGUMENT); + AssertIntEQ(wolfSSH_CTX_GetStrictKex(NULL), WS_BAD_ARGUMENT); + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + + /* Default-enabled: a caller has to ask to be vulnerable. */ + AssertIntEQ(wolfSSH_CTX_GetStrictKex(ctx), 1); + AssertIntEQ(wolfSSH_GetStrictKex(ssh), 1); + + AssertIntEQ(wolfSSH_SetStrictKex(ssh, 0), WS_SUCCESS); + AssertIntEQ(wolfSSH_GetStrictKex(ssh), 0); + + AssertIntEQ(wolfSSH_SetStrictKex(ssh, 1), WS_SUCCESS); + AssertIntEQ(wolfSSH_GetStrictKex(ssh), 1); + + /* Any non-zero enables, and the getter normalizes to 1. */ + AssertIntEQ(wolfSSH_SetStrictKex(ssh, 200), WS_SUCCESS); + AssertIntEQ(wolfSSH_GetStrictKex(ssh), 1); + + /* Nothing has been negotiated yet, so the peer and session flags are + * still clear. */ + AssertIntEQ(ssh->peerStrictKex, 0); + AssertIntEQ(ssh->strictKexEnabled, 0); + AssertIntEQ(ssh->initialKexDone, 0); + + /* The CTX setting seeds the sessions made after it and leaves the ones + * already made alone, so the session above keeps what it was born + * with across the change. */ + AssertIntEQ(wolfSSH_CTX_SetStrictKex(ctx, 0), WS_SUCCESS); + AssertIntEQ(wolfSSH_CTX_GetStrictKex(ctx), 0); + AssertIntEQ(wolfSSH_GetStrictKex(ssh), 1); + + wolfSSH_free(ssh); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AssertIntEQ(wolfSSH_GetStrictKex(ssh), 0); + + /* The session setting overrides it for that session alone. */ + AssertIntEQ(wolfSSH_SetStrictKex(ssh, 1), WS_SUCCESS); + AssertIntEQ(wolfSSH_GetStrictKex(ssh), 1); + AssertIntEQ(wolfSSH_CTX_GetStrictKex(ctx), 0); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + + printf("\tstrict KEX runtime controls.\n"); +} + + static void test_wolfSSH_SetMaxAuthAttempts(void) { WOLFSSH_CTX* ctx; @@ -8361,6 +8424,7 @@ int wolfSSH_ApiTest(int argc, char** argv) test_wolfSSH_ReadKey_sshNoComment(); test_wolfSSH_QueryAlgoList(); test_wolfSSH_SetMaxAuthAttempts(); + test_wolfSSH_StrictKex(); test_wolfSSH_AlgoListKeyInSync(); test_wolfSSH_SetAlgoList(); test_wolfSSH_DefaultAlgoListsExcludeWeak(); diff --git a/tests/regress.c b/tests/regress.c index bb6de33d0..3a57a52be 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -682,6 +682,11 @@ static word32 LoadFileBuffer(const char* path, byte* buf, word32 bufSz) } #endif /* KEXDH_REPLY_REGRESS_KEX_ALGO || WOLFSSH_TEST_INTERNAL */ +/* A transport-layer message id with no meaning assigned to it. Anything in + * 8..19 will do; the receiver has no handler for it and answers with + * UNIMPLEMENTED, having already counted the packet. */ +#define REGRESS_UNASSIGNED_TRANS_MSGID 9 + #ifdef KEXDH_REPLY_REGRESS_KEX_ALGO #define REGRESS_DUPLEX_QUEUE_SZ 32768U @@ -733,6 +738,23 @@ static word32 LoadFileBuffer(const char* path, byte* buf, word32 bufSz) #define REGRESS_MUTATE_E_EMPTY 5 #define REGRESS_MUTATE_GEX_GROUP_SHRINK 6 #define REGRESS_MUTATE_GEX_GEN_BAD 7 +#define REGRESS_MUTATE_TERRAPIN 8 +/* The same splice aimed the other way: the packet goes in ahead of the + * client's NEWKEYS, so the server is the one asked to accept it. */ +#define REGRESS_MUTATE_TERRAPIN_C2S 9 + +/* 4 (len) + 1 (padLen) + 1 (msgId) + 4 (empty string payload) + 6 (pad) */ +#define REGRESS_INJECT_PACKET_SZ 16U + +/* An injected DISCONNECT carries a reason code and two strings, RFC 4253 + * section 11.1, so it decodes and the receiver's answer is the gate's + * doing rather than a short payload's. + * 4 (len) + 1 (padLen) + 1 (msgId) + 4 (reason) + 4 + 4 (empty strings) + * + 6 (pad) */ +#define REGRESS_INJECT_DISCONNECT_SZ 24U + +/* Every injected packet fits in a buffer of this size. */ +#define REGRESS_INJECT_MAX_SZ REGRESS_INJECT_DISCONNECT_SZ typedef struct { byte data[REGRESS_DUPLEX_QUEUE_SZ]; @@ -748,6 +770,9 @@ typedef struct { byte scratch[REGRESS_MUTATION_SCRATCH_SZ]; word32 scratchSz; byte mode; + /* Terrapin bookkeeping */ + byte injectMsgId; + word32 injectedPackets; } KexReplyMutator; typedef struct DuplexEndpoint { @@ -757,6 +782,9 @@ typedef struct DuplexEndpoint { word32 disconnectReason; byte isServer; byte sawDisconnect; + /* What this endpoint put in its own plaintext KEXINIT */ + byte sentStrictKexMarker; + byte sentExtInfoMarker; } DuplexEndpoint; typedef struct { @@ -1186,11 +1214,192 @@ static int RewriteSingleKexDhGexGroupPacket(const byte* packet, } #endif /* REGRESS_GEX_KEX_ALGO */ -/* SIG_*, F_TRUNC and the GEX_* modes rewrite the server's messages; - * E_TRUNC and E_EMPTY the client's init. */ +/* Substring search over a wire buffer, which is not NUL terminated. */ +static int BufferContainsString(const byte* buf, word32 bufSz, const char* str) +{ + word32 strSz = (word32)WSTRLEN(str); + word32 i; + + if (strSz == 0 || bufSz < strSz) { + return 0; + } + + for (i = 0; i + strSz <= bufSz; i++) { + if (WMEMCMP(buf + i, str, strSz) == 0) { + return 1; + } + } + + return 0; +} + +/* Note the pseudo-KEX markers this endpoint advertises. Only the initial + * KEXINIT travels in the clear, which is the one the strict KEX marker is + * allowed on, so a rekey that wrongly re-advertises it is caught by the + * marker never appearing rather than by reading the encrypted packet. */ +static void NoteOutboundKexInitMarkers(DuplexEndpoint* endpoint, + const byte* packet, word32 packetSz) +{ + const char* strictName = endpoint->isServer + ? "kex-strict-s" : "kex-strict-c"; + const char* extInfoName = endpoint->isServer ? "ext-info-s" : "ext-info-c"; + + if (BufferContainsString(packet, packetSz, strictName)) { + endpoint->sentStrictKexMarker = 1; + } + if (BufferContainsString(packet, packetSz, extInfoName)) { + endpoint->sentExtInfoMarker = 1; + } +} + +/* Build the packet a Terrapin attacker splices into the initial KEX. The + * payload is four zero bytes, which reads as an empty string for IGNORE + * (RFC 4253 section 11.2, and DoIgnore() only skips it), as a sequence + * number for UNIMPLEMENTED, and as an empty extension list for EXT_INFO. + * An unassigned id has no handler to read it at all. Sized to a multiple + * of MIN_BLOCK_SZ because the packet travels in the clear. */ +static void BuildInjectedPacketPlain(byte* out, word32 outSz, byte msgId) +{ + const byte padLen = 6; + /* padLen field + msg id + empty string + padding */ + const word32 packetLen = PAD_LENGTH_SZ + MSG_ID_SZ + UINT32_SZ + padLen; + + AssertIntEQ(outSz, REGRESS_INJECT_PACKET_SZ); + AssertIntEQ(UINT32_SZ + packetLen, REGRESS_INJECT_PACKET_SZ); + + (void)AppendUint32(out, outSz, 0, packetLen); + out[UINT32_SZ] = padLen; + out[UINT32_SZ + PAD_LENGTH_SZ] = msgId; + /* empty string payload, then zero padding */ + WMEMSET(out + UINT32_SZ + PAD_LENGTH_SZ + MSG_ID_SZ, 0, + UINT32_SZ + padLen); +} + +/* The DISCONNECT form of the same splice. Strict KEX lets this one through + * on purpose, so unlike the packets above it has to decode: DoDisconnect() + * reads the reason code and both strings before it latches WS_DISCONNECT. */ +static void BuildInjectedDisconnectPlain(byte* out, word32 outSz) +{ + const byte padLen = 6; + /* padLen field + msg id + reason + two empty strings + padding */ + const word32 packetLen = PAD_LENGTH_SZ + MSG_ID_SZ + (3 * UINT32_SZ) + + padLen; + word32 idx; + + AssertIntEQ(outSz, REGRESS_INJECT_DISCONNECT_SZ); + AssertIntEQ(UINT32_SZ + packetLen, REGRESS_INJECT_DISCONNECT_SZ); + + idx = AppendUint32(out, outSz, 0, packetLen); + out[idx++] = padLen; + out[idx++] = MSGID_DISCONNECT; + idx = AppendUint32(out, outSz, idx, WOLFSSH_DISCONNECT_BY_APPLICATION); + /* empty description and language strings, then zero padding */ + WMEMSET(out + idx, 0, outSz - idx); +} + +/* Lay down the packet to splice in, reporting how long it came out. */ +static word32 BuildInjectedPacket(byte* out, word32 outSz, byte msgId) +{ + AssertIntEQ(outSz, REGRESS_INJECT_MAX_SZ); + + if (msgId == MSGID_DISCONNECT) { + BuildInjectedDisconnectPlain(out, REGRESS_INJECT_DISCONNECT_SZ); + return REGRESS_INJECT_DISCONNECT_SZ; + } + + BuildInjectedPacketPlain(out, REGRESS_INJECT_PACKET_SZ, msgId); + return REGRESS_INJECT_PACKET_SZ; +} + +/* Walk a plaintext write looking for the packet carrying msgId, reporting + * the offset it starts at. Only sound before NEWKEYS, where the framing is + * readable; the Terrapin splice point is by definition in that window. */ +static int FindPlainPacketOffset(const byte* packet, word32 packetSz, + byte msgId, word32* offsetOut) +{ + word32 offset = 0; + + while (packetSz - offset >= UINT32_SZ + PAD_LENGTH_SZ + MSG_ID_SZ) { + word32 curPacketSz = ReadUint32(packet + offset) + UINT32_SZ; + + if (curPacketSz > packetSz - offset || + curPacketSz < UINT32_SZ + PAD_LENGTH_SZ + MSG_ID_SZ + + MIN_PAD_LENGTH) { + return 0; + } + + if (packet[offset + UINT32_SZ + PAD_LENGTH_SZ] == msgId) { + *offsetOut = offset; + return 1; + } + + offset += curPacketSz; + } + + return 0; +} + +/* The Terrapin prefix-truncation attack (CVE-2023-48795). Nothing in the + * initial KEX is authenticated, so a man in the middle can splice an extra + * SSH_MSG_IGNORE in ahead of the victim's NEWKEYS. The victim accepts it, + * and from then on its inbound sequence number runs one ahead of what the + * peer is MACing against. + * + * The published attack pairs that with deleting a packet after NEWKEYS to + * put the counters back in step, silently truncating the stream. That step + * only works when the cipher takes its nonce from the sequence number -- + * chacha20-poly1305@openssh.com or the *-etm@openssh.com MACs. wolfSSH + * implements neither: its AES-GCM, CTR, and CBC modes all carry chained + * cipher state, so a deletion breaks decryption outright. The injection + * alone is what these tests exercise, and refusing it is what the + * mitigation has to do. */ +static void TerrapinInjectBeforeNewKeys(DuplexEndpoint* endpoint, + const byte** output, word32* outputSz) +{ + KexReplyMutator* mutator = endpoint->mutator; + byte injectPkt[REGRESS_INJECT_MAX_SZ]; + word32 injectPktSz; + word32 newKeysOffset; + + if (mutator->injectedPackets > 0) { + return; + } + if (!FindPlainPacketOffset(*output, *outputSz, MSGID_NEWKEYS, + &newKeysOffset)) { + return; + } + + injectPktSz = BuildInjectedPacket(injectPkt, (word32)sizeof(injectPkt), + mutator->injectMsgId); + + /* Everything ahead of NEWKEYS passes through untouched, then the forged + * packet; the caller forwards the NEWKEYS tail. */ + if (newKeysOffset > 0) { + AssertIntEQ(QueueAppend(&endpoint->peer->inbound, *output, + newKeysOffset), WS_SUCCESS); + } + AssertIntEQ(QueueAppend(&endpoint->peer->inbound, injectPkt, + injectPktSz), WS_SUCCESS); + + mutator->injectedPackets++; + *output += newKeysOffset; + *outputSz -= newKeysOffset; +} + +/* Both splice a packet into the initial KEX, they only differ in which + * side's NEWKEYS they go in ahead of. */ +static int IsTerrapinMode(byte mode) +{ + return mode == REGRESS_MUTATE_TERRAPIN || + mode == REGRESS_MUTATE_TERRAPIN_C2S; +} + +/* SIG_*, F_TRUNC, TERRAPIN and the GEX_* modes rewrite the server's + * messages; E_TRUNC, E_EMPTY and TERRAPIN_C2S the client's. */ static int MutatorTargetsEndpoint(byte mode, byte isServer) { - if (mode == REGRESS_MUTATE_E_TRUNC || mode == REGRESS_MUTATE_E_EMPTY) { + if (mode == REGRESS_MUTATE_E_TRUNC || mode == REGRESS_MUTATE_E_EMPTY || + mode == REGRESS_MUTATE_TERRAPIN_C2S) { return !isServer; } return isServer != 0; @@ -1276,6 +1485,18 @@ static int DuplexSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) if (endpoint->mutator != NULL && endpoint->mutator->enabled && + IsTerrapinMode(endpoint->mutator->mode) && + MutatorTargetsEndpoint(endpoint->mutator->mode, + endpoint->isServer) && + !(outputSz >= REGRESS_SSH_PROTO_PREFIX_SZ && + WMEMCMP(output, REGRESS_SSH_PROTO_PREFIX, + REGRESS_SSH_PROTO_PREFIX_SZ) == 0)) { + TerrapinInjectBeforeNewKeys(endpoint, &output, &outputSz); + } + + if (endpoint->mutator != NULL && + endpoint->mutator->enabled && + !IsTerrapinMode(endpoint->mutator->mode) && MutatorTargetsEndpoint(endpoint->mutator->mode, endpoint->isServer) && endpoint->mutator->mutatedPackets == 0 && @@ -1329,6 +1550,7 @@ static int DuplexSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) WMEMCMP(output, REGRESS_SSH_PROTO_PREFIX, REGRESS_SSH_PROTO_PREFIX_SZ) == 0)) { NoteOutboundDisconnect(endpoint, output, outputSz); + NoteOutboundKexInitMarkers(endpoint, output, outputSz); } ret = QueueAppend(&endpoint->peer->inbound, output, outputSz); @@ -2100,6 +2322,495 @@ static void TestKexDhGexGroupBadGeneratorSendsDisconnect(void) } #endif /* REGRESS_GEX_KEX_ALGO */ +/* ---- Strict KEX, the Terrapin mitigation (CVE-2023-48795) ---- */ + +/* KEXINIT, KEXDH_INIT/KEXDH_REPLY, NEWKEYS. Each side sends exactly three + * packets before its NEWKEYS, so strict KEX zeroing the counters there + * leaves both of them three short of an unmitigated run. */ +#define REGRESS_PRE_NEWKEYS_PACKETS 3 + +/* injectMsgId is the message a man in the middle splices in ahead of the + * NEWKEYS the mode names, or MSGID_NONE for a clean run. */ +static void InitStrictKexHarnessMode(KexReplyHarness* harness, + byte injectMsgId, byte clientStrict, byte serverStrict, byte mode) +{ + InitKexReplyHarnessEx(harness, REGRESS_DEFAULT_KEY_ALGO, + REGRESS_DEFAULT_KEY_PATH, injectMsgId != MSGID_NONE, + mode, NULL, 0); + harness->mutator.injectMsgId = injectMsgId; + AssertIntEQ(wolfSSH_SetStrictKex(harness->client, clientStrict), + WS_SUCCESS); + AssertIntEQ(wolfSSH_SetStrictKex(harness->server, serverStrict), + WS_SUCCESS); +} + +/* The injection the client has to answer: spliced in ahead of the server's + * NEWKEYS. */ +static void InitStrictKexHarness(KexReplyHarness* harness, byte injectMsgId, + byte clientStrict, byte serverStrict) +{ + InitStrictKexHarnessMode(harness, injectMsgId, clientStrict, serverStrict, + REGRESS_MUTATE_TERRAPIN); +} + +/* Two wolfSSH peers with the default settings must come out of the initial + * KEX with strict KEX on at both ends. */ +static void TestStrictKexNegotiatedByDefault(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarness(&harness, MSGID_NONE, 1, 1); + AssertIntEQ(wolfSSH_GetStrictKex(harness.client), 1); + AssertIntEQ(wolfSSH_GetStrictKex(harness.server), 1); + + RunKexReplyHandshake(&harness, &result); + + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + AssertIntEQ(harness.client->peerStrictKex, 1); + AssertIntEQ(harness.client->strictKexEnabled, 1); + AssertIntEQ(harness.server->peerStrictKex, 1); + AssertIntEQ(harness.server->strictKexEnabled, 1); + + /* Both sides saw the peer's NEWKEYS, so the initial-KEX message + * restriction has lifted. */ + AssertIntEQ(harness.client->initialKexDone, 1); + AssertIntEQ(harness.server->initialKexDone, 1); + + /* Both markers really went out on the initial KEXINIT. */ + AssertIntEQ(harness.clientIo.sentStrictKexMarker, 1); + AssertIntEQ(harness.serverIo.sentStrictKexMarker, 1); + + /* Control for the Terrapin tests below: an unattacked client does + * receive the server's EXT_INFO. */ + AssertTrue(harness.client->peerSigIdSz > 0); + + FreeKexReplyHarness(&harness); +} + +/* The marker rides on the KEXINIT algorithm list, so a peer that opts out + * never advertises it and neither side turns the mitigation on. The + * handshake still has to complete -- an old peer must stay interoperable. */ +static void AssertStrictKexOptOut(byte clientStrict, byte serverStrict) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarness(&harness, MSGID_NONE, clientStrict, serverStrict); + RunKexReplyHandshake(&harness, &result); + + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + + /* The opted-out side left the marker off its KEXINIT, and the peer + * agrees it never arrived. */ + AssertIntEQ(harness.clientIo.sentStrictKexMarker, clientStrict); + AssertIntEQ(harness.serverIo.sentStrictKexMarker, serverStrict); + AssertIntEQ(harness.client->peerStrictKex, serverStrict); + AssertIntEQ(harness.server->peerStrictKex, clientStrict); + AssertIntEQ(harness.client->strictKexEnabled, 0); + AssertIntEQ(harness.server->strictKexEnabled, 0); + + /* Only the strict KEX marker is conditional; ext-info still goes out. */ + AssertIntEQ(harness.clientIo.sentExtInfoMarker, 1); + AssertIntEQ(harness.serverIo.sentExtInfoMarker, 1); + + FreeKexReplyHarness(&harness); +} + +static void TestStrictKexClientOptOut(void) +{ + AssertStrictKexOptOut(0, 1); +} + +static void TestStrictKexServerOptOut(void) +{ + AssertStrictKexOptOut(1, 0); +} + +/* A handshake freezes the strict KEX setting when it starts, so the + * decision to advertise the marker and the decision to enforce it cannot + * read different values. + * + * The server is where the two can be prised apart: its KEXINIT goes out on + * the version exchange, in a different wolfSSH_accept() call from the one + * that reads the client's KEXINIT and decides whether to enforce. Turn the + * setting off in between. Without the freeze the server would advertise a + * mitigation it then declined to apply, leaving it counting sequence + * numbers its peer had already zeroed. */ +static void TestStrictKexSettingFrozenForHandshake(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + word32 step; + + InitStrictKexHarness(&harness, MSGID_NONE, 1, 1); + + /* The client's version string, which is all the server needs to answer + * with its own version and its KEXINIT. */ + (void)wolfSSH_connect(harness.client); + + /* Drive the server alone: the client sends its KEXINIT on its next + * call, so the server cannot reach DoKexInit() while this runs. */ + for (step = 0; step < REGRESS_MAX_HANDSHAKE_STEPS && + !harness.serverIo.sentStrictKexMarker; step++) { + (void)wolfSSH_accept(harness.server); + } + AssertIntEQ(harness.serverIo.sentStrictKexMarker, 1); + + /* Advertised, but nothing has been negotiated yet. */ + AssertIntEQ(harness.server->peerStrictKex, 0); + AssertIntEQ(harness.server->strictKexEnabled, 0); + + /* The caller changes its mind with the handshake already running. */ + AssertIntEQ(wolfSSH_SetStrictKex(harness.server, 0), WS_SUCCESS); + AssertIntEQ(wolfSSH_GetStrictKex(harness.server), 0); + + RunKexReplyHandshake(&harness, &result); + + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + + /* Both ends came out of it mitigated, on the value the handshake + * started with rather than the one the session now holds. */ + AssertIntEQ(harness.server->strictKexEnabled, 1); + AssertIntEQ(harness.client->strictKexEnabled, 1); + AssertIntEQ(wolfSSH_GetStrictKex(harness.server), 0); + + FreeKexReplyHarness(&harness); +} + +/* draft-miller-sshm-strict-kex puts the marker in the first KEXINIT only. + * A rekey KEXINIT is encrypted, so rather than read it off the wire, force + * SendKexInit down its rekey path -- a session id is already established -- + * and check the marker never appears in the clear. The handshake itself is + * expected to fall over on the planted session id; only what went out + * before that matters. */ +static void TestStrictKexMarkerNotSentOnRekey(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarness(&harness, MSGID_NONE, 1, 1); + + /* A non-empty session id is what SendKexInit reads as "this is a + * rekey", the same test DoKexInit makes on the receiving side. */ + WMEMSET(harness.client->sessionId, 0x5A, WC_SHA256_DIGEST_SIZE); + harness.client->sessionIdSz = WC_SHA256_DIGEST_SIZE; + + RunKexReplyHandshake(&harness, &result); + + /* The client did send a KEXINIT, and it carried ext-info-c but not the + * strict KEX marker. */ + AssertIntEQ(harness.clientIo.sentExtInfoMarker, 1); + AssertIntEQ(harness.clientIo.sentStrictKexMarker, 0); + + /* The server saw no marker either, so it cannot have turned the + * mitigation on off the back of a rekey. */ + AssertIntEQ(harness.server->peerStrictKex, 0); + AssertIntEQ(harness.server->strictKexEnabled, 0); + + FreeKexReplyHarness(&harness); +} + +/* NEWKEYS must zero both sequence numbers once strict KEX is negotiated. + * Run the same handshake with and without it and compare the counters: the + * mitigated run has to be short by exactly the packets each side sent + * before its NEWKEYS. */ +static void TestStrictKexResetsSequenceNumbers(void) +{ + KexReplyHarness strict; + KexReplyHarness plain; + KexReplyRunResult result; + + InitStrictKexHarness(&strict, MSGID_NONE, 1, 1); + RunKexReplyHandshake(&strict, &result); + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + AssertIntEQ(strict.client->strictKexEnabled, 1); + + InitStrictKexHarness(&plain, MSGID_NONE, 0, 0); + RunKexReplyHandshake(&plain, &result); + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + AssertIntEQ(plain.client->strictKexEnabled, 0); + + AssertIntEQ(strict.client->seq + REGRESS_PRE_NEWKEYS_PACKETS, + plain.client->seq); + AssertIntEQ(strict.client->peerSeq + REGRESS_PRE_NEWKEYS_PACKETS, + plain.client->peerSeq); + AssertIntEQ(strict.server->seq + REGRESS_PRE_NEWKEYS_PACKETS, + plain.server->seq); + AssertIntEQ(strict.server->peerSeq + REGRESS_PRE_NEWKEYS_PACKETS, + plain.server->peerSeq); + + FreeKexReplyHarness(&plain); + FreeKexReplyHarness(&strict); +} + +/* The injection, with the mitigation off. Nothing rejects the forged + * SSH_MSG_IGNORE: the client processes it, accepts the server's NEWKEYS, + * and finishes the handshake with its inbound counter one ahead of what + * the server is sending against. Nothing notices, now or later -- the + * harness negotiates aes256-gcm@openssh.com, and CreateMac()/VerifyMac() + * are reached only on the non-AEAD paths, so under an AEAD cipher the + * sequence number is not an authentication input at all. The counters + * simply stay apart. That undetected shift is what Terrapin is built on. */ +static void TestTerrapinInjectionAcceptedWithoutStrictKex(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarness(&harness, MSGID_IGNORE, 0, 0); + RunKexReplyHandshake(&harness, &result); + + AssertIntEQ(harness.mutator.injectedPackets, 1); + AssertIntEQ(harness.mutator.parseError, 0); + AssertIntEQ(harness.client->strictKexEnabled, 0); + + /* The client accepted the injected packet and kept going all the way + * through the server's NEWKEYS. Nothing in the KEX objected, and the + * handshake came up. */ + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + AssertIntEQ(harness.client->initialKexDone, 1); + AssertTrue(harness.client->error != WS_MSGID_NOT_ALLOWED_E); + + /* The shift: the client has counted one more inbound packet than the + * server has sent. */ + AssertIntEQ(harness.client->peerSeq, harness.server->seq + 1); + + FreeKexReplyHarness(&harness); +} + +/* The same injection with strict KEX negotiated, for each message a man in + * the middle could reach for. All of them are refused for as long as the + * initial KEX is running, so the sequence number is never shifted and the + * connection dies loudly instead. */ +static void AssertTerrapinInjectionRejected(byte injectMsgId) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarness(&harness, injectMsgId, 1, 1); + RunKexReplyHandshake(&harness, &result); + + AssertIntEQ(harness.mutator.injectedPackets, 1); + AssertIntEQ(harness.mutator.parseError, 0); + AssertIntEQ(harness.client->strictKexEnabled, 1); + + AssertFalse(result.clientSuccess); + AssertIntEQ(result.clientErr, WS_MSGID_NOT_ALLOWED_E); + + /* Rejected where it was injected: ahead of the server's NEWKEYS, so + * the client counted only the server's KEXINIT and KEXDH_REPLY and its + * sequence number was never shifted. */ + AssertIntEQ(harness.client->initialKexDone, 0); + AssertIntEQ(harness.client->peerSeq, 2); + + /* Strict KEX calls for ending the connection, not just dropping the + * packet, so the client tells the server why before it goes. */ + AssertIntEQ(harness.client->disconnected, 1); + + /* The runner stops as soon as the client fails, so give the server one + * more turn to read what the client sent on its way out. */ + (void)wolfSSH_accept(harness.server); + AssertIntEQ(harness.server->disconnected, 1); + + /* The server decoded it as a DISCONNECT rather than as a broken + * packet, which is as far as this can be checked from here: the client + * sent its own NEWKEYS before it read the forgery, so the message goes + * out encrypted and the plaintext sniffer never sees the reason code + * itself. */ + AssertIntEQ(harness.server->error, WS_DISCONNECT); + + FreeKexReplyHarness(&harness); +} + +static void TestTerrapinInjectionRejectedWithStrictKex(void) +{ + /* IGNORE, DEBUG and UNIMPLEMENTED are the messages the attack is + * written up with. EXT_INFO and an unassigned transport id are here + * because the receiver used to take those during the initial KEX just + * as readily, and each one buys the attacker the same step of the + * sequence number. */ + static const byte injectable[] = { + MSGID_IGNORE, MSGID_DEBUG, MSGID_UNIMPLEMENTED, MSGID_EXT_INFO, + REGRESS_UNASSIGNED_TRANS_MSGID + }; + word32 i; + + for (i = 0; i < (word32)(sizeof(injectable) / sizeof(injectable[0])); i++) { + AssertTerrapinInjectionRejected(injectable[i]); + } +} + +/* The same splice aimed at the server. The attack does not care which end + * it desynchronizes, and the two sides run the gate off their own state, + * so the server has to refuse an injected message just as the client does. + * The client's NEWKEYS is the splice point here, by which time the server + * has counted the client's KEXINIT and KEXDH_INIT and nothing else. */ +static void TestTerrapinInjectionRejectedByServer(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarnessMode(&harness, MSGID_IGNORE, 1, 1, + REGRESS_MUTATE_TERRAPIN_C2S); + RunKexReplyHandshake(&harness, &result); + + AssertIntEQ(harness.mutator.injectedPackets, 1); + AssertIntEQ(harness.mutator.parseError, 0); + AssertIntEQ(harness.server->strictKexEnabled, 1); + + AssertFalse(result.serverSuccess); + AssertIntEQ(result.serverErr, WS_MSGID_NOT_ALLOWED_E); + + AssertIntEQ(harness.server->initialKexDone, 0); + AssertIntEQ(harness.server->peerSeq, 2); + AssertIntEQ(harness.server->disconnected, 1); + + FreeKexReplyHarness(&harness); +} + +/* DISCONNECT is the one message the gate lets through mid-KEX, so that a + * peer can still give up on the exchange. The client takes the spliced-in + * one and ends the session on it: the error is the peer's disconnect, not + * the gate's refusal, and the packet is counted because it was accepted. */ +static void TestStrictKexTakesInjectedDisconnect(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarness(&harness, MSGID_DISCONNECT, 1, 1); + RunKexReplyHandshake(&harness, &result); + + AssertIntEQ(harness.mutator.injectedPackets, 1); + AssertIntEQ(harness.mutator.parseError, 0); + AssertIntEQ(harness.client->strictKexEnabled, 1); + + AssertFalse(result.clientSuccess); + AssertIntEQ(result.clientErr, WS_DISCONNECT); + AssertIntEQ(harness.client->disconnected, 1); + + /* The server's KEXINIT and KEXDH_REPLY, then the forgery. */ + AssertIntEQ(harness.client->peerSeq, 3); + + /* It landed ahead of the server's NEWKEYS, so the initial KEX never + * finished and the gate was still up when the message went through. */ + AssertIntEQ(harness.client->initialKexDone, 0); + + FreeKexReplyHarness(&harness); +} + +/* What a worker call is allowed to come back with while a rekey runs: an + * empty queue, a packet handed to the application, or the rekey itself + * still in progress. Anything else is a real failure. */ +static int IsPumpRetryable(int err) +{ + return IsHandshakeRetryable(err) || err == WS_CHAN_RXD || + err == WS_REKEYING; +} + +/* Run both ends until their queues are empty and neither is keying. Only + * sound once the handshake is done, where the only traffic left is what + * the test asked for. */ +static void PumpDuplexPair(KexReplyHarness* harness) +{ + word32 step; + + for (step = 0; step < REGRESS_MAX_HANDSHAKE_STEPS; step++) { + int ret; + + ret = wolfSSH_worker(harness->client, NULL); + if (ret < WS_SUCCESS) { + int err = wolfSSH_get_error(harness->client); + AssertTrue(IsPumpRetryable(err)); + } + + ret = wolfSSH_worker(harness->server, NULL); + if (ret < WS_SUCCESS) { + int err = wolfSSH_get_error(harness->server); + AssertTrue(IsPumpRetryable(err)); + } + + if (harness->clientIo.inbound.len == 0 && + harness->serverIo.inbound.len == 0 && + harness->client->isKeying == 0 && + harness->server->isKeying == 0) { + return; + } + } + + Fail(("the rekey to finish"), ("it ran out of steps")); +} + +/* draft-miller-sshm-strict-kex resets the sequence numbers at every + * NEWKEYS, not just the initial one. Establish a session, note where the + * counters have got to, then rekey: with the mitigation on they fall back + * to count only what followed the new NEWKEYS, and with it off they carry + * on climbing. A counter only ever goes up on its own, so a smaller number + * after the rekey is the reset and nothing else. */ +static void AssertRekeyResetsSequenceNumbers(byte strict) +{ + KexReplyHarness harness; + KexReplyRunResult result; + word32 clientSeq, clientPeerSeq, serverSeq, serverPeerSeq; + + InitStrictKexHarness(&harness, MSGID_NONE, strict, strict); + RunKexReplyHandshake(&harness, &result); + + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + AssertIntEQ(harness.client->strictKexEnabled, strict); + AssertIntEQ(harness.server->strictKexEnabled, strict); + + clientSeq = harness.client->seq; + clientPeerSeq = harness.client->peerSeq; + serverSeq = harness.server->seq; + serverPeerSeq = harness.server->peerSeq; + + /* Userauth and the session channel put every counter well clear of + * zero, so a reset is visible as a drop. */ + AssertTrue(clientSeq > REGRESS_PRE_NEWKEYS_PACKETS); + AssertTrue(clientPeerSeq > REGRESS_PRE_NEWKEYS_PACKETS); + AssertTrue(serverSeq > REGRESS_PRE_NEWKEYS_PACKETS); + AssertTrue(serverPeerSeq > REGRESS_PRE_NEWKEYS_PACKETS); + + AssertIntEQ(wolfSSH_TriggerKeyExchange(harness.client), WS_SUCCESS); + PumpDuplexPair(&harness); + + /* The rekey ran to the end: both sides are done keying and neither + * fell over on a packet it could not authenticate. */ + AssertIntEQ(harness.client->isKeying, 0); + AssertIntEQ(harness.server->isKeying, 0); + AssertIntEQ(harness.client->disconnected, 0); + AssertIntEQ(harness.server->disconnected, 0); + + if (strict) { + AssertTrue(harness.client->seq < clientSeq); + AssertTrue(harness.client->peerSeq < clientPeerSeq); + AssertTrue(harness.server->seq < serverSeq); + AssertTrue(harness.server->peerSeq < serverPeerSeq); + } + else { + AssertTrue(harness.client->seq > clientSeq); + AssertTrue(harness.client->peerSeq > clientPeerSeq); + AssertTrue(harness.server->seq > serverSeq); + AssertTrue(harness.server->peerSeq > serverPeerSeq); + } + + FreeKexReplyHarness(&harness); +} + +static void TestStrictKexResetsSequenceNumbersOnRekey(void) +{ + AssertRekeyResetsSequenceNumbers(1); + /* The control: the same rekey with the mitigation off. */ + AssertRekeyResetsSequenceNumbers(0); +} + #endif /* KEXDH_REPLY_REGRESS_KEX_ALGO */ /* Shared with the client-side forwarding tests below. */ @@ -2491,6 +3202,102 @@ static void TestAuthMessageBlockedDuringKeying(WOLFSSH* ssh) AssertIntEQ(ssh->handshake->expectMsgId, MSGID_NONE); } +/* Strict KEX gates the messages an attacker can inject for free during the + * initial KEX. They are otherwise legal at any time, which is exactly what + * makes them useful for shifting the peer's sequence number + * (CVE-2023-48795). */ +static void TestStrictKexBlocksInjectableMessages(WOLFSSH* ssh) +{ + /* Every message id the receiver would otherwise take during the initial + * KEX. IGNORE, DEBUG and UNIMPLEMENTED are the ones the attack is + * usually described with, but EXT_INFO and the unassigned transport ids + * are just as good: MSGIDLIMIT_TRANS_GEN() covers 1 through 19 and used + * to wave all of them through, and any one of them steps peerSeq. */ + static const byte injectable[] = { + MSGID_IGNORE, MSGID_DEBUG, MSGID_UNIMPLEMENTED, MSGID_EXT_INFO, + REGRESS_UNASSIGNED_TRANS_MSGID, MSGIDLIMIT_TRANS_GEN_MAX + }; + word32 i; + + for (i = 0; i < (word32)(sizeof(injectable) / sizeof(injectable[0])); i++) { + byte msg = injectable[i]; + + /* Baseline: allowed with the mitigation off, so the rejections + * below are the strict KEX gate and not some other state check. */ + ResetSession(ssh); + ssh->strictKexEnabled = 0; + ssh->initialKexDone = 0; + AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, msg, WS_MSG_RECV)); + AssertIntEQ(ssh->error, 0); + + /* Negotiated, peer's NEWKEYS not yet seen: refused. */ + ResetSession(ssh); + ssh->strictKexEnabled = 1; + ssh->initialKexDone = 0; + AssertFalse(wolfSSH_TestIsMessageAllowed(ssh, msg, WS_MSG_RECV)); + AssertIntEQ(ssh->error, WS_MSGID_NOT_ALLOWED_E); + + /* The gate is on the receive path only; sending one is our own + * business and does not desynchronize anything. */ + ResetSession(ssh); + ssh->strictKexEnabled = 1; + ssh->initialKexDone = 0; + AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, msg, WS_MSG_SEND)); + + /* Peer's NEWKEYS arrived, so the restriction lifts. */ + ResetSession(ssh); + ssh->strictKexEnabled = 1; + ssh->initialKexDone = 1; + AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, msg, WS_MSG_RECV)); + AssertIntEQ(ssh->error, 0); + } + + ResetSession(ssh); + ssh->strictKexEnabled = 0; + ssh->initialKexDone = 0; +} + +/* A peer has to be able to tear the connection down mid-KEX, so DISCONNECT + * stays allowed even while the injectable messages are refused. */ +static void TestStrictKexAllowsDisconnectDuringInitialKex(WOLFSSH* ssh) +{ + ResetSession(ssh); + ssh->strictKexEnabled = 1; + ssh->initialKexDone = 0; + + AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, MSGID_DISCONNECT, + WS_MSG_RECV)); + AssertIntEQ(ssh->error, 0); + + /* The KEX itself must still run, too. NEWKEYS is what lifts the gate, + * and the algorithm-specific exchange in between has to reach its + * handler or nothing ever completes. */ + AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, MSGID_KEXINIT, WS_MSG_RECV)); + AssertIntEQ(ssh->error, 0); + + { + static const byte kexMsgs[] = { + MSGID_KEXDH_REPLY, MSGID_KEXDH_GEX_REPLY, MSGID_NEWKEYS + }; + word32 i; + + for (i = 0; i < (word32)(sizeof(kexMsgs) / sizeof(kexMsgs[0])); i++) { + ResetSession(ssh); + ssh->strictKexEnabled = 1; + ssh->initialKexDone = 0; + ssh->isKeying = WOLFSSH_PEER_IS_KEYING; + ssh->handshake = AllocHandshake(ssh); + ssh->handshake->expectMsgId = kexMsgs[i]; + AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, kexMsgs[i], + WS_MSG_RECV)); + AssertIntEQ(ssh->error, 0); + } + } + + ResetSession(ssh); + ssh->strictKexEnabled = 0; +} + /* Reject USERAUTH_FAILURE with password list during keying (password-leak PoC). */ static void TestUserauthFailureDuringKeying(WOLFSSH* ssh) { @@ -13731,15 +14538,13 @@ static void TestKeyboardResponseNullCtx(WOLFSSH* ssh) #endif /* WOLFSSH_KEYBOARD_INTERACTIVE */ -#if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) \ - && !defined(WOLFSSH_NO_RSA) \ - && !defined(WOLFSSH_NO_CURVE25519_SHA256) \ - && !defined(WOLFSSH_NO_RSA_SHA2_256) +/* KEXINIT payload builders. Used by the first_packet_follows coverage and by + * the strict KEX marker cases below, so they are guarded by what they need + * rather than by either caller's narrower conditions. */ +#if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_RSA) -#define FPF_KEX_GOOD "ecdh-sha2-nistp256" -#define FPF_KEX_BAD "curve25519-sha256" -#define FPF_KEY_GOOD "ssh-rsa" -#define FPF_KEY_BAD "rsa-sha2-256" +#define REGRESS_KEXINIT_KEX_ALGO "ecdh-sha2-nistp256" +#define REGRESS_KEXINIT_KEY_ALGO "ssh-rsa" /* AppendString for one of the library's own canned algorithm lists. Those are * built by concatenating "name," fragments, so they carry a trailing comma @@ -13788,6 +14593,18 @@ static word32 BuildKexInitPayload(WOLFSSH* ssh, const char* kexList, return idx; } +#endif /* KEXINIT payload builder guard */ + +#if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) \ + && !defined(WOLFSSH_NO_RSA) \ + && !defined(WOLFSSH_NO_CURVE25519_SHA256) \ + && !defined(WOLFSSH_NO_RSA_SHA2_256) + +#define FPF_KEX_GOOD "ecdh-sha2-nistp256" +#define FPF_KEX_BAD "curve25519-sha256" +#define FPF_KEY_GOOD "ssh-rsa" +#define FPF_KEY_BAD "rsa-sha2-256" + #if !defined(WOLFSSH_NO_AES_CBC) && !defined(WOLFSSH_NO_AES_CTR) \ && !defined(WOLFSSH_NO_HMAC_SHA1) && !defined(WOLFSSH_NO_HMAC_SHA2_256) /* Like BuildKexInitPayload but with explicit per-direction cipher/MAC lists. */ @@ -15592,8 +16409,180 @@ static void TestDoKexInitRejectsWhenPeerIsKeying(void) wolfSSH_CTX_free(ctx); } + #endif /* first_packet_follows coverage guard */ +#if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_RSA) + +/* ---- Strict KEX marker negotiation (CVE-2023-48795) ---- */ + +#define SK_OPENSSH_S "kex-strict-s-v00@openssh.com" +#define SK_OPENSSH_C "kex-strict-c-v00@openssh.com" +#define SK_DRAFT_S "kex-strict-s" +#define SK_DRAFT_C "kex-strict-c" + +typedef struct { + const char* description; + const char* peerKexList; /* what the peer put in its KEXINIT */ + byte side; /* endpoint under test */ + byte sendStrictKex; /* local opt-in */ + byte rekey; /* pretend a session id is already established */ + byte expectPeerStrictKex; + byte expectStrictKexEnabled; +} StrictKexMarkerCase; + +static const StrictKexMarkerCase strictKexMarkerCases[] = { + /* A peer that says nothing leaves the mitigation off. */ + { "no marker", + REGRESS_KEXINIT_KEX_ALGO, WOLFSSH_ENDPOINT_SERVER, 1, 0, 0, 0 }, + + /* Deployed OpenSSH only ever sends the -v00@openssh.com spelling, so + * accepting it is what makes the mitigation work against a real peer. */ + { "openssh marker only", + REGRESS_KEXINIT_KEX_ALGO "," SK_OPENSSH_C, + WOLFSSH_ENDPOINT_SERVER, 1, 0, 1, 1 }, + + /* draft-miller-sshm-strict-kex names it without the vendor suffix. */ + { "draft marker only", + REGRESS_KEXINIT_KEX_ALGO "," SK_DRAFT_C, + WOLFSSH_ENDPOINT_SERVER, 1, 0, 1, 1 }, + + { "both spellings", + REGRESS_KEXINIT_KEX_ALGO "," SK_OPENSSH_C "," SK_DRAFT_C, + WOLFSSH_ENDPOINT_SERVER, 1, 0, 1, 1 }, + + /* The marker is directional. A server must ignore the server-side + * names, which are its own to send, not the client's. */ + { "server-side names from a client", + REGRESS_KEXINIT_KEX_ALGO "," SK_OPENSSH_S "," SK_DRAFT_S, + WOLFSSH_ENDPOINT_SERVER, 1, 0, 0, 0 }, + + /* Both sides have to ask for it. */ + { "peer asks, local opted out", + REGRESS_KEXINIT_KEX_ALGO "," SK_OPENSSH_C, + WOLFSSH_ENDPOINT_SERVER, 0, 0, 1, 0 }, + + /* draft-miller-sshm-strict-kex: the marker is meaningful only in the + * first KEXINIT, and a rekey carrying it must not turn anything on. */ + { "marker in a rekey KEXINIT", + REGRESS_KEXINIT_KEX_ALGO "," SK_OPENSSH_C "," SK_DRAFT_C, + WOLFSSH_ENDPOINT_SERVER, 1, 1, 0, 0 }, + + /* The client side of the same negotiation. */ + { "client sees openssh marker", + REGRESS_KEXINIT_KEX_ALGO "," SK_OPENSSH_S, + WOLFSSH_ENDPOINT_CLIENT, 1, 0, 1, 1 }, + { "client sees draft marker", + REGRESS_KEXINIT_KEX_ALGO "," SK_DRAFT_S, + WOLFSSH_ENDPOINT_CLIENT, 1, 0, 1, 1 }, + { "client-side names from a server", + REGRESS_KEXINIT_KEX_ALGO "," SK_OPENSSH_C "," SK_DRAFT_C, + WOLFSSH_ENDPOINT_CLIENT, 1, 0, 0, 0 }, +}; + +static void RunStrictKexMarkerCase(const StrictKexMarkerCase* tc) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + byte payload[768]; + word32 payloadSz; + word32 idx = 0; + + ctx = wolfSSH_CTX_new(tc->side, NULL); + AssertNotNull(ctx); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + + AssertIntEQ(wolfSSH_SetAlgoListKex(ssh, REGRESS_KEXINIT_KEX_ALGO), + WS_SUCCESS); + AssertIntEQ(wolfSSH_SetAlgoListKey(ssh, REGRESS_KEXINIT_KEY_ALGO), + WS_SUCCESS); + AssertIntEQ(wolfSSH_SetStrictKex(ssh, tc->sendStrictKex), WS_SUCCESS); + + if (tc->rekey) { + /* DoKexInit reads a non-empty session id as "this is a rekey". */ + WMEMSET(ssh->sessionId, 0x5A, WC_SHA256_DIGEST_SIZE); + ssh->sessionIdSz = WC_SHA256_DIGEST_SIZE; + } + + payloadSz = BuildKexInitPayload(ssh, tc->peerKexList, + REGRESS_KEXINIT_KEY_ALGO, 0, + payload, (word32)sizeof(payload)); + + /* DoKexInit's tail hashes and answers the KEXINIT, which fails on a + * stripped-down WOLFSSH with no host key loaded. Only the negotiation + * this test asserts on happens before that. */ + (void)wolfSSH_TestDoKexInit(ssh, payload, payloadSz, &idx); + + if (ssh->peerStrictKex != tc->expectPeerStrictKex) { + Fail(("peerStrictKex == %u (%s)", + tc->expectPeerStrictKex, tc->description), + ("%u", ssh->peerStrictKex)); + } + if (ssh->strictKexEnabled != tc->expectStrictKexEnabled) { + Fail(("strictKexEnabled == %u (%s)", + tc->expectStrictKexEnabled, tc->description), + ("%u", ssh->strictKexEnabled)); + } + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + +static void TestStrictKexMarkerNegotiation(void) +{ + word32 i; + + for (i = 0; i < (word32)(sizeof(strictKexMarkerCases) / + sizeof(strictKexMarkerCases[0])); i++) { + RunStrictKexMarkerCase(&strictKexMarkerCases[i]); + } +} + +/* A rekey must not clear a mitigation the initial KEX turned on, either. */ +static void TestStrictKexSurvivesRekeyKexInit(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + byte payload[768]; + word32 payloadSz; + word32 idx = 0; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + + AssertIntEQ(wolfSSH_SetAlgoListKex(ssh, REGRESS_KEXINIT_KEX_ALGO), + WS_SUCCESS); + AssertIntEQ(wolfSSH_SetAlgoListKey(ssh, REGRESS_KEXINIT_KEY_ALGO), + WS_SUCCESS); + + /* Established session, strict KEX already negotiated. */ + WMEMSET(ssh->sessionId, 0x5A, WC_SHA256_DIGEST_SIZE); + ssh->sessionIdSz = WC_SHA256_DIGEST_SIZE; + ssh->peerStrictKex = 1; + ssh->strictKexEnabled = 1; + + /* A rekey KEXINIT that no longer carries the marker, which is what a + * conforming peer sends. */ + payloadSz = BuildKexInitPayload(ssh, REGRESS_KEXINIT_KEX_ALGO, + REGRESS_KEXINIT_KEY_ALGO, 0, + payload, (word32)sizeof(payload)); + (void)wolfSSH_TestDoKexInit(ssh, payload, payloadSz, &idx); + + AssertIntEQ(ssh->peerStrictKex, 1); + AssertIntEQ(ssh->strictKexEnabled, 1); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + +#endif /* strict KEX marker guard */ + + /* Regression coverage for issue 5575: the documented ssh://hostname form must * set the hostname even without an explicit port, and a malformed destination @@ -16212,6 +17201,8 @@ int main(int argc, char** argv) #endif #ifndef NO_WOLFSSH_CLIENT TestAuthMessageBlockedDuringKeying(ssh); + TestStrictKexBlocksInjectableMessages(ssh); + TestStrictKexAllowsDisconnectDuringInitialKex(ssh); TestUserauthFailureDuringKeying(ssh); TestPasswordLeakAborts(ssh); TestPrematureUserauthSuccess(ssh); @@ -16455,6 +17446,10 @@ int main(int argc, char** argv) TestKexInitLanguageLengthOverflow(); TestDoKexInitRejectsWhenPeerIsKeying(); #endif +#if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_RSA) + TestStrictKexMarkerNegotiation(); + TestStrictKexSurvivesRekeyKexInit(); +#endif #if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_RSA) \ && !defined(WOLFSSH_NO_CURVE25519_SHA256) \ && !defined(WOLFSSH_NO_RSA_SHA2_256) \ @@ -16568,6 +17563,18 @@ int main(int argc, char** argv) TestKexDhGexGroupShrunkPrimeSendsDisconnect(); TestKexDhGexGroupBadGeneratorSendsDisconnect(); #endif + /* Strict KEX, the Terrapin mitigation (CVE-2023-48795) */ + TestStrictKexNegotiatedByDefault(); + TestStrictKexClientOptOut(); + TestStrictKexServerOptOut(); + TestStrictKexSettingFrozenForHandshake(); + TestStrictKexMarkerNotSentOnRekey(); + TestStrictKexResetsSequenceNumbers(); + TestStrictKexResetsSequenceNumbersOnRekey(); + TestTerrapinInjectionAcceptedWithoutStrictKex(); + TestTerrapinInjectionRejectedWithStrictKex(); + TestTerrapinInjectionRejectedByServer(); + TestStrictKexTakesInjectedDisconnect(); #endif #ifdef WOLFSSH_SFTP diff --git a/wolfssh/internal.h b/wolfssh/internal.h index c44bccf5a..c1ca45f61 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -529,8 +529,12 @@ enum { ID_CURVE25519_SHA256, ID_CURVE25519_SHA256_LIBSSH, #endif - ID_EXTINFO_S, /* Pseudo-KEX to indicate server extensions. */ - ID_EXTINFO_C, /* Pseudo-KEX to indicate client extensions. */ + ID_EXT_INFO_S, /* Pseudo-KEX to indicate server extensions. */ + ID_EXT_INFO_C, /* Pseudo-KEX to indicate client extensions. */ + ID_EXT_STRICT_KEX_S, /* Pseudo-KEX to indicate server strict KEX. */ + ID_EXT_STRICT_KEX_C, /* Pseudo-KEX to indicate client strict KEX. */ + ID_EXT_PRE_STRICT_KEX_S, /* OpenSSH -v00 spelling of the server's. */ + ID_EXT_PRE_STRICT_KEX_C, /* OpenSSH -v00 spelling of the client's. */ /* Public Key IDs */ ID_SSH_RSA, @@ -947,6 +951,7 @@ struct WOLFSSH_CTX { word32 windowSz; word32 maxPacketSz; word32 maxAuthAttempts; /* server cap on failed userauth */ + byte sendStrictKex; /* offer the strict KEX marker */ byte side; /* client or server */ byte showBanner; byte appChannels; /* app drives channels, see ssh.h */ @@ -980,6 +985,8 @@ typedef struct Keys { typedef struct HandshakeInfo { byte expectMsgId; + byte strictKex; /* the strict KEX setting this handshake runs on */ + byte strictKexSet; /* strictKex has been read from the session */ byte kexId; byte kexHashId; byte pubKeyId; @@ -1351,6 +1358,10 @@ struct WOLFSSH { byte userAuthPkDone; byte sendExtInfo; byte extInfoSent; /* track if the ext info has already been sent */ + byte sendStrictKex; /* offer the strict KEX marker on initial KEXINIT */ + byte peerStrictKex; /* peer offered the strict KEX marker (initial KEX) */ + byte strictKexEnabled; /* both sides negotiated strict KEX for this session */ + byte initialKexDone; /* peer's initial KEX finished (its NEWKEYS arrived) */ byte* peerSigId; word32 peerSigIdSz; diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index d1e2d10b0..9f0cc6f5d 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -203,6 +203,21 @@ WOLFSSH_API const char* wolfSSH_GetAlgoListKeyAccepted(WOLFSSH* ssh); WOLFSSH_API int wolfSSH_CheckAlgoName(const char* name); +/* Strict KEX (Terrapin mitigation, CVE-2023-48795) controls. Defaults to + * enabled. The CTX setting seeds every session made from it; the session + * setting overrides it for that session alone. Only the initial KEXINIT + * carries the marker and only that exchange decides whether the mitigation + * is on, so the session call has to be made before the connection starts: + * once a session is keying, its setting is fixed for the life of that + * session and a rekey does not revisit it. The CTX calls report + * WS_BAD_ARGUMENT on a NULL ctx, the session calls WS_SSH_NULL_E on a NULL + * ssh; the getters otherwise report the setting, 1 for enabled and 0 for + * disabled, not whether strict KEX was negotiated. */ +WOLFSSH_API int wolfSSH_CTX_SetStrictKex(WOLFSSH_CTX* ctx, byte enable); +WOLFSSH_API int wolfSSH_CTX_GetStrictKex(WOLFSSH_CTX* ctx); +WOLFSSH_API int wolfSSH_SetStrictKex(WOLFSSH* ssh, byte enable); +WOLFSSH_API int wolfSSH_GetStrictKex(WOLFSSH* ssh); + WOLFSSH_API const char* wolfSSH_QueryKex(word32* idx); WOLFSSH_API const char* wolfSSH_QueryKey(word32* idx); WOLFSSH_API const char* wolfSSH_QueryCipher(word32* idx); From 983dc714e41c0f8782ed10098fe0fba342a279af Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 22 Sep 2026 10:00:14 -0700 Subject: [PATCH 2/8] scripts: add an OpenSSH strict KEX interop test Runs the OpenSSH client against the echoserver and the wolfSSH example client against sshd, checking that each pairing negotiates the strict KEX marker and that a session then runs over the connection. - skip when ssh is missing or predates OpenSSH 9.6 - fall back from the ECC keys to RSA, then Ed25519, on a build without them, and give sshd an Ed25519 host key for the same case - skip the sshd half alone when sshd or ssh-keygen is missing, or sshd will not start unprivileged - report the sequence number resets OpenSSH logs rather than test them, the wording being no contract - run from make check through dist_noinst_SCRIPTS --- scripts/include.am | 4 + scripts/openssh-interop.test | 440 +++++++++++++++++++++++++++++++++++ 2 files changed, 444 insertions(+) create mode 100755 scripts/openssh-interop.test diff --git a/scripts/include.am b/scripts/include.am index 1e1c86149..abf2b4f5c 100644 --- a/scripts/include.am +++ b/scripts/include.am @@ -20,6 +20,10 @@ endif # app wasn't built. dist_noinst_SCRIPTS+= scripts/sshclient.test +# Not gated on anything: the script skips itself when OpenSSH is missing +# or predates strict KEX. +dist_noinst_SCRIPTS+= scripts/openssh-interop.test + dist_noinst_SCRIPTS+= scripts/fwd.test dist_noinst_SCRIPTS+= scripts/fwd-bulk.test endif diff --git a/scripts/openssh-interop.test b/scripts/openssh-interop.test new file mode 100755 index 000000000..d5ce0524d --- /dev/null +++ b/scripts/openssh-interop.test @@ -0,0 +1,440 @@ +#!/bin/sh + +# OpenSSH interop test for strict key exchange, the Terrapin mitigation +# (CVE-2023-48795). +# +# Part 1 puts the OpenSSH client on the wolfSSH echoserver, part 2 the +# wolfSSH example client on OpenSSH's sshd. Each one checks that the marker +# is negotiated and that a session then runs over the connection: the +# mitigation zeroes the packet sequence numbers at SSH_MSG_NEWKEYS, and a +# peer that disagreed about the counters would fail the first packet that +# had to authenticate. +# +# The checks read OpenSSH's own debug output, so they are worded the way +# OpenSSH words them. Strict KEX arrived in OpenSSH 9.6; anything older is +# skipped. + +no_pid=-1 +server_pid=$no_pid +sshd_pid=$no_pid +work_dir="`pwd`/openssh_interop_test$$" +ready_file="$work_dir/ready" +client_out="$work_dir/client.out" +client_log="$work_dir/client.log" +sshd_log="$work_dir/sshd.log" +port=0 +counter=0 +# What the echo session sends and expects back. +probe="strictkexprobe$$" +# Seconds any one connection is given before it is killed. Nothing here +# takes more than a moment; the limit only keeps a stuck session from +# hanging make check. +run_limit=60 +killer_pid=-1 +# The test keys are not copied into the build directory, so reach them +# through srcdir the way scripts/fwd.test does, or an out-of-tree build +# and make distcheck cannot find them. +keys_dir="${srcdir:-.}/keys" + +do_cleanup() { + if [ $killer_pid != $no_pid ] + then + kill $killer_pid 2>/dev/null + killer_pid=$no_pid + fi + if [ $sshd_pid != $no_pid ] + then + kill -9 $sshd_pid 2>/dev/null + sshd_pid=$no_pid + fi + if [ $server_pid != $no_pid ] + then + kill -9 $server_pid 2>/dev/null + server_pid=$no_pid + fi + rm -rf -- "$work_dir" +} + +do_trap() { + echo "got trap" + do_cleanup + exit 1 +} + +trap do_trap INT TERM +# sshd holds a fixed port, so clean up on the way out however that happens. +trap do_cleanup EXIT + +# Wait for a backgrounded command, killing it if it outstays the limit. +# timeout(1) is GNU coreutils and is on neither stock macOS nor the BSDs, +# so the watchdog is run here, the way scripts/sshclient.test does it. +# Polls rather than sleeping the whole limit: killing a subshell that is +# waiting on a sleep leaves the sleep running. +wait_with_limit() { + watched_pid=$1 + + ( + watched=0 + while kill -0 $watched_pid 2>/dev/null; do + if [ $watched -ge $run_limit ]; then + kill -9 $watched_pid 2>/dev/null + break + fi + sleep 1 + watched=$((watched + 1)) + done + ) 2>/dev/null & + killer_pid=$! + + wait $watched_pid + watched_status=$? + + kill $killer_pid 2>/dev/null + killer_pid=$no_pid + + return $watched_status +} + +fail() { + printf '\n\n%s\n' "$1" + do_cleanup + exit 1 +} + +skip() { + printf '%s, skipping\n' "$1" + rm -rf -- "$work_dir" + exit 77 +} + +command -v ssh > /dev/null 2>&1 || skip "no ssh in the path" + +ssh_version=`ssh -V 2>&1` +case "$ssh_version" in + OpenSSH_*) ;; + *) skip "ssh is not OpenSSH ($ssh_version)" ;; +esac + +# 0 when the OpenSSH_X.Y in $1 is 9.6 or newer, 1 when it is older, and 2 +# when there is no version to read. Strict KEX arrived in 9.6. +has_strict_kex() { + hsk_major=`echo "$1" | sed -n 's/.*OpenSSH_\([0-9][0-9]*\)\..*/\1/p'` + hsk_minor=`echo "$1" | \ + sed -n 's/.*OpenSSH_[0-9][0-9]*\.\([0-9][0-9]*\).*/\1/p'` + + [ -z "$hsk_major" ] && return 2 + [ -z "$hsk_minor" ] && hsk_minor=0 + + if [ "$hsk_major" -lt 9 ] || + { [ "$hsk_major" -eq 9 ] && [ "$hsk_minor" -lt 6 ]; } + then + return 1 + fi + + return 0 +} + +has_strict_kex "$ssh_version" +case $? in + 1) skip "$ssh_version predates strict KEX" ;; + 2) skip "can't read the OpenSSH version ($ssh_version)" ;; +esac + +[ ! -x ./examples/echoserver/echoserver ] && skip "echoserver doesn't exist" +./examples/echoserver/echoserver '-?' 2>&1 | grep -q "^echoserver " \ + || skip "echoserver doesn't run" + +mkdir -p "$work_dir" || exit 1 + +# The echoserver is one shot, so start a new one per connection. It picks +# an ephemeral port and writes it to the ready file. +# +# -f keeps it in echo mode; without it a build with shell support tries to +# fork a login shell for a user who doesn't exist on this machine. Any +# argument given here is passed along, which is how the ECC sample key set +# is selected. +start_echoserver() { + if [ $server_pid != $no_pid ] + then + kill -9 $server_pid 2>/dev/null + wait $server_pid 2>/dev/null + server_pid=$no_pid + fi + + rm -f "$ready_file" + ./examples/echoserver/echoserver -1 -f "$@" -R "$ready_file" \ + > "$work_dir/server.log" 2>&1 & + server_pid=$! + + counter=0 + while [ ! -s "$ready_file" ] && [ "$counter" -lt 100 ]; do + sleep 0.1 + counter=$((counter + 1)) + done + + [ ! -s "$ready_file" ] && fail "NO ready file, the echoserver never came up" + + port=`cat "$ready_file"` +} + +# Run one echo session with the OpenSSH client. +# +# $1 - the private key to authenticate with, in a form OpenSSH reads +# rest - extra echoserver arguments +# +# The key is copied because OpenSSH refuses one that is readable by anyone +# else, and the checked-in keys are group and world readable. The 0x03 in +# the input is what tells the echoserver's worker the session is over; the +# newline before it is the line the server echoes back. +run_openssh_client() { + key_file=$1 + shift + + rm -f "$client_out" "$client_log" + cp "$key_file" "$work_dir/id" || fail "couldn't copy $key_file" + chmod 600 "$work_dir/id" + + start_echoserver "$@" + + printf '%s\n\003' "$probe" | \ + ssh -vvv -T -p $port \ + -o StrictHostKeyChecking=no \ + -o UserKnownHostsFile=/dev/null \ + -o IdentitiesOnly=yes \ + -o PreferredAuthentications=publickey \ + -i "$work_dir/id" hansel@127.0.0.1 \ + > "$client_out" 2> "$client_log" & + wait_with_limit $! +} + +echo "Test the OpenSSH client against the wolfSSH echoserver" + +# -e switches the echoserver to its ECC sample keys. A build without ECDSA +# has only the RSA set, so fall back to that rather than call it a failure. +# A build with neither has only Ed25519, and the checked-in Ed25519 key is +# PKCS#8, which an OpenSSH on LibreSSL won't load, so make a key of +# OpenSSH's own and hand the echoserver its public half. +run_openssh_client "$keys_dir/hansel-key-ecc.pem" -e +session_status=$? +if [ $session_status -ne 0 ]; then + echo "the ECC attempt failed, retrying with the RSA sample key" + run_openssh_client "$keys_dir/hansel-key-rsa.pem" + session_status=$? +fi +if [ $session_status -ne 0 ] && command -v ssh-keygen > /dev/null 2>&1; then + echo "the RSA attempt failed, retrying with an Ed25519 key" + ssh-keygen -q -t ed25519 -N '' -C hansel -f "$work_dir/ed25519" \ + < /dev/null || fail "couldn't make an Ed25519 client key" + run_openssh_client "$work_dir/ed25519" -j "$work_dir/ed25519.pub" + session_status=$? +fi +if [ $session_status -ne 0 ]; then + cat "$client_log" + fail "couldn't open a session" +fi + +# What the echoserver put in its KEXINIT. Both spellings go out; this is +# the one deployed OpenSSH matches on. +grep -q 'kex-strict-s-v00@openssh.com' "$client_log" \ + || fail "the echoserver didn't offer the strict KEX marker" + +# And what OpenSSH decided to do about it. +grep -qi 'strict KEX ordering' "$client_log" \ + || fail "the OpenSSH client didn't turn strict KEX on" + +grep -q "$probe" "$client_out" \ + || fail "the echoserver's reply didn't make it back" + +# OpenSSH logs each reset as it happens. Informational: it is the one check +# here that a reworded debug line would break, and the round trip above +# already proves the two ends agree on the sequence numbers. +if grep -q 'resetting read seqnr' "$client_log"; then + echo "the OpenSSH client reset its inbound sequence number" +else + echo "note: no sequence number reset in the client log" +fi + +echo "the OpenSSH client negotiated strict KEX with the echoserver" + +# Part 2 needs an sshd to talk to, and a host key to give it. Neither is +# worth failing the test over. +sshd_bin= +for candidate in /usr/sbin/sshd /usr/local/sbin/sshd /opt/homebrew/sbin/sshd +do + [ -x "$candidate" ] && sshd_bin="$candidate" && break +done +if [ -z "$sshd_bin" ]; then + sshd_bin=`command -v sshd 2>/dev/null` +fi + +if [ -z "$sshd_bin" ] || [ ! -x "$sshd_bin" ]; then + echo "no sshd on this machine, skipping the wolfSSH client half" + do_cleanup + exit 0 +fi +if ! command -v ssh-keygen > /dev/null 2>&1; then + echo "no ssh-keygen, skipping the wolfSSH client half" + do_cleanup + exit 0 +fi +if [ ! -x ./examples/client/client ]; then + echo "the example client wasn't built, skipping the wolfSSH client half" + do_cleanup + exit 0 +fi + +echo "Test the wolfSSH client against OpenSSH's sshd" + +# An Ed25519 host key beside the ECDSA one, for a build without ECDSA. +ssh-keygen -q -t ecdsa -N '' -f "$work_dir/hostkey" < /dev/null \ + || fail "couldn't make a host key for sshd" +ssh-keygen -q -t ed25519 -N '' -f "$work_dir/hostkey-ed25519" < /dev/null \ + || fail "couldn't make an Ed25519 host key for sshd" + +# sshd takes a fixed port, so try a few. -f /dev/null keeps the machine's +# own sshd_config out of it, and -ddd both raises the log level and keeps +# sshd in the foreground for one connection. ForceCommand stands in for the +# user's login shell: the client exits with the session's exit status, and +# whatever the user's dotfiles leave behind is not this test's business. +start_sshd() { + rm -f "$sshd_log" + "$sshd_bin" -ddd -f /dev/null -p "$1" -h "$work_dir/hostkey" \ + -h "$work_dir/hostkey-ed25519" \ + -o "ListenAddress=127.0.0.1" \ + -o "AuthorizedKeysFile=$work_dir/authorized_keys" \ + -o "StrictModes=no" \ + -o "PasswordAuthentication=no" \ + -o "ForceCommand=true" \ + -o "PidFile=none" > "$sshd_log" 2>&1 & + sshd_pid=$! + + # Its own counter: the loop that calls this one is counting too. A + # failed bind is checked for as well as a good one -- sshd carries on + # after one address fails, so "Server listening" alone does not mean + # the port the client dials is answering. + sshd_wait=0 + while [ "$sshd_wait" -lt 100 ]; do + grep -q 'failed: Address already in use' "$sshd_log" 2>/dev/null \ + && return 1 + grep -q 'Server listening on' "$sshd_log" 2>/dev/null && return 0 + kill -0 $sshd_pid 2>/dev/null || return 1 + sleep 0.1 + sshd_wait=$((sshd_wait + 1)) + done + + return 1 +} + +sshd_port=0 +candidate_port=$((22500 + ($$ % 1000))) +counter=0 +while [ "$counter" -lt 5 ]; do + if start_sshd $candidate_port; then + sshd_port=$candidate_port + break + fi + kill -9 $sshd_pid 2>/dev/null + sshd_pid=$no_pid + candidate_port=$((candidate_port + 1)) + counter=$((counter + 1)) +done + +if [ "$sshd_port" -eq 0 ]; then + echo "sshd wouldn't start here, skipping the wolfSSH client half" + [ -s "$sshd_log" ] && tail -n 5 "$sshd_log" + do_cleanup + exit 0 +fi + +# The version gate above covered the client. This sshd was found on its own +# and can be the older vendor one while a newer ssh sits earlier in PATH, so +# read what it says about itself before asking it for a marker it has never +# heard of. +sshd_version=`sed -n 's/.*sshd version \(OpenSSH_[^,]*\).*/\1/p' \ + "$sshd_log" | head -n 1` +has_strict_kex "$sshd_version" +if [ $? -ne 0 ]; then + echo "$sshd_bin is ${sshd_version:-an unreadable version}, no strict KEX," + echo "skipping the wolfSSH client half" + do_cleanup + exit 0 +fi + +# sshd authenticates the user running this test, with one of the canned +# client keys put in an authorized_keys file of its own. +login_user=${USER:-`id -un`} +[ -z "$login_user" ] && fail "couldn't work out who is running this" + +run_wolfssh_client() { + cat "$2" > "$work_dir/authorized_keys" \ + || fail "couldn't write the authorized_keys file" + chmod 600 "$work_dir/authorized_keys" + + # -x stops once the session is up: the KEX and the user auth are what + # this half is about, and the echo path was covered above. stdin is + # closed because the client prompts for a password when publickey is + # refused, and a prompt with nothing behind it hangs the run. + HOME="$work_dir" ./examples/client/client \ + -h 127.0.0.1 -p $sshd_port -u "$login_user" -i "$1" -j "$2" -x \ + < /dev/null > "$client_out" 2>&1 & + wait_with_limit $! +} + +# sshd handles one connection per -d run, so it needs restarting between +# attempts. Wait for the old one to go first, or the restart races it for +# the port. +restart_sshd() { + kill -9 $sshd_pid 2>/dev/null + wait $sshd_pid 2>/dev/null + sshd_pid=$no_pid + start_sshd $sshd_port || fail "couldn't restart sshd" +} + +run_wolfssh_client "$keys_dir/hansel-key-ecc.der" \ + "$keys_dir/hansel-key-ecc.pub" +if [ $? -ne 0 ]; then + echo "the ECC attempt failed, retrying with the RSA client key" + restart_sshd + run_wolfssh_client "$keys_dir/hansel-key-rsa.der" \ + "$keys_dir/hansel-key-rsa.pub" +fi +if [ $? -ne 0 ]; then + echo "the RSA attempt failed, retrying with the Ed25519 client key" + restart_sshd + run_wolfssh_client "$keys_dir/hansel-key-ed25519.der" \ + "$keys_dir/hansel-key-ed25519.pub" +fi +if [ $? -ne 0 ]; then + cat "$client_out" + tail -n 20 "$sshd_log" + fail "the wolfSSH client couldn't connect to sshd" +fi + +# Give sshd a moment to finish writing its side of the session. +counter=0 +while [ "$counter" -lt 50 ]; do + grep -q 'Accepted publickey' "$sshd_log" 2>/dev/null && break + sleep 0.1 + counter=$((counter + 1)) +done + +grep -q 'kex-strict-c-v00@openssh.com' "$sshd_log" \ + || fail "the wolfSSH client didn't offer the strict KEX marker" + +grep -qi 'strict KEX ordering' "$sshd_log" \ + || fail "sshd didn't turn strict KEX on" + +grep -q 'Accepted publickey' "$sshd_log" \ + || fail "sshd never authenticated the wolfSSH client" + +if grep -q 'resetting send seqnr' "$sshd_log"; then + echo "sshd reset its outbound sequence number" +else + echo "note: no sequence number reset in the sshd log" +fi + +echo "the wolfSSH client negotiated strict KEX with sshd" + +do_cleanup +echo "OpenSSH strict KEX interop tests passed" +exit 0 From d7851e7103aba9b962f9fa7a08a59c18e02dc92e Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 22 Sep 2026 11:34:19 -0700 Subject: [PATCH 3/8] sftpclient: report why an autopilot copy failed The "Unable to copy" message now carries the return code and the session error with its name. The return alone is usually the generic WS_FATAL_ERROR, which says nothing about the cause. --- examples/sftpclient/sftpclient.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/examples/sftpclient/sftpclient.c b/examples/sftpclient/sftpclient.c index 011d1c9a9..5ccfbe908 100644 --- a/examples/sftpclient/sftpclient.c +++ b/examples/sftpclient/sftpclient.c @@ -1570,13 +1570,18 @@ static int doAutopilot(int cmd, char* local, char* remote) ret == WS_FATAL_ERROR); if (ret != WS_SUCCESS) { + /* ret is a generic failure code; the cause is in the session */ + err = wolfSSH_get_error(ssh); + if (cmd == AUTOPILOT_PUT) { - fprintf(stderr, "Unable to copy local file %s to remote file %s\n", - local, fullpath); + fprintf(stderr, "Unable to copy local file %s to remote file %s" + ": ret %d, error %d, %s\n", + local, fullpath, ret, err, wolfSSH_ErrorToName(err)); } else if (cmd == AUTOPILOT_GET) { - fprintf(stderr, "Unable to copy remote file %s to local file %s\n", - fullpath, local); + fprintf(stderr, "Unable to copy remote file %s to local file %s" + ": ret %d, error %d, %s\n", + fullpath, local, ret, err, wolfSSH_ErrorToName(err)); } } From b898b75ea3f613b7fc8bdc228d0025f94021b301 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 22 Sep 2026 11:50:30 -0700 Subject: [PATCH 4/8] echoserver: wait out a partial packet in SFTP When select() reports the socket readable but only part of a packet has arrived, wolfSSH_worker() returns WS_WANT_READ. sftp_worker() now goes back to waiting on the socket instead of ending the session, which cut off SFTP transfers whose packets arrived split across reads. --- examples/echoserver/echoserver.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index 39c240191..b50dbc00a 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -1915,6 +1915,10 @@ static int sftp_worker(thread_ctx_t* threadCtx) continue; } #endif + if (ret == WS_WANT_READ) { + /* Part of a packet arrived; wait for the rest. */ + continue; + } if (ret == WS_WANT_WRITE) { /* recall wolfSSH_worker here because is likely our custom * highwater callback that returned up a WS_WANT_WRITE */ From 2c0b8ea365434b9ce8cdb76b80ef56f5abb2be87 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 22 Sep 2026 11:58:31 -0700 Subject: [PATCH 5/8] internal: keep input past a packet in DoReceive GetInputLine() reads up to 255 bytes for the version line, so a peer that sends its first packets in the same segment leaves them buffered behind it. DoReceive() force-freed the input buffer after each packet, dropping any of those bytes past the first packet. It now keeps them and frees the buffer only once it is empty. --- src/internal.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index 85dbed456..4251074e3 100644 --- a/src/internal.c +++ b/src/internal.c @@ -14872,7 +14872,9 @@ int DoReceive(WOLFSSH* ssh) ssh->inputBuffer.idx += peerMacSz; WLOG(WS_LOG_DEBUG, "PR4: Shrinking input buffer"); - ShrinkBuffer(&ssh->inputBuffer, 1); + /* Keep bytes past this packet; DoProtoId() can leave some. */ + ShrinkBuffer(&ssh->inputBuffer, + ssh->inputBuffer.idx >= ssh->inputBuffer.length); ssh->processReplyState = PROCESS_INIT; } From 3d5b683f25acbae2c960132b8f5f27f5f1cce451 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 22 Sep 2026 11:58:31 -0700 Subject: [PATCH 6/8] internal: require KEXINIT first under strict KEX The strict KEX allow list is armed only once the peer's KEXINIT has negotiated it, so a packet spliced in ahead of that KEXINIT got through. DoKexInit() now ends the connection with KEY_EXCHANGE_FAILED when strict KEX is on and the KEXINIT is not the peer's first packet. - regress: a harness mode that splices a packet in ahead of the server's KEXINIT - regress: IGNORE, UNIMPLEMENTED, EXT_INFO and an unassigned id there are refused with strict KEX; an IGNORE is accepted without it --- src/internal.c | 11 ++++++ tests/regress.c | 101 ++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 96 insertions(+), 16 deletions(-) diff --git a/src/internal.c b/src/internal.c index 4251074e3..d9abe2751 100644 --- a/src/internal.c +++ b/src/internal.c @@ -6736,6 +6736,17 @@ static int DoKexInit(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) if (ssh->strictKexEnabled) { WLOG(WS_LOG_DEBUG, "DKI: strict KEX negotiated"); } + + /* Strict KEX requires KEXINIT to be the peer's first packet. + * The allow list is not armed until now. peerSeq is still + * this packet's number here. */ + if (ssh->strictKexEnabled && ssh->peerSeq != 0) { + WLOG(WS_LOG_DEBUG, + "DKI: strict KEX, KEXINIT was not the first packet"); + (void)SendDisconnect(ssh, + WOLFSSH_DISCONNECT_KEY_EXCHANGE_FAILED); + ret = WS_MSGID_NOT_ALLOWED_E; + } } else { WLOG(WS_LOG_DEBUG, diff --git a/tests/regress.c b/tests/regress.c index 3a57a52be..bed34bab1 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -742,6 +742,8 @@ static word32 LoadFileBuffer(const char* path, byte* buf, word32 bufSz) /* The same splice aimed the other way: the packet goes in ahead of the * client's NEWKEYS, so the server is the one asked to accept it. */ #define REGRESS_MUTATE_TERRAPIN_C2S 9 +/* The splice ahead of the server's KEXINIT, before the allow list arms. */ +#define REGRESS_MUTATE_TERRAPIN_PRE_KEXINIT 10 /* 4 (len) + 1 (padLen) + 1 (msgId) + 4 (empty string payload) + 6 (pad) */ #define REGRESS_INJECT_PACKET_SZ 16U @@ -1353,49 +1355,51 @@ static int FindPlainPacketOffset(const byte* packet, word32 packetSz, * cipher state, so a deletion breaks decryption outright. The injection * alone is what these tests exercise, and refusing it is what the * mitigation has to do. */ -static void TerrapinInjectBeforeNewKeys(DuplexEndpoint* endpoint, +static void TerrapinInject(DuplexEndpoint* endpoint, const byte** output, word32* outputSz) { KexReplyMutator* mutator = endpoint->mutator; byte injectPkt[REGRESS_INJECT_MAX_SZ]; word32 injectPktSz; - word32 newKeysOffset; + word32 anchorOffset; + byte anchorMsgId = (mutator->mode == REGRESS_MUTATE_TERRAPIN_PRE_KEXINIT) + ? MSGID_KEXINIT : MSGID_NEWKEYS; if (mutator->injectedPackets > 0) { return; } - if (!FindPlainPacketOffset(*output, *outputSz, MSGID_NEWKEYS, - &newKeysOffset)) { + if (!FindPlainPacketOffset(*output, *outputSz, anchorMsgId, + &anchorOffset)) { return; } injectPktSz = BuildInjectedPacket(injectPkt, (word32)sizeof(injectPkt), mutator->injectMsgId); - /* Everything ahead of NEWKEYS passes through untouched, then the forged - * packet; the caller forwards the NEWKEYS tail. */ - if (newKeysOffset > 0) { + /* Pass through what precedes the anchor, then the forged packet; the + * caller forwards the rest. */ + if (anchorOffset > 0) { AssertIntEQ(QueueAppend(&endpoint->peer->inbound, *output, - newKeysOffset), WS_SUCCESS); + anchorOffset), WS_SUCCESS); } AssertIntEQ(QueueAppend(&endpoint->peer->inbound, injectPkt, injectPktSz), WS_SUCCESS); mutator->injectedPackets++; - *output += newKeysOffset; - *outputSz -= newKeysOffset; + *output += anchorOffset; + *outputSz -= anchorOffset; } -/* Both splice a packet into the initial KEX, they only differ in which - * side's NEWKEYS they go in ahead of. */ +/* Each splices a packet into the initial KEX ahead of a different one. */ static int IsTerrapinMode(byte mode) { return mode == REGRESS_MUTATE_TERRAPIN || - mode == REGRESS_MUTATE_TERRAPIN_C2S; + mode == REGRESS_MUTATE_TERRAPIN_C2S || + mode == REGRESS_MUTATE_TERRAPIN_PRE_KEXINIT; } -/* SIG_*, F_TRUNC, TERRAPIN and the GEX_* modes rewrite the server's - * messages; E_TRUNC, E_EMPTY and TERRAPIN_C2S the client's. */ +/* E_TRUNC, E_EMPTY and TERRAPIN_C2S rewrite the client's messages; the + * rest rewrite the server's. */ static int MutatorTargetsEndpoint(byte mode, byte isServer) { if (mode == REGRESS_MUTATE_E_TRUNC || mode == REGRESS_MUTATE_E_EMPTY || @@ -1491,7 +1495,7 @@ static int DuplexSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) !(outputSz >= REGRESS_SSH_PROTO_PREFIX_SZ && WMEMCMP(output, REGRESS_SSH_PROTO_PREFIX, REGRESS_SSH_PROTO_PREFIX_SZ) == 0)) { - TerrapinInjectBeforeNewKeys(endpoint, &output, &outputSz); + TerrapinInject(endpoint, &output, &outputSz); } if (endpoint->mutator != NULL && @@ -2704,6 +2708,69 @@ static void TestStrictKexTakesInjectedDisconnect(void) FreeKexReplyHarness(&harness); } +/* A packet ahead of the server's KEXINIT passes the unarmed allow list, + * so the KEXINIT arrives at a nonzero sequence number and the client + * disconnects. */ +static void AssertPreKexInitInjectionRejected(byte injectMsgId) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarnessMode(&harness, injectMsgId, 1, 1, + REGRESS_MUTATE_TERRAPIN_PRE_KEXINIT); + RunKexReplyHandshake(&harness, &result); + + AssertIntEQ(harness.mutator.injectedPackets, 1); + AssertIntEQ(harness.mutator.parseError, 0); + AssertIntEQ(harness.client->strictKexEnabled, 1); + + AssertFalse(result.clientSuccess); + AssertIntEQ(result.clientErr, WS_MSGID_NOT_ALLOWED_E); + AssertIntEQ(harness.client->initialKexDone, 0); + AssertIntEQ(harness.client->disconnected, 1); + + /* Still plaintext, so the reason is readable. */ + AssertIntEQ(harness.clientIo.sawDisconnect, 1); + AssertIntEQ(harness.clientIo.disconnectReason, + WOLFSSH_DISCONNECT_KEY_EXCHANGE_FAILED); + + FreeKexReplyHarness(&harness); +} + +static void TestPreKexInitInjectionRejectedWithStrictKex(void) +{ + /* EXT_INFO would replace the client's server-sig-algs list. DEBUG is + * omitted: it gets decoded here, and the payload is too short. */ + static const byte injectable[] = { + MSGID_IGNORE, MSGID_UNIMPLEMENTED, MSGID_EXT_INFO, + REGRESS_UNASSIGNED_TRANS_MSGID + }; + word32 i; + + for (i = 0; i < (word32)(sizeof(injectable) / sizeof(injectable[0])); i++) { + AssertPreKexInitInjectionRejected(injectable[i]); + } +} + +/* Without strict KEX the same splice is accepted. */ +static void TestPreKexInitInjectionAcceptedWithoutStrictKex(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarnessMode(&harness, MSGID_IGNORE, 0, 0, + REGRESS_MUTATE_TERRAPIN_PRE_KEXINIT); + RunKexReplyHandshake(&harness, &result); + + AssertIntEQ(harness.mutator.injectedPackets, 1); + AssertIntEQ(harness.mutator.parseError, 0); + AssertIntEQ(harness.client->strictKexEnabled, 0); + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + + FreeKexReplyHarness(&harness); +} + /* What a worker call is allowed to come back with while a rekey runs: an * empty queue, a packet handed to the application, or the rekey itself * still in progress. Anything else is a real failure. */ @@ -17575,6 +17642,8 @@ int main(int argc, char** argv) TestTerrapinInjectionRejectedWithStrictKex(); TestTerrapinInjectionRejectedByServer(); TestStrictKexTakesInjectedDisconnect(); + TestPreKexInitInjectionRejectedWithStrictKex(); + TestPreKexInitInjectionAcceptedWithoutStrictKex(); #endif #ifdef WOLFSSH_SFTP From 3eed3e31bb2c6891c7d3baea45067accd2a63d62 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 22 Sep 2026 20:55:11 -0700 Subject: [PATCH 7/8] internal: send the v00 strict KEX marker last SendKexInit() now lists kex-strict-*-v00@openssh.com after the plain kex-strict-* name. Paramiko lets the last kex-strict-* name it sees decide strict KEX and knows only the v00 form, so with it first Paramiko turned strict KEX off. --- src/internal.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/internal.c b/src/internal.c index d9abe2751..188b7eb2b 100644 --- a/src/internal.c +++ b/src/internal.c @@ -15393,7 +15393,9 @@ int SendKexInit(WOLFSSH* ssh) * (RFC draft-miller-sshm-strict-kex), distinguished here by an * empty session id. The sendStrictKex flag lets callers opt out * at runtime; the handshake freezes it so this and the enforcement - * decision in DoKexInit() cannot read different values. */ + * decision in DoKexInit() cannot read different values. The + * -v00@openssh.com marker goes last: Paramiko lets the last + * kex-strict-* name it sees decide, and knows only that one. */ int includeStrictKex = (ssh->sessionIdSz == 0) && HandshakeStrictKex(ssh); @@ -15401,8 +15403,8 @@ int SendKexInit(WOLFSSH* ssh) if (includeStrictKex) { kexAlgoNamesPlus = ",ext-info-c" - ",kex-strict-c-v00@openssh.com" - ",kex-strict-c"; + ",kex-strict-c" + ",kex-strict-c-v00@openssh.com"; } else { kexAlgoNamesPlus = ",ext-info-c"; @@ -15413,8 +15415,8 @@ int SendKexInit(WOLFSSH* ssh) if (includeStrictKex) { kexAlgoNamesPlus = ",ext-info-s" - ",kex-strict-s-v00@openssh.com" - ",kex-strict-s"; + ",kex-strict-s" + ",kex-strict-s-v00@openssh.com"; } else { kexAlgoNamesPlus = ",ext-info-s"; From 48fa84ee19ecd9c4c592e7588bbf42463f0e5b5c Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 22 Sep 2026 20:55:11 -0700 Subject: [PATCH 8/8] scripts: add an RSA sshd host key to interop test A build with neither ECDSA nor Ed25519 can verify only an RSA host key, so openssh-interop.test now starts sshd with an RSA host key beside the ECDSA and Ed25519 ones. --- scripts/openssh-interop.test | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/openssh-interop.test b/scripts/openssh-interop.test index d5ce0524d..2eb37dc32 100755 --- a/scripts/openssh-interop.test +++ b/scripts/openssh-interop.test @@ -285,9 +285,12 @@ fi echo "Test the wolfSSH client against OpenSSH's sshd" -# An Ed25519 host key beside the ECDSA one, for a build without ECDSA. +# RSA and Ed25519 host keys beside the ECDSA one, for a build without +# ECDSA. A build with neither ECDSA nor Ed25519 can verify only RSA. ssh-keygen -q -t ecdsa -N '' -f "$work_dir/hostkey" < /dev/null \ || fail "couldn't make a host key for sshd" +ssh-keygen -q -t rsa -b 3072 -N '' -f "$work_dir/hostkey-rsa" < /dev/null \ + || fail "couldn't make an RSA host key for sshd" ssh-keygen -q -t ed25519 -N '' -f "$work_dir/hostkey-ed25519" < /dev/null \ || fail "couldn't make an Ed25519 host key for sshd" @@ -299,7 +302,7 @@ ssh-keygen -q -t ed25519 -N '' -f "$work_dir/hostkey-ed25519" < /dev/null \ start_sshd() { rm -f "$sshd_log" "$sshd_bin" -ddd -f /dev/null -p "$1" -h "$work_dir/hostkey" \ - -h "$work_dir/hostkey-ed25519" \ + -h "$work_dir/hostkey-rsa" -h "$work_dir/hostkey-ed25519" \ -o "ListenAddress=127.0.0.1" \ -o "AuthorizedKeysFile=$work_dir/authorized_keys" \ -o "StrictModes=no" \