From 6db3b7fcaa0cc84b91d5b467b5559f518e0e8119 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 23 Jun 2026 15:36:35 -0700 Subject: [PATCH 1/3] Discard guessed KEX packet in DoKexDhGexGroup - DoKexDhGexGroup didn't check ignoreNextKexMsg, so a wrong first_kex_packet_follows guess was parsed as a real group and errored instead of being silently discarded (RFC 4253 7.1). - Add the guard already used by DoKexDhReply / DoKexDhInit: consume the packet, clear the flag, return WS_SUCCESS. - Extend TestFirstPacketFollowsSkipped via a new wolfSSH_TestDoKexDhGexGroup wrapper. Issue: F-2866 --- src/internal.c | 18 +++++++++++++++++- tests/regress.c | 2 ++ wolfssh/internal.h | 2 ++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index dfb92d728..c9973ac67 100644 --- a/src/internal.c +++ b/src/internal.c @@ -6765,10 +6765,20 @@ static int DoKexDhGexGroup(WOLFSSH* ssh, word32 begin; int ret = WS_SUCCESS; - if (ssh == NULL || buf == NULL || len == 0 || idx == NULL) + if (ssh == NULL || ssh->handshake == NULL || buf == NULL || len == 0 || + idx == NULL) ret = WS_BAD_ARGUMENT; if (ret == WS_SUCCESS) { + if (ssh->handshake->ignoreNextKexMsg) { + /* skip this message. */ + WLOG(WS_LOG_DEBUG, "Skipping server's KEXDH_GEX_GROUP message due " + "to first_packet_follows guess mismatch."); + ssh->handshake->ignoreNextKexMsg = 0; + *idx += len; + return WS_SUCCESS; + } + begin = *idx; ret = GetMpint(&primeGroupSz, &primeGroup, buf, len, &begin); if (ret == WS_SUCCESS && primeGroupSz > (MAX_KEX_KEY_SZ + 1)) { @@ -18542,6 +18552,12 @@ int wolfSSH_TestDoKexDhGexRequest(WOLFSSH* ssh, byte* buf, word32 len, return DoKexDhGexRequest(ssh, buf, len, idx); } +int wolfSSH_TestDoKexDhGexGroup(WOLFSSH* ssh, byte* buf, word32 len, + word32* idx) +{ + return DoKexDhGexGroup(ssh, buf, len, idx); +} + int wolfSSH_TestValidateKexDhGexGroup(const byte* primeGroup, word32 primeGroupSz, const byte* generator, word32 generatorSz, word32 minBits, word32 maxBits, WC_RNG* rng) diff --git a/tests/regress.c b/tests/regress.c index de22281bd..3c2defa1a 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -2634,6 +2634,8 @@ static void TestFirstPacketFollowsSkipped(void) #ifndef WOLFSSH_NO_DH_GEX_SHA256 RunFirstPacketFollowsSkipCase(wolfSSH_TestDoKexDhGexRequest, "DoKexDhGexRequest", WOLFSSH_ENDPOINT_SERVER, CLIENT_KEXINIT_DONE); + RunFirstPacketFollowsSkipCase(wolfSSH_TestDoKexDhGexGroup, + "DoKexDhGexGroup", WOLFSSH_ENDPOINT_CLIENT, SERVER_KEXINIT_DONE); #endif RunFirstPacketFollowsSkipCase(wolfSSH_TestDoKexDhReply, "DoKexDhReply", WOLFSSH_ENDPOINT_CLIENT, SERVER_KEXINIT_DONE); diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 858c9a9bc..cf26da3f5 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1434,6 +1434,8 @@ enum WS_MessageIdLimits { #ifndef WOLFSSH_NO_DH_GEX_SHA256 WOLFSSH_API int wolfSSH_TestDoKexDhGexRequest(WOLFSSH* ssh, byte* buf, word32 len, word32* idx); + WOLFSSH_API int wolfSSH_TestDoKexDhGexGroup(WOLFSSH* ssh, byte* buf, + word32 len, word32* idx); WOLFSSH_API int wolfSSH_TestValidateKexDhGexGroup(const byte* primeGroup, word32 primeGroupSz, const byte* generator, word32 generatorSz, word32 minBits, word32 maxBits, WC_RNG* rng); From 20a64633dcb55b6f5ba6bae2b41a9415bd4b7205 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 23 Jun 2026 15:43:33 -0700 Subject: [PATCH 2/3] Select DH GEX group from client size window - Server GEX ignored the client's min/preferred/max and always sent group 14, silently downgrading a 4096-min client (RFC 4419). - Add SelectKexDhGexGroup(): pick the built-in group within [min, max] closest to preferred, ties favoring the smaller. - Reject with WS_DH_SIZE_E when no built-in group fits. - GetDHPrimeGroup() now takes the WOLFSSH so the group sent, the exchange hash, and the shared secret all reselect consistently. - Add a unit test (default, max-cap, out-of-window, 4096-min, 1024-only). Issue: F-55 --- src/internal.c | 147 ++++++++++++++++++++++++++++++++--- tests/unit.c | 190 +++++++++++++++++++++++++++++++++++++++++++++ wolfssh/internal.h | 6 ++ 3 files changed, 334 insertions(+), 9 deletions(-) diff --git a/src/internal.c b/src/internal.c index c9973ac67..763f8d9f5 100644 --- a/src/internal.c +++ b/src/internal.c @@ -11767,11 +11767,83 @@ static int BuildRFC6187Info(WOLFSSH* ssh, int pubKeyID, #endif /* WOLFSSH_CERTS */ +#ifndef WOLFSSH_NO_DH_GEX_SHA256 +/* Select a built-in DH group for a GEX exchange that fits the client's + * requested window. Picks the group whose modulus size in bits lies within + * [minBits, maxBits] and is closest to preferredBits; ties favor the smaller + * group, which preserves the historical 2048-bit choice for a default client. + * Returns WS_SUCCESS with primeGroup/primeGroupSz set, or WS_DH_SIZE_E if no + * built-in group falls inside the client's window -- the server rejects the + * exchange rather than silently handing back a group the client did not ask + * for (RFC 4419 sec. 3). */ +static int SelectKexDhGexGroup(word32 minBits, word32 preferredBits, + word32 maxBits, const byte** primeGroup, word32* primeGroupSz) +{ + static const struct { + word32 bits; + const byte* group; + const word32* groupSz; + } candidates[] = { + #ifndef WOLFSSH_NO_DH_GROUP1_SHA1 + { 1024, dhPrimeGroup1, &dhPrimeGroup1Sz }, + #endif + { 2048, dhPrimeGroup14, &dhPrimeGroup14Sz }, + #ifndef WOLFSSH_NO_DH_GROUP16_SHA512 + { 4096, dhPrimeGroup16, &dhPrimeGroup16Sz }, + #endif + }; + word32 i; + word32 best = 0; + word32 bestDelta = 0; + int haveBest = 0; + int ret = WS_SUCCESS; + + if (primeGroup == NULL || primeGroupSz == NULL) + return WS_BAD_ARGUMENT; + + for (i = 0; i < (word32)(sizeof(candidates) / sizeof(candidates[0])); i++) { + word32 bits = candidates[i].bits; + word32 delta; + + if (bits < minBits || bits > maxBits) + continue; + delta = (bits > preferredBits) ? (bits - preferredBits) + : (preferredBits - bits); + /* Ascending scan with a strict '<' keeps the smaller group on a tie. */ + if (!haveBest || delta < bestDelta) { + best = i; + bestDelta = delta; + haveBest = 1; + } + } + + if (!haveBest) { + WLOG(WS_LOG_DEBUG, + "DH GEX: no built-in group within client window [%u, %u]", + minBits, maxBits); + ret = WS_DH_SIZE_E; + } + else { + *primeGroup = candidates[best].group; + *primeGroupSz = *candidates[best].groupSz; + } + + return ret; +} +#endif /* !WOLFSSH_NO_DH_GEX_SHA256 */ + + #ifndef WOLFSSH_NO_DH -static int GetDHPrimeGroup(int kexId, const byte** primeGroup, +static int GetDHPrimeGroup(WOLFSSH* ssh, const byte** primeGroup, word32* primeGroupSz, const byte** generator, word32* generatorSz) { int ret = WS_SUCCESS; + int kexId; + + if (ssh == NULL || ssh->handshake == NULL) + return WS_BAD_ARGUMENT; + + kexId = ssh->handshake->kexId; switch (kexId) { #ifndef WOLFSSH_NO_DH_GROUP1_SHA1 @@ -11808,10 +11880,27 @@ static int GetDHPrimeGroup(int kexId, const byte** primeGroup, #endif #ifndef WOLFSSH_NO_DH_GEX_SHA256 case ID_DH_GEX_SHA256: - *primeGroup = dhPrimeGroup14; - *primeGroupSz = dhPrimeGroup14Sz; - *generator = dhGenerator; - *generatorSz = dhGeneratorSz; + /* Reuse the group SendKexDhGexGroup cached on the handshake so the + * exchange hash and the shared secret match the group that was put + * on the wire. Fall back to selecting from the client's window if + * the cache is somehow unset, so this path can never desynchronize + * from the wire group. */ + if (ssh->handshake->primeGroup != NULL) { + *primeGroup = ssh->handshake->primeGroup; + *primeGroupSz = ssh->handshake->primeGroupSz; + *generator = dhGenerator; + *generatorSz = dhGeneratorSz; + } + else { + ret = SelectKexDhGexGroup(ssh->handshake->dhGexMinSz, + ssh->handshake->dhGexPreferredSz, + ssh->handshake->dhGexMaxSz, + primeGroup, primeGroupSz); + if (ret == WS_SUCCESS) { + *generator = dhGenerator; + *generatorSz = dhGeneratorSz; + } + } break; #endif default: @@ -12142,7 +12231,7 @@ static int SendKexGetSigningKey(WOLFSSH* ssh, if (ssh->handshake->kexId == ID_DH_GEX_SHA256) { byte primeGroupPad = 0, generatorPad = 0; - if (GetDHPrimeGroup(ssh->handshake->kexId, &primeGroup, + if (GetDHPrimeGroup(ssh, &primeGroup, &primeGroupSz, &generator, &generatorSz) != WS_SUCCESS) { ret = WS_BAD_ARGUMENT; } @@ -12375,7 +12464,7 @@ static int KeyAgreeDh_server(WOLFSSH* ssh, byte hashId, byte* f, word32* fSz) WOLFSSH_UNUSED(hashId); if (ret == WS_SUCCESS) { - ret = GetDHPrimeGroup(ssh->handshake->kexId, &primeGroup, + ret = GetDHPrimeGroup(ssh, &primeGroup, &primeGroupSz, &generator, &generatorSz); if (ret == WS_SUCCESS) { @@ -13796,8 +13885,8 @@ int SendKexDhGexGroup(WOLFSSH* ssh) byte* output; word32 idx = 0; word32 payloadSz; - const byte* primeGroup = dhPrimeGroup14; - word32 primeGroupSz = dhPrimeGroup14Sz; + const byte* primeGroup = NULL; + word32 primeGroupSz = 0; const byte* generator = dhGenerator; word32 generatorSz = dhGeneratorSz; byte primePad = 0; @@ -13808,6 +13897,32 @@ int SendKexDhGexGroup(WOLFSSH* ssh) if (ssh == NULL || ssh->handshake == NULL) ret = WS_BAD_ARGUMENT; + /* Pick a group that satisfies the client's requested min/preferred/max + * rather than always sending the 2048-bit group 14. */ + if (ret == WS_SUCCESS) { + ret = SelectKexDhGexGroup(ssh->handshake->dhGexMinSz, + ssh->handshake->dhGexPreferredSz, + ssh->handshake->dhGexMaxSz, + &primeGroup, &primeGroupSz); + } + + /* Cache the selected group on the handshake so the exchange hash and the + * shared secret (GetDHPrimeGroup) reuse exactly what goes on the wire here, + * making the wire group the single source of truth instead of re-running + * the selection independently. */ + if (ret == WS_SUCCESS) { + if (ssh->handshake->primeGroup) + WFREE(ssh->handshake->primeGroup, ssh->ctx->heap, DYNTYPE_MPINT); + ssh->handshake->primeGroup = + (byte*)WMALLOC(primeGroupSz, ssh->ctx->heap, DYNTYPE_MPINT); + if (ssh->handshake->primeGroup == NULL) + ret = WS_MEMORY_E; + else { + WMEMCPY(ssh->handshake->primeGroup, primeGroup, primeGroupSz); + ssh->handshake->primeGroupSz = primeGroupSz; + } + } + if (ret == WS_SUCCESS) { if (primeGroup[0] & 0x80) primePad = 1; @@ -18542,6 +18657,13 @@ int wolfSSH_TestKeyAgreeDh_server(WOLFSSH* ssh, byte hashId, return KeyAgreeDh_server(ssh, hashId, f, fSz); } +int wolfSSH_TestGetDHPrimeGroup(WOLFSSH* ssh, const byte** primeGroup, + word32* primeGroupSz, const byte** generator, word32* generatorSz) +{ + return GetDHPrimeGroup(ssh, primeGroup, primeGroupSz, + generator, generatorSz); +} + #endif /* !WOLFSSH_NO_DH */ #ifndef WOLFSSH_NO_DH_GEX_SHA256 @@ -18566,6 +18688,13 @@ int wolfSSH_TestValidateKexDhGexGroup(const byte* primeGroup, generator, generatorSz, minBits, maxBits, rng); } +int wolfSSH_TestSelectKexDhGexGroup(word32 minBits, word32 preferredBits, + word32 maxBits, const byte** primeGroup, word32* primeGroupSz) +{ + return SelectKexDhGexGroup(minBits, preferredBits, maxBits, + primeGroup, primeGroupSz); +} + #endif /* !WOLFSSH_NO_DH_GEX_SHA256 */ #ifndef WOLFSSH_NO_RSA diff --git a/tests/unit.c b/tests/unit.c index e18981ef7..1ef8aba0b 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -1131,6 +1131,186 @@ static int test_DhGexGroupValidate(void) return result; } +/* Server-side group selection for DH GEX. Confirms the server honors the + * client's requested min/preferred/max window instead of always handing back + * the 2048-bit group 14, and rejects windows no built-in group can satisfy. */ +static int test_DhGexGroupSelect(void) +{ + const byte* group; + word32 groupSz; + int ret; + + /* Default-ish window: with group 16 enabled, preferred 3072 is equidistant + * from 2048 and 4096 and the tie favors the smaller group; without group 16, + * 2048 is simply the closest in-window candidate. Either way a stock client + * still receives the historical 2048-bit group (256 bytes). */ + group = NULL; groupSz = 0; + ret = wolfSSH_TestSelectKexDhGexGroup(1024, 3072, 8192, &group, &groupSz); + if (ret != WS_SUCCESS || groupSz != 256) { + printf("DhGexGroupSelect: default window got ret %d sz %u\n", + ret, groupSz); + return -200; + } + + /* A max below 4096 must cap the choice even when preferred is huge. */ + group = NULL; groupSz = 0; + ret = wolfSSH_TestSelectKexDhGexGroup(1024, 8192, 2048, &group, &groupSz); + if (ret != WS_SUCCESS || groupSz != 256) { + printf("DhGexGroupSelect: max-cap window got ret %d sz %u\n", + ret, groupSz); + return -201; + } + + /* A window no built-in group falls inside must be rejected, not silently + * served a group outside it. */ + group = NULL; groupSz = 0; + ret = wolfSSH_TestSelectKexDhGexGroup(3000, 3000, 3500, &group, &groupSz); + if (ret != WS_DH_SIZE_E) { + printf("DhGexGroupSelect: impossible window got ret %d\n", ret); + return -202; + } + +#ifndef WOLFSSH_NO_DH_GROUP16_SHA512 + /* A client demanding a 4096-bit minimum gets the 4096-bit group (512 + * bytes), never a silent downgrade to 2048. */ + group = NULL; groupSz = 0; + ret = wolfSSH_TestSelectKexDhGexGroup(4096, 4096, 8192, &group, &groupSz); + if (ret != WS_SUCCESS || groupSz != 512) { + printf("DhGexGroupSelect: 4096 min got ret %d sz %u\n", ret, groupSz); + return -203; + } +#else + /* Without group 16 there is nothing >= 4096, so the request is rejected + * rather than downgraded. */ + group = NULL; groupSz = 0; + ret = wolfSSH_TestSelectKexDhGexGroup(4096, 4096, 8192, &group, &groupSz); + if (ret != WS_DH_SIZE_E) { + printf("DhGexGroupSelect: 4096 min (no group16) got ret %d\n", ret); + return -203; + } +#endif + +#ifndef WOLFSSH_NO_DH_GROUP1_SHA1 + /* A window only the 1024-bit group fits returns group 1 (128 bytes). */ + group = NULL; groupSz = 0; + ret = wolfSSH_TestSelectKexDhGexGroup(1024, 1024, 1536, &group, &groupSz); + if (ret != WS_SUCCESS || groupSz != 128) { + printf("DhGexGroupSelect: 1024 window got ret %d sz %u\n", + ret, groupSz); + return -204; + } +#endif + + return 0; +} + +/* Send sink so SendKexDhGexGroup can run the real server path (select the + * group, build the KEXDH_GEX_GROUP packet, drain it) without a live transport. + * We only care that the group it selects gets cached on the handshake. */ +static int GexSinkSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) +{ + (void)ssh; + (void)buf; + (void)ctx; + return (int)sz; +} + +/* Write a big-endian uint32 without the internal-only c32toa(). */ +static void PutU32BE(byte* out, word32 v) +{ + out[0] = (byte)(v >> 24); + out[1] = (byte)(v >> 16); + out[2] = (byte)(v >> 8); + out[3] = (byte)(v); +} + +/* End-to-end consistency check for DH GEX server group selection: the group + * SendKexDhGexGroup puts on the wire must be the exact group GetDHPrimeGroup + * hands the exchange-hash and key-agreement path. The default window resolves + * to the historical 2048-bit group 14, so every stock GEX handshake test would + * pass even if the two paths disagreed. This drives a client window that forces + * the 4096-bit group 16 and confirms the cached wire group and the group that + * GetDHPrimeGroup returns are one and the same -- the property the send/hash + * cache exists to guarantee. */ +static int test_DhGexGroupSendHashConsistency(void) +{ +#ifndef WOLFSSH_NO_DH_GROUP16_SHA512 + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + byte request[UINT32_SZ * 3]; + word32 idx = 0; + const byte* hashGroup = NULL; + word32 hashGroupSz = 0; + const byte* generator = NULL; + word32 generatorSz = 0; + int ret; + int result = 0; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + if (ctx == NULL) + return -210; + wolfSSH_SetIOSend(ctx, GexSinkSend); + ssh = wolfSSH_new(ctx); + if (ssh == NULL || ssh->handshake == NULL) { + result = -211; + goto out; + } + ssh->handshake->kexId = ID_DH_GEX_SHA256; + + /* Client window demanding a 4096-bit minimum -> forces group 16, never the + * default group 14 a desync could silently fall back to. */ + PutU32BE(request, 4096); + PutU32BE(request + UINT32_SZ, 4096); + PutU32BE(request + UINT32_SZ * 2, 8192); + + /* Real server entry point: parse the window, select the group, send it on + * the wire, and cache it on the handshake. */ + ret = wolfSSH_TestDoKexDhGexRequest(ssh, request, sizeof(request), &idx); + if (ret != WS_SUCCESS) { + printf("DhGexGroupSendHashConsistency: request ret %d\n", ret); + result = -212; + goto out; + } + /* The group put on the wire / cached must be the 4096-bit group 16 + * (512 bytes), not a silent fallback to the 256-byte group 14. */ + if (ssh->handshake->primeGroup == NULL || + ssh->handshake->primeGroupSz != 512) { + printf("DhGexGroupSendHashConsistency: wire group sz %u\n", + ssh->handshake->primeGroupSz); + result = -213; + goto out; + } + + /* The exchange-hash / key-agreement path must hand back the identical group + * -- same pointer, same size -- not an independently re-selected one. */ + ret = wolfSSH_TestGetDHPrimeGroup(ssh, &hashGroup, &hashGroupSz, + &generator, &generatorSz); + if (ret != WS_SUCCESS) { + printf("DhGexGroupSendHashConsistency: hash group ret %d\n", ret); + result = -214; + goto out; + } + if (hashGroup != ssh->handshake->primeGroup || + hashGroupSz != ssh->handshake->primeGroupSz) { + printf("DhGexGroupSendHashConsistency: hash group sz %u != wire %u\n", + hashGroupSz, ssh->handshake->primeGroupSz); + result = -215; + goto out; + } + +out: + if (ssh != NULL) + wolfSSH_free(ssh); + if (ctx != NULL) + wolfSSH_CTX_free(ctx); + return result; +#else + /* Without group 16 nothing forces a non-default selection, so there is no + * meaningful send-vs-hash divergence to guard against here. */ + return 0; +#endif /* !WOLFSSH_NO_DH_GROUP16_SHA512 */ +} + #endif /* WOLFSSH_TEST_INTERNAL && !WOLFSSH_NO_DH_GEX_SHA256 */ @@ -5119,6 +5299,16 @@ int wolfSSH_UnitTest(int argc, char** argv) printf("DhGexGroupValidate: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + + unitResult = test_DhGexGroupSelect(); + printf("DhGexGroupSelect: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; + + unitResult = test_DhGexGroupSendHashConsistency(); + printf("DhGexGroupSendHashConsistency: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; #endif #ifdef WOLFSSH_TEST_INTERNAL diff --git a/wolfssh/internal.h b/wolfssh/internal.h index cf26da3f5..3cd9072c4 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1430,6 +1430,9 @@ enum WS_MessageIdLimits { const byte* f, word32 fSz); WOLFSSH_API int wolfSSH_TestKeyAgreeDh_server(WOLFSSH* ssh, byte hashId, byte* f, word32* fSz); + WOLFSSH_API int wolfSSH_TestGetDHPrimeGroup(WOLFSSH* ssh, + const byte** primeGroup, word32* primeGroupSz, + const byte** generator, word32* generatorSz); #endif /* !WOLFSSH_NO_DH */ #ifndef WOLFSSH_NO_DH_GEX_SHA256 WOLFSSH_API int wolfSSH_TestDoKexDhGexRequest(WOLFSSH* ssh, byte* buf, @@ -1439,6 +1442,9 @@ enum WS_MessageIdLimits { WOLFSSH_API int wolfSSH_TestValidateKexDhGexGroup(const byte* primeGroup, word32 primeGroupSz, const byte* generator, word32 generatorSz, word32 minBits, word32 maxBits, WC_RNG* rng); + WOLFSSH_API int wolfSSH_TestSelectKexDhGexGroup(word32 minBits, + word32 preferredBits, word32 maxBits, const byte** primeGroup, + word32* primeGroupSz); #endif /* !WOLFSSH_NO_DH_GEX_SHA256 */ #ifndef WOLFSSH_NO_RSA WOLFSSH_API int wolfSSH_TestRsaVerify(const byte* sig, word32 sigSz, From 167c037cd1a94d9d79158ad3ee140f8572162e6b Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 23 Jun 2026 17:19:38 -0700 Subject: [PATCH 3/3] Enforce 2048-bit floor on DH GEX group select - Clamp client's lower bound up to WOLFSSH_DH_GEX_MIN_BITS (default 2048) before the candidate scan; prevents downgrade to group 1. - Reject a window whose max is below the floor with WS_DH_SIZE_E. - Keep group 1 in the candidate set so a lowered floor stays usable. - Cap the client window to MAX_KEX_KEY_SZ so selection can never pick a group larger than the server's own key buffers (defense-in-depth for builds enabling group 16 with a lowered WOLFSSH_DEFAULT_GEXDH_MAX). - Relabel the no-candidate log "effective window" so the clamped min is not mistaken for the raw client request. - Note the client-side DoKexDhGexGroup ignoreNextKexMsg skip as defensive symmetry not expected to fire. - Update test_DhGexGroupSelect for rejection and clamp-up cases; add test_DhGexGroupCacheMissFallback covering the GetDHPrimeGroup cache-miss re-selection path. Issue: F-55 --- src/internal.c | 35 ++++++++++++++++-- tests/unit.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 125 insertions(+), 9 deletions(-) diff --git a/src/internal.c b/src/internal.c index 763f8d9f5..3a117b899 100644 --- a/src/internal.c +++ b/src/internal.c @@ -6771,7 +6771,10 @@ static int DoKexDhGexGroup(WOLFSSH* ssh, if (ret == WS_SUCCESS) { if (ssh->handshake->ignoreNextKexMsg) { - /* skip this message. */ + /* A conformant server sends GROUP only in response to the client's + * REQUEST, so it should never set first_packet_follows here. + * Discard the message defensively if a peer sets it anyway (RFC + * 4253 7.1), mirroring the other Do* handlers. */ WLOG(WS_LOG_DEBUG, "Skipping server's KEXDH_GEX_GROUP message due " "to first_packet_follows guess mismatch."); ssh->handshake->ignoreNextKexMsg = 0; @@ -11768,12 +11771,24 @@ static int BuildRFC6187Info(WOLFSSH* ssh, int pubKeyID, #ifndef WOLFSSH_NO_DH_GEX_SHA256 + +/* Lower bound on the GEX modulus the server will hand out, regardless of how + * small a window the client requests. RFC 8270 (updating RFC 4419) says a + * server SHOULD NOT select a group smaller than 2048 bits; clamping here keeps + * the 1024-bit group 1 from being reachable via GEX even when it is otherwise + * enabled for direct group1-sha1 negotiation. */ +#ifndef WOLFSSH_DH_GEX_MIN_BITS + #define WOLFSSH_DH_GEX_MIN_BITS 2048 +#endif + /* Select a built-in DH group for a GEX exchange that fits the client's * requested window. Picks the group whose modulus size in bits lies within * [minBits, maxBits] and is closest to preferredBits; ties favor the smaller * group, which preserves the historical 2048-bit choice for a default client. - * Returns WS_SUCCESS with primeGroup/primeGroupSz set, or WS_DH_SIZE_E if no - * built-in group falls inside the client's window -- the server rejects the + * The client's lower bound is first raised to WOLFSSH_DH_GEX_MIN_BITS so the + * server never downgrades below the 2048-bit floor (RFC 8270). Returns + * WS_SUCCESS with primeGroup/primeGroupSz set, or WS_DH_SIZE_E if no built-in + * group falls inside the (clamped) client window -- the server rejects the * exchange rather than silently handing back a group the client did not ask * for (RFC 4419 sec. 3). */ static int SelectKexDhGexGroup(word32 minBits, word32 preferredBits, @@ -11801,6 +11816,18 @@ static int SelectKexDhGexGroup(word32 minBits, word32 preferredBits, if (primeGroup == NULL || primeGroupSz == NULL) return WS_BAD_ARGUMENT; + /* Never select below the 2048-bit floor even if the client asks for less. + * If the client's max is also below the floor, no candidate matches and we + * reject (WS_DH_SIZE_E) rather than downgrade. */ + if (minBits < WOLFSSH_DH_GEX_MIN_BITS) + minBits = WOLFSSH_DH_GEX_MIN_BITS; + + /* Cap to the server's own key buffer (MAX_KEX_KEY_SZ sizes the private + * exponent in KeyAgreeDh_server) so the client window can't select a group + * larger than it holds. */ + if (maxBits > (word32)(MAX_KEX_KEY_SZ * 8)) + maxBits = (word32)(MAX_KEX_KEY_SZ * 8); + for (i = 0; i < (word32)(sizeof(candidates) / sizeof(candidates[0])); i++) { word32 bits = candidates[i].bits; word32 delta; @@ -11819,7 +11846,7 @@ static int SelectKexDhGexGroup(word32 minBits, word32 preferredBits, if (!haveBest) { WLOG(WS_LOG_DEBUG, - "DH GEX: no built-in group within client window [%u, %u]", + "DH GEX: no built-in group within effective window [%u, %u]", minBits, maxBits); ret = WS_DH_SIZE_E; } diff --git a/tests/unit.c b/tests/unit.c index 1ef8aba0b..c671587cc 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -1190,16 +1190,27 @@ static int test_DhGexGroupSelect(void) } #endif -#ifndef WOLFSSH_NO_DH_GROUP1_SHA1 - /* A window only the 1024-bit group fits returns group 1 (128 bytes). */ + /* A sub-2048 window must be rejected, not downgraded to the 1024-bit group + * 1, even when group 1 is compiled in for direct group1-sha1: the GEX path + * enforces the WOLFSSH_DH_GEX_MIN_BITS (2048) floor from RFC 8270. The + * client's max (1536) is below the floor, so no candidate fits. */ group = NULL; groupSz = 0; ret = wolfSSH_TestSelectKexDhGexGroup(1024, 1024, 1536, &group, &groupSz); - if (ret != WS_SUCCESS || groupSz != 128) { - printf("DhGexGroupSelect: 1024 window got ret %d sz %u\n", + if (ret != WS_DH_SIZE_E) { + printf("DhGexGroupSelect: sub-2048 window got ret %d sz %u\n", ret, groupSz); return -204; } -#endif + + /* A window whose minimum is below the floor but whose max admits 2048 must + * clamp up to the 2048-bit group (256 bytes), never the 1024-bit group. */ + group = NULL; groupSz = 0; + ret = wolfSSH_TestSelectKexDhGexGroup(1024, 1024, 4096, &group, &groupSz); + if (ret != WS_SUCCESS || groupSz != 256) { + printf("DhGexGroupSelect: floor-clamp window got ret %d sz %u\n", + ret, groupSz); + return -205; + } return 0; } @@ -1311,6 +1322,79 @@ static int test_DhGexGroupSendHashConsistency(void) #endif /* !WOLFSSH_NO_DH_GROUP16_SHA512 */ } +/* Exercise the GetDHPrimeGroup cache-miss fallback: with the cached group + * cleared, it must re-select the same group from the client window (and set a + * generator), not desync from the wire. */ +static int test_DhGexGroupCacheMissFallback(void) +{ +#ifndef WOLFSSH_NO_DH_GROUP16_SHA512 + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + byte request[UINT32_SZ * 3]; + word32 idx = 0; + const byte* group = NULL; + word32 groupSz = 0; + const byte* generator = NULL; + word32 generatorSz = 0; + int ret; + int result = 0; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + if (ctx == NULL) + return -220; + wolfSSH_SetIOSend(ctx, GexSinkSend); + ssh = wolfSSH_new(ctx); + if (ssh == NULL || ssh->handshake == NULL) { + result = -221; + goto out; + } + ssh->handshake->kexId = ID_DH_GEX_SHA256; + + PutU32BE(request, 4096); + PutU32BE(request + UINT32_SZ, 4096); + PutU32BE(request + UINT32_SZ * 2, 8192); + + ret = wolfSSH_TestDoKexDhGexRequest(ssh, request, sizeof(request), &idx); + if (ret != WS_SUCCESS) { + result = -222; + goto out; + } + + /* Drop the cache to force the fallback branch. primeGroup is a WMALLOC'd + * copy; the fallback returns a pointer into static group data. */ + if (ssh->handshake->primeGroup != NULL) { + WFREE(ssh->handshake->primeGroup, ssh->ctx->heap, DYNTYPE_MPINT); + ssh->handshake->primeGroup = NULL; + } + + ret = wolfSSH_TestGetDHPrimeGroup(ssh, &group, &groupSz, + &generator, &generatorSz); + if (ret != WS_SUCCESS) { + printf("DhGexGroupCacheMissFallback: fallback ret %d\n", ret); + result = -223; + goto out; + } + /* Must re-select the same 4096-bit group 16 the wire used, with a + * non-NULL generator. */ + if (group == NULL || groupSz != 512 || generator == NULL || + generatorSz == 0) { + printf("DhGexGroupCacheMissFallback: group sz %u gen %p\n", + groupSz, (void*)generator); + result = -224; + goto out; + } + +out: + if (ssh != NULL) + wolfSSH_free(ssh); + if (ctx != NULL) + wolfSSH_CTX_free(ctx); + return result; +#else + return 0; +#endif /* !WOLFSSH_NO_DH_GROUP16_SHA512 */ +} + #endif /* WOLFSSH_TEST_INTERNAL && !WOLFSSH_NO_DH_GEX_SHA256 */ @@ -5309,6 +5393,11 @@ int wolfSSH_UnitTest(int argc, char** argv) printf("DhGexGroupSendHashConsistency: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + + unitResult = test_DhGexGroupCacheMissFallback(); + printf("DhGexGroupCacheMissFallback: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; #endif #ifdef WOLFSSH_TEST_INTERNAL