Skip to content

MemZero: Add more checks of buffers. #10988

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

MemZero: Add more checks of buffers. #10988

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
11 changes: 11 additions & 0 deletions src/dtls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,10 +571,21 @@ static void FindPskSuiteFromExt(const WOLFSSL* ssl, TLSX* extensions,
byte psk_key[MAX_PSK_KEY_LEN];
word32 psk_keySz;
byte foundSuite[SUITE_LEN];
#ifdef WOLFSSL_CHECK_MEM_ZERO
/* Register before the key is populated so any future path that
* fails to clear it before scope exit is caught. Baseline the
* buffer so it is defined at registration time. */
XMEMSET(psk_key, 0, sizeof(psk_key));
wc_MemZero_Add("FindPskSuiteFromExt psk_key", psk_key,
sizeof(psk_key));
#endif
ret = FindPskSuite(ssl, current, psk_key, &psk_keySz,
suites->suites + i, &found, foundSuite);
/* Clear the key just in case */
ForceZero(psk_key, sizeof(psk_key));
#ifdef WOLFSSL_CHECK_MEM_ZERO
wc_MemZero_Check(psk_key, sizeof(psk_key));
#endif
if (ret == 0 && found) {
pskInfo->cipherSuite0 = foundSuite[0];
pskInfo->cipherSuite = foundSuite[1];
Expand Down
32 changes: 32 additions & 0 deletions src/pk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4933,6 +4933,13 @@ static int _DH_compute_key(unsigned char* key, const WOLFSSL_BIGNUM* otherPub,
if (privSz <= 0) {
ret = WOLFSSL_FATAL_ERROR;
}
#if defined(WOLFSSL_CHECK_MEM_ZERO) && !defined(WOLFSSL_SMALL_STACK)
/* Register as soon as the stack array holds the private key so any
* future path that fails to zeroize it before exit is caught. */
else {
wc_MemZero_Add("_DH_compute_key priv", priv, (word32)privSz);
}
#endif
}
if (ret == 0) {
/* Get the public key into the array. */
Expand Down Expand Up @@ -4996,6 +5003,9 @@ static int _DH_compute_key(unsigned char* key, const WOLFSSL_BIGNUM* otherPub,
{
/* Zeroize sensitive data. */
ForceZero(priv, (word32)privSz);
#if defined(WOLFSSL_CHECK_MEM_ZERO) && !defined(WOLFSSL_SMALL_STACK)
wc_MemZero_Check(priv, sizeof(priv));
#endif
}
}
WC_FREE_VAR_EX(pub, NULL, DYNAMIC_TYPE_PUBLIC_KEY);
Expand Down Expand Up @@ -7226,6 +7236,14 @@ int wolfSSL_PEM_do_header(EncryptedInfo* cipher, unsigned char* data, long* len,
if (passwordSz < 0) {
ret = 0;
}
#ifdef WOLFSSL_CHECK_MEM_ZERO
/* Register as soon as the stack buffer holds the secret so any future
* path that fails to zeroize it before exit is caught. */
else if (passwordSz > 0) {
wc_MemZero_Add("wolfSSL_PEM_do_header password", password,
(word32)passwordSz);
}
#endif
}

if (ret == 1) {
Expand All @@ -7239,6 +7257,9 @@ int wolfSSL_PEM_do_header(EncryptedInfo* cipher, unsigned char* data, long* len,
if (passwordSz > 0) {
/* Ensure password is erased from memory. */
ForceZero(password, (word32)passwordSz);
#ifdef WOLFSSL_CHECK_MEM_ZERO
wc_MemZero_Check(password, NAME_SZ);
#endif
}

return ret;
Expand Down Expand Up @@ -7476,6 +7497,14 @@ static int pem_write_mem_pkcs8privatekey(byte** pem, int* pemSz,
res = 0;
}
passwd = password;
#ifdef WOLFSSL_CHECK_MEM_ZERO
/* Register as soon as the stack buffer holds the secret so any
* future path that fails to zeroize it before exit is caught. */
if (passwdSz > 0) {
wc_MemZero_Add("pem_write_mem_pkcs8privatekey password",
password, (word32)passwdSz);
}
#endif
}

if (res == 1) {
Expand All @@ -7489,6 +7518,9 @@ static int pem_write_mem_pkcs8privatekey(byte** pem, int* pemSz,
/* Zeroize the password from memory. */
if ((password == passwd) && (passwdSz > 0)) {
ForceZero(password, (word32)passwdSz);
#ifdef WOLFSSL_CHECK_MEM_ZERO
wc_MemZero_Check(password, NAME_SZ);
#endif
}
}
else if ((res == 1) && (enc == NULL)) {
Expand Down
26 changes: 26 additions & 0 deletions src/sniffer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7594,6 +7594,22 @@ static int parseKeyLogFile(const char* fileName, char* error)
return WOLFSSL_SNIFFER_ERROR;
}

#ifdef WOLFSSL_CHECK_MEM_ZERO
/* Register the secret-bearing stack buffers as high as possible (right
* after the last early return that bypasses the ForceZeros, i.e. the
* fopen failure above) so any future path that fails to clear them before
* scope exit is caught. Every exit path below (the in-loop error return
* and the normal post-loop return) ForceZeros and Checks each of them, so
* exactly one Add is balanced by one Check. Baseline the not-yet-written
* buffers so they are defined at registration time (secretHex is already
* zero-initialized at declaration). */
XMEMSET(secret, 0, sizeof(secret));
XMEMSET(line, 0, sizeof(line));
wc_MemZero_Add("parseKeyLogFile secret", secret, sizeof(secret));
wc_MemZero_Add("parseKeyLogFile secretHex", secretHex, sizeof(secretHex));
wc_MemZero_Add("parseKeyLogFile line", line, sizeof(line));
#endif

while (fgets(line, (int)sizeof(line), file) != NULL) {
/* RFC 9850 Section 1: ignore empty lines and lines whose first
* character is the octothorpe ('#') comment marker. */
Expand Down Expand Up @@ -7658,6 +7674,11 @@ static int parseKeyLogFile(const char* fileName, char* error)
ForceZero(secret, SECRET_LENGTH);
ForceZero(secretHex, sizeof(secretHex));
ForceZero(line, sizeof(line));
#ifdef WOLFSSL_CHECK_MEM_ZERO
wc_MemZero_Check(secret, sizeof(secret));
wc_MemZero_Check(secretHex, sizeof(secretHex));
wc_MemZero_Check(line, sizeof(line));
#endif
return ret;
}
}
Expand All @@ -7666,6 +7687,11 @@ static int parseKeyLogFile(const char* fileName, char* error)
ForceZero(secret, SECRET_LENGTH);
ForceZero(secretHex, sizeof(secretHex));
ForceZero(line, sizeof(line));
#ifdef WOLFSSL_CHECK_MEM_ZERO
wc_MemZero_Check(secret, sizeof(secret));
wc_MemZero_Check(secretHex, sizeof(secretHex));
wc_MemZero_Check(line, sizeof(line));
#endif
return 0;
}

Expand Down
33 changes: 31 additions & 2 deletions src/tls13.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,14 @@ int Tls13_Exporter(WOLFSSL* ssl, unsigned char *out, size_t outLen,
return BAD_FUNC_ARG;
}

#ifdef WOLFSSL_CHECK_MEM_ZERO
/* Poison and register firstExpand before it is written so that any path
* below (all of which funnel through cleanup) is covered. */
XMEMSET(firstExpand, 0xff, sizeof(firstExpand));
wc_MemZero_Add("Tls13_Exporter firstExpand", firstExpand,
sizeof(firstExpand));
#endif

/* Derive-Secret(Secret, label, "") */
ret = Tls13HKDFExpandLabel(ssl, firstExpand, hashLen,
ssl->arrays->exporterSecret, hashLen,
Expand All @@ -1076,6 +1084,9 @@ int Tls13_Exporter(WOLFSSL* ssl, unsigned char *out, size_t outLen,
* Hash(context_value); wipe both before the stack frame is reclaimed. */
ForceZero(firstExpand, sizeof(firstExpand));
ForceZero(hashOut, sizeof(hashOut));
#ifdef WOLFSSL_CHECK_MEM_ZERO
wc_MemZero_Check(firstExpand, sizeof(firstExpand));
#endif
return ret;
}
#endif
Expand Down Expand Up @@ -1533,6 +1544,11 @@ int DeriveTls13Keys(WOLFSSL* ssl, int secret, int side, int store)
WC_ALLOC_VAR_EX(key_dig, byte, MAX_PRF_DIG, ssl->heap,
DYNAMIC_TYPE_DIGEST, return MEMORY_E);

#ifdef WOLFSSL_CHECK_MEM_ZERO
XMEMSET(key_dig, 0xff, MAX_PRF_DIG);
wc_MemZero_Add("DeriveTls13Keys key_dig", key_dig, MAX_PRF_DIG);
#endif

if (side == ENCRYPT_AND_DECRYPT_SIDE) {
provision = PROVISION_CLIENT_SERVER;
}
Expand Down Expand Up @@ -1665,7 +1681,8 @@ int DeriveTls13Keys(WOLFSSL* ssl, int secret, int side, int store)
WOLFSSL_SERVER_END);
if (ret != 0)
goto end;
i += ssl->specs.iv_size;
/* Server IV is the last key material written to key_dig, so i is not
* advanced here; the whole buffer is zeroed at end regardless. */
}

/* Store keys and IVs but don't activate them. */
Expand Down Expand Up @@ -1717,7 +1734,9 @@ int DeriveTls13Keys(WOLFSSL* ssl, int secret, int side, int store)
#endif /* WOLFSSL_DTLS13 */

end:
ForceZero(key_dig, (word32)i);
/* Zero the whole key_dig buffer (not just the i bytes derived) so no
* key-schedule material can linger in the unused tail. */
ForceZero(key_dig, MAX_PRF_DIG);
#ifdef WOLFSSL_SMALL_STACK
XFREE(key_dig, ssl->heap, DYNAMIC_TYPE_DIGEST);
#elif defined(WOLFSSL_CHECK_MEM_ZERO)
Expand Down Expand Up @@ -6523,6 +6542,13 @@ static int DoPreSharedKeys(WOLFSSL* ssl, const byte* input, word32 inputSz,

(void)suite;

#ifdef WOLFSSL_CHECK_MEM_ZERO
/* Poison and register binderKey up front; every exit below (including the
* error paths) funnels through the cleanup label which zeroes it. */
XMEMSET(binderKey, 0xff, sizeof(binderKey));
wc_MemZero_Add("DoPreSharedKeys binderKey", binderKey, sizeof(binderKey));
#endif

ext = TLSX_Find(ssl->extensions, TLSX_PRE_SHARED_KEY);
if (ext == NULL) {
WOLFSSL_MSG("No pre shared extension keys found");
Expand Down Expand Up @@ -6739,6 +6765,9 @@ static int DoPreSharedKeys(WOLFSSL* ssl, const byte* input, word32 inputSz,
cleanup:
ForceZero(binderKey, sizeof(binderKey));
ForceZero(binder, sizeof(binder));
#ifdef WOLFSSL_CHECK_MEM_ZERO
wc_MemZero_Check(binderKey, sizeof(binderKey));
#endif
WOLFSSL_LEAVE("DoPreSharedKeys", ret);

return ret;
Expand Down
Loading
Loading