From a643aeac41f883f9bf41fa2f5f5a30dd3784321e Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 21 Sep 2018 09:33:40 -0700 Subject: [PATCH] * Fixes for async with TLS where keys are being free'd too soon. * Fix for possible NULL RNG case in mp_rand. * Fix for memory macros to handle expression for `HEAP`. * Fix for possible unknown uint32_t type with mem track. * Fix for double Alloc/Free print when using track and debug memory at same time. * Fix for building with `./configure CFLAGS="-DECC_USER_CURVES -DNO_ECC256 -DHAVE_ECC160"` * Performance improvements for cases with `WC_ASYNC_NO_HASH` and `WC_ASYNC_ENABLE_SHA256`. --- src/internal.c | 65 ++++++++++++++++++++++------------- src/tls.c | 12 +++++++ wolfcrypt/src/memory.c | 4 +-- wolfcrypt/src/random.c | 24 +++++++++++++ wolfcrypt/src/wolfmath.c | 4 +-- wolfcrypt/test/test.c | 2 ++ wolfssl/wolfcrypt/mem_track.h | 2 +- wolfssl/wolfcrypt/types.h | 10 +++--- 8 files changed, 89 insertions(+), 34 deletions(-) diff --git a/src/internal.c b/src/internal.c index 1e41be4f7..46cac7926 100644 --- a/src/internal.c +++ b/src/internal.c @@ -5322,12 +5322,12 @@ void FreeHandshakeResources(WOLFSSL* ssl) #endif #if defined(WOLFSSL_TLS13) #if !defined(WOLFSSL_POST_HANDSHAKE_AUTH) - || ssl->options.tls1_3 + || ssl->options.tls1_3 #elif !defined(HAVE_SESSION_TICKET) - || (ssl->options.tls1_3 && ssl->options.side == WOLFSSL_SERVER_END) + || (ssl->options.tls1_3 && ssl->options.side == WOLFSSL_SERVER_END) #endif #endif - ) { + ) { if (ssl->options.weOwnRng) { wc_FreeRng(ssl->rng); XFREE(ssl->rng, ssl->heap, DYNAMIC_TYPE_RNG); @@ -10704,10 +10704,17 @@ static int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx, !defined(NO_ED25519_CLIENT_AUTH) if (ssl->options.resuming || !IsAtLeastTLSv1_2(ssl) || IsAtLeastTLSv1_3(ssl->version)) { - ssl->options.cacheMessages = 0; - if (ssl->hsHashes->messages != NULL) { - XFREE(ssl->hsHashes->messages, ssl->heap, DYNAMIC_TYPE_HASHES); - ssl->hsHashes->messages = NULL; + + #if defined(WOLFSSL_ASYNC_CRYPT) || defined(WOLFSSL_NONBLOCK_OCSP) + if (ret != WC_PENDING_E && ret != OCSP_WANT_READ) + #endif + { + ssl->options.cacheMessages = 0; + if (ssl->hsHashes->messages != NULL) { + XFREE(ssl->hsHashes->messages, ssl->heap, + DYNAMIC_TYPE_HASHES); + ssl->hsHashes->messages = NULL; + } } } #endif @@ -10776,10 +10783,15 @@ static int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx, !defined(NO_ED25519_CLIENT_AUTH) if (ssl->options.resuming || !ssl->options.verifyPeer || \ !IsAtLeastTLSv1_2(ssl) || IsAtLeastTLSv1_3(ssl->version)) { - ssl->options.cacheMessages = 0; - if (ssl->hsHashes->messages != NULL) { - XFREE(ssl->hsHashes->messages, ssl->heap, DYNAMIC_TYPE_HASHES); - ssl->hsHashes->messages = NULL; + #if defined(WOLFSSL_ASYNC_CRYPT) || defined(WOLFSSL_NONBLOCK_OCSP) + if (ret != WC_PENDING_E && ret != OCSP_WANT_READ) + #endif + { + ssl->options.cacheMessages = 0; + if (ssl->hsHashes->messages != NULL) { + XFREE(ssl->hsHashes->messages, ssl->heap, DYNAMIC_TYPE_HASHES); + ssl->hsHashes->messages = NULL; + } } } #endif @@ -18075,7 +18087,7 @@ static int DoServerKeyExchange(WOLFSSL* ssl, const byte* input, } ssl->buffers.serverDH_P.buffer = (byte*)XMALLOC(length, - ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); + ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); if (ssl->buffers.serverDH_P.buffer) { ssl->buffers.serverDH_P.length = length; } @@ -18102,7 +18114,7 @@ static int DoServerKeyExchange(WOLFSSL* ssl, const byte* input, } ssl->buffers.serverDH_G.buffer = (byte*)XMALLOC(length, - ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); + ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); if (ssl->buffers.serverDH_G.buffer) { ssl->buffers.serverDH_G.length = length; } @@ -18129,7 +18141,7 @@ static int DoServerKeyExchange(WOLFSSL* ssl, const byte* input, } ssl->buffers.serverDH_Pub.buffer = (byte*)XMALLOC(length, - ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); + ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); if (ssl->buffers.serverDH_Pub.buffer) { ssl->buffers.serverDH_Pub.length = length; } @@ -18480,11 +18492,12 @@ static int DoServerKeyExchange(WOLFSSL* ssl, const byte* input, args->sigSz = (word16)ret; ret = 0; } - - /* peerRsaKey */ - FreeKey(ssl, DYNAMIC_TYPE_RSA, + if (ret == 0) { + /* peerRsaKey */ + FreeKey(ssl, DYNAMIC_TYPE_RSA, (void**)&ssl->peerRsaKey); - ssl->peerRsaKeyPresent = 0; + ssl->peerRsaKeyPresent = 0; + } break; } #endif /* !NO_RSA */ @@ -18503,10 +18516,12 @@ static int DoServerKeyExchange(WOLFSSL* ssl, const byte* input, #endif ); - /* peerEccDsaKey */ - FreeKey(ssl, DYNAMIC_TYPE_ECC, + if (ret == 0) { + /* peerEccDsaKey */ + FreeKey(ssl, DYNAMIC_TYPE_ECC, (void**)&ssl->peerEccDsaKey); - ssl->peerEccDsaKeyPresent = 0; + ssl->peerEccDsaKeyPresent = 0; + } break; } #endif /* HAVE_ECC */ @@ -18525,10 +18540,12 @@ static int DoServerKeyExchange(WOLFSSL* ssl, const byte* input, #endif ); - /* peerEccDsaKey */ - FreeKey(ssl, DYNAMIC_TYPE_ED25519, + if (ret == 0) { + /* peerEccDsaKey */ + FreeKey(ssl, DYNAMIC_TYPE_ED25519, (void**)&ssl->peerEd25519Key); - ssl->peerEd25519KeyPresent = 0; + ssl->peerEd25519KeyPresent = 0; + } break; } #endif /* HAVE_ED25519 */ diff --git a/src/tls.c b/src/tls.c index 0b931cc7e..09070fede 100644 --- a/src/tls.c +++ b/src/tls.c @@ -339,7 +339,11 @@ static int PRF(byte* digest, word32 digLen, const byte* secret, word32 secLen, int ret = 0; if (useAtLeastSha256) { + #ifndef WC_ASYNC_NO_HASH DECLARE_VAR(labelSeed, byte, MAX_PRF_LABSEED, heap); + #else + byte labelSeed[MAX_PRF_LABSEED]; + #endif if (labLen + seedLen > MAX_PRF_LABSEED) return BUFFER_E; @@ -354,7 +358,9 @@ static int PRF(byte* digest, word32 digLen, const byte* secret, word32 secLen, ret = p_hash(digest, digLen, secret, secLen, labelSeed, labLen + seedLen, hash_type, heap, devId); + #ifndef WC_ASYNC_NO_HASH FREE_VAR(labelSeed, heap); + #endif } #ifndef NO_OLD_TLS else { @@ -517,7 +523,11 @@ static int _DeriveTlsKeys(byte* key_dig, word32 key_dig_len, void* heap, int devId) { int ret; +#ifndef WC_ASYNC_NO_HASH DECLARE_VAR(seed, byte, SEED_LEN, heap); +#else + byte seed[SEED_LEN]; +#endif XMEMCPY(seed, sr, RAN_LEN); XMEMCPY(seed + RAN_LEN, cr, RAN_LEN); @@ -525,7 +535,9 @@ static int _DeriveTlsKeys(byte* key_dig, word32 key_dig_len, ret = PRF(key_dig, key_dig_len, ms, msLen, key_label, KEY_LABEL_SZ, seed, SEED_LEN, tls1_2, hash_type, heap, devId); +#ifndef WC_ASYNC_NO_HASH FREE_VAR(seed, heap); +#endif return ret; } diff --git a/wolfcrypt/src/memory.c b/wolfcrypt/src/memory.c index a457475e7..e55fad202 100644 --- a/wolfcrypt/src/memory.c +++ b/wolfcrypt/src/memory.c @@ -131,7 +131,7 @@ void* wolfSSL_Malloc(size_t size) } #ifdef WOLFSSL_DEBUG_MEMORY -#ifdef WOLFSSL_DEBUG_MEMORY_PRINT +#if defined(WOLFSSL_DEBUG_MEMORY_PRINT) && !defined(WOLFSSL_TRACK_MEMORY) printf("Alloc: %p -> %u at %s:%d\n", res, (word32)size, func, line); #else (void)func; @@ -172,7 +172,7 @@ void wolfSSL_Free(void *ptr) #endif { #ifdef WOLFSSL_DEBUG_MEMORY -#ifdef WOLFSSL_DEBUG_MEMORY_PRINT +#if defined(WOLFSSL_DEBUG_MEMORY_PRINT) && !defined(WOLFSSL_TRACK_MEMORY) printf("Free: %p at %s:%d\n", ptr, func, line); #else (void)func; diff --git a/wolfcrypt/src/random.c b/wolfcrypt/src/random.c index db1a6edac..e4a18c486 100755 --- a/wolfcrypt/src/random.c +++ b/wolfcrypt/src/random.c @@ -301,7 +301,11 @@ static int Hash_df(DRBG* drbg, byte* out, word32 outSz, byte type, #else wc_Sha256 sha[1]; #endif +#ifdef WC_ASYNC_ENABLE_SHA256 DECLARE_VAR(digest, byte, WC_SHA256_DIGEST_SIZE, drbg->heap); +#else + byte digest[WC_SHA256_DIGEST_SIZE]; +#endif (void)drbg; #ifdef WOLFSSL_ASYNC_CRYPT @@ -362,7 +366,9 @@ static int Hash_df(DRBG* drbg, byte* out, word32 outSz, byte type, ForceZero(digest, WC_SHA256_DIGEST_SIZE); +#ifdef WC_ASYNC_ENABLE_SHA256 FREE_VAR(digest, drbg->heap); +#endif return (ret == 0) ? DRBG_SUCCESS : DRBG_FAILURE; } @@ -427,7 +433,11 @@ static int Hash_gen(DRBG* drbg, byte* out, word32 outSz, const byte* V) #else wc_Sha256 sha[1]; #endif +#ifdef WC_ASYNC_ENABLE_SHA256 DECLARE_VAR(digest, byte, WC_SHA256_DIGEST_SIZE, drbg->heap); +#else + byte digest[WC_SHA256_DIGEST_SIZE]; +#endif /* Special case: outSz is 0 and out is NULL. wc_Generate a block to save for * the continuous test. */ @@ -487,7 +497,9 @@ static int Hash_gen(DRBG* drbg, byte* out, word32 outSz, const byte* V) } ForceZero(data, sizeof(data)); +#ifdef WC_ASYNC_ENABLE_SHA256 FREE_VAR(digest, drbg->heap); +#endif return (ret == 0) ? DRBG_SUCCESS : DRBG_FAILURE; } @@ -529,7 +541,11 @@ static int Hash_DRBG_Generate(DRBG* drbg, byte* out, word32 outSz) if (drbg->reseedCtr == RESEED_INTERVAL) { return DRBG_NEED_RESEED; } else { + #ifdef WC_ASYNC_ENABLE_SHA256 DECLARE_VAR(digest, byte, WC_SHA256_DIGEST_SIZE, drbg->heap); + #else + byte digest[WC_SHA256_DIGEST_SIZE]; + #endif type = drbgGenerateH; reseedCtr = drbg->reseedCtr; @@ -566,7 +582,9 @@ static int Hash_DRBG_Generate(DRBG* drbg, byte* out, word32 outSz) drbg->reseedCtr++; } ForceZero(digest, WC_SHA256_DIGEST_SIZE); + #ifdef WC_ASYNC_ENABLE_SHA256 FREE_VAR(digest, drbg->heap); + #endif } return (ret == 0) ? DRBG_SUCCESS : DRBG_FAILURE; @@ -718,7 +736,11 @@ static int _InitRng(WC_RNG* rng, byte* nonce, word32 nonceSz, seedSz = MAX_SEED_SZ; if (wc_RNG_HealthTestLocal(0) == 0) { + #ifdef WC_ASYNC_ENABLE_SHA256 DECLARE_VAR(seed, byte, MAX_SEED_SZ, rng->heap); + #else + byte seed[MAX_SEED_SZ]; + #endif rng->drbg = (struct DRBG*)XMALLOC(sizeof(DRBG), rng->heap, @@ -743,7 +765,9 @@ static int _InitRng(WC_RNG* rng, byte* nonce, word32 nonceSz, } ForceZero(seed, seedSz); + #ifdef WC_ASYNC_ENABLE_SHA256 FREE_VAR(seed, rng->heap); + #endif } else ret = DRBG_CONT_FAILURE; diff --git a/wolfcrypt/src/wolfmath.c b/wolfcrypt/src/wolfmath.c index 2cdff1539..0d2c9896a 100644 --- a/wolfcrypt/src/wolfmath.c +++ b/wolfcrypt/src/wolfmath.c @@ -97,7 +97,7 @@ int get_rand_digit(WC_RNG* rng, mp_digit* d) int mp_rand(mp_int* a, int digits, WC_RNG* rng) { int ret = 0; - DECLARE_VAR(d, mp_digit, 1, rng->heap); + DECLARE_VAR(d, mp_digit, 1, rng ? rng->heap : NULL); if (rng == NULL) { ret = MISSING_RNG_E; goto exit; @@ -141,7 +141,7 @@ int mp_rand(mp_int* a, int digits, WC_RNG* rng) } exit: - FREE_VAR(d, rng->heap); + FREE_VAR(d, rng ? rng->heap : NULL); return ret; } diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 0995b7663..0673f6cfc 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -15607,6 +15607,7 @@ static int ecc_test_curve(WC_RNG* rng, int keySize) return 0; } +#if !defined(NO_ECC256) || defined(HAVE_ALL_CURVES) #if !defined(WOLFSSL_ATECC508A) && defined(HAVE_ECC_KEY_IMPORT) && \ defined(HAVE_ECC_KEY_EXPORT) static int ecc_point_test(void) @@ -16106,6 +16107,7 @@ done: wc_ecc_free(&key); return ret; } +#endif /* !NO_ECC256 || HAVE_ALL_CURVES */ #ifdef WOLFSSL_CERT_EXT static int ecc_decode_test(void) diff --git a/wolfssl/wolfcrypt/mem_track.h b/wolfssl/wolfcrypt/mem_track.h index 2fa50a5b8..2c963f3e7 100644 --- a/wolfssl/wolfcrypt/mem_track.h +++ b/wolfssl/wolfcrypt/mem_track.h @@ -104,7 +104,7 @@ typedef struct memoryList { memHint* head; memHint* tail; - uint32_t count; + word32 count; } memoryList; #endif diff --git a/wolfssl/wolfcrypt/types.h b/wolfssl/wolfcrypt/types.h index 0d5afb2bd..777001a96 100644 --- a/wolfssl/wolfcrypt/types.h +++ b/wolfssl/wolfcrypt/types.h @@ -282,10 +282,10 @@ /* declare/free variable handling for async */ #ifdef WOLFSSL_ASYNC_CRYPT #define DECLARE_VAR(VAR_NAME, VAR_TYPE, VAR_SIZE, HEAP) \ - VAR_TYPE* VAR_NAME = (VAR_TYPE*)XMALLOC(sizeof(VAR_TYPE) * VAR_SIZE, HEAP, DYNAMIC_TYPE_WOLF_BIGINT); + VAR_TYPE* VAR_NAME = (VAR_TYPE*)XMALLOC(sizeof(VAR_TYPE) * VAR_SIZE, (HEAP), DYNAMIC_TYPE_WOLF_BIGINT); #define DECLARE_VAR_INIT(VAR_NAME, VAR_TYPE, VAR_SIZE, INIT_VALUE, HEAP) \ VAR_TYPE* VAR_NAME = ({ \ - VAR_TYPE* ptr = (VAR_TYPE*)XMALLOC(sizeof(VAR_TYPE) * VAR_SIZE, HEAP, DYNAMIC_TYPE_WOLF_BIGINT); \ + VAR_TYPE* ptr = (VAR_TYPE*)XMALLOC(sizeof(VAR_TYPE) * VAR_SIZE, (HEAP), DYNAMIC_TYPE_WOLF_BIGINT); \ if (ptr && INIT_VALUE) { \ XMEMCPY(ptr, INIT_VALUE, sizeof(VAR_TYPE) * VAR_SIZE); \ } \ @@ -295,13 +295,13 @@ VAR_TYPE* VAR_NAME[VAR_ITEMS]; \ int idx##VAR_NAME; \ for (idx##VAR_NAME=0; idx##VAR_NAME