Skip to content

Fix: WolfCrypt Fenrir - 12 fixes #10786

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

Fix: WolfCrypt Fenrir - 12 fixes #10786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
84e553a
https://fenrir.wolfssl.com/finding/6145
aidankeefe2022 Jun 24, 2026
983e25d
Added new field to WC_DEVCRYPTO that singals if the object has been i…
aidankeefe2022 Jul 7, 2026
f02c0b9
atomic mutex init fix
aidankeefe2022 Jul 7, 2026
a5b0f26
aes test fix
aidankeefe2022 Jul 8, 2026
6ed96c8
possible windows fix
aidankeefe2022 Jul 8, 2026
5cdc977
Added new thread yield macro and added new header with CAAM_ADDRESS d…
aidankeefe2022 Jul 14, 2026
1241e7c
skoll fixes
aidankeefe2022 Jul 15, 2026
a6029ca
freertos compile error
aidankeefe2022 Jul 15, 2026
2b3617a
zepher fix for when thread yeild is not present
aidankeefe2022 Jul 15, 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
4 changes: 2 additions & 2 deletions doc/dox_comments/header_files/sha256.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,11 +366,11 @@ int wc_Sha256_Grow(wc_Sha256* sha256, const byte* in, int inSz);
\return negative on error

\param src Source SHA256 structure

@dgarske dgarske Jul 31, 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] sha256.h doc: contract change documented only for wc_Sha256Copy, and the example leaves src uninitialized

The PR makes wc_Sha256Free(dst) the first operation on dst and codifies 'must be zeroed/initialized' in the dox comment. I verified every in-tree caller already satisfies that contract — src/internal.c:7670 XMEMSETs *destination, src/tls13.c:12893 XMEMSETs the Digest union, wolfcrypt/src/hmac.c:391 XMEMSETs dst->hash inside wc_HmacCopy, src/ssl_crypto.c, evp.c:6155, wc_slhdsa.c and the tests — and the generic implementation in sha256.c already behaved this way, so the contract is consistent repo-wide. Two documentation gaps remain: the updated example still leaves src uninitialized, and only the SHA-256 comment was touched though wc_ShaCopy, wc_Sha384Copy, wc_Sha512Copy and wc_Sm3Copy carry the identical contract.

Fix: Fix the example to initialize src, and propagate the must be zeroed/initialized wording to wc_ShaCopy / wc_Sha384Copy / wc_Sha512Copy / wc_Sm3Copy.

\param dst Destination SHA256 structure
\param dst Destination SHA256 structure; must be zeroed/initialized

_Example_
\code
wc_Sha256 src, dst;
wc_Sha256 src, dst = {0};
int ret = wc_Sha256Copy(&src, &dst);
\endcode

Expand Down
14 changes: 9 additions & 5 deletions tests/api/test_aes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3260,13 +3260,15 @@ int test_wc_AesGcmEncryptDecrypt_Sizes(void)
int sz;
int i;
WC_DECLARE_VAR(plain, byte, GCM_LEN, NULL);
WC_DECLARE_VAR(cipher, byte, GCM_LEN, NULL);
/* enlarged size is to accommodate devcrypto build which assumes
* space in buffer to append auth tag */
WC_DECLARE_VAR(cipher, byte, GCM_LEN+WC_AES_BLOCK_SIZE, NULL);
#ifdef HAVE_AES_DECRYPT
WC_DECLARE_VAR(decrypted, byte, GCM_LEN, NULL);
#endif

WC_ALLOC_VAR(plain, byte, GCM_LEN, NULL);
WC_ALLOC_VAR(cipher, byte, GCM_LEN, NULL);
WC_ALLOC_VAR(cipher, byte, GCM_LEN+WC_AES_BLOCK_SIZE, NULL);
#ifdef HAVE_AES_DECRYPT
WC_ALLOC_VAR(decrypted, byte, GCM_LEN, NULL);
#endif
Expand All @@ -3286,7 +3288,7 @@ int test_wc_AesGcmEncryptDecrypt_Sizes(void)

