DH GEX group selection hardening

src/internal.c

@@ -7411,10 +7411,23 @@ 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) {
+            /* 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;
+            *idx += len;
+            return WS_SUCCESS;
+        }
+
         begin = *idx;
         ret = GetMpint(&primeGroupSz, &primeGroup, buf, len, &begin);
         if (ret == WS_SUCCESS && primeGroupSz > (MAX_KEX_KEY_SZ + 1)) {
@@ -7426,9 +7439,16 @@ static int DoKexDhGexGroup(WOLFSSH* ssh,
         ret = GetMpint(&generatorSz, &generator, buf, len, &begin);
 
     if (ret == WS_SUCCESS) {
+        /* Enforce the floor on accept as well as on select, so a server can't
+         * hand us a weak group by ignoring the min we asked for (RFC 8270). */
+        word32 minBits = ssh->handshake->dhGexMinSz;
+
+        if (minBits < WOLFSSH_DH_GEX_MIN_BITS)
+            minBits = WOLFSSH_DH_GEX_MIN_BITS;
+
         ret = ValidateKexDhGexGroup(primeGroup, primeGroupSz,
                 generator, generatorSz,
-                ssh->handshake->dhGexMinSz,
+                minBits,
                 ssh->handshake->dhGexMaxSz,
                 ssh->rng);
     }
@@ -12617,19 +12637,6 @@ struct wolfSSH_sigKeyBlockFull {
         } sk;
 };
 
-#ifndef WOLFSSH_NO_NISTP384_MLKEM1024_SHA384
-    /* Size of ML-KEM-1024 ciphertext (1568) plus ECC P-384 component (97). */
-    #define KEX_F_SIZE 1700
-#elif !defined(WOLFSSH_NO_NISTP256_MLKEM768_SHA256) || \
-      !defined(WOLFSSH_NO_CURVE25519_MLKEM768_SHA256)
-    /* Size of ML-KEM-768 public key (1184) plus ECC/X25519 component. */
-    #define KEX_F_SIZE 1300
-#elif !defined(WOLFSSH_NO_DH_GROUP16_SHA512)
-    #define KEX_F_SIZE (512 + 1)
-#else
-    #define KEX_F_SIZE (256 + 1)
-#endif
-
 #ifdef WOLFSSH_NO_MLDSA
     #define KEX_SIG_SIZE (512)
 #else
