Skip to content

Improve HMAC and SHA-2 Zeroing #9785

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
rizlik wants to merge 2 commits into
base: master
Choose a base branch
from
+81 −77
Open

Improve HMAC and SHA-2 Zeroing #9785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
614940a
hmac: refactor out hmac->hash clean-up
rizlik Feb 17, 2026
3c8af83
wc: ForceZero only sensitive fields in HMAC and SHA-2
rizlik Feb 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
122 changes: 45 additions & 77 deletions wolfcrypt/src/hmac.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1307,140 +1307,108 @@ int wc_HmacInit_Label(Hmac* hmac, const char* label, void* heap, int devId)
}
#endif /* WOLF_PRIVATE_KEY_ID */

/* Free Hmac from use with async device */
void wc_HmacFree(Hmac* hmac)
/* Free hash state. */
static void HmacFreeHash(int macType, wc_HmacHash* hash)
{
if (hmac == NULL)
return;

#ifdef WOLF_CRYPTO_CB
/* handle cleanup case where final is not called */
if (hmac->devId != INVALID_DEVID && hmac->devCtx != NULL) {
int ret;
byte finalHash[WC_HMAC_BLOCK_SIZE];
ret = wc_CryptoCb_Hmac(hmac, hmac->macType, NULL, 0, finalHash);
(void)ret; /* must ignore return code here */
(void)finalHash;
}
#endif

#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_HMAC)
wolfAsync_DevCtxFree(&hmac->asyncDev, WOLFSSL_ASYNC_MARKER_HMAC);
#endif /* WOLFSSL_ASYNC_CRYPT */

switch (hmac->macType) {
switch (macType) {
#ifndef NO_MD5
case WC_MD5:
wc_Md5Free(&hmac->hash.md5);
#ifdef WOLFSSL_HMAC_COPY_HASH
wc_Md5Free(&hmac->i_hash.md5);
wc_Md5Free(&hmac->o_hash.md5);
#endif
wc_Md5Free(&hash->md5);
break;
#endif /* !NO_MD5 */

#ifndef NO_SHA
case WC_SHA:
wc_ShaFree(&hmac->hash.sha);
#ifdef WOLFSSL_HMAC_COPY_HASH
wc_ShaFree(&hmac->i_hash.sha);
wc_ShaFree(&hmac->o_hash.sha);
#endif
wc_ShaFree(&hash->sha);
break;
#endif /* !NO_SHA */

#ifdef WOLFSSL_SHA224
case WC_SHA224:
wc_Sha224Free(&hmac->hash.sha224);
#ifdef WOLFSSL_HMAC_COPY_HASH
wc_Sha224Free(&hmac->i_hash.sha224);
wc_Sha224Free(&hmac->o_hash.sha224);
#endif
wc_Sha224Free(&hash->sha224);
break;
#endif /* WOLFSSL_SHA224 */
#ifndef NO_SHA256
case WC_SHA256:
wc_Sha256Free(&hmac->hash.sha256);
#ifdef WOLFSSL_HMAC_COPY_HASH
wc_Sha256Free(&hmac->i_hash.sha256);
wc_Sha256Free(&hmac->o_hash.sha256);
#endif
wc_Sha256Free(&hash->sha256);
break;
#endif /* !NO_SHA256 */

#ifdef WOLFSSL_SHA384
case WC_SHA384:
wc_Sha384Free(&hmac->hash.sha384);
#ifdef WOLFSSL_HMAC_COPY_HASH
wc_Sha384Free(&hmac->i_hash.sha384);
wc_Sha384Free(&hmac->o_hash.sha384);
#endif
wc_Sha384Free(&hash->sha384);
break;
#endif /* WOLFSSL_SHA384 */
#ifdef WOLFSSL_SHA512
case WC_SHA512:
wc_Sha512Free(&hmac->hash.sha512);
#ifdef WOLFSSL_HMAC_COPY_HASH
wc_Sha512Free(&hmac->i_hash.sha512);
wc_Sha512Free(&hmac->o_hash.sha512);
#endif
wc_Sha512Free(&hash->sha512);
break;
#endif /* WOLFSSL_SHA512 */

#ifdef WOLFSSL_SHA3
#ifndef WOLFSSL_NOSHA3_224
case WC_SHA3_224:
wc_Sha3_224_Free(&hmac->hash.sha3);
#ifdef WOLFSSL_HMAC_COPY_HASH
wc_Sha3_224_Free(&hmac->i_hash.sha3);
wc_Sha3_224_Free(&hmac->o_hash.sha3);
#endif
wc_Sha3_224_Free(&hash->sha3);
break;
#endif
#ifndef WOLFSSL_NOSHA3_256
case WC_SHA3_256:
wc_Sha3_256_Free(&hmac->hash.sha3);
#ifdef WOLFSSL_HMAC_COPY_HASH
wc_Sha3_256_Free(&hmac->i_hash.sha3);
wc_Sha3_256_Free(&hmac->o_hash.sha3);
#endif
wc_Sha3_256_Free(&hash->sha3);
break;
#endif
#ifndef WOLFSSL_NOSHA3_384
case WC_SHA3_384:
wc_Sha3_384_Free(&hmac->hash.sha3);
#ifdef WOLFSSL_HMAC_COPY_HASH
wc_Sha3_384_Free(&hmac->i_hash.sha3);
wc_Sha3_384_Free(&hmac->o_hash.sha3);
#endif
wc_Sha3_384_Free(&hash->sha3);
break;
#endif
#ifndef WOLFSSL_NOSHA3_512
case WC_SHA3_512:
wc_Sha3_512_Free(&hmac->hash.sha3);
#ifdef WOLFSSL_HMAC_COPY_HASH
wc_Sha3_512_Free(&hmac->i_hash.sha3);
wc_Sha3_512_Free(&hmac->o_hash.sha3);
#endif
wc_Sha3_512_Free(&hash->sha3);
break;
#endif
#endif /* WOLFSSL_SHA3 */

#ifdef WOLFSSL_SM3
case WC_SM3:
wc_Sm3Free(&hmac->hash.sm3);
#ifdef WOLFSSL_HMAC_COPY_HASH
wc_Sm3Free(&hmac->i_hash.sm3);
wc_Sm3Free(&hmac->o_hash.sm3);
#endif
wc_Sm3Free(&hash->sm3);
break;
#endif

default:
break;
}
}

/* Free Hmac from use with async device */
void wc_HmacFree(Hmac* hmac)
{
if (hmac == NULL)
return;

#ifdef WOLF_CRYPTO_CB
/* handle cleanup case where final is not called */
if (hmac->devId != INVALID_DEVID && hmac->devCtx != NULL) {
int ret;
byte finalHash[WC_HMAC_BLOCK_SIZE];
ret = wc_CryptoCb_Hmac(hmac, hmac->macType, NULL, 0, finalHash);
(void)ret; /* must ignore return code here */
(void)finalHash;
}
#endif

#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_HMAC)
wolfAsync_DevCtxFree(&hmac->asyncDev, WOLFSSL_ASYNC_MARKER_HMAC);
#endif /* WOLFSSL_ASYNC_CRYPT */

HmacFreeHash(hmac->macType, &hmac->hash);
#ifdef WOLFSSL_HMAC_COPY_HASH
HmacFreeHash(hmac->macType, &hmac->i_hash);
HmacFreeHash(hmac->macType, &hmac->o_hash);
#endif

ForceZero(hmac, sizeof(*hmac));
ForceZero(hmac->ipad, sizeof(hmac->ipad));
ForceZero(hmac->opad, sizeof(hmac->opad));
ForceZero(hmac->innerHash, sizeof(hmac->innerHash));
Comment on lines +1403 to +1411
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

wc_HmacFree no longer wipes the hmac->hash / i_hash / o_hash unions themselves. Several underlying hash *Free functions (e.g. wc_Md5Free, wc_ShaFree) do not zero their internal digest/buffer state, so keyed intermediate state from HMAC can be left in memory after wc_HmacFree. Please explicitly zero the hash unions after calling HmacFreeHash (or otherwise ensure each possible hash *Free reliably wipes sensitive state) to avoid a security regression.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@rizlik rizlik Feb 17, 2026

Choose a reason for hiding this comment

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

MD5 and Sha zeroing should be handled in their respective freeing function imo, however I thought to do that in a following PR instead of this one.

}
#endif /* WOLFSSL_KCAPI_HMAC */

Expand Down
24 changes: 24 additions & 0 deletions wolfcrypt/src/sha256.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2347,7 +2347,19 @@ static WC_INLINE int Transform_Sha256_Len(wc_Sha256* sha256, const byte* data,
#if defined(PSOC6_HASH_SHA2)
wc_Psoc6_Sha_Free();
#endif
#if !defined(FREESCALE_LTC_SHA) && \
!(defined(WOLFSSL_SE050) && defined(WOLFSSL_SE050_HASH)) && \
!defined(STM32_HASH_SHA2) && \
!defined(WOLFSSL_SILABS_SE_ACCEL) && \
!defined(WOLFSSL_IMXRT_DCP) && \
!defined(PSOC6_HASH_SHA2)
/* PSA compiles out the free function completely */
ForceZero(sha224->buffer, sizeof(sha224->buffer));
if (sha224->hiLen != 0 || sha224->loLen != 0)
ForceZero(sha224->digest, sizeof(sha224->digest));
Comment on lines +2357 to +2359
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

wc_Sha224 is typedef'd to struct wc_Sha256, but that struct does not always contain buffer, digest, hiLen, or loLen (e.g. STM32_HASH_SHA2, FREESCALE_LTC_SHA, WOLFSSL_IMXRT_DCP, PSOC6_HASH_SHA2, etc. in sha256.h). These unguarded member accesses will break builds for those configurations. Please conditionally compile this wiping logic only when the software digest/buffer fields are present, or provide an alternative wipe path for the other struct variants.

Copilot uses AI. Check for mistakes.
#else
ForceZero(sha224, sizeof(*sha224));
#endif
}
#endif /* !defined(WOLFSSL_HAVE_PSA) || defined(WOLFSSL_PSA_NO_HASH) */
#endif /* WOLFSSL_SHA224 */
Expand Down Expand Up @@ -2494,7 +2506,19 @@ void wc_Sha256Free(wc_Sha256* sha256)
wc_Psoc6_Sha_Free();
#endif

#if !defined(FREESCALE_LTC_SHA) && \
!(defined(WOLFSSL_SE050) && defined(WOLFSSL_SE050_HASH)) && \
!defined(STM32_HASH_SHA2) && \
!defined(WOLFSSL_SILABS_SE_ACCEL) && \
!defined(WOLFSSL_IMXRT_DCP) && \
!defined(PSOC6_HASH_SHA2)
/* PSA compiles out the free function completely */
ForceZero(sha256->buffer, sizeof(sha256->buffer));
if (sha256->hiLen != 0 || sha256->loLen != 0)
ForceZero(sha256->digest, sizeof(sha256->digest));
Comment on lines +2516 to +2518
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

struct wc_Sha256 does not always define buffer, digest, hiLen, or loLen (see the multiple hardware/PSA variants in sha256.h). These unguarded member accesses will fail to compile when any of those alternative layouts are enabled. Suggest wrapping this zeroing logic in preprocessor checks that match the struct definition, or falling back to wiping the whole struct when the software fields are not available.

Suggested change
ForceZero(sha256->buffer, sizeof(sha256->buffer));
if (sha256->hiLen != 0 || sha256->loLen != 0)
ForceZero(sha256->digest, sizeof(sha256->digest));
ForceZero(sha256, sizeof(*sha256));

Copilot uses AI. Check for mistakes.
#else
ForceZero(sha256, sizeof(*sha256));
#endif
} /* wc_Sha256Free */

