From 64ba151c358ab7106464b5926ff677d36ddc2546 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 6 Jun 2018 16:38:48 -0700 Subject: [PATCH 1/6] Experimental fixes for async to resolve runtime fsanitize issues with invalid memory access due to attempting realloc on non NUMA type. Tested with `./configure --with-intelqa=../QAT1.6 --enable-asynccrypt CC="clang -fsanitize=address" --enable-debug --disable-shared --enable-trackmemory CFLAGS="-DWOLFSSL_DEBUG_MEMORY -DWOLFSSL_DEBUG_MEMORY_PRINT" && make` and `sudo ./tests/unit.test`. --- src/internal.c | 13 ++++++++++--- src/tls.c | 38 +++++++++++++++----------------------- wolfcrypt/src/ecc.c | 16 ++-------------- 3 files changed, 27 insertions(+), 40 deletions(-) diff --git a/src/internal.c b/src/internal.c index dfb3a2fe9..0433c643c 100644 --- a/src/internal.c +++ b/src/internal.c @@ -13286,7 +13286,7 @@ typedef struct BuildMsgArgs { word32 headerSz; word16 size; word32 ivSz; /* TLSv1.1 IV */ - byte iv[AES_BLOCK_SIZE]; /* max size */ + byte* iv; } BuildMsgArgs; static void FreeBuildMsgArgs(WOLFSSL* ssl, void* pArgs) @@ -13296,7 +13296,10 @@ static void FreeBuildMsgArgs(WOLFSSL* ssl, void* pArgs) (void)ssl; (void)args; - /* no allocations in BuildMessage */ + if (args->iv) { + XFREE(args->iv, ssl->heap, DYNAMIC_TYPE_SALT); + args->iv = NULL; + } } #endif @@ -13400,7 +13403,7 @@ int BuildMessage(WOLFSSL* ssl, byte* output, int outSz, const byte* input, args->ivSz = blockSz; args->sz += args->ivSz; - if (args->ivSz > (word32)sizeof(args->iv)) + if (args->ivSz > AES_BLOCK_SIZE) ERROR_OUT(BUFFER_E, exit_buildmsg); } args->sz += 1; /* pad byte */ @@ -13431,6 +13434,10 @@ int BuildMessage(WOLFSSL* ssl, byte* output, int outSz, const byte* input, } if (args->ivSz > 0) { + args->iv = (byte*)XMALLOC(args->ivSz, ssl->heap, DYNAMIC_TYPE_SALT); + if (args->iv == NULL) + ERROR_OUT(MEMORY_E, exit_buildmsg); + ret = wc_RNG_GenerateBlock(ssl->rng, args->iv, args->ivSz); if (ret != 0) goto exit_buildmsg; diff --git a/src/tls.c b/src/tls.c index 9f0c49497..766613a5a 100644 --- a/src/tls.c +++ b/src/tls.c @@ -260,16 +260,15 @@ static int doPRF(byte* digest, word32 digLen, const byte* secret,word32 secLen, #ifdef WOLFSSL_SMALL_STACK byte* md5_half; byte* sha_half; - byte* labelSeed; byte* md5_result; byte* sha_result; #else byte md5_half[MAX_PRF_HALF]; /* half is real size */ byte sha_half[MAX_PRF_HALF]; /* half is real size */ - byte labelSeed[MAX_PRF_LABSEED]; /* labLen + seedLen is real size */ byte md5_result[MAX_PRF_DIG]; /* digLen is real size */ byte sha_result[MAX_PRF_DIG]; /* digLen is real size */ #endif + DECLARE_VAR(labelSeed, byte, MAX_PRF_LABSEED, heap); if (half > MAX_PRF_HALF) return BUFFER_E; @@ -281,17 +280,16 @@ static int doPRF(byte* digest, word32 digLen, const byte* secret,word32 secLen, #ifdef WOLFSSL_SMALL_STACK md5_half = (byte*)XMALLOC(MAX_PRF_HALF, heap, DYNAMIC_TYPE_DIGEST); sha_half = (byte*)XMALLOC(MAX_PRF_HALF, heap, DYNAMIC_TYPE_DIGEST); - labelSeed = (byte*)XMALLOC(MAX_PRF_LABSEED, heap, DYNAMIC_TYPE_SEED); md5_result = (byte*)XMALLOC(MAX_PRF_DIG, heap, DYNAMIC_TYPE_DIGEST); sha_result = (byte*)XMALLOC(MAX_PRF_DIG, heap, DYNAMIC_TYPE_DIGEST); - if (md5_half == NULL || sha_half == NULL || labelSeed == NULL || - md5_result == NULL || sha_result == NULL) { + if (md5_half == NULL || sha_half == NULL || md5_result == NULL || + sha_result == NULL) { if (md5_half) XFREE(md5_half, heap, DYNAMIC_TYPE_DIGEST); if (sha_half) XFREE(sha_half, heap, DYNAMIC_TYPE_DIGEST); - if (labelSeed) XFREE(labelSeed, heap, DYNAMIC_TYPE_SEED); if (md5_result) XFREE(md5_result, heap, DYNAMIC_TYPE_DIGEST); if (sha_result) XFREE(sha_result, heap, DYNAMIC_TYPE_DIGEST); + FREE_VAR(labelSeed, heap); return MEMORY_E; } @@ -317,11 +315,12 @@ static int doPRF(byte* digest, word32 digLen, const byte* secret,word32 secLen, #ifdef WOLFSSL_SMALL_STACK XFREE(md5_half, heap, DYNAMIC_TYPE_DIGEST); XFREE(sha_half, heap, DYNAMIC_TYPE_DIGEST); - XFREE(labelSeed, heap, DYNAMIC_TYPE_SEED); XFREE(md5_result, heap, DYNAMIC_TYPE_DIGEST); XFREE(sha_result, heap, DYNAMIC_TYPE_DIGEST); #endif + FREE_VAR(labelSeed, heap); + return ret; } @@ -339,21 +338,11 @@ static int PRF(byte* digest, word32 digLen, const byte* secret, word32 secLen, int ret = 0; if (useAtLeastSha256) { -#ifdef WOLFSSL_SMALL_STACK - byte* labelSeed; -#else - byte labelSeed[MAX_PRF_LABSEED]; /* labLen + seedLen is real size */ -#endif + DECLARE_VAR(labelSeed, byte, MAX_PRF_LABSEED, heap); if (labLen + seedLen > MAX_PRF_LABSEED) return BUFFER_E; -#ifdef WOLFSSL_SMALL_STACK - labelSeed = (byte*)XMALLOC(MAX_PRF_LABSEED, heap, DYNAMIC_TYPE_SEED); - if (labelSeed == NULL) - return MEMORY_E; -#endif - XMEMCPY(labelSeed, label, labLen); XMEMCPY(labelSeed + labLen, seed, seedLen); @@ -364,9 +353,7 @@ 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); -#ifdef WOLFSSL_SMALL_STACK - XFREE(labelSeed, heap, DYNAMIC_TYPE_SEED); -#endif + FREE_VAR(labelSeed, heap); } #ifndef NO_OLD_TLS else { @@ -528,13 +515,18 @@ static int _DeriveTlsKeys(byte* key_dig, word32 key_dig_len, int tls1_2, int hash_type, void* heap, int devId) { - byte seed[SEED_LEN]; + int ret; + DECLARE_VAR(seed, byte, SEED_LEN, heap); XMEMCPY(seed, sr, RAN_LEN); XMEMCPY(seed + RAN_LEN, cr, RAN_LEN); - return PRF(key_dig, key_dig_len, ms, msLen, key_label, KEY_LABEL_SZ, + ret = PRF(key_dig, key_dig_len, ms, msLen, key_label, KEY_LABEL_SZ, seed, SEED_LEN, tls1_2, hash_type, heap, devId); + + FREE_VAR(seed, heap); + + return ret; } /* External facing wrapper so user can call as well, 0 on success */ diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 8daa52e63..dd551bd54 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -3116,17 +3116,7 @@ int wc_ecc_point_is_at_infinity(ecc_point* p) static int wc_ecc_gen_k(WC_RNG* rng, int size, mp_int* k, mp_int* order) { int err; -#ifdef WOLFSSL_SMALL_STACK - byte* buf; -#else - byte buf[ECC_MAXSIZE_GEN]; -#endif - -#ifdef WOLFSSL_SMALL_STACK - buf = (byte*)XMALLOC(ECC_MAXSIZE_GEN, NULL, DYNAMIC_TYPE_ECC_BUFFER); - if (buf == NULL) - return MEMORY_E; -#endif + DECLARE_VAR(buf, byte, ECC_MAXSIZE_GEN, rng->heap); /*generate 8 extra bytes to mitigate bias from the modulo operation below*/ /*see section A.1.2 in 'Suite B Implementor's Guide to FIPS 186-3 (ECDSA)'*/ @@ -3153,9 +3143,7 @@ static int wc_ecc_gen_k(WC_RNG* rng, int size, mp_int* k, mp_int* order) } ForceZero(buf, ECC_MAXSIZE); -#ifdef WOLFSSL_SMALL_STACK - XFREE(buf, NULL, DYNAMIC_TYPE_ECC_BUFFER); -#endif + FREE_VAR(buf, rng->heap); return err; } From 623f1b58acae592e53aea8b38afb5e2ecc3e62ff Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 7 Jun 2018 12:45:11 -0700 Subject: [PATCH 2/6] Fix for min IV size check. Cleanup of the max IV to use new enum `MAX_IV_SZ`. --- src/internal.c | 6 +++--- wolfssl/internal.h | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/internal.c b/src/internal.c index 0433c643c..1f81d6ce6 100644 --- a/src/internal.c +++ b/src/internal.c @@ -13403,7 +13403,7 @@ int BuildMessage(WOLFSSL* ssl, byte* output, int outSz, const byte* input, args->ivSz = blockSz; args->sz += args->ivSz; - if (args->ivSz > AES_BLOCK_SIZE) + if (args->ivSz > MAX_IV_SZ) ERROR_OUT(BUFFER_E, exit_buildmsg); } args->sz += 1; /* pad byte */ @@ -13455,9 +13455,9 @@ int BuildMessage(WOLFSSL* ssl, byte* output, int outSz, const byte* input, AddRecordHeader(output, args->size, (byte)type, ssl); /* write to output */ - if (args->ivSz) { + if (args->ivSz > 0) { XMEMCPY(output + args->idx, args->iv, - min(args->ivSz, sizeof(args->iv))); + min(args->ivSz, MAX_IV_SZ)); args->idx += args->ivSz; } XMEMCPY(output + args->idx, input, inSz); diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 9c5beadbf..55b9216e7 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -1191,6 +1191,8 @@ enum Misc { AES_128_KEY_SIZE = 16, #endif + MAX_IV_SZ = AES_BLOCK_SIZE, + AEAD_SEQ_OFFSET = 4, /* Auth Data: Sequence number */ AEAD_TYPE_OFFSET = 8, /* Auth Data: Type */ AEAD_VMAJ_OFFSET = 9, /* Auth Data: Major Version */ From ec132cd3f4cc6242ca0f0aeeba706f839bb22f37 Mon Sep 17 00:00:00 2001 From: David Garske Date: Tue, 12 Jun 2018 15:23:50 -0700 Subject: [PATCH 3/6] Fix fsanitize issue for `mp_rand`. --- wolfcrypt/src/wolfmath.c | 43 ++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/wolfcrypt/src/wolfmath.c b/wolfcrypt/src/wolfmath.c index ececf0133..0c81427d4 100644 --- a/wolfcrypt/src/wolfmath.c +++ b/wolfcrypt/src/wolfmath.c @@ -99,44 +99,53 @@ int get_rand_digit(WC_RNG* rng, mp_digit* d) #ifdef WC_RSA_BLINDING int mp_rand(mp_int* a, int digits, WC_RNG* rng) { - int ret; - mp_digit d; + int ret = 0; + DECLARE_VAR(d, mp_digit, 1, rng->heap); - if (rng == NULL) - return MISSING_RNG_E; + if (rng == NULL) { + ret = MISSING_RNG_E; goto exit; + } - if (a == NULL) - return BAD_FUNC_ARG; + if (a == NULL + #ifdef WOLFSSL_ASYNC_CRYPT + || d == NULL + #endif + ) { + ret = BAD_FUNC_ARG; goto exit; + } mp_zero(a); if (digits <= 0) { - return MP_OKAY; + ret = MP_OKAY; goto exit; } /* first place a random non-zero digit */ do { - ret = get_rand_digit(rng, &d); + ret = get_rand_digit(rng, d); if (ret != 0) { - return ret; + goto exit; } - } while (d == 0); + } while (*d == 0); - if ((ret = mp_add_d(a, d, a)) != MP_OKAY) { - return ret; + if ((ret = mp_add_d(a, *d, a)) != MP_OKAY) { + goto exit; } while (--digits > 0) { if ((ret = mp_lshd(a, 1)) != MP_OKAY) { - return ret; + goto exit; } - if ((ret = get_rand_digit(rng, &d)) != 0) { - return ret; + if ((ret = get_rand_digit(rng, d)) != 0) { + goto exit; } - if ((ret = mp_add_d(a, d, a)) != MP_OKAY) { - return ret; + if ((ret = mp_add_d(a, *d, a)) != MP_OKAY) { + goto exit; } } +exit: + FREE_VAR(d, rng->heap); + return ret; } #endif /* WC_RSA_BLINDING */ From 71606dde4543ca72563a63f2c11f098e15fc2ec6 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 20 Jun 2018 14:51:32 -0700 Subject: [PATCH 4/6] Fixes for a few wolfCrypt test memory leaks. Fix for HMAC with empty input not supported on QuickAssist. --- wolfcrypt/test/test.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 4d6f929d4..b72afe667 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -1502,6 +1502,8 @@ int md5_test(void) if (ret != 0) ERROR_OUT(-1605 - i, exit); + wc_Md5Free(&md5Copy); + if (XMEMCMP(hash, test_md5[i].output, WC_MD5_DIGEST_SIZE) != 0) ERROR_OUT(-1606 - i, exit); @@ -1696,6 +1698,7 @@ int sha_test(void) ret = wc_ShaFinal(&sha, hash); if (ret != 0) ERROR_OUT(-1805 - i, exit); + wc_ShaFree(&shaCopy); if (XMEMCMP(hash, test_sha[i].output, WC_SHA_DIGEST_SIZE) != 0) ERROR_OUT(-1806 - i, exit); @@ -1937,6 +1940,7 @@ int sha224_test(void) ret = wc_Sha224Final(&sha, hash); if (ret != 0) ERROR_OUT(-2105 - i, exit); + wc_Sha224Free(&shaCopy); if (XMEMCMP(hash, test_sha[i].output, WC_SHA224_DIGEST_SIZE) != 0) ERROR_OUT(-2106 - i, exit); @@ -2013,6 +2017,7 @@ int sha256_test(void) ret = wc_Sha256Final(&sha, hash); if (ret != 0) ERROR_OUT(-2205 - i, exit); + wc_Sha256Free(&shaCopy); if (XMEMCMP(hash, test_sha[i].output, WC_SHA256_DIGEST_SIZE) != 0) ERROR_OUT(-2206 - i, exit); @@ -2123,6 +2128,7 @@ int sha512_test(void) ret = wc_Sha512Final(&sha, hash); if (ret != 0) ERROR_OUT(-2305 - i, exit); + wc_Sha512Free(&shaCopy); if (XMEMCMP(hash, test_sha[i].output, WC_SHA512_DIGEST_SIZE) != 0) ERROR_OUT(-2306 - i, exit); @@ -2229,6 +2235,7 @@ int sha384_test(void) ret = wc_Sha384Final(&sha, hash); if (ret != 0) ERROR_OUT(-2405 - i, exit); + wc_Sha384Free(&shaCopy); if (XMEMCMP(hash, test_sha[i].output, WC_SHA384_DIGEST_SIZE) != 0) ERROR_OUT(-2406 - i, exit); @@ -3291,6 +3298,10 @@ int hmac_sha256_test(void) if (i == 1) continue; /* cavium can't handle short keys, fips not allowed */ #endif +#if defined(HAVE_INTEL_QA) || defined(HAVE_CAVIUM) + if (i == 3) + continue; /* QuickAssist can't handle empty HMAC */ +#endif if (wc_HmacInit(&hmac, HEAP_HINT, devId) != 0) return -3500 - i; @@ -14177,6 +14188,9 @@ static int ecc_test_vector_item(const eccVector* vector) done: wc_ecc_free(&userA); +#if !defined(NO_ASN) && !defined(HAVE_SELFTEST) + FREE_VAR(sigRaw, HEAP_HINT); +#endif FREE_VAR(sig, HEAP_HINT); return ret; From 1cb5bbf8eaf17b3d5065f2aac0267ceb5b3772c4 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 20 Jun 2018 17:30:56 -0700 Subject: [PATCH 5/6] Fixes for some async issues. Fixes an async issue with BuildMessage. Fixes for PKCS7 tests to not use async since it is not supported. --- src/internal.c | 7 +++++-- wolfcrypt/test/test.c | 12 +++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/internal.c b/src/internal.c index 1f81d6ce6..6c52f9048 100644 --- a/src/internal.c +++ b/src/internal.c @@ -13370,11 +13370,11 @@ int BuildMessage(WOLFSSL* ssl, byte* output, int outSz, const byte* input, { /* catch mistaken sizeOnly parameter */ if (!sizeOnly && (output == NULL || input == NULL) ) { - return BAD_FUNC_ARG; + ERROR_OUT(BAD_FUNC_ARG, exit_buildmsg); } if (sizeOnly && (output || input) ) { WOLFSSL_MSG("BuildMessage w/sizeOnly doesn't need input/output"); - return BAD_FUNC_ARG; + ERROR_OUT(BAD_FUNC_ARG, exit_buildmsg); } ssl->options.buildMsgState = BUILD_MSG_SIZE; @@ -13564,6 +13564,9 @@ exit_buildmsg: /* Final cleanup */ FreeBuildMsgArgs(ssl, args); +#ifdef WOLFSSL_ASYNC_CRYPT + ssl->async.freeArgs = NULL; +#endif return ret; #endif /* !WOLFSSL_NO_TLS12 */ diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index b72afe667..31c7c5159 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -9634,7 +9634,7 @@ static int rsa_certgen_test(RsaKey* key, RsaKey* keypub, WC_RNG* rng, byte* tmp) ret = 0; do { #if defined(WOLFSSL_ASYNC_CRYPT) - ret = wc_AsyncWait(ret, &key.asyncDev, WC_ASYNC_FLAG_CALL_AGAIN); + ret = wc_AsyncWait(ret, &key->asyncDev, WC_ASYNC_FLAG_CALL_AGAIN); #endif if (ret >= 0) { ret = wc_MakeSelfCert(myCert, der, FOURK_BUF, key, rng); @@ -17946,8 +17946,13 @@ static int pkcs7enveloped_run_vectors(byte* rsaCert, word32 rsaCertSz, testSz = sizeof(testVectors) / sizeof(pkcs7EnvelopedVector); for (i = 0; i < testSz; i++) { - - ret = wc_PKCS7_Init(&pkcs7, HEAP_HINT, devId); + ret = wc_PKCS7_Init(&pkcs7, HEAP_HINT, + #ifdef WOLFSSL_ASYNC_CRYPT + INVALID_DEVID /* async PKCS7 is not supported */ + #else + devId + #endif + ); if (ret != 0) return -9214; @@ -18494,6 +18499,7 @@ static int pkcs7signed_run_vectors(byte* rsaCert, word32 rsaCertSz, for (i = 0; i < testSz; i++) { pkcs7.heap = HEAP_HINT; + pkcs7.devId = INVALID_DEVID; ret = wc_PKCS7_InitWithCert(&pkcs7, testVectors[i].cert, (word32)testVectors[i].certSz); From 522f365279b293b8756e364ae53ee0f91edcba78 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 20 Jun 2018 17:33:09 -0700 Subject: [PATCH 6/6] Fix one more issue with PKCS7 and async, which is not supported. --- tests/api.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/api.c b/tests/api.c index baec67e45..253448fbd 100644 --- a/tests/api.c +++ b/tests/api.c @@ -14464,6 +14464,7 @@ static void test_wc_PKCS7_EncodeSignedData (void) pkcs7.encryptOID = RSAk; pkcs7.hashOID = SHAh; pkcs7.rng = &rng; + pkcs7.devId = INVALID_DEVID; AssertIntGT(wc_PKCS7_EncodeSignedData(&pkcs7, output, outputSz), 0);