Skip to content

Enforce DH minimum prime size on the parameters-only generation path #427

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yosuke-wolfssl wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Enforce DH minimum prime size on the parameters-only generation path #427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions src/wp_dh_kmgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1584,8 +1584,15 @@ static int wp_dh_gen_set_params(wp_DhGenCtx* ctx, const OSSL_PARAM params[])
WOLFPROV_ENTER(WP_LOG_COMP_DH, "wp_dh_gen_set_params");

p = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_FFC_PBITS);
if ((p != NULL) && (!OSSL_PARAM_get_int(p, &ctx->bits))) {
ok = 0;
if (p != NULL) {
if (!OSSL_PARAM_get_int(p, &ctx->bits)) {
ok = 0;
}
/* Reject a prime size below the provider minimum. */
if (ok && (ctx->bits < WP_DH_MIN_BITS)) {
ERR_raise(ERR_LIB_PROV, PROV_R_KEY_SIZE_TOO_SMALL);
ok = 0;
}
}
if (ok) {
p = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_DH_PRIV_LEN);
Expand Down Expand Up @@ -1860,12 +1867,13 @@ static wp_Dh* wp_dh_gen(wp_DhGenCtx *ctx, OSSL_CALLBACK *cb, void *cbArg)
else if (!wp_dh_gen_copy_parameters(ctx, dh)) {
ok = 0;
}
/* Validate parameters meet minimum requirements in every path. */
if (ok && (!wp_dh_params_validate(dh))) {
ok = 0;
}
/* Generate key pair if requested. */
if (ok && ((ctx->selection & OSSL_KEYMGMT_SELECT_KEYPAIR) != 0)) {
if (!wp_dh_params_validate(dh)) {
ok = 0;
}
if (ok && (!wp_dh_gen_keypair(ctx, dh))) {
if (!wp_dh_gen_keypair(ctx, dh)) {
ok = 0;
}
}
Expand Down
63 changes: 63 additions & 0 deletions test/test_dh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1331,4 +1331,67 @@ int test_dh_fromdata_oversize(void *data)
return err;
}

/* Enforce the minimum prime size: a below-minimum request must not produce
* parameters, while a request at the minimum must still succeed. Gated on the
* same HAVE_FIPS condition as WP_DH_MIN_BITS so the two cannot diverge. */
#ifdef HAVE_FIPS
#define TEST_DH_MIN_BITS 2048
#define TEST_DH_WEAK_BITS 1024
#else
#define TEST_DH_MIN_BITS 1024
#define TEST_DH_WEAK_BITS 512
#endif
int test_dh_pgen_min_bits(void *data)
{
int err = 0;
int rc;
EVP_PKEY_CTX *ctx = NULL;
EVP_PKEY *keyParams = NULL;

(void)data;

PRINT_MSG("Testing DH parameter generation minimum prime size enforcement");

/* Below-minimum size must be rejected at set-params time. */

@aidangarske aidangarske Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [Medium] Regression test does not exercise relocated generation-time validation
💡 SUGGEST test

The PR moves wp_dh_params_validate(dh) so generated or copied parameters are validated in every selection path, but the new weak-size test stops at EVP_PKEY_CTX_set_dh_paramgen_prime_len(). With the fail-fast guard in wp_dh_gen_set_params() present, this test would still pass if the relocated validation in wp_dh_gen() were accidentally reverted. That leaves the main behavioral change under-tested, especially for the caller pattern described in the PR where a caller ignores the set-params return and continues to parameter generation.

Recommendation: Keep the existing fail-fast assertion, but add a second assertion covering generation-time rejection so the test fails if the wp_dh_gen() validation move is removed. Add a FIPS-specific check that deliberately continues after the below-minimum set_dh_paramgen_prime_len() failure and asserts EVP_PKEY_paramgen() still fails, exercising the relocated wp_dh_params_validate() path for 1024-bit DH in FIPS.

ctx = EVP_PKEY_CTX_new_from_name(wpLibCtx, "DH", NULL);
if (ctx == NULL) {
err = 1;
}
if (err == 0) {
err = EVP_PKEY_paramgen_init(ctx) != 1;
}
if (err == 0) {
rc = EVP_PKEY_CTX_set_dh_paramgen_prime_len(ctx, TEST_DH_WEAK_BITS);
if (rc == 1) {
PRINT_MSG("set_dh_paramgen_prime_len accepted a below-minimum size");
err = 1;
}
}
EVP_PKEY_CTX_free(ctx);
ctx = NULL;

/* Size at the minimum must still succeed. */
if (err == 0) {
ctx = EVP_PKEY_CTX_new_from_name(wpLibCtx, "DH", NULL);
err = ctx == NULL;
}
if (err == 0) {
err = EVP_PKEY_paramgen_init(ctx) != 1;
}
if (err == 0) {
err = EVP_PKEY_CTX_set_dh_paramgen_prime_len(ctx, TEST_DH_MIN_BITS)
!= 1;
}
if (err == 0) {
err = EVP_PKEY_paramgen(ctx, &keyParams) != 1;
if (err != 0) {
PRINT_MSG("DH paramgen failed at the minimum prime size");
}
}

EVP_PKEY_free(keyParams);
EVP_PKEY_CTX_free(ctx);
return err;
}

#endif /* WP_HAVE_DH */
1 change: 1 addition & 0 deletions test/unit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ TEST_CASE test_case[] = {
TEST_DECL(test_dh_x963_kdf, NULL),
#endif
TEST_DECL(test_dh_fromdata_oversize, NULL),
TEST_DECL(test_dh_pgen_min_bits, NULL),
#ifndef WOLFPROV_QUICKTEST
TEST_DECL(test_dh_get_params, NULL),
#endif
Expand Down
1 change: 1 addition & 0 deletions test/unit.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ int test_dh_pad(void *data);
int test_dh_x963_kdf(void *data);
#endif
int test_dh_fromdata_oversize(void *data);
int test_dh_pgen_min_bits(void *data);
#endif /* WP_HAVE_DH */

#ifdef WP_HAVE_ECC
Expand Down
Loading