ExpectIntEQ(wc_AesGcmSetKey(&aes, key32, sizeof(key32)/sizeof(byte)), 0);
for (sz = 0; sz < WC_AES_BLOCK_SIZE; sz++) {
XMEMSET(cipher, 0, GCM_LEN);
XMEMSET(cipher, 0, GCM_LEN + WC_AES_BLOCK_SIZE);
ExpectIntEQ(wc_AesGcmEncrypt(&aes, cipher, plain, sz, iv, ivLen, tag,
sizeof(tag), NULL, 0), 0);
ExpectBufEQ(cipher, expected, sz);
Expand All @@ -3302,7 +3304,7 @@ int test_wc_AesGcmEncryptDecrypt_Sizes(void)

i = 0;
for (sz = WC_AES_BLOCK_SIZE; sz <= GCM_LEN; sz *= 2) {
XMEMSET(cipher, 0, GCM_LEN);
XMEMSET(cipher, 0, GCM_LEN + WC_AES_BLOCK_SIZE);
ExpectIntEQ(wc_AesGcmEncrypt(&aes, cipher, plain, sz, iv, ivLen, tag,
sizeof(tag), NULL, 0), 0);
ExpectBufEQ(tag, expTagLong[i], WC_AES_BLOCK_SIZE);
Expand Down Expand Up @@ -3991,7 +3993,9 @@ int test_wc_AesGcmNonStdNonce(void)
* and cannot exercise the GHASH-based counter derivation. */
#if !defined(NO_AES) && defined(HAVE_AESGCM) && \
!defined(HAVE_FIPS) && \
!defined(WOLFSSL_AFALG) && !defined(WOLFSSL_KCAPI)
!defined(WOLFSSL_AFALG) && !defined(WOLFSSL_KCAPI) && \
!defined(WOLFSSL_DEVCRYPTO_AES)
/* DEVCRYPTO does not support Non std Nonce */

/* ------------------------------------------------------------------
* Section 1: 1-byte IV, AES-128
Expand Down
6 changes: 4 additions & 2 deletions wolfcrypt/src/aes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5848,7 +5848,8 @@ static void AesSetKey_C(Aes* aes, const byte* key, word32 keySz, int dir)

#if defined(WOLFSSL_DEVCRYPTO) && \
(defined(WOLFSSL_DEVCRYPTO_AES) || defined(WOLFSSL_DEVCRYPTO_CBC))
aes->ctx.cfd = -1;
aes->ctx.inited = 0;
aes->ctx.cfd = -1;
#endif
#ifdef WOLFSSL_IMX6_CAAM_BLOB
ForceZero(local, sizeof(local));
Expand Down Expand Up @@ -14891,7 +14892,8 @@ int wc_AesInit(Aes* aes, void* heap, int devId)
#endif
#if defined(WOLFSSL_DEVCRYPTO) && \
(defined(WOLFSSL_DEVCRYPTO_AES) || defined(WOLFSSL_DEVCRYPTO_CBC))
aes->ctx.cfd = -1;
aes->ctx.inited = 0;
aes->ctx.cfd = -1;
#endif
#if defined(WOLFSSL_IMXRT_DCP)
DCPAesInit(aes);
Expand Down
14 changes: 9 additions & 5 deletions wolfcrypt/src/des3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1570,11 +1570,15 @@
pc1m[(l = j + totrot[i]) < (j < 28 ? 28 : 56) ? l : l-28];

@dgarske dgarske Jul 31, 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] des3.c: secret-dependent ternary remains in the pc1 bit-extraction loop

The PR removes the secret-dependent branch from the pc2 selection loop, but the loop fifteen lines above — which produces the very values it consumes — still branches on a secret key bit via ? 1 : 0. That is the same construct class being eliminated. GCC and Clang both lower a ? 1 : 0 on a masked value to a branchless setne/cset, which is why it has not shown up; but the same was true of the if (pcr[...]) form just rewritten. Fixing one and leaving the other means the function is still branchless only by compiler goodwill, not by construction. (I verified the rewrite itself is bit-identical: pc1m[] is filled by this exact ? 1 : 0, and pcr[j] only copies pc1m[], so mask = (byte)(0 - bit) is provably 0x00/0xFF. bytebit appears nowhere else in the repo, so no duplicate loop was missed, and DES3 KATs at wolfcrypt/test/test.c:12588/12645 cover it.)

Fix: Convert to arithmetic extraction so the whole key schedule is branchless by construction rather than by optimizer behaviour.

Note: Referenced line (wolfcrypt/src/des3.c:1557-1563) is outside the diff hunk. Comment anchored to nearest changed region.


/* rotate left and right halves independently */
for (j = 0; j < 48; j++) { /* select bits individually */
if (pcr[pc2[j] - 1]) { /* check bit that goes to ks[j] */
l= j % 6; /* mask it in if it's there */
ks[j/6] |= (byte)(bytebit[l] >> 2);
}
for (j = 0; j < 48; j++) { /* select bits individually */
byte bit;
byte mask;
bit =

@dgarske dgarske Jul 31, 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] des3.c: new line exceeds the 80-column limit and will fail check-source-text

Line 1577, added by this PR, is 87 characters. wolfSSL enforces 80 columns via testing/git-hooks/wolfssl-multi-test.sh check-source-text, wired into .github/workflows/check-source-text.yml, so this fails CI. The awkward statement splitting (bit = on 1576 with the value and trailing comment on 1577; ks[j/6] |= on 1580 with the expression on 1581) exists only because of the long trailing comments.

Fix: Reflow under 80 columns by moving comments above the statements, then run the repo's check-source-text lint before pushing.

(byte)(pcr[pc2[j] - 1]); /* all pcr values are either 0 or 1 */
mask = (byte)(0 - bit); /* mask is either 0xFF or 0x00 */
/* only set to bytebit value if bit == 1 */
ks[j/6] |=
(byte)((bytebit[j % 6] >> 2) & mask);
}

/* Now convert to odd/even interleaved form for use in F */
Expand Down
7 changes: 7 additions & 0 deletions wolfcrypt/src/dsa.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1128,6 +1128,13 @@ int wc_DsaVerify_ex(const byte* digest, word32 digestSz, const byte* sig,
if (digest == NULL || sig == NULL || key == NULL || answer == NULL)
return BAD_FUNC_ARG;

/* assign default value so verification is always failed on error */
*answer = 0;

/* Note the min allowed digestSz here is WC_SHA_DIGEST_SIZE, not

@dgarske dgarske Jul 31, 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] dsa.c: added comment misstates what WC_MIN_DIGEST_SIZE_FOR_VERIFY expands to

The new comment claims 'the min allowed digestSz here is WC_SHA_DIGEST_SIZE, not WC_MIN_DIGEST_SIZE'. Per hash.h:272-287 both halves are wrong in the general case: in a non-FIPS build the macro is literally #define WC_MIN_DIGEST_SIZE_FOR_VERIFY WC_MIN_DIGEST_SIZE, and WC_MIN_DIGEST_SIZE can be 16 (MD2/MD4/MD5) or 32 or more depending on enabled hashes (hash.h:185-228). Only under WC_FIPS_186_4_PLUS is it max(WC_MIN_DIGEST_SIZE, 20), which equals WC_SHA_DIGEST_SIZE only when WC_MIN_DIGEST_SIZE <= 20. The FIPS references are accurate; it is the concrete value claim that misleads.

Fix: Reword to describe intent rather than a concrete value, and point at hash.h instead of restating it.

* WC_MIN_DIGEST_SIZE, to allow verify-only legacy DSA operations, as
* expressly allowed under FIPS 186-5, FIPS 140-3, and SP 800-131A.
*/
if ((digestSz > WC_MAX_DIGEST_SIZE) ||
(digestSz < WC_MIN_DIGEST_SIZE_FOR_VERIFY))
{
Expand Down
6 changes: 3 additions & 3 deletions wolfcrypt/src/ecc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10401,7 +10401,7 @@ static int _ecc_export_x963(ecc_key* key, byte* out, word32* outLen)
/* store byte point type */
out[0] = ECC_POINT_UNCOMP;

if (caamReadPartition((CAAM_ADDRESS)key->securePubKey, out+1, keySz*2) != 0)
if (caamReadPartition(key->securePubKey, out+1, keySz*2) != 0)
return WC_HW_E;

*outLen = 1 + 2*keySz;
Expand Down Expand Up @@ -12024,15 +12024,15 @@ static int _ecc_import_private_key_ex(const byte* priv, word32 privSz,
}

key->partNum = part;
key->blackKey = (word32)vaddr;
key->blackKey = vaddr;
if (caamWriteToPartition(vaddr, priv, privSz) != 0)
return WC_HW_E;

if (pub != NULL) {
/* +1 to account for x963 compressed bit */
if (caamWriteToPartition(vaddr + privSz, pub + 1, pubSz - 1) != 0)
return WC_HW_E;
key->securePubKey = (word32)vaddr + privSz;
key->securePubKey = vaddr + privSz;
}
}
else {
Expand Down
3 changes: 2 additions & 1 deletion wolfcrypt/src/hmac.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1519,7 +1519,8 @@ int wc_HmacInit(Hmac* hmac, void* heap, int devId)
hmac->devCtx = NULL;
#endif
#if defined(WOLFSSL_DEVCRYPTO_HMAC)
hmac->ctx.cfd = -1;
hmac->ctx.inited = 0;
hmac->ctx.cfd = -1;
#endif

#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_HMAC)
Expand Down
8 changes: 3 additions & 5 deletions wolfcrypt/src/port/Espressif/esp_crt_bundle/esp_crt_bundle.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -983,14 +983,12 @@ static CB_INLINE int wolfssl_ssl_conf_verify_cb_no_signer(int preverify,
/* Clean up and exit */
if ((_crt_found == 0) && (bundle_cert != NULL)) {
ESP_LOGW(TAG, "Cert not found, free bundle_cert");

@dgarske dgarske Jul 31, 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] esp_crt_bundle.c: missing space before comment terminator

The fix is correct and I verified it thoroughly: this_issuer and this_subject are only ever assigned from wolfSSL_X509_get_issuer_name(bundle_cert) (line 778) and wolfSSL_X509_get_subject_name(bundle_cert) (line 788), which return &cert->issuer / &cert->subject (src/x509.c:6283-6298) — interior pointers, never dups. The removed wolfSSL_X509_NAME_free would have double-freed the name internals (since wolfSSL_X509_free → FreeX509 already calls FreeX509Name on both) and then XFREEd a pointer into the middle of the X509 allocation. It also ran after wolfSSL_X509_free(bundle_cert), and in the loop-exhausted case the pointers already dangled into a cert freed at line 862 — so it was a use-after-free too. The _crt_found != 0 branch correctly does not free them. Only the comment formatting is off: no space before */.

Fix: Add the missing space before */ and name wolfSSL_X509_free explicitly so the ownership reasoning is obvious to the next reader.

/* this_subject and this_issuer are a part of bundle_cert and will be
* freed here*/
wolfSSL_X509_free(bundle_cert);
bundle_cert = NULL;
/* this_subject and this_issuer are pointers into cert used.
* Don't free if the cert was found. */
wolfSSL_X509_NAME_free(this_subject);
this_subject = NULL;
wolfSSL_X509_NAME_free(this_issuer);
this_issuer = NULL;
this_subject = NULL;
}

/* We don't clean up the store_cert and x509 as we are in a callback,
Expand Down
20 changes: 13 additions & 7 deletions wolfcrypt/src/port/Renesas/renesas_fspsm_aes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,13 @@ int wc_fspsm_AesGcmEncrypt(struct Aes* aes, byte* out,
aes->heap, DYNAMIC_TYPE_AES);
key_server_aes = (FSPSM_AES_PWKEY)XMALLOC(sizeof(FSPSM_AES_WKEY),
aes->heap, DYNAMIC_TYPE_AES);
if (key_client_aes == NULL || key_server_aes == NULL) {
XFREE(plainBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(cipherBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(aTagBuf, aes->heap, DYNAMIC_TYPE_AES);
if (key_server_aes == NULL || key_client_aes == NULL) {

@dgarske dgarske Jul 31, 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] Renesas FSPSM AES: inconsistent condition order and argument alignment between encrypt and decrypt

The fix is correct and complete — I verified the lock is genuinely held (if ((ret = wc_fspsm_hw_lock()) == 0) at line 379 encrypt, 613 decrypt), that the pre-lock early returns at 354/359/363 and 588/593/598 correctly do not unlock, that this MEMORY_E return was the only unbalanced exit between lock and the shared unlock at 535/756, and that the new XFREEs can never free a wrapped_key alias because those assignments (445, 674) live in the mutually exclusive non-TLS branch reached only after this block. Only cosmetics: the encrypt path reorders the condition to key_server_aes == NULL || key_client_aes == NULL while decrypt keeps client-then-server, and the XFREE argument columns do not line up (key_client_aes, followed by 2 spaces, plainBuf, by 7).

Fix: Use client-then-server in both (matching declaration and allocation order) and align the XFREE arguments identically so the two blocks stay diffable.

Note: Referenced line (wolfcrypt/src/port/Renesas/renesas_fspsm_aes.c:413-421,644-652) is outside the diff hunk. Comment anchored to nearest changed region.

XFREE(key_client_aes, aes->heap, DYNAMIC_TYPE_AES);
XFREE(key_server_aes, aes->heap, DYNAMIC_TYPE_AES);
XFREE(plainBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(cipherBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(aTagBuf, aes->heap, DYNAMIC_TYPE_AES);
wc_fspsm_hw_unlock();
return MEMORY_E;
}

Expand Down Expand Up @@ -638,9 +641,12 @@ int wc_fspsm_AesGcmDecrypt(struct Aes* aes, byte* out,
key_server_aes = (FSPSM_AES_PWKEY)XMALLOC(sizeof(FSPSM_AES_WKEY),
aes->heap, DYNAMIC_TYPE_AES);
if (key_client_aes == NULL || key_server_aes == NULL) {
XFREE(plainBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(cipherBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(aTagBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(key_client_aes, aes->heap, DYNAMIC_TYPE_AES);
XFREE(key_server_aes, aes->heap, DYNAMIC_TYPE_AES);
XFREE(plainBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(cipherBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(aTagBuf, aes->heap, DYNAMIC_TYPE_AES);
wc_fspsm_hw_unlock();
return MEMORY_E;
}

Expand Down
6 changes: 5 additions & 1 deletion wolfcrypt/src/port/Renesas/renesas_fspsm_sha.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ static int FSPSM_HashFinal(wolfssl_FSPSM_Hash* hash, byte* out, word32 outSz)
#endif
wc_fspsm_hw_lock();

if (Init(&handle) == FSP_SUCCESS) {
if ((ret = Init(&handle)) == FSP_SUCCESS) {
ret = Update(&handle, (uint8_t*)hash->msg, hash->used);
if (ret == FSP_SUCCESS) {
ret = Final(&handle, out, (uint32_t*)&sz);
Expand All @@ -433,6 +433,10 @@ static int FSPSM_HashFinal(wolfssl_FSPSM_Hash* hash, byte* out, word32 outSz)
}
}
}

@dgarske dgarske Jul 31, 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] FSPSM_HashFinal maps Init/Update failures to WC_HW_E without a WOLFSSL_ERROR trace

The inner Final-failure path logs before converting (WOLFSSL_MSG + WOLFSSL_ERROR(WC_HW_E) at 429-431), and the RSIP branch does the same at 474-476. The new blanket conversion at 436-438 does neither, so an Init() or Update() hardware failure — the case this hunk exists to surface — now returns WC_HW_E with no trace entry and the original FSP error code discarded. On a bring-up-heavy platform that is the difference between a diagnosable failure and a silent -248. (I verified the mapping is otherwise correct and clobbers nothing: FSP_SUCCESS is 0, the only wolfSSL code assigned inside the block is WC_HW_E itself, and the pre-lock FSPSM_FuncGet return at 414-416 is unaffected.)

Fix: Log before converting, matching the file's existing style, and apply the same to FSPSM_HashGet when fixing the BLOCK finding above.

if (ret != FSP_SUCCESS)
ret = WC_HW_E;

wc_fspsm_hw_unlock();

#elif defined(WOLFSSL_RENESAS_RSIP)
Expand Down
2 changes: 1 addition & 1 deletion wolfcrypt/src/port/caam/wolfcaam_ecdsa.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ int wc_CAAM_EccSign(const byte* in, int inlen, byte* out, word32* outlen,

/* private key */
if (key->blackKey == CAAM_BLACK_KEY_SM) {
buf[idx].TheAddress = (CAAM_ADDRESS)key->blackKey;
buf[idx].TheAddress = key->blackKey;
args[0] = CAAM_BLACK_KEY_SM; /* is a black key in sm */
buf[idx].Length = keySz;
}
Expand Down
2 changes: 1 addition & 1 deletion wolfcrypt/src/port/caam/wolfcaam_hmac.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ int wc_CAAM_Hmac(Hmac* hmac, int macType, const byte* msg, int msgSz,
{
int ret = 0;

if (hmac->ctx.cfd == -1 && hmac->keyLen > 0) {
if (hmac->ctx.inited == 0 && hmac->keyLen > 0) {
ret = wc_DevCrypto_HmacSetKey(hmac, macType, hmac->keyRaw,
hmac->keyLen);
if (ret != 0) {
Expand Down
16 changes: 7 additions & 9 deletions wolfcrypt/src/port/devcrypto/devcrypto_aes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ int wc_AesCbcEncrypt(Aes* aes, byte* out, const byte* in, word32 sz)
if (sz == 0) {
return 0;
}
if (aes->ctx.cfd == -1) {
if (aes->ctx.inited == 0) {
ret = wc_DevCryptoCreate(&aes->ctx, CRYPTO_AES_CBC,
(byte*)aes->devKey, aes->keylen);
if (ret != 0)
Expand Down Expand Up @@ -82,7 +82,7 @@ int wc_AesCbcDecrypt(Aes* aes, byte* out, const byte* in, word32 sz)
}

XMEMCPY(aes->tmp, in + sz - WC_AES_BLOCK_SIZE, WC_AES_BLOCK_SIZE);
if (aes->ctx.cfd == -1) {
if (aes->ctx.inited == 0) {
ret = wc_DevCryptoCreate(&aes->ctx, CRYPTO_AES_CBC,
(byte*)aes->devKey, aes->keylen);
if (ret != 0)
Expand Down Expand Up @@ -110,7 +110,7 @@ int wc_AesSetKey(Aes* aes, const byte* userKey, word32 keylen,
const word32 max_key_len = (AES_MAX_KEY_SIZE / 8);
#endif

if (aes == NULL ||
if (aes == NULL || userKey == NULL ||
!((keylen == 16) || (keylen == 24) || (keylen == 32))) {
return BAD_FUNC_ARG;
}
Expand All @@ -129,6 +129,7 @@ int wc_AesSetKey(Aes* aes, const byte* userKey, word32 keylen,
aes->left = 0;
#endif
aes->ctx.cfd = -1;

@dgarske dgarske Jul 31, 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] wc_AesSetKey clears the devcrypto context without releasing an already-open session

The PR touches these exact two lines (adding aes->ctx.inited = 0;). Calling wc_AesSetKey()/wc_AesGcmSetKey() twice on the same Aes — a supported pattern, e.g. TLS key update — resets cfd and inited without wc_DevCryptoFree(). The previously-opened cloned /dev/crypto descriptor and kernel session are orphaned: the fd leaks, and because the teardown guard is now inited, wc_AesFree() will not reclaim it either. Same pattern in wc_DevCrypto_HmacSetKey (devcrypto_hmac.c:53-54) and wc_AesSetKeyLocal (aes.c:5851-5852). The leak predates the PR, but the PR introduces the very lifecycle flag that should close it and instead cements it.

Fix: Replace the two assignments with wc_DevCryptoFree(&aes->ctx); (which already sets cfd = -1 and inited = 0 when a session exists) plus an explicit aes->ctx.cfd = -1; for the never-opened case. Apply the same to wc_DevCrypto_HmacSetKey.

aes->ctx.inited = 0;
XMEMCPY(aes->devKey, userKey, keylen);

(void)dir;
Expand All @@ -151,7 +152,7 @@ static int wc_DevCrypto_AesDirect(Aes* aes, byte* out, const byte* in,
return BAD_FUNC_ARG;
}

if (aes->ctx.cfd == -1) {
if (aes->ctx.inited == 0) {
ret = wc_DevCryptoCreate(&aes->ctx, CRYPTO_AES_ECB, (byte*)aes->devKey,
aes->keylen);
if (ret != 0)
Expand Down Expand Up @@ -221,7 +222,7 @@ int wc_AesCtrEncrypt(Aes* aes, byte* out, const byte* in, word32 sz)
sz--;
}

if (aes->ctx.cfd == -1) {
if (aes->ctx.inited == 0) {
ret = wc_DevCryptoCreate(&aes->ctx, CRYPTO_AES_CTR, (byte*)aes->devKey,
aes->keylen);
if (ret != 0)
Expand Down Expand Up @@ -284,8 +285,6 @@ int wc_AesGcmSetKey(Aes* aes, const byte* key, word32 len)
return wc_AesSetKey(aes, key, len, NULL, AES_ENCRYPTION);
}



/* common code for AES-GCM encrypt/decrypt */
static int wc_DevCrypto_AesGcm(Aes* aes, byte* out, byte* in, word32 sz,
const byte* iv, word32 ivSz,
Expand All @@ -310,7 +309,7 @@ static int wc_DevCrypto_AesGcm(Aes* aes, byte* out, byte* in, word32 sz,
in = scratch;

XMEMSET(scratch, 0, WC_AES_BLOCK_SIZE);
if (aes->ctx.cfd == -1) {
if (aes->ctx.inited == 0) {
ret = wc_DevCryptoCreate(&aes->ctx, CRYPTO_AES_GCM, (byte*)aes->devKey,
aes->keylen);
if (ret != 0)
Expand Down Expand Up @@ -399,4 +398,3 @@ int wc_AesEcbDecrypt(Aes* aes, byte* out, const byte* in, word32 sz)
#endif /* HAVE_AES_ECB */
#endif /* WOLFSSL_DEVCRYPTO_AES */
#endif /* !NO_AES && WOLFSSL_DEVCRYPTO */

Loading
Loading