Validate DSA parameters when verifying DSA key.

kareem-wolfssl · kareem-wolfssl · commit e886d50d1b97 · 2026-05-06T15:10:43.000-07:00
Thanks to Kr0emer for the report.
diff --git a/tests/api/test_dsa.c b/tests/api/test_dsa.c
@@ -578,3 +578,142 @@ int test_wc_DsaExportKeyRaw(void)
     return EXPECT_RESULT();
 } /* END test_wc_DsaExportParamsRaw */
 
+
+/*
+ * Testing wc_DsaCheckPubKey() and DSA verify rejecting malformed public
+ * keys / domain parameters (e.g. g = 1, y = 1 forgery class).
+ *
+ * Requires WOLFSSL_PUBLIC_MP so the test can manipulate mp_int fields
+ * directly to construct malformed keys without going through the (already
+ * partially validating) import paths.
+ */
+int test_wc_DsaCheckPubKey(void)
+{
+    EXPECT_DECLS;
+#if !defined(NO_DSA) && !defined(WC_FIPS_186_5_PLUS) && \
+    !defined(HAVE_SELFTEST) && !defined(HAVE_FIPS) && defined(WOLFSSL_PUBLIC_MP)
+    DsaKey key;
+    int    answer = -1;
+    int    ret;
+    /* Well-formed FIPS 186-4 [L=1024, N=160] domain parameters.
+     * Same vector as used by test_wc_DsaImportParamsRaw above. */
+    const char* p =
+        "d38311e2cd388c3ed698e82fdf88eb92b5a9a483dc88005d"
+        "4b725ef341eabb47cf8a7a8a41e792a156b7ce97206c4f9c"
+        "5ce6fc5ae7912102b6b502e59050b5b21ce263dddb2044b6"
+        "52236f4d42ab4b5d6aa73189cef1ace778d7845a5c1c1c71"
+        "47123188f8dc551054ee162b634d60f097f719076640e209"
+        "80a0093113a8bd73";
+    const char* q = "96c5390a8b612c0e422bb2b0ea194a3ec935a281";
+    const char* g =
+        "06b7861abbd35cc89e79c52f68d20875389b127361ca66822"
+        "138ce4991d2b862259d6b4548a6495b195aa0e0b6137ca37e"
+        "b23b94074d3c3d300042bdf15762812b6333ef7b07ceba786"
+        "07610fcc9ee68491dbc1e34cd12615474e52b18bc934fb00c"
+        "61d39e7da8902291c4434a4e2224c3f4fd9f93cd6f4f17fc0"
+        "76341a7e7d9";
+    /* For verify: a SHA-1-sized digest (any value) — without the fix the
+     * forgery (r=1, s=1) verifies for ANY digest. */
+    byte digest[WC_SHA_DIGEST_SIZE];
+    /* signature is r || s, each q-sized (20 bytes for 160-bit q). */
+    byte sig[2 * 20];
+
+    XMEMSET(&key, 0, sizeof(DsaKey));
+    XMEMSET(digest, 0xAA, sizeof(digest));
+
+    ExpectIntEQ(wc_InitDsaKey(&key), 0);
+
+    /* --- Bad-arg coverage. --- */
+    ExpectIntEQ(wc_DsaCheckPubKey(NULL), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+
+    /* Load good (p, q, g). */
+    ExpectIntEQ(wc_DsaImportParamsRaw(&key, p, q, g), 0);
+    /* Compute a well-formed y = g^x mod p using x = 2 so the baseline
+     * passes wc_DsaCheckPubKey. */
+    ExpectIntEQ(mp_set(&key.x, 2), 0);
+    ExpectIntEQ(mp_exptmod(&key.g, &key.x, &key.p, &key.y), 0);
+    key.type = DSA_PUBLIC;
+    /* Sanity: a well-formed key should pass validation. */
+    ExpectIntEQ(wc_DsaCheckPubKey(&key), 0);
+
+    /* Now set g = 1, y = 1, sig = (1, 1).
+       This should fail validation. */
+    ExpectIntEQ(mp_set(&key.g, 1), 0);
+    ExpectIntEQ(mp_set(&key.y, 1), 0);
+    XMEMSET(sig, 0, sizeof(sig));
+    sig[19] = 0x01; /* r = 1 */
+    sig[39] = 0x01; /* s = 1 */
+    answer = -1;
+    ret = wc_DsaVerify(digest, sig, &key, &answer);
+    ExpectIntEQ(ret, WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+    ExpectIntNE(answer, 1);
+    ExpectIntEQ(wc_DsaCheckPubKey(&key), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+
+    /* g out of range: g = 0 */
+    ExpectIntEQ(mp_set(&key.g, 0), 0);
+    /* restore a valid y for the remaining checks */
+    ExpectIntEQ(mp_read_radix(&key.g, g, MP_RADIX_HEX), 0);
+    ExpectIntEQ(mp_exptmod(&key.g, &key.x, &key.p, &key.y), 0);
+    ExpectIntEQ(mp_set(&key.g, 0), 0);
+    ExpectIntEQ(wc_DsaCheckPubKey(&key), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+
+    /* g out of range: g = 1 */
+    ExpectIntEQ(mp_set(&key.g, 1), 0);
+    ExpectIntEQ(wc_DsaCheckPubKey(&key), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+
+    /* g = p (>= p) */
+    ExpectIntEQ(mp_copy(&key.p, &key.g), 0);
+    ExpectIntEQ(wc_DsaCheckPubKey(&key), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+
+    /* g in range [2, p-1] but NOT in the order-q subgroup.
+     * g = 2 will generate a subgroup of order != q, so 2^q mod p != 1. */
+    ExpectIntEQ(mp_set(&key.g, 2), 0);
+    ExpectIntEQ(wc_DsaCheckPubKey(&key), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+
+    /* y out of range: restore good g and a valid y between cases. */
+    ExpectIntEQ(mp_read_radix(&key.g, g, MP_RADIX_HEX), 0);
+    ExpectIntEQ(mp_exptmod(&key.g, &key.x, &key.p, &key.y), 0);
+    /* Confirm the restoration produced a valid key. */
+    ExpectIntEQ(wc_DsaCheckPubKey(&key), 0);
+
+    /* y = 0 */
+    ExpectIntEQ(mp_set(&key.y, 0), 0);
+    ExpectIntEQ(wc_DsaCheckPubKey(&key), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+
+    /* y = 1 */
+    ExpectIntEQ(mp_set(&key.y, 1), 0);
+    ExpectIntEQ(wc_DsaCheckPubKey(&key), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+
+    /* y = p */
+    ExpectIntEQ(mp_copy(&key.p, &key.y), 0);
+    ExpectIntEQ(wc_DsaCheckPubKey(&key), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+
+    /* y in range but NOT in the order-q subgroup: y = 2. */
+    ExpectIntEQ(mp_set(&key.y, 2), 0);
+    ExpectIntEQ(wc_DsaCheckPubKey(&key), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+
+    /* q does not divide (p - 1). Replace q with (p - 2). This is plain
+     * integer arithmetic (no primality assumption on p): for any integer
+     * p > 3, p - 1 = 1 * (p - 2) + 1, so (p - 1) mod (p - 2) = 1, which
+     * is deterministically non-zero. q' = p-2 is also > 1 and is not
+     * compared against p in DsaCheckPubKey, so the divisibility check
+     * is the only one that fires. */
+    ExpectIntEQ(mp_exptmod(&key.g, &key.x, &key.p, &key.y), 0);
+    ExpectIntEQ(mp_copy(&key.p, &key.q), 0);     /* q = p   */
+    ExpectIntEQ(mp_sub_d(&key.q, 2, &key.q), 0); /* q = p-2 */
+    ExpectIntEQ(wc_DsaCheckPubKey(&key), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+    /* Restore the original q for any subsequent checks. */
+    ExpectIntEQ(mp_read_radix(&key.q, q, MP_RADIX_HEX), 0);
+
+    /* p, q sanity floors: p = 1 or q = 1 must be rejected. */
+    ExpectIntEQ(mp_set(&key.p, 1), 0);
+    ExpectIntEQ(wc_DsaCheckPubKey(&key), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+    ExpectIntEQ(mp_read_radix(&key.p, p, MP_RADIX_HEX), 0);
+    ExpectIntEQ(mp_set(&key.q, 1), 0);
+    ExpectIntEQ(wc_DsaCheckPubKey(&key), WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+
+    wc_FreeDsaKey(&key);
+#endif
+    return EXPECT_RESULT();
+} /* END test_wc_DsaCheckPubKey */
+
diff --git a/tests/api/test_dsa.h b/tests/api/test_dsa.h
@@ -34,6 +34,7 @@ int test_wc_DsaImportParamsRaw(void);
 int test_wc_DsaImportParamsRawCheck(void);
 int test_wc_DsaExportParamsRaw(void);
 int test_wc_DsaExportKeyRaw(void);
+int test_wc_DsaCheckPubKey(void);
 
 #define TEST_DSA_DECLS                                          \
     TEST_DECL_GROUP("dsa", test_wc_InitDsaKey),                 \
@@ -45,6 +46,7 @@ int test_wc_DsaExportKeyRaw(void);
     TEST_DECL_GROUP("dsa", test_wc_DsaImportParamsRaw),         \
     TEST_DECL_GROUP("dsa", test_wc_DsaImportParamsRawCheck),    \
     TEST_DECL_GROUP("dsa", test_wc_DsaExportParamsRaw),         \
-    TEST_DECL_GROUP("dsa", test_wc_DsaExportKeyRaw)
+    TEST_DECL_GROUP("dsa", test_wc_DsaExportKeyRaw),            \
+    TEST_DECL_GROUP("dsa", test_wc_DsaCheckPubKey)
 
 #endif /* WOLFCRYPT_TEST_DSA_H */
diff --git a/wolfcrypt/src/dsa.c b/wolfcrypt/src/dsa.c
@@ -94,6 +94,102 @@ void wc_FreeDsaKey(DsaKey* key)
 }
 
 
+/* Validate DSA domain parameters and public key.
+ *
+ * Performs the following checks (subset of FIPS 186-4 / SP 800-89):
+ *   - p > 1 and q > 1
+ *   - q divides (p - 1)            (FIPS 186-4 A.1.1.2)
+ *   - 1 < g < p
+ *   - 1 < y < p
+ *   - g^q mod p == 1  (g generates the order-q subgroup)
+ *   - y^q mod p == 1  (y is in the order-q subgroup)
+ *
+ * Note: this routine does not run primality tests on p or q. Full FIPS
+ * 186-4 domain-parameter validation additionally requires that p and q be
+ * prime; callers that need that level of assurance should use
+ * wc_DsaImportParamsRawCheck() (which exercises p) and/or run
+ * mp_prime_is_prime_ex() on q at import time.
+ *
+ * key - pointer to DsaKey populated with p, q, g, and y.
+ * return 0 on success, BAD_FUNC_ARG when the key fails validation, or a
+ *        negative error code on internal failure.
+ */
+int wc_DsaCheckPubKey(DsaKey* key)
+{
+    int err = MP_OKAY;
+#if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC)
+    mp_int* tmp = NULL;
+    mp_int* tmp2 = NULL;
+#else
+    mp_int tmp[1];
+    mp_int tmp2[1];
+#endif
+
+    if (key == NULL)
+        return BAD_FUNC_ARG;
+
+    /* p and q must be at least 2 */
+    if (mp_cmp_d(&key->p, 1) != MP_GT || mp_cmp_d(&key->q, 1) != MP_GT)
+        return BAD_FUNC_ARG;
+
+    /* 1 < g < p */
+    if (mp_cmp_d(&key->g, 1) != MP_GT || mp_cmp(&key->g, &key->p) != MP_LT)
+        return BAD_FUNC_ARG;
+
+    /* 1 < y < p */
+    if (mp_cmp_d(&key->y, 1) != MP_GT || mp_cmp(&key->y, &key->p) != MP_LT)
+        return BAD_FUNC_ARG;
+
+#if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC)
+    tmp = (mp_int*)XMALLOC(sizeof(*tmp), key->heap, DYNAMIC_TYPE_TMP_BUFFER);
+    if (tmp == NULL)
+        return MEMORY_E;
+    tmp2 = (mp_int*)XMALLOC(sizeof(*tmp2), key->heap, DYNAMIC_TYPE_TMP_BUFFER);
+    if (tmp2 == NULL) {
+        XFREE(tmp, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
+        return MEMORY_E;
+    }
+#endif
+
+    err = mp_init_multi(tmp, tmp2, NULL, NULL, NULL, NULL);
+    if (err != MP_OKAY) {
+#if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC)
+        XFREE(tmp2, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
+        XFREE(tmp, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
+#endif
+        return err;
+    }
+
+    /* q divides (p - 1): tmp2 = (p - 1) mod q, must be 0. */
+    if (err == MP_OKAY)
+        err = mp_sub_d(&key->p, 1, tmp);
+    if (err == MP_OKAY)
+        err = mp_mod(tmp, &key->q, tmp2);
+    if (err == MP_OKAY && !mp_iszero(tmp2))
+        err = BAD_FUNC_ARG;
+
+    /* g^q mod p == 1 */
+    if (err == MP_OKAY)
+        err = mp_exptmod(&key->g, &key->q, &key->p, tmp);
+    if (err == MP_OKAY && mp_cmp_d(tmp, 1) != MP_EQ)
+        err = BAD_FUNC_ARG;
+
+    /* y^q mod p == 1 */
+    if (err == MP_OKAY)
+        err = mp_exptmod(&key->y, &key->q, &key->p, tmp);
+    if (err == MP_OKAY && mp_cmp_d(tmp, 1) != MP_EQ)
+        err = BAD_FUNC_ARG;
+
+    mp_clear(tmp);
+    mp_clear(tmp2);
+#if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC)
+    XFREE(tmp2, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
+    XFREE(tmp, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
+#endif
+
+    return err;
+}
+
 /* validate that (L,N) match allowed sizes from FIPS 186-4, Section 4.2.
  * modLen - represents L, the size of p (prime modulus) in bits
  * divLen - represents N, the size of q (prime divisor) in bits
@@ -1070,6 +1166,13 @@ int wc_DsaVerify_ex(const byte* digest, word32 digestSz, const byte* sig,
             break;
         }
 
+        /* Validate domain parameters and public key before doing any
+         * signature math. */
+        ret = wc_DsaCheckPubKey(key);
+        if (ret != 0) {
+            break;
+        }
+
         /* set r and s from signature */
         if (mp_read_unsigned_bin(r, sig, (word32)qSz) != MP_OKAY ||
             mp_read_unsigned_bin(s, sig + qSz, (word32)qSz) != MP_OKAY) {
diff --git a/wolfssl/wolfcrypt/dsa.h b/wolfssl/wolfcrypt/dsa.h
@@ -95,6 +95,7 @@ WOLFSSL_API int wc_DsaKeyToDer(DsaKey* key, byte* output, word32 inLen);
 WOLFSSL_API int wc_SetDsaPublicKey(byte* output, DsaKey* key,
                                    int outLen, int with_header);
 WOLFSSL_API int wc_DsaKeyToPublicDer(DsaKey* key, byte* output, word32 inLen);
+WOLFSSL_API int wc_DsaCheckPubKey(DsaKey* key);
 
 #ifdef WOLFSSL_KEY_GEN
 WOLFSSL_API int wc_MakeDsaKey(WC_RNG *rng, DsaKey *dsa);
