Skip to content

Fix fenrir findings across wolfProvider #387

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
JeremiahM37 wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fix fenrir findings across wolfProvider #387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6e8bb19
Chunk large inputs to prevent word32 truncation
JeremiahM37 Apr 6, 2026
a0fedc3
Fix mutex guards to use WP_SINGLE_THREADED
JeremiahM37 Apr 6, 2026
d3d620d
Fix prefix matches, sizeof errors in string ops
JeremiahM37 Apr 6, 2026
eba09b8
Secure-clear key material on free and stack
JeremiahM37 Apr 6, 2026
1ebe98a
Fix error handling, NULL checks, and TLS padding
JeremiahM37 Apr 6, 2026
4aeb35e
Fix PEM parsing and hex dump off-by-one
JeremiahM37 Apr 6, 2026
1d0fb43
Add unit tests for Fenrir fixes
JeremiahM37 Apr 6, 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
6 changes: 4 additions & 2 deletions src/wp_aes_aead.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,9 @@ static int wp_aead_set_param_tag(wp_AeadCtx* ctx,
if (ok && ((sz == 0) || ((p->data != NULL) && ctx->enc))) {
ok = 0;
}
ctx->tagLen = sz;
if (ok) {
ctx->tagLen = sz;
}

WOLFPROV_LEAVE(WP_LOG_COMP_AES, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
return ok;
Expand Down Expand Up @@ -1122,7 +1124,7 @@ static int wp_aesgcm_dinit(wp_AeadCtx *ctx, const unsigned char *key,
ok = 0;
}
}
if (ok) {
if (ok && (iv != NULL)) {
XMEMCPY(ctx->iv, iv, ivLen);
ctx->ivState = IV_STATE_BUFFERED;
ctx->ivSet = 0;
Expand Down
84 changes: 52 additions & 32 deletions src/wp_aes_block.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,45 +431,65 @@ static int wp_aes_block_dinit(wp_AesBlockCtx *ctx, const unsigned char *key,
static int wp_aes_block_doit(wp_AesBlockCtx *ctx, unsigned char *out,
const unsigned char *in, size_t inLen)
{
int rc;

#ifdef WP_HAVE_AESCBC
if (ctx->mode == EVP_CIPH_CBC_MODE) {
if (ctx->enc) {
rc = wc_AesCbcEncrypt(&ctx->aes, out, in, (word32)inLen);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_AesCbcEncrypt", rc);
int rc = 0;

while ((rc == 0) && (inLen > 0)) {
/* Chunk must be block-aligned (AES block size = 16). */
word32 chunk = (inLen > 0xFFFFFFF0U) ? 0xFFFFFFF0U : (word32)inLen;

#ifdef WP_HAVE_AESCBC
if (ctx->mode == EVP_CIPH_CBC_MODE) {
if (ctx->enc) {
rc = wc_AesCbcEncrypt(&ctx->aes, out, in, chunk);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG,
"wc_AesCbcEncrypt", rc);
}
}
}
else {
rc = wc_AesCbcDecrypt(&ctx->aes, out, in, (word32)inLen);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_AesCbcDecrypt", rc);
else {
rc = wc_AesCbcDecrypt(&ctx->aes, out, in, chunk);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG,
"wc_AesCbcDecrypt", rc);
}
}
}
XMEMCPY(ctx->iv, ctx->aes.reg, ctx->ivLen);
}
else
#endif
#ifdef WP_HAVE_AESECB
if (ctx->mode == EVP_CIPH_ECB_MODE) {
if (ctx->enc) {
rc = wc_AesEcbEncrypt(&ctx->aes, out, in, (word32)inLen);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_AesEcbEncrypt", rc);
else
#endif
#ifdef WP_HAVE_AESECB
if (ctx->mode == EVP_CIPH_ECB_MODE) {
if (ctx->enc) {
rc = wc_AesEcbEncrypt(&ctx->aes, out, in, chunk);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG,
"wc_AesEcbEncrypt", rc);
}
}
}
else {
rc = wc_AesEcbDecrypt(&ctx->aes, out, in, (word32)inLen);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_AesEcbDecrypt", rc);
else {
rc = wc_AesEcbDecrypt(&ctx->aes, out, in, chunk);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG,
"wc_AesEcbDecrypt", rc);
}
}
}
else
#endif
{
rc = -1;
}

in += chunk;
out += chunk;
inLen -= chunk;
}
else
#endif
{
rc = -1;

if (rc == 0) {
#ifdef WP_HAVE_AESCBC
if (ctx->mode == EVP_CIPH_CBC_MODE) {
XMEMCPY(ctx->iv, ctx->aes.reg, ctx->ivLen);
}
#endif
}

return rc == 0;
Expand Down
48 changes: 31 additions & 17 deletions src/wp_aes_stream.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,13 +532,18 @@ static int wp_aes_stream_doit(wp_AesStreamCtx *ctx, unsigned char *out,

#ifdef WP_HAVE_AESCTR
if (ctx->mode == EVP_CIPH_CTR_MODE) {
int rc;

XMEMCPY(&ctx->aes.reg, ctx->iv, ctx->ivLen);
rc = wc_AesCtrEncrypt(&ctx->aes, out, in, (word32)inLen);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_AesCtrEncrypt", rc);
ok = 0;
while (ok && (inLen > 0)) {
word32 chunk = (inLen > 0xFFFFFFFFU) ? 0xFFFFFFFFU : (word32)inLen;
int rc = wc_AesCtrEncrypt(&ctx->aes, out, in, chunk);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG,
"wc_AesCtrEncrypt", rc);
ok = 0;
}
in += chunk;
out += chunk;
inLen -= chunk;
}
if (ok) {
XMEMCPY(ctx->iv, ctx->aes.reg, ctx->ivLen);
Expand All @@ -548,17 +553,24 @@ static int wp_aes_stream_doit(wp_AesStreamCtx *ctx, unsigned char *out,
#endif
#ifdef WP_HAVE_AESCFB
if (ctx->mode == EVP_CIPH_CFB_MODE) {
int rc;

XMEMCPY(&ctx->aes.reg, ctx->iv, ctx->ivLen);
if (ctx->enc) {
rc = wc_AesCfbEncrypt(&ctx->aes, out, in, (word32)inLen);
}else {
rc = wc_AesCfbDecrypt(&ctx->aes, out, in, (word32)inLen);
}
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_AesCfbEncrypt/wc_AesCfbDecrypt", rc);
ok = 0;
while (ok && (inLen > 0)) {
word32 chunk = (inLen > 0xFFFFFFFFU) ? 0xFFFFFFFFU : (word32)inLen;
int rc;
if (ctx->enc) {
rc = wc_AesCfbEncrypt(&ctx->aes, out, in, chunk);
}
else {
rc = wc_AesCfbDecrypt(&ctx->aes, out, in, chunk);
}
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG,
"wc_AesCfbEncrypt/wc_AesCfbDecrypt", rc);
ok = 0;
}
in += chunk;
out += chunk;
inLen -= chunk;
}
if (ok) {
XMEMCPY(ctx->iv, ctx->aes.reg, ctx->ivLen);
Expand Down Expand Up @@ -682,7 +694,9 @@ static int wp_aes_stream_cipher(wp_AesStreamCtx* ctx, unsigned char* out,
ok = 0;
}

*outLen = inLen;
if (ok) {
*outLen = inLen;
}
WOLFPROV_LEAVE(WP_LOG_COMP_AES, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
return ok;
}
Expand Down
17 changes: 11 additions & 6 deletions src/wp_cmac.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ static void wp_cmac_free(wp_CmacCtx* macCtx)
{
Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 9, 2026

Choose a reason for hiding this comment

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

test inline

Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 9, 2026

Choose a reason for hiding this comment

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

finding 3

Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 9, 2026

Choose a reason for hiding this comment

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

finding 3 body

Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 9, 2026

Choose a reason for hiding this comment

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

🟡 [Medium] CMAC free does not call wc_CmacFree unlike GMAC's wc_AesFree addition
💡 SUGGEST convention

The PR correctly added wc_AesFree(&macCtx->gmac.aes) in wp_gmac_free to properly release wolfSSL AES resources. However, the analogous wp_cmac_free (also modified in this PR to use OPENSSL_clear_free) does not call wc_CmacFree on the Cmac struct. wolfSSL's wc_CmacFree exists and is used in the codebase (e.g., wp_kbkdf.c:521). While the OPENSSL_clear_free zeroes memory, it doesn't run wolfSSL's cleanup for the internal AES state (which may hold hardware crypto handles on some platforms). Since this PR already touches wp_cmac_free, it would be consistent to add the wolfSSL cleanup call here too.

Suggestion:

Suggested change
{
static void wp_cmac_free(wp_CmacCtx* macCtx)
{
if (macCtx != NULL) {
#ifndef HAVE_FIPS
wc_CmacFree(&macCtx->cmac);
#endif
OPENSSL_cleanse(macCtx->key, macCtx->keyLen);
OPENSSL_clear_free(macCtx, sizeof(*macCtx));
}
}

if (macCtx != NULL) {
OPENSSL_cleanse(macCtx->key, macCtx->keyLen);
OPENSSL_free(macCtx);
OPENSSL_clear_free(macCtx, sizeof(*macCtx));
}
}

Expand Down Expand Up @@ -222,14 +222,19 @@ static int wp_cmac_update(wp_CmacCtx* macCtx, const unsigned char* data,
size_t dataLen)
{
int ok = 1;
int rc;

WOLFPROV_ENTER(WP_LOG_COMP_MAC, "wp_cmac_update");

rc = wc_CmacUpdate(&macCtx->cmac, data, (word32)dataLen);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_CmacUpdate", rc);
ok = 0;
while (ok && (dataLen > 0)) {
word32 chunk = (dataLen > 0xFFFFFFFFU) ? 0xFFFFFFFFU : (word32)dataLen;
int rc = wc_CmacUpdate(&macCtx->cmac, data, chunk);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_CmacUpdate",
rc);
ok = 0;
}
data += chunk;
dataLen -= chunk;
}

WOLFPROV_LEAVE(WP_LOG_COMP_MAC, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
Expand Down
2 changes: 1 addition & 1 deletion src/wp_dec_epki2pki.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ static int wp_epki2pki_decode(wp_Epki2Pki* ctx, OSSL_CORE_BIO* coreBio,
}

/* Dispose of the EPKI data buffer. */
OPENSSL_free(data);
OPENSSL_clear_free(data, len);

OPENSSL_cleanse(password, sizeof(password));

Expand Down
2 changes: 1 addition & 1 deletion src/wp_dec_pem2der.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ static int wp_pem2der_convert(const char* data, word32 len, DerBuffer** pDer,

/* Skip '-----BEGIN <name>-----\n'. */
base64Data = data + 16 + nameLen + 1;
base64Len = len - 16 + nameLen + 1;
base64Len = len - (16 + nameLen + 1);
footer = XSTRSTR(base64Data, "-----END ");
if (footer == NULL) {
info->consumed = len;
Expand Down
21 changes: 15 additions & 6 deletions src/wp_des.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,16 +374,25 @@ static int wp_des3_block_dinit(wp_Des3BlockCtx *ctx, const unsigned char *key,
static int wp_des3_block_doit(wp_Des3BlockCtx *ctx, unsigned char *out,
const unsigned char *in, size_t inLen)
{
int rc;
int rc = 0;

if (ctx->mode == EVP_CIPH_CBC_MODE) {
if (ctx->enc) {
rc = wc_Des3_CbcEncrypt(&ctx->des3, out, in, (word32)inLen);
while ((rc == 0) && (inLen > 0)) {
/* Chunk must be block-aligned (DES3 block size = 8). */
word32 chunk = (inLen > 0xFFFFFFF8U) ? 0xFFFFFFF8U : (word32)inLen;
if (ctx->enc) {
rc = wc_Des3_CbcEncrypt(&ctx->des3, out, in, chunk);
}
else {
rc = wc_Des3_CbcDecrypt(&ctx->des3, out, in, chunk);
}
in += chunk;
out += chunk;
inLen -= chunk;
}
else {
rc = wc_Des3_CbcDecrypt(&ctx->des3, out, in, (word32)inLen);
if (rc == 0) {
XMEMCPY(ctx->iv, ctx->des3.reg, ctx->ivLen);
}
XMEMCPY(ctx->iv, ctx->des3.reg, ctx->ivLen);
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion src/wp_dh_kmgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ static wp_Dh* wp_dh_new(WOLFPROV_CTX *provCtx)
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_InitDhKey_ex", rc);
ok = 0;
}
#ifndef SINGLE_THREADED
#ifndef WP_SINGLE_THREADED
if (ok) {
rc = wc_InitMutex(&dh->mutex);
if (rc != 0) {
Expand Down
13 changes: 9 additions & 4 deletions src/wp_digests.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,15 @@ static int name##_update(void* ctx, const unsigned char* in, size_t inLen) \
{ \
int ok = 1; \
WOLFPROV_ENTER(WP_LOG_COMP_DIGEST, #name "_update"); \
int rc = upd(ctx, in, (word32)inLen); \
if (rc != 0) { \
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, #upd, rc); \
ok = 0; \
while (ok && (inLen > 0)) { \
word32 chunk = (inLen > 0xFFFFFFFFU) ? 0xFFFFFFFFU : (word32)inLen; \
int rc = upd(ctx, in, chunk); \
if (rc != 0) { \
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, #upd, rc); \
ok = 0; \
} \
in += chunk; \
inLen -= chunk; \
} \
WOLFPROV_LEAVE(WP_LOG_COMP_DIGEST, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);\
return ok; \
Expand Down
2 changes: 1 addition & 1 deletion src/wp_ecc_kmgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ static wp_Ecc* wp_ecc_new(WOLFPROV_CTX *provCtx)
}
}

#ifndef SINGLE_THREADED
#ifndef WP_SINGLE_THREADED
if (ok) {
rc = wc_InitMutex(&ecc->mutex);
if (rc != 0) {
Expand Down
35 changes: 23 additions & 12 deletions src/wp_ecx_exch.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,21 +250,32 @@ static int wp_x25519_derive(wp_EcxCtx* ctx, unsigned char* secret,
ok = 0;
}
if (ok) {
/* Constant-time: always subtract, then select based on
* whether secret >= order. */
unsigned char reduced[CURVE25519_KEYSIZE];
int16_t carry = 0;
byte gt = 0;
byte eq = 0xFF;

for (i = CURVE25519_KEYSIZE - 1; i >= 0; i--) {
carry += secret[i];
carry -= wp_curve25519_order[i];
reduced[i] = (unsigned char)carry;
carry >>= 8;
}
/* Determine if secret >= order in constant time. */
for (i = 0; i < CURVE25519_KEYSIZE; i++) {
if (secret[i] != wp_curve25519_order[i]) {
break;
}
gt |= eq & wp_ct_int_mask_gte(secret[i],
wp_curve25519_order[i] + 1);
eq &= wp_ct_byte_mask_eq(secret[i],
wp_curve25519_order[i]);
}
if ((i < CURVE25519_KEYSIZE) &&
(secret[i] > wp_curve25519_order[i])) {
int16_t carry = 0;
for (i = CURVE25519_KEYSIZE - 1; i >= 0; i--) {
carry += secret[i];
carry -= wp_curve25519_order[i];
secret[i] = (unsigned char)carry;
carry >>= 8;
}
/* Select reduced if secret >= order. */
for (i = 0; i < CURVE25519_KEYSIZE; i++) {
secret[i] = wp_ct_byte_mask_sel(gt | eq, reduced[i],
secret[i]);
}
OPENSSL_cleanse(reduced, sizeof(reduced));
}
if (ok) {
*secLen = len;
Expand Down
8 changes: 5 additions & 3 deletions src/wp_ecx_kmgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ static wp_Ecx* wp_ecx_new(WOLFPROV_CTX* provCtx, const wp_EcxData* data)
ok = 0;
}

#ifndef SINGLE_THREADED
#ifndef WP_SINGLE_THREADED
if (ok) {
rc = wc_InitMutex(&ecx->mutex);
if (rc != 0) {
Expand Down Expand Up @@ -698,6 +698,9 @@ static int wp_ecx_match_priv_key(const wp_Ecx* ecx1, const wp_Ecx* ecx2)
ok = 0;
}

OPENSSL_cleanse(key1, sizeof(key1));
OPENSSL_cleanse(key2, sizeof(key2));

WOLFPROV_LEAVE(WP_LOG_COMP_KE, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
return ok;
}
Expand Down Expand Up @@ -1273,8 +1276,7 @@ static int wp_ecx_gen_set_params(wp_EcxGenCtx* ctx, const OSSL_PARAM params[])
&name)) {
ok = 0;
}
if (ok && (name != NULL) && (XSTRNCMP(name, ctx->name,
XSTRLEN(name)) != 0)) {
if (ok && (name != NULL) && (XSTRCMP(name, ctx->name) != 0)) {
ok = 0;
}

Expand Down
3 changes: 2 additions & 1 deletion src/wp_gmac.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,11 @@ static wp_GmacCtx* wp_gmac_new(WOLFPROV_CTX* provCtx)
static void wp_gmac_free(wp_GmacCtx* macCtx)
{
if (macCtx != NULL) {
wc_AesFree(&macCtx->gmac.aes);
OPENSSL_cleanse(macCtx->key, macCtx->keyLen);
OPENSSL_cleanse(macCtx->iv, macCtx->ivLen);
OPENSSL_clear_free(macCtx->data, macCtx->dataLen);
OPENSSL_free(macCtx);
OPENSSL_clear_free(macCtx, sizeof(*macCtx));
}
}

Expand Down
Loading
Loading