#endif /* !defined(WOLFSSL_HAVE_PSA) || defined(WOLFSSL_PSA_NO_HASH) */
Expand Down
12 changes: 12 additions & 0 deletions wolfcrypt/src/sha512.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1688,7 +1688,13 @@ void wc_Sha512Free(wc_Sha512* sha512)
wc_Psoc6_Sha_Free();
#endif

#if !defined(PSOC6_HASH_SHA2)
ForceZero(sha512->buffer, sizeof(sha512->buffer));
if (!(sha512->hiLen == 0 && sha512->loLen == 0))
ForceZero(sha512->digest, sizeof(sha512->digest));
Comment on lines +1692 to +1694
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

wc_Sha512 does not always have buffer, digest, hiLen, or loLen fields (e.g. when PSOC6_HASH_SHA2 is enabled in sha512.h). These direct member accesses will cause build failures for those configurations. Consider guarding these zeroing statements with the same preprocessor conditions as the struct layout (or falling back to ForceZero(sha512, sizeof(*sha512)) when the software fields are not present).

Copilot uses AI. Check for mistakes.
#else
ForceZero(sha512, sizeof(*sha512));
#endif
}
#endif

Expand Down Expand Up @@ -2176,7 +2182,13 @@ void wc_Sha384Free(wc_Sha384* sha384)
wc_MXC_TPU_SHA_Free(&(sha384->mxcCtx));
#endif

#if !defined(PSOC6_HASH_SHA2)
ForceZero(sha384->buffer, sizeof(sha384->buffer));
if (!(sha384->hiLen == 0 && sha384->loLen == 0))
ForceZero(sha384->digest, sizeof(sha384->digest));
Comment on lines +2186 to +2188
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

wc_Sha384 is a typedef of wc_Sha512, and in some configurations (notably PSOC6_HASH_SHA2) the struct layout does not include buffer, digest, hiLen, or loLen. These member accesses will not compile in those builds. Please add appropriate #if/#else guards (or a safe fallback wipe) that matches the struct definition in sha512.h.

Copilot uses AI. Check for mistakes.
#else
ForceZero(sha384, sizeof(*sha384));
#endif
}

#endif
Expand Down