fix for wc_DhAgree public key validation

Reported by: Nicholas Carlini <npc@anthropic.com>

Author: Frauschi

tests/api.c

@@ -34686,6 +34686,78 @@ static int test_sniffer_chain_input_overflow(void)
 }
 #endif /* WOLFSSL_SNIFFER && WOLFSSL_SNIFFER_CHAIN_INPUT */
 
+/* Test: wc_DhAgree must reject p-1 as peer public key.
+ * ffdhe2048 p ends with ...FFFFFFFFFFFFFFFF so p-1 ends ...FFFFFFFFFFFFFFFE */
+static int test_DhAgree_rejects_p_minus_1(void)
+{
+    EXPECT_DECLS;
+#if !defined(HAVE_SELFTEST) && !defined(NO_DH) && \
+    defined(HAVE_FFDHE_2048) && !defined(WOLFSSL_NO_MALLOC) && \
+    (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0))
+    DhKey key;
+    WC_RNG rng;
+    byte priv[256], pub[256], agree[256];
+    word32 privSz = sizeof(priv), pubSz = sizeof(pub), agreeSz;
+
+    /* ffdhe2048 p - copied from wolfcrypt/src/dh.c dh_ffdhe2048_p.
+     * p ends with 0xFF*8 so p-1 ends with ...0xFE */
+    static const byte ffdhe2048_p[256] = {
+        0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,
+        0xAD,0xF8,0x54,0x58,0xA2,0xBB,0x4A,0x9A,
+        0xAF,0xDC,0x56,0x20,0x27,0x3D,0x3C,0xF1,
+        0xD8,0xB9,0xC5,0x83,0xCE,0x2D,0x36,0x95,
+        0xA9,0xE1,0x36,0x41,0x14,0x64,0x33,0xFB,
+        0xCC,0x93,0x9D,0xCE,0x24,0x9B,0x3E,0xF9,
+        0x7D,0x2F,0xE3,0x63,0x63,0x0C,0x75,0xD8,
+        0xF6,0x81,0xB2,0x02,0xAE,0xC4,0x61,0x7A,
+        0xD3,0xDF,0x1E,0xD5,0xD5,0xFD,0x65,0x61,
+        0x24,0x33,0xF5,0x1F,0x5F,0x06,0x6E,0xD0,
+        0x85,0x63,0x65,0x55,0x3D,0xED,0x1A,0xF3,
+        0xB5,0x57,0x13,0x5E,0x7F,0x57,0xC9,0x35,
+        0x98,0x4F,0x0C,0x70,0xE0,0xE6,0x8B,0x77,
+        0xE2,0xA6,0x89,0xDA,0xF3,0xEF,0xE8,0x72,
+        0x1D,0xF1,0x58,0xA1,0x36,0xAD,0xE7,0x35,
+        0x30,0xAC,0xCA,0x4F,0x48,0x3A,0x79,0x7A,
+        0xBC,0x0A,0xB1,0x82,0xB3,0x24,0xFB,0x61,
+        0xD1,0x08,0xA9,0x4B,0xB2,0xC8,0xE3,0xFB,
+        0xB9,0x6A,0xDA,0xB7,0x60,0xD7,0xF4,0x68,
+        0x1D,0x4F,0x42,0xA3,0xDE,0x39,0x4D,0xF4,
+        0xAE,0x56,0xED,0xE7,0x63,0x72,0xBB,0x19,
+        0x0B,0x07,0xA7,0xC8,0xEE,0x0A,0x6D,0x70,
+        0x9E,0x02,0xFC,0xE1,0xCD,0xF7,0xE2,0xEC,
+        0xC0,0x34,0x04,0xCD,0x28,0x34,0x2F,0x61,
+        0x91,0x72,0xFE,0x9C,0xE9,0x85,0x83,0xFF,
+        0x8E,0x4F,0x12,0x32,0xEE,0xF2,0x81,0x83,
+        0xC3,0xFE,0x3B,0x1B,0x4C,0x6F,0xAD,0x73,
+        0x3B,0xB5,0xFC,0xBC,0x2E,0xC2,0x20,0x05,
+        0xC5,0x8E,0xF1,0x83,0x7D,0x16,0x83,0xB2,
+        0xC6,0xF3,0x4A,0x26,0xC1,0xB2,0xEF,0xFA,
+        0x88,0x6B,0x42,0x38,0x61,0x28,0x5C,0x97,
+        0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF
+    };
+    byte pMinus1[256];
+
+    ExpectIntEQ(wc_InitRng(&rng), 0);
+    ExpectIntEQ(wc_InitDhKey(&key), 0);
+    ExpectIntEQ(wc_DhSetNamedKey(&key, WC_FFDHE_2048), 0);
+    ExpectIntEQ(wc_DhGenerateKeyPair(&key, &rng, priv, &privSz,
+                                      pub, &pubSz), 0);
+
+    /* Construct p-1: last byte FF -> FE */
+    XMEMCPY(pMinus1, ffdhe2048_p, sizeof(ffdhe2048_p));
+    pMinus1[255] -= 1;
+
+    /* wc_DhAgree must reject p-1 */
+    agreeSz = sizeof(agree);
+    ExpectIntNE(wc_DhAgree(&key, agree, &agreeSz, priv, privSz,
+                            pMinus1, sizeof(pMinus1)), 0);
+
+    wc_FreeDhKey(&key);
+    wc_FreeRng(&rng);
+#endif
+    return EXPECT_RESULT();
+}
+
 TEST_CASE testCases[] = {
     TEST_DECL(test_fileAccess),
 
@@ -35500,6 +35572,7 @@ TEST_CASE testCases[] = {
     TEST_DECL(test_ocsp_responder),
     TEST_TLS_DECLS,
     TEST_DECL(test_wc_DhSetNamedKey),
+    TEST_DECL(test_DhAgree_rejects_p_minus_1),
 
 #if defined(WOLFSSL_SNIFFER) && defined(WOLFSSL_SNIFFER_CHAIN_INPUT)
     TEST_DECL(test_sniffer_chain_input_overflow),

wolfcrypt/src/dh.c

@@ -2030,12 +2030,13 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
         WOLFSSL_MSG("wc_DhAgree wc_DhCheckPrivKey failed");
         return DH_CHECK_PRIV_E;
     }
+#endif
 
+    /* Always validate peer public key (2 <= y <= p-2) per SP 800-56A */
     if (wc_DhCheckPubKey(key, otherPub, pubSz) != 0) {
         WOLFSSL_MSG("wc_DhAgree wc_DhCheckPubKey failed");
         return DH_CHECK_PUB_E;
     }
-#endif
 
 #if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC)
     y = (mp_int*)XMALLOC(sizeof(mp_int), key->heap, DYNAMIC_TYPE_DH);