From d1e43496618cb63005f611bcf99de8eb74df7a4e Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Thu, 23 Mar 2023 12:27:38 +1000 Subject: [PATCH] MemZero check fixes ForceZero the client and server secret regardless of whether TLS 1.3 as it may change but have been copied in. ForceZero the input buffer in wolfSSL_Clear() when encryption was on. Changed wc_PRF_TLS to only check the parts of data used. Changed where scatch is added for checking in wc_AesCtrEncrypt. Change wc_MakeRsaKey to memset p, q, tmp1, tmp2 and tmp3 to all zeros so that MemZero check works. Memset not needed otherwise. Changes for new compiler - thinks uninitialized. --- src/internal.c | 9 ++++----- src/ssl.c | 10 ++++++++++ wolfcrypt/src/aes.c | 10 +++++++--- wolfcrypt/src/cmac.c | 2 ++ wolfcrypt/src/kdf.c | 4 ++-- wolfcrypt/src/rsa.c | 7 +++++++ wolfcrypt/src/wc_encrypt.c | 2 ++ 7 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/internal.c b/src/internal.c index 8c4328e7c..f7b759b97 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7600,10 +7600,8 @@ void SSL_ResourceFree(WOLFSSL* ssl) ForceZero(&ssl->keys, sizeof(Keys)); #ifdef WOLFSSL_TLS13 - if (ssl->options.tls1_3) { - ForceZero(&ssl->clientSecret, sizeof(ssl->clientSecret)); - ForceZero(&ssl->serverSecret, sizeof(ssl->serverSecret)); - } + ForceZero(&ssl->clientSecret, sizeof(ssl->clientSecret)); + ForceZero(&ssl->serverSecret, sizeof(ssl->serverSecret)); #if defined(HAVE_ECH) if (ssl->options.useEch == 1) { @@ -34858,7 +34856,8 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, } #ifdef WOLFSSL_CHECK_MEM_ZERO /* Internal ticket successfully decrypted. */ - wc_MemZero_Add("Do Client Ticket internal", it, sizeof(InternalTicket)); + wc_MemZero_Add("Do Client Ticket internal", psk->it, + sizeof(InternalTicket)); #endif ret = DoClientTicketCheckVersion(ssl, psk->it); diff --git a/src/ssl.c b/src/ssl.c index a413c43f2..57a3b48d2 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -20029,6 +20029,16 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, ssl->extensions = NULL; #endif + if (ssl->keys.encryptionOn) { + ForceZero(ssl->buffers.inputBuffer.buffer - + ssl->buffers.inputBuffer.offset, + ssl->buffers.inputBuffer.bufferSize); + #ifdef WOLFSSL_CHECK_MEM_ZERO + wc_MemZero_Check(ssl->buffers.inputBuffer.buffer - + ssl->buffers.inputBuffer.offset, + ssl->buffers.inputBuffer.bufferSize); + #endif + } ssl->keys.encryptionOn = 0; XMEMSET(&ssl->msgsReceived, 0, sizeof(ssl->msgsReceived)); diff --git a/wolfcrypt/src/aes.c b/wolfcrypt/src/aes.c index a93da6ea4..6eb3dff35 100644 --- a/wolfcrypt/src/aes.c +++ b/wolfcrypt/src/aes.c @@ -2970,6 +2970,7 @@ static WARN_UNUSED_RESULT int wc_AesDecrypt( defined(WOLFSSL_AES_128) case 16: #ifdef WOLFSSL_CHECK_MEM_ZERO + temp = (word32)-1; wc_MemZero_Add("wc_AesSetKeyLocal temp", &temp, sizeof(temp)); #endif while (1) @@ -3002,6 +3003,7 @@ static WARN_UNUSED_RESULT int wc_AesDecrypt( defined(WOLFSSL_AES_192) case 24: #ifdef WOLFSSL_CHECK_MEM_ZERO + temp = (word32)-1; wc_MemZero_Add("wc_AesSetKeyLocal temp", &temp, sizeof(temp)); #endif /* for (;;) here triggers a bug in VC60 SP4 w/ Pro Pack */ @@ -3037,6 +3039,7 @@ static WARN_UNUSED_RESULT int wc_AesDecrypt( defined(WOLFSSL_AES_256) case 32: #ifdef WOLFSSL_CHECK_MEM_ZERO + temp = (word32)-1; wc_MemZero_Add("wc_AesSetKeyLocal temp", &temp, sizeof(temp)); #endif while (1) @@ -4459,9 +4462,6 @@ int wc_AesSetIV(Aes* aes, const byte* iv) sz--; } - #ifdef WOLFSSL_CHECK_MEM_ZERO - wc_MemZero_Add("wc_AesCtrEncrypt scratch", scratch, AES_BLOCK_SIZE); - #endif #if defined(HAVE_AES_ECB) && !defined(WOLFSSL_PIC32MZ_CRYPT) && \ !defined(XTRANSFORM_AESCTRBLOCK) if (in != out && sz >= AES_BLOCK_SIZE) { @@ -4485,6 +4485,10 @@ int wc_AesSetIV(Aes* aes, const byte* iv) else #endif { + #ifdef WOLFSSL_CHECK_MEM_ZERO + wc_MemZero_Add("wc_AesCtrEncrypt scratch", scratch, + AES_BLOCK_SIZE); + #endif /* do as many block size ops as possible */ while (sz >= AES_BLOCK_SIZE) { #ifdef XTRANSFORM_AESCTRBLOCK diff --git a/wolfcrypt/src/cmac.c b/wolfcrypt/src/cmac.c index 587e381ee..24ddcc635 100644 --- a/wolfcrypt/src/cmac.c +++ b/wolfcrypt/src/cmac.c @@ -293,6 +293,8 @@ int wc_AesCmacGenerate(byte* out, word32* outSz, } #endif #ifdef WOLFSSL_CHECK_MEM_ZERO + XMEMSET(((unsigned char *)cmac) + sizeof(Aes), 0xff, + sizeof(Cmac) - sizeof(Aes)); /* Aes part is checked by wc_AesFree. */ wc_MemZero_Add("wc_AesCmacGenerate cmac", ((unsigned char *)cmac) + sizeof(Aes), sizeof(Cmac) - sizeof(Aes)); diff --git a/wolfcrypt/src/kdf.c b/wolfcrypt/src/kdf.c index d9b968962..556ee9ea7 100644 --- a/wolfcrypt/src/kdf.c +++ b/wolfcrypt/src/kdf.c @@ -149,8 +149,8 @@ int wc_PRF(byte* result, word32 resLen, const byte* secret, return MEMORY_E; } #endif - #ifdef WOLFSSL_CHECK_MEM_ZERO + XMEMSET(previous, 0xff, P_HASH_MAX_SIZE); wc_MemZero_Add("wc_PRF previous", previous, P_HASH_MAX_SIZE); wc_MemZero_Add("wc_PRF current", current, P_HASH_MAX_SIZE); wc_MemZero_Add("wc_PRF hmac", hmac, sizeof(Hmac)); @@ -486,7 +486,7 @@ int wc_PRF_TLS(byte* digest, word32 digLen, const byte* secret, word32 secLen, ForceZero(data, idx); #ifdef WOLFSSL_CHECK_MEM_ZERO - wc_MemZero_Check(data, MAX_TLS13_HKDF_LABEL_SZ); + wc_MemZero_Check(data, idx); #endif #ifdef WOLFSSL_SMALL_STACK XFREE(data, NULL, DYNAMIC_TYPE_TMP_BUFFER); diff --git a/wolfcrypt/src/rsa.c b/wolfcrypt/src/rsa.c index 071436e6f..c91599ec0 100644 --- a/wolfcrypt/src/rsa.c +++ b/wolfcrypt/src/rsa.c @@ -4775,6 +4775,13 @@ int wc_MakeRsaKey(RsaKey* key, int size, long e, WC_RNG* rng) goto out; } #endif +#ifdef WOLFSSL_CHECK_MEM_ZERO + XMEMSET(p, 0, sizeof(*p)); + XMEMSET(q, 0, sizeof(*q)); + XMEMSET(tmp1, 0, sizeof(*tmp1)); + XMEMSET(tmp2, 0, sizeof(*tmp2)); + XMEMSET(tmp3, 0, sizeof(*tmp3)); +#endif #ifdef WOLF_CRYPTO_CB if (key->devId != INVALID_DEVID) { diff --git a/wolfcrypt/src/wc_encrypt.c b/wolfcrypt/src/wc_encrypt.c index 853ff7474..2d36f2a82 100644 --- a/wolfcrypt/src/wc_encrypt.c +++ b/wolfcrypt/src/wc_encrypt.c @@ -341,6 +341,7 @@ int wc_BufferKeyEncrypt(EncryptedInfo* info, byte* der, word32 derSz, } #endif /* WOLFSSL_SMALL_STACK */ #ifdef WOLFSSL_CHECK_MEM_ZERO + XMEMSET(key, 0xff, WC_MAX_SYM_KEY_SIZE); wc_MemZero_Add("wc_BufferKeyDecrypt key", key, WC_MAX_SYM_KEY_SIZE); #endif @@ -503,6 +504,7 @@ int wc_CryptKey(const char* password, int passwordSz, byte* salt, if (ret == 0) { #ifdef WOLFSSL_CHECK_MEM_ZERO + XMEMSET(key, 0xff, PKCS_MAX_KEY_SIZE); wc_MemZero_Add("wc_CryptKey key", key, PKCS_MAX_KEY_SIZE); #endif