Skip to content

Fix generic bugs, memory leaks, and migrate to wc_ForceZero #274

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

Merged
philljj merged 1 commit into from
Jul 17, 2026
+158 −72
Merged

Fix generic bugs, memory leaks, and migrate to wc_ForceZero #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
154 changes: 101 additions & 53 deletions src/sign-verify/clu_sign.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,67 +25,86 @@
#include <wolfclu/x509/clu_cert.h>
#include <wolfclu/genkey/clu_genkey.h> /* for xmss callback functions */

/* Fallback for older wolfSSL builds that have HAVE_DILITHIUM but predate the
* PEM size constants. DILITHIUM_LEVEL5_BOTH_KEY_PEM_SIZE (10267) is the
* largest possible key PEM size and is used as a safe upper bound. */
#ifdef HAVE_DILITHIUM
#ifndef DILITHIUM_MAX_BOTH_KEY_PEM_SIZE
#define DILITHIUM_MAX_BOTH_KEY_PEM_SIZE 10267
#endif
#endif
#include <limits.h>

/* Upper bound on DER size out of wolfCLU_KeyPemToDer, covering all key types. */
#ifndef WOLFCLU_MAX_KEY_PEM_DER_SZ
#define WOLFCLU_MAX_KEY_PEM_DER_SZ 65536
#endif /* WOLFCLU_MAX_KEY_PEM_DER_SZ */

#ifndef WOLFCLU_NO_FILESYSTEM