@@ -12754,11 +12761,91 @@ static int BuildRFC6187Info(WOLFSSH* ssh, int pubKeyID,
 #endif /* WOLFSSH_CERTS */
 
 
+#ifndef WOLFSSH_NO_DH_GEX_SHA256
+
+/* Pick the smallest in-window group >= preferredBits, else the largest
+ * in-window group (RFC 4419 sec. 3, as OpenSSH's choose_dh() reads it: the
+ * comparison is >=, not the RFC's literal >). WS_DH_SIZE_E if none fits.
+ * See WOLFSSH_DH_GEX_MIN_BITS. */
+static int SelectKexDhGexGroup(word32 minBits, word32 preferredBits,
+        word32 maxBits, const byte** primeGroup, word32* primeGroupSz)
+{
+    /* Ascending by bits; the scan below depends on that order. No entry may
+     * exceed (KEX_F_SIZE - 1) * 8 bits, the server's f buffer. */
+    static const struct {
+        word32 bits;
+        const byte* group;
+        word32 groupSz;
+    } candidates[] = {
+        #ifndef WOLFSSH_NO_DH_GROUP1_SHA1
+        /* Only reachable with a lowered WOLFSSH_DH_GEX_MIN_BITS. */
+        { 1024, dhPrimeGroup1,  sizeof(dhPrimeGroup1)  },
+        #endif
+        { 2048, dhPrimeGroup14, sizeof(dhPrimeGroup14) },
+        #ifndef WOLFSSH_NO_DH_GROUP16_SHA512
+        { 4096, dhPrimeGroup16, sizeof(dhPrimeGroup16) },
+        #endif
+    };
+    word32 i;
+    word32 best = 0;
+    int haveBest = 0;
+    int ret = WS_SUCCESS;
+
+    if (primeGroup == NULL || primeGroupSz == NULL)
+        return WS_BAD_ARGUMENT;
+
+    /* Clamp up to the floor; a max below it leaves no candidate, so we reject
+     * rather than downgrade. */
+    if (minBits < WOLFSSH_DH_GEX_MIN_BITS)
+        minBits = WOLFSSH_DH_GEX_MIN_BITS;
+
+    /* Cap to the smaller of the shared secret buffer in KeyAgreeDh_server and
+     * the f buffer in SendKexDhReply, less its mpint sign pad. */
+    if (maxBits > (word32)(MAX_KEX_KEY_SZ * 8))
+        maxBits = (word32)(MAX_KEX_KEY_SZ * 8);
+    if (maxBits > (word32)((KEX_F_SIZE - 1) * 8))
+        maxBits = (word32)((KEX_F_SIZE - 1) * 8);
+
+    /* Ascending scan: keep the last in-window candidate, and stop at the first
+     * one that reaches preferredBits. */
+    for (i = 0; i < (word32)(sizeof(candidates) / sizeof(candidates[0])); i++) {
+        word32 bits = candidates[i].bits;
+
+        if (bits < minBits || bits > maxBits)
+            continue;
+        best = i;
+        haveBest = 1;
+        if (bits >= preferredBits)
+            break;
+    }
+
+    if (!haveBest) {
+        WLOG(WS_LOG_DEBUG,
+                "DH GEX: no built-in group within effective 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
@@ -12795,10 +12882,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. An unset cache means no GROUP was ever sent (the
+             * peer skipped GEX_REQUEST); there is no wire group to match, so
+             * re-select from whatever window the handshake holds. */
+            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:
@@ -13212,7 +13316,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;
             }
@@ -13460,7 +13564,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) {
@@ -14921,6 +15025,21 @@ int SendKexDhGexRequest(WOLFSSH* ssh)
     if (ssh == NULL || ssh->handshake == NULL)
         ret = WS_BAD_ARGUMENT;
 
+    /* Advertise the floor DoKexDhGexGroup enforces on the reply, so a lowered
+     * WOLFSSH_DEFAULT_GEXDH_MIN can't ask for a group we would then reject. */
+    if (ret == WS_SUCCESS) {
+        if (ssh->handshake->dhGexMinSz < WOLFSSH_DH_GEX_MIN_BITS)
+            ssh->handshake->dhGexMinSz = WOLFSSH_DH_GEX_MIN_BITS;
+        if (ssh->handshake->dhGexPreferredSz < ssh->handshake->dhGexMinSz)
+            ssh->handshake->dhGexPreferredSz = ssh->handshake->dhGexMinSz;
+        if (ssh->handshake->dhGexMaxSz < ssh->handshake->dhGexMinSz) {
+            WLOG(WS_LOG_DEBUG, "DH GEX: max %u below the %u-bit floor",
+                    ssh->handshake->dhGexMaxSz,
+                    (word32)WOLFSSH_DH_GEX_MIN_BITS);
+            ret = WS_DH_SIZE_E;
+        }
+    }
+
     if (ret == WS_SUCCESS) {
         payloadSz = MSG_ID_SZ + (UINT32_SZ * 3);
         ret = PreparePacket(ssh, payloadSz);
@@ -14964,8 +15083,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;
@@ -14976,6 +15095,33 @@ 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 so the exchange hash and shared secret
+     * (GetDHPrimeGroup) reuse exactly what goes on the wire here. */
+    if (ret == WS_SUCCESS) {
+        if (ssh->handshake->primeGroup != NULL)
+            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) {
+            /* Keep the pointer and size consistent. */
+            ssh->handshake->primeGroupSz = 0;
+            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;
@@ -20024,7 +20170,7 @@ int wolfSSH_TestSetDhKexKey(WOLFSSH* ssh)
     word32 eSz = (word32)sizeof(e);
     int keyInited = 0;
 
-    ret = GetDHPrimeGroup(ssh->handshake->kexId, &primeGroup, &primeGroupSz,
+    ret = GetDHPrimeGroup(ssh, &primeGroup, &primeGroupSz,
             &generator, &generatorSz);
     if (ret == WS_SUCCESS) {
         ret = wc_InitDhKey(privKey);
@@ -20054,6 +20200,13 @@ int wolfSSH_TestSetDhKexKey(WOLFSSH* ssh)
     return ret;
 }
 
+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_ECDH
@@ -20072,12 +20225,23 @@ int wolfSSH_TestKeyAgreeEcdh_client(WOLFSSH* ssh, byte hashId,
 
 #ifndef WOLFSSH_NO_DH_GEX_SHA256
 
+int wolfSSH_TestSendKexDhGexRequest(WOLFSSH* ssh)
+{
+    return SendKexDhGexRequest(ssh);
+}
+
 int wolfSSH_TestDoKexDhGexRequest(WOLFSSH* ssh, byte* buf, word32 len,
         word32* idx)
 {
     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)
@@ -20086,6 +20250,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

tests/regress.c

@@ -2948,6 +2948,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);