wp_drbg: gate wc_RNG_DRBG_Reseed on FIPS version#408
Merged
aidangarske merged 1 commit intoJun 23, 2026
Conversation
aidangarske
reviewed
Jun 16, 2026
aidangarske
left a comment
Member
There was a problem hiding this comment.
Skoll Multi-Scan Review
Modes: review + review-security + bugsOverall recommendation: COMMENT
Findings: 5 total — 4 posted, 1 skipped
4 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Medium] [review+review-security] FIPS fallback reseed re-instantiates from OS entropy, bypassing the parent-seed/seccomp design and treating caller entropy as nonce —
src/wp_drbg.c:440-485 - [Medium] [review] New error-state and explicit-entropy reseed paths have no test coverage —
src/wp_drbg.c:328-333,440-485,612-644 - [Low] [review-security+bugs] wp_drbg_get_seed does not check the new rngError state, operating on a de-instantiated RNG after a failed fallback reseed —
src/wp_drbg.c:734-756 - [Low] [review] wc_FreeRng() return code ignored in fallback reseed —
src/wp_drbg.c:468
Skipped findings
- [Low]
utils-wolfssl.sh adds untested v5.2.3 - --enable-fips=v5.2.3 mapping that changes prior behavior
Review generated by Skoll
aidangarske
reviewed
Jun 22, 2026
aidangarske
left a comment
Member
There was a problem hiding this comment.
Skoll Multi-Scan Review
Modes: review + review-securityOverall recommendation: COMMENT
Findings: 6 total — 4 posted, 2 skipped
4 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Medium] [review] Untested v5.2.3 FIPS tag mapping added without validation —
scripts/utils-wolfssl.sh:186-188 - [Medium] [review] Bare scope block introduced in wp_drbg_reseed native path —
src/wp_drbg.c:400-455 - [Low] [review] Non-ASCII em-dash in shell comment —
scripts/utils-wolfssl.sh:183 - [Low] [review-security] Bare scope block introduced in wp_drbg_reseed native reseed path —
src/wp_drbg.c:400-455
Skipped findings
- [Info]
FIPS v5.x fallback reseed deviates from SP 800-90A reseed semantics - [Low]
Missing unit coverage for fallback reseed error-state path
Review generated by Skoll
Use native wc_RNG_DRBG_Reseed only where reliably exported (non-FIPS >=5.7.2, FIPS v6+); else re-instantiate via wc_FreeRng()+wc_InitRngNonce(). Parent-path instantiate uses the FIPS-validated wc_InitRngNonce. Also fix install_wolfssl cwd imbalance on FIPS-bundle builds and add a reseed-with-entropy test.
1f1026d to
05f28c3
Compare
aidangarske
approved these changes
Jun 23, 2026
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.
wp_drbg_reseed references wc_RNG_DRBG_Reseed, but that is not always an exported symbol. It is WOLFSSL_API in non-FIPS >= 5.7.2 and in FIPS v6+, while FIPS v5.x commercial bundles are inconsistent -- the same v5.2.4 cert is WOLFSSL_LOCAL on a 5.8.4 wrapper but WOLFSSL_API on a 5.9.1 wrapper. Linking it fails (undefined reference to wc_RNG_DRBG_Reseed) on the bundles that keep it LOCAL, which the previous version-only gate did not account for.
Validated: build + full unit suite pass (136/0) against the 5.8.4 and 5.9.1 wolfSSL wrappers x v5.2.1 and v5.2.4 commercial FIPS bundles, plus non-FIPS.
Note on the FIPS v5.x fallback path: caller-supplied entropy/addIn is folded into wc_InitRngNonce as the nonce, not mixed as DRBG entropy_input, and predResist is not honored -- wolfCrypt re-seeds internally via its seed source. This is the available behavior where the module does not export wc_RNG_DRBG_Reseed, and differs from the native path where caller entropy is reseed input.