Skip to content

wp_drbg: gate wc_RNG_DRBG_Reseed on FIPS version #408

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
ColtonWilley wants to merge 3 commits into
base: master
Choose a base branch
from
+229 −45
Open

wp_drbg: gate wc_RNG_DRBG_Reseed on FIPS version #408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a05f200
wp_drbg: gate wc_RNG_DRBG_Reseed on FIPS version
ColtonWilley Jun 16, 2026
4d22a1b
wp_drbg: handle null reseed entropy, keep reseed seccomp-safe
ColtonWilley Jun 16, 2026
1f1026d
settings: gate DRBG reseed conservatively (FIPS v5.x linkage varies b…
ColtonWilley Jun 17, 2026
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
18 changes: 18 additions & 0 deletions include/wolfprovider/settings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,24 @@
#include <wolfssl/version.h>
#include <wolfssl/wolfcrypt/settings.h>

/* wc_RNG_DRBG_Reseed is WOLFSSL_API in non-FIPS >= 5.7.2 and in FIPS v6+.
* Across FIPS v5.x commercial bundles its linkage is inconsistent: some v5.2.4
* bundles export it and others keep it WOLFSSL_LOCAL (it depends on both the
* wolfSSL wrapper version and the FIPS cert), so it cannot be predicted from
* the version macros. Use the native in-place reseed only where the symbol is
* reliably exported and fall back to DRBG re-instantiation otherwise (which
* links on every build). Define WP_NO_DRBG_RESEED to force the fallback for a
* bundle that keeps the symbol WOLFSSL_LOCAL despite its version. */
#if defined(WP_NO_DRBG_RESEED)
/* caller forced the re-instantiation fallback */
#elif !defined(HAVE_FIPS)
#if LIBWOLFSSL_VERSION_HEX >= 0x05007002
#define WP_HAVE_DRBG_RESEED
#endif
#elif defined(HAVE_FIPS_VERSION_MAJOR) && HAVE_FIPS_VERSION_MAJOR >= 6
#define WP_HAVE_DRBG_RESEED
#endif

#define WP_HAVE_DIGEST
#if !defined(NO_MD5)
#define WP_HAVE_MD5
Expand Down
8 changes: 8 additions & 0 deletions scripts/utils-wolfssl.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,15 @@ install_wolfssl() {
# Determine configure option from tag
local fips_configure_arg=""
case "$fips_tag" in
v5.2.4|linuxv5.2.4)
# Distinct module (SP math, PATCH 4) — not the v5/cert4718 base

@aidangarske aidangarske Jun 22, 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.

🔵 [Low] Non-ASCII em-dash in shell comment

The added comment uses a Unicode em-dash (—) in an otherwise ASCII source tree. For consistency with the rest of the C/shell sources and to avoid encoding surprises in tooling/diffs, prefer a plain ASCII hyphen.

Fix: Replace the em-dash with an ASCII hyphen.

fips_configure_arg="v5.2.4"
;;
v5.2.3|linuxv5.2.3)

@aidangarske aidangarske Jun 22, 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] Untested v5.2.3 FIPS tag mapping added without validation

The PR adds a new case mapping v5.2.3|linuxv5.2.3 to --enable-fips=v5.2.3. The PR description and validation only cover v5.2.1 and v5.2.4 bundles; v5.2.3 is neither mentioned nor tested, and unlike the v5.2.4 case it carries no comment explaining why it is a distinct module. If wolfSSL's configure does not accept --enable-fips=v5.2.3 as a recognized value, builds invoked with that tag will now fail at configure time, whereas before this change they fell through to the working v5 mapping. This silently broadens behavior for an unvalidated tag.

Fix: Confirm --enable-fips=v5.2.3 is a valid configure value for the targeted wolfSSL versions. If it is not validated, drop the v5.2.3 case so it falls through to v5 (the prior, working behavior), or add a comment documenting which cert/module it targets like the v5.2.4 case does.

fips_configure_arg="v5.2.3"
;;
v5.2.*|v5.3.*|v5.4.*|v5.5.*|linuxv5.*)
# v5.2.1/cert4718 and other v5.x map to the base v5 module
fips_configure_arg="v5"
;;
v6.*|linuxv6.*)
Expand Down
227 changes: 182 additions & 45 deletions src/wp_drbg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <openssl/evp.h>
#include <openssl/hmac.h>

#include <wolfprovider/settings.h>
#include <wolfprovider/alg_funcs.h>
#include <wolfprovider/internal.h>

