Skip to content

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

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_4082
Open

Enforce DH minimum prime size on the parameters-only generation path#427
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_4082

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown

Summary

DH parameter generation could produce keys weaker than the provider's own declared minimum on the parameters-only path.

wp_dh_params_validate — which enforces WP_DH_MIN_BITS — was only invoked when OSSL_KEYMGMT_SELECT_KEYPAIR was set. A caller requesting only OSSL_KEYMGMT_SELECT_DOMAIN_PARAMETERS (parameter-only generation) could therefore have the provider generate DH parameters below the minimum with no check, and wp_dh_gen_set_params read OSSL_PKEY_PARAM_FFC_PBITS into ctx->bits with no range check.

This is most impactful in FIPS builds: WP_DH_MIN_BITS is 2048, but wolfCrypt's own wc_DhGenerateParams floor is 1024, so sub-2048 parameters could be generated unchecked. In non-FIPS builds wolfCrypt's floor (1024) already equals WP_DH_MIN_BITS, so there the change is defense-in-depth.

Changes

  • wp_dh_gen: run wp_dh_params_validate after parameters are generated or copied, in every selection path, before key pair generation (previously only under the keypair branch).
  • wp_dh_gen_set_params: reject an FFC_PBITS request below WP_DH_MIN_BITS at set-params time (fail-fast for callers that check the return).
  • test_dh_pgen_min_bits (new): asserts a below-minimum prime length is rejected and a request at exactly the minimum still succeeds. Gated on HAVE_FIPS to mirror WP_DH_MIN_BITS (1024 non-FIPS / 2048 FIPS).

Testing

  • Full DH unit suite passes (tests 77–86), including the new test_dh_pgen_min_bits.
  • Negative control: disabling the size guard makes the new test fail at the set-params assertion, confirming it is non-vacuous.

Notes

  • WP_DH_MIN_BITS is left at 1024 for non-FIPS to avoid breaking interop with legitimate 1024-bit DH consumers. Raising it to 2048 (per NIST SP 800-131A / Logjam) is a separate policy decision and not included here.
  • The relocated wp_dh_params_validate is the actual enforcer; OpenSSL buffers prime_len and the set-params guard is a secondary fail-fast.

@yosuke-wolfssl yosuke-wolfssl self-assigned this Jul 3, 2026
Copilot AI review requested due to automatic review settings July 3, 2026 01:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR closes a security gap in the DH keymgmt generation path where parameters-only generation could bypass the provider’s minimum prime-size enforcement, particularly impacting FIPS builds.

Changes:

  • Enforce WP_DH_MIN_BITS for DH parameter generation across all selection paths by validating generated/copied parameters before any keypair generation.
  • Fail fast in wp_dh_gen_set_params when a caller requests OSSL_PKEY_PARAM_FFC_PBITS below WP_DH_MIN_BITS.
  • Add a new unit test to assert below-minimum DH prime sizes are rejected while the minimum size still succeeds (with FIPS/non-FIPS thresholds).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
test/unit.h Registers the new DH minimum-prime-size test prototype.
test/unit.c Adds the new DH minimum-prime-size test to the unit test table.
test/test_dh.c Implements test_dh_pgen_min_bits to exercise set-params rejection and minimum-size success.
src/wp_dh_kmgmt.c Enforces minimum DH prime size in set-params and ensures parameter validation runs for parameters-only generation as well.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/wp_dh_kmgmt.c

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #427

Scan targets checked: wolfprovider-bugs, wolfprovider-src

No new issues found in the changed files. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants