Enforce DH minimum prime size on the parameters-only generation path#427
Open
yosuke-wolfssl wants to merge 1 commit into
Open
Enforce DH minimum prime size on the parameters-only generation path#427yosuke-wolfssl wants to merge 1 commit into
yosuke-wolfssl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
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_BITSfor DH parameter generation across all selection paths by validating generated/copied parameters before any keypair generation. - Fail fast in
wp_dh_gen_set_paramswhen a caller requestsOSSL_PKEY_PARAM_FFC_PBITSbelowWP_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.
5d1f266 to
06af890
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #427
Scan targets checked: wolfprovider-bugs, wolfprovider-src
No new issues found in the changed files. ✅
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 enforcesWP_DH_MIN_BITS— was only invoked whenOSSL_KEYMGMT_SELECT_KEYPAIRwas set. A caller requesting onlyOSSL_KEYMGMT_SELECT_DOMAIN_PARAMETERS(parameter-only generation) could therefore have the provider generate DH parameters below the minimum with no check, andwp_dh_gen_set_paramsreadOSSL_PKEY_PARAM_FFC_PBITSintoctx->bitswith no range check.This is most impactful in FIPS builds:
WP_DH_MIN_BITSis 2048, but wolfCrypt's ownwc_DhGenerateParamsfloor is 1024, so sub-2048 parameters could be generated unchecked. In non-FIPS builds wolfCrypt's floor (1024) already equalsWP_DH_MIN_BITS, so there the change is defense-in-depth.Changes
wp_dh_gen: runwp_dh_params_validateafter 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 anFFC_PBITSrequest belowWP_DH_MIN_BITSat 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 onHAVE_FIPSto mirrorWP_DH_MIN_BITS(1024 non-FIPS / 2048 FIPS).Testing
test_dh_pgen_min_bits.Notes
WP_DH_MIN_BITSis 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.wp_dh_params_validateis the actual enforcer; OpenSSL buffersprime_lenand the set-params guard is a secondary fail-fast.