Expand Down Expand Up @@ -61,6 +62,10 @@ typedef struct wp_DrbgCtx {
OSSL_FUNC_rand_get_seed_fn* parentGetSeed;
/** Parent's clear_seed function. */
OSSL_FUNC_rand_clear_seed_fn* parentClearSeed;
#ifndef WP_HAVE_DRBG_RESEED
Comment thread
aidangarske marked this conversation as resolved.
/** Set when a failed reseed re-instantiation left ctx->rng de-instantiated. */
int rngError;
#endif
} wp_DrbgCtx;


Expand Down Expand Up @@ -143,6 +148,8 @@ static void wp_drbg_free(wp_DrbgCtx* ctx)
}
}

static int wp_drbg_uninstantiate(wp_DrbgCtx* ctx);

/**
* Instantiate a new DRBG.
*
Expand Down Expand Up @@ -171,6 +178,11 @@ static int wp_drbg_instantiate(wp_DrbgCtx* ctx, unsigned int strength,
ok = 0;
}

/* Free any existing DRBG before re-allocating to avoid a leak. */
if (ok && ctx->rng != NULL) {
wp_drbg_uninstantiate(ctx);
}

if (ok && ctx->parentGetSeed != NULL) {
/* Get entropy from parent DRBG (no file I/O needed) */
WOLFPROV_MSG_DEBUG(WP_LOG_COMP_RNG,
Expand All @@ -189,22 +201,29 @@ static int wp_drbg_instantiate(wp_DrbgCtx* ctx, unsigned int strength,
}

if (ok) {
/* Initialize wolfCrypt RNG with parent-provided seed */
ctx->rng = OPENSSL_zalloc(sizeof(*ctx->rng));
/* Use wc_rng_new (>= 5.0) so alloc and free use the same allocator,
* matching the root path and wc_rng_free() in teardown. */
#if LIBWOLFSSL_VERSION_HEX >= 0x05000000
ctx->rng = wc_rng_new(seed, (word32)seedLen, NULL);
if (ctx->rng == NULL) {
Comment thread
aidangarske marked this conversation as resolved.
ok = 0;
}
}

if (ok) {
int rc = wc_InitRngNonce(ctx->rng, seed, (word32)seedLen);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_COMP_RNG,
"wc_InitRngNonce", rc);
OPENSSL_free(ctx->rng);
ctx->rng = NULL;
#else
ctx->rng = OPENSSL_zalloc(sizeof(*ctx->rng));
if (ctx->rng == NULL) {
ok = 0;
}
if (ok) {
int rc = wc_InitRngNonce(ctx->rng, seed, (word32)seedLen);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_COMP_RNG,
"wc_InitRngNonce", rc);
OPENSSL_clear_free(ctx->rng, sizeof(*ctx->rng));
ctx->rng = NULL;
ok = 0;
}
}
#endif
}

/* Clear the seed from parent */
Expand Down Expand Up @@ -242,6 +261,13 @@ static int wp_drbg_instantiate(wp_DrbgCtx* ctx, unsigned int strength,
#endif
}

#ifndef WP_HAVE_DRBG_RESEED
if (ok) {
/* Clear any prior reseed error state. */
ctx->rngError = 0;
}
#endif

