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
+117 −19
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
7 changes: 7 additions & 0 deletions include/wolfprovider/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@
/** Maximum supported digest name size. */
#define WP_MAX_PROPS_SIZE 80

/** Minimum accepted DH prime size in bits for parameter/key generation. */
#ifdef HAVE_FIPS
#define WP_DH_MIN_BITS 2048
#else
#define WP_DH_MIN_BITS 1024
#endif

/* DER key encoding/decoding formats. */
/** SubjectPublicKeyInfo encoding format. */
#define WP_ENC_FORMAT_SPKI 1
Expand Down
40 changes: 21 additions & 19 deletions src/wp_dh_kmgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,6 @@ static const int wp_dh_decode_nids[] = {
/** Maximum size of the group name string. */
#define WP_MAX_DH_GROUP_NAME_SZ 10

/* Min accepted bitlen for keygen */
#ifdef HAVE_FIPS
#define WP_DH_MIN_BITS 2048
#else
#define WP_DH_MIN_BITS 1024
#endif

/**
* DH key.
*/
Expand Down Expand Up @@ -1546,18 +1539,19 @@ static wp_DhGenCtx* wp_dh_gen_init(WOLFPROV_CTX* provCtx,
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_InitRng", rc);
ok = 0;
}
if (ok) {
if (!wp_dh_gen_set_params(ctx, params)) {
wc_FreeRng(&ctx->rng);
ok = 0;
}
}
/* Defaults first so init-time parameters override them. */
if (ok) {
ctx->provCtx = provCtx;
ctx->selection = selection;
ctx->bits = 2048;
ctx->generator = 2;
}
if (ok) {
if (!wp_dh_gen_set_params(ctx, params)) {
wc_FreeRng(&ctx->rng);
ok = 0;
}
}

if (!ok) {
/* Rng freed when parameters fail to set. */
Expand Down Expand Up @@ -1621,8 +1615,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 @@ -1897,12 +1898,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
87 changes: 87 additions & 0 deletions test/test_dh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "unit.h"
#include <openssl/core_names.h>
#include <openssl/decoder.h>
#include <wolfprovider/internal.h>

#ifdef WP_HAVE_DH

Expand Down Expand Up @@ -1428,4 +1429,90 @@ int test_dh_import_group_no_nul(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. Sizes come
* from the provider's own WP_DH_MIN_BITS so the two cannot diverge. */
#define TEST_DH_MIN_BITS WP_DH_MIN_BITS
#define TEST_DH_WEAK_BITS (WP_DH_MIN_BITS / 2)
int test_dh_pgen_min_bits(void *data)
{
int err = 0;
int rc;
EVP_PKEY_CTX *ctx = NULL;
EVP_PKEY *keyParams = NULL;
EVP_PKEY *weakParams = NULL;
BIGNUM *prime = 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.

@yosuke-wolfssl yosuke-wolfssl Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I reworked the regression test.

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;

/* A caller that ignores the set-params failure must never end up with
* below-minimum parameters: generation either fails, or the prime it
* produced is at least the minimum size. */
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) {
(void)EVP_PKEY_CTX_set_dh_paramgen_prime_len(ctx, TEST_DH_WEAK_BITS);
if (EVP_PKEY_paramgen(ctx, &weakParams) == 1) {
err = EVP_PKEY_get_bn_param(weakParams, OSSL_PKEY_PARAM_FFC_P,
&prime) != 1;
if ((err == 0) && (BN_num_bits(prime) < TEST_DH_MIN_BITS)) {
PRINT_MSG("paramgen produced a prime below the 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");
}
}

BN_free(prime);
EVP_PKEY_free(weakParams);
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 @@ -321,6 +321,7 @@ TEST_CASE test_case[] = {
TEST_DECL(test_dh_fromdata_oversize, NULL),
TEST_DECL(test_dh_param_check_explicit, NULL),
TEST_DECL(test_dh_import_group_no_nul, 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 @@ -331,6 +331,7 @@ int test_dh_x963_kdf(void *data);
int test_dh_fromdata_oversize(void *data);
int test_dh_param_check_explicit(void *data);
int test_dh_import_group_no_nul(void *data);
int test_dh_pgen_min_bits(void *data);
#endif /* WP_HAVE_DH */

#ifdef WP_HAVE_ECC
Expand Down
Loading