int wolfCLU_KeyPemToDer(unsigned char** pkeyBuf, int pkeySz, int pubIn) {
int ret = 0;
byte* der = NULL;
const unsigned char* keyBuf = *pkeyBuf;
const unsigned char* keyBuf;

if (pkeyBuf == NULL || *pkeyBuf == NULL || pkeySz <= 0) {
return BAD_FUNC_ARG;
}
keyBuf = *pkeyBuf;

if (pubIn == 0) {
ret = wc_KeyPemToDer(keyBuf, pkeySz, NULL, 0, NULL);
if (ret > 0) {
wolfCLU_Log(WOLFCLU_L0, "DER size: %d", ret);
int derSz = ret;
der = (byte*)XMALLOC(derSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
if (der == NULL) {
wolfCLU_Log(WOLFCLU_L0, "Failed to allocate memory for DER");
ret = MEMORY_E;
if (derSz > WOLFCLU_MAX_KEY_PEM_DER_SZ) {
/* Leave *pkeyBuf intact; caller frees it. */
wolfCLU_LogError("PEM private key DER size exceeds limit");
ret = WOLFCLU_FATAL_ERROR;
}
else {
ret = wc_KeyPemToDer(keyBuf, pkeySz, der, derSz, NULL);
if (ret > 0) {
wolfCLU_Log(WOLFCLU_L0, "DER size2: %d", ret);
/* replace incoming pkeyBuf with new der buf */
XFREE(*pkeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
*pkeyBuf = der;
der = (byte*)XMALLOC(derSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
if (der == NULL) {
wolfCLU_LogError("Failed to allocate memory for DER");
ret = MEMORY_E;
}
else {
wolfCLU_Log(WOLFCLU_L0, "Failed to convert PEM to DER");
/* failure, so cleanup */
XFREE(der, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
ret = wc_KeyPemToDer(keyBuf, pkeySz, der, derSz, NULL);
if (ret > 0) {
/* *pkeyBuf still points to the original PEM alloc here
* (pkeySz is its size); 'der' is the new DER alloc. */
wolfCLU_ForceZero(*pkeyBuf, (unsigned int)pkeySz);
XFREE(*pkeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
*pkeyBuf = der;
}
else {
wolfCLU_LogError("Failed to convert PEM to DER");
/* failure, so cleanup */
wolfCLU_ForceZero(der, (unsigned int)derSz);
XFREE(der, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
Comment thread
stenslae marked this conversation as resolved.
}
}
}
}
wolfCLU_Log(WOLFCLU_L0, "DER size3: %d", ret);
}
else {
ret = wc_PubKeyPemToDer(keyBuf, pkeySz, NULL, 0);
if (ret > 0) {
int derSz = ret;
der = (byte*)XMALLOC(derSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
if (der == NULL) {
ret = MEMORY_E;
if (derSz > WOLFCLU_MAX_KEY_PEM_DER_SZ) {
wolfCLU_LogError("PEM public key DER size exceeds limit");
ret = WOLFCLU_FATAL_ERROR;
}
else {
ret = wc_PubKeyPemToDer(keyBuf, pkeySz, der, derSz);
if (ret > 0) {
/* replace incoming pkeyBuf with new der buf */
XFREE(*pkeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
*pkeyBuf = der;
der = (byte*)XMALLOC(derSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
if (der == NULL) {
ret = MEMORY_E;
}
else {
/* failure, so cleanup */
XFREE(der, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
ret = wc_PubKeyPemToDer(keyBuf, pkeySz, der, derSz);
if (ret > 0) {
/* replace incoming pkeyBuf with new der buf */
wolfCLU_ForceZero(*pkeyBuf, (unsigned int)pkeySz);
XFREE(*pkeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
*pkeyBuf = der;
}
else {
wolfCLU_LogError(
"Failed to convert public key PEM to DER");
wolfCLU_ForceZero(der, (unsigned int)derSz);
XFREE(der, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
}
}
}
}
Expand Down Expand Up @@ -121,13 +140,14 @@ int wolfCLU_sign_data(char* in, char* out, char* privKey, int keyType,
}
fSz = (int)fTell;

data = (byte*)XMALLOC(fSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
data = (byte*)XMALLOC((size_t)fSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
if (data == NULL) {
XFCLOSE(f);
return MEMORY_E;
}

if (XFSEEK(f, 0, SEEK_SET) != 0 || (int)XFREAD(data, 1, fSz, f) != fSz) {
if (XFSEEK(f, 0, SEEK_SET) != 0 ||
XFREAD(data, 1, (size_t)fSz, f) != (size_t)fSz) {
XFREE(data, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
XFCLOSE(f);
return WOLFCLU_FATAL_ERROR;
Expand All @@ -137,20 +157,22 @@ int wolfCLU_sign_data(char* in, char* out, char* privKey, int keyType,
switch(keyType) {

case RSA_SIG_VER:
ret = wolfCLU_sign_data_rsa(data, out, fSz, privKey, inForm);
ret = wolfCLU_sign_data_rsa(data, out, (word32)fSz, privKey, inForm);
break;

case ECC_SIG_VER:
ret = wolfCLU_sign_data_ecc(data, out, fSz, privKey, inForm);
ret = wolfCLU_sign_data_ecc(data, out, (word32)fSz, privKey, inForm);
break;

case ED25519_SIG_VER:
ret = wolfCLU_sign_data_ed25519(data, out, fSz, privKey, inForm);
ret = wolfCLU_sign_data_ed25519(data, out, (word32)fSz, privKey,
inForm);
break;

#ifdef HAVE_DILITHIUM
case DILITHIUM_SIG_VER:
ret = wolfCLU_sign_data_dilithium(data, out, fSz, privKey, inForm);
ret = wolfCLU_sign_data_dilithium(data, out, (word32)fSz, privKey,
inForm);
break;
#endif

Expand Down Expand Up @@ -256,7 +278,14 @@ int wolfCLU_sign_data_rsa(byte* data, char* out, word32 dataSz, char* privKey,
if (inForm == PEM_FORM && ret == 0) {
ret = wolfCLU_KeyPemToDer(&keyBuf, privFileSz, 0);
if (ret < 0) {
wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret);
if (ret == WC_NO_ERR_TRACE(ASN_NO_PEM_HEADER)) {
WOLFCLU_LOG(WOLFCLU_L0,
"No PEM header found, treating as DER.");
ret = 0;
}
else {
wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret);
}
}
else {
privFileSz = ret;
Expand Down Expand Up @@ -317,7 +346,7 @@ int wolfCLU_sign_data_rsa(byte* data, char* out, word32 dataSz, char* privKey,
}

if (keyBuf!= NULL) {
wolfCLU_ForceZero(keyBuf, privFileSz);
wolfCLU_ForceZero(keyBuf, (unsigned int)privFileSz);
XFREE(keyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
}
if (outBuf!= NULL) {
Expand Down Expand Up @@ -409,7 +438,14 @@ int wolfCLU_sign_data_ecc(byte* data, char* out, word32 fSz, char* privKey,
if (inForm == PEM_FORM && ret == 0) {
ret = wolfCLU_KeyPemToDer(&keyBuf, privFileSz, 0);
if (ret < 0) {
wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret);
if (ret == WC_NO_ERR_TRACE(ASN_NO_PEM_HEADER)) {
WOLFCLU_LOG(WOLFCLU_L0,
"No PEM header found, treating as DER.");
ret = 0;
}
else {
wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret);
}
}
else {
privFileSz = ret;
Expand Down Expand Up @@ -494,7 +530,7 @@ int wolfCLU_sign_data_ecc(byte* data, char* out, word32 fSz, char* privKey,
}

if (keyBuf!= NULL) {
wolfCLU_ForceZero(keyBuf, privFileSz);
wolfCLU_ForceZero(keyBuf, (unsigned int)privFileSz);
XFREE(keyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
}
if (outBuf!= NULL) {
Expand Down Expand Up @@ -585,7 +621,14 @@ int wolfCLU_sign_data_ed25519 (byte* data, char* out, word32 fSz, char* privKey,
if (inForm == PEM_FORM && ret == 0) {
ret = wolfCLU_KeyPemToDer(&keyBuf, privFileSz, 0);
if (ret < 0) {
wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret);
if (ret == WC_NO_ERR_TRACE(ASN_NO_PEM_HEADER)) {
WOLFCLU_LOG(WOLFCLU_L0,
"No PEM header found, treating as DER.");
ret = 0;
}
else {
wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret);
}
}
else {
privFileSz = ret;
Expand Down Expand Up @@ -659,7 +702,7 @@ int wolfCLU_sign_data_ed25519 (byte* data, char* out, word32 fSz, char* privKey,
}

if (keyBuf!= NULL) {
wolfCLU_ForceZero(keyBuf, privFileSz);
wolfCLU_ForceZero(keyBuf, (unsigned int)privFileSz);
XFREE(keyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
}
if (outBuf!= NULL) {
Expand All @@ -679,7 +722,7 @@ int wolfCLU_sign_data_ed25519 (byte* data, char* out, word32 fSz, char* privKey,
int wolfCLU_sign_data_dilithium (byte* data, char* out, word32 dataSz, char* privKey,
int inForm)
{
#ifdef HAVE_DILITHIUM
#if defined(HAVE_DILITHIUM)
int ret = 0;
int privFileSz = 0;
word32 index = 0;
Expand All @@ -705,7 +748,6 @@ int wolfCLU_sign_data_dilithium (byte* data, char* out, word32 dataSz, char* pri
dilithium_key key[1];
#endif

/* zero before init for defensive security */
XMEMSET(&rng, 0, sizeof(rng));
XMEMSET(key, 0, sizeof(dilithium_key));

Expand Down Expand Up @@ -770,7 +812,14 @@ int wolfCLU_sign_data_dilithium (byte* data, char* out, word32 dataSz, char* pri
if (inForm == PEM_FORM && ret == 0) {
ret = wolfCLU_KeyPemToDer(&privBuf, privFileSz, 0);
if (ret < 0) {
wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret);
if (ret == WC_NO_ERR_TRACE(ASN_NO_PEM_HEADER)) {
WOLFCLU_LOG(WOLFCLU_L0,
"No PEM header found, treating as DER.");
ret = 0;
}
else {
wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret);
}
}
else {
/* update privBuf and privFileSz with the converted DER data */
Expand Down Expand Up @@ -827,7 +876,7 @@ int wolfCLU_sign_data_dilithium (byte* data, char* out, word32 dataSz, char* pri
XFCLOSE(privKeyFile);

if (privBuf != NULL) {
wolfCLU_ForceZero(privBuf, privBufSz);
wolfCLU_ForceZero(privBuf, (unsigned int)privBufSz);
XFREE(privBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
}

Expand All @@ -836,9 +885,8 @@ int wolfCLU_sign_data_dilithium (byte* data, char* out, word32 dataSz, char* pri
}

wc_dilithium_free(key);
/* rng was zeroed via XMEMSET before wc_InitRng, so wc_FreeRng is safe
* even if wc_InitRng failed: wolfSSL checks rng->drbg != NULL internally
* before freeing any resources. */
/* rng zeroed via XMEMSET before wc_InitRng, so even if wc_InitRng failed:
* wolfSSL checks rng->drbg internally before freeing. */
wc_FreeRng(&rng);
#ifdef WOLFSSL_SMALL_STACK
XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
Expand Down
Loading
Loading