WOLFPROV_LEAVE(WP_LOG_COMP_RNG, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
return ok;
}
Expand All @@ -262,6 +288,9 @@ static int wp_drbg_uninstantiate(wp_DrbgCtx* ctx)
OPENSSL_clear_free(ctx->rng, sizeof(*ctx->rng));
#endif
ctx->rng = NULL;
#ifndef WP_HAVE_DRBG_RESEED
ctx->rngError = 0;
#endif
WOLFPROV_LEAVE(WP_LOG_COMP_RNG, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), 1);
return 1;
}
Expand Down Expand Up @@ -292,6 +321,16 @@ static int wp_drbg_generate(wp_DrbgCtx* ctx, unsigned char* out,
if (strength > WP_DRBG_STRENGTH) {
ok = 0;
}
if (ok && ctx->rng == NULL) {
WOLFPROV_MSG_DEBUG(WP_LOG_COMP_RNG, "DRBG not instantiated");
ok = 0;
}
#ifndef WP_HAVE_DRBG_RESEED
if (ok && ctx->rngError) {
WOLFPROV_MSG_DEBUG(WP_LOG_COMP_RNG, "DRBG in error state");
ok = 0;
}
#endif
#if 0
if (ok && (addInLen > 0)) {
rc = wc_RNG_DRBG_Reseed(ctx->rng, addIn, addInLen);
Expand Down Expand Up @@ -322,6 +361,10 @@ static int wp_drbg_generate(wp_DrbgCtx* ctx, unsigned char* out,
/**
* Reseed DRBG.
*
* When WP_HAVE_DRBG_RESEED is undefined (FIPS modules without an exported
* wc_RNG_DRBG_Reseed), this re-instantiates rather than reseeding: @p entropy /
* @p addIn are used as the nonce, not as DRBG entropy_input.
*
* @param [in, out] ctx DRBG context object.
* @param [in] predResist Prediction resistance required.
* @param [in] entropy Entropy data to reseed with.
Expand All @@ -338,52 +381,130 @@ static int wp_drbg_reseed(wp_DrbgCtx* ctx, int predResist,
{
int ok = 1;
int rc;
unsigned char *seed = NULL;
size_t seedLen = 0;

WOLFPROV_ENTER(WP_LOG_COMP_RNG, "wp_drbg_reseed");

/* If no entropy provided, get fresh entropy from the OS source. */
if (entropy == NULL || entropyLen == 0) {
seedLen = 48;
seed = OPENSSL_malloc(seedLen);
if (seed == NULL) {
ok = 0;
}
if (ok) {
OS_Seed osSeed;
rc = wc_GenerateSeed(&osSeed, seed, (word32)seedLen);
if (rc != 0) {
/* Reseed requires an instantiated DRBG. */
if (ctx->rng == NULL) {
ok = 0;
}

/* wolfCrypt RNG APIs take word32 lengths; reject oversized inputs. */
if (ok && entropy != NULL && entropyLen > 0xFFFFFFFFU) {
ok = 0;
}
if (ok && addIn != NULL && addInLen > 0xFFFFFFFFU) {
ok = 0;
}

#ifdef WP_HAVE_DRBG_RESEED

@aidangarske aidangarske Jun 22, 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] Bare scope block introduced in wp_drbg_reseed native path

The #ifdef WP_HAVE_DRBG_RESEED branch opens a standalone { ... } block solely to scope the local declarations unsigned char* seed and size_t seedLen. This is a bare scope block that does not belong to a function, control statement, switch/case, or type definition. The wolfSSL/wolfProvider C style explicitly bans introducing bare scope blocks in new code to make C89 declarations work — declarations should move to the top of the enclosing function (or the logic split into a helper). Note the sibling #else fallback branch correctly declares its locals inside an if (ok) { control block, so the two halves of the same function are inconsistent.

Fix: Remove the bare scope block to comply with the no-bare-scope-block convention; the empty-brace-scan skill flags exactly this pattern.

@aidangarske aidangarske Jun 22, 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.

🔵 [Low] Bare scope block introduced in wp_drbg_reseed native reseed path · Logic

The new native (WP_HAVE_DRBG_RESEED) branch opens a standalone { ... } scope solely to declare unsigned char* seed and size_t seedLen mid-function. This is a bare scope block, which the wolfSSL/wolfProvider C coding standard prohibits in new code (scope blocks are only allowed for function/control/switch/type bodies). The parallel fallback branch correctly uses a control-statement block (if (ok) { ... }), so only the native path violates the rule. BEFORE: these two locals were declared at the top of the function. AFTER: they were moved into a new bare { } block under the #ifdef. This is a style/maintainability issue, not a security defect.

Fix: Declare seed/seedLen at the top of wp_drbg_reseed (guarded as needed) and remove the bare { } scope, or move the native logic into a small helper function, to comply with the no-bare-scope coding standard. Run the empty-brace-scan to confirm no other new scope blocks were introduced.

{
unsigned char* seed = NULL;
size_t seedLen = 0;

/* An in-place reseed needs explicit entropy. If the caller supplied
* none, draw fresh entropy from the cached /dev/urandom fd (SEED-SRC),
* which stays open across a seccomp sandbox, rather than
* wc_GenerateSeed() which opens /dev/urandom directly and would be
* blocked under the sandbox. */
if (ok && (entropy == NULL || entropyLen == 0)) {
seedLen = 48;
seed = OPENSSL_malloc(seedLen);
if (seed == NULL) {
ok = 0;
}
else {
if (ok) {
#if defined(WP_HAVE_SEED_SRC) && defined(WP_HAVE_RANDOM)
if (wp_urandom_read(seed, seedLen) != (int)seedLen) {
ok = 0;
}
#else
OS_Seed osSeed;
if (wc_GenerateSeed(&osSeed, seed, (word32)seedLen) != 0) {
ok = 0;
}
#endif
}
if (ok) {
entropy = seed;
entropyLen = seedLen;
}
}
}

if (ok && entropy != NULL && entropyLen > 0) {
rc = wc_RNG_DRBG_Reseed(ctx->rng, entropy, (word32)entropyLen);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_COMP_RNG,
"wc_RNG_DRBG_Reseed", rc);
ok = 0;
/* In-place SP 800-90A reseed via wolfCrypt's public DRBG API. */
if (ok && entropy != NULL && entropyLen > 0) {
rc = wc_RNG_DRBG_Reseed(ctx->rng, entropy, (word32)entropyLen);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_COMP_RNG,
"wc_RNG_DRBG_Reseed", rc);
ok = 0;
}
}
}
if (ok && (addInLen > 0) && (addIn != NULL)) {
rc = wc_RNG_DRBG_Reseed(ctx->rng, addIn, (word32)addInLen);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_COMP_RNG,
"wc_RNG_DRBG_Reseed", rc);
ok = 0;
if (ok && (addInLen > 0) && (addIn != NULL)) {
rc = wc_RNG_DRBG_Reseed(ctx->rng, addIn, (word32)addInLen);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_COMP_RNG,
"wc_RNG_DRBG_Reseed", rc);
ok = 0;
}
}

if (seed != NULL) {
OPENSSL_clear_free(seed, seedLen);
}
}
#else
Comment thread
aidangarske marked this conversation as resolved.
/* FIPS modules without an exported wc_RNG_DRBG_Reseed (e.g. cert4718):
* re-instantiate in place via wc_FreeRng() + wc_InitRngNonce() on the same
* WC_RNG struct. wc_InitRngNonce self-seeds fresh entropy_input; with
* SEED-SRC that entropy comes from the cached /dev/urandom fd via the seed
* callback (so it stays sandbox-safe), otherwise from wc_GenerateSeed().
* Caller entropy/addIn are folded in only as the nonce (which may be empty).
* A failed re-init leaves the DRBG de-instantiated and sets rngError. */
if (ok) {
unsigned char* nonce = NULL;
word32 nonceLen = 0;
word32 eLen = (entropy != NULL) ? (word32)entropyLen : 0;
word32 aLen = (addIn != NULL) ? (word32)addInLen : 0;

/* Securely clear and free locally allocated seed buffer. */
if (seed != NULL) {
OPENSSL_clear_free(seed, seedLen);
/* Build nonce = entropy || addIn (either may be absent). */
if (aLen > (0xFFFFFFFFU - eLen)) {
ok = 0;
}
if (ok && (eLen + aLen) > 0) {
nonceLen = eLen + aLen;
nonce = OPENSSL_malloc(nonceLen);
if (nonce == NULL) {
ok = 0;
}
else {
if (eLen > 0) {
XMEMCPY(nonce, entropy, eLen);
}
if (aLen > 0) {
XMEMCPY(nonce + eLen, addIn, aLen);
}
}
}
if (ok) {
wc_FreeRng(ctx->rng);
Comment thread
aidangarske marked this conversation as resolved.
rc = wc_InitRngNonce(ctx->rng, nonce, nonceLen);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_COMP_RNG,
"wc_InitRngNonce", rc);
ctx->rngError = 1;
ok = 0;
}
else {
/* Recovered: clear any prior reseed error. */
ctx->rngError = 0;
}
}
if (nonce != NULL) {
OPENSSL_clear_free(nonce, nonceLen);
}
}
#endif /* WP_HAVE_DRBG_RESEED */

(void)predResist;

Expand Down Expand Up @@ -512,15 +633,25 @@ static int wp_drbg_get_ctx_params(wp_DrbgCtx* ctx, OSSL_PARAM params[])

WOLFPROV_ENTER(WP_LOG_COMP_RNG, "wp_drbg_get_ctx_params");

(void)ctx;

p = OSSL_PARAM_locate(params, OSSL_RAND_PARAM_MAX_REQUEST);
if ((p != NULL) && (!OSSL_PARAM_set_size_t(p, WP_DRBG_MAX_REQUESTS))) {
ok = 0;
}
if (ok) {
int state = EVP_RAND_STATE_READY;

if (ctx->rng == NULL) {
state = EVP_RAND_STATE_UNINITIALISED;
}
#ifndef WP_HAVE_DRBG_RESEED
/* Failed reseed re-instantiation left the DRBG de-instantiated. */
else if (ctx->rngError) {
state = EVP_RAND_STATE_ERROR;
}
#endif

p = OSSL_PARAM_locate(params, OSSL_RAND_PARAM_STATE);
if ((p != NULL) && (!OSSL_PARAM_set_int(p, EVP_RAND_STATE_READY))) {
if ((p != NULL) && (!OSSL_PARAM_set_int(p, state))) {
ok = 0;
}
}
Expand Down Expand Up @@ -622,6 +753,12 @@ static size_t wp_drbg_get_seed(wp_DrbgCtx* ctx, unsigned char** pSeed,
"DRBG not instantiated");
goto end;
}
#ifndef WP_HAVE_DRBG_RESEED
if (ctx->rngError) {
WOLFPROV_MSG_DEBUG(WP_LOG_COMP_RNG, "DRBG in error state");
goto end;
}
#endif

buffer = OPENSSL_secure_malloc(minLen);
if (buffer == NULL) {
Expand Down
Loading
Loading