From 1493b94b27b825d2bef88cf913ba23406ca00909 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 19 Sep 2019 11:34:59 -0700 Subject: [PATCH 1/3] Eliminate async NUMA allocation for `wc_ecc_gen_k`. Additional DECLARE_VAR checks. Improve `mp_rand` to avoid alloc in async case. --- wolfcrypt/src/ecc.c | 7 ------- wolfcrypt/src/random.c | 10 +++++++++- wolfcrypt/src/wolfmath.c | 20 +++++++------------- wolfssl/wolfcrypt/types.h | 0 4 files changed, 16 insertions(+), 21 deletions(-) mode change 100755 => 100644 wolfssl/wolfcrypt/types.h diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 6eda8490b..06863a4a8 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -3815,11 +3815,7 @@ static int wc_ecc_gen_k(WC_RNG* rng, int size, mp_int* k, mp_int* order) { #ifndef WC_NO_RNG int err; -#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_ECC) - DECLARE_VAR(buf, byte, ECC_MAXSIZE_GEN, rng->heap); -#else byte buf[ECC_MAXSIZE_GEN]; -#endif /*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)'*/ @@ -3846,9 +3842,6 @@ static int wc_ecc_gen_k(WC_RNG* rng, int size, mp_int* k, mp_int* order) } ForceZero(buf, ECC_MAXSIZE); -#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_ECC) - FREE_VAR(buf, rng->heap); -#endif return err; #else diff --git a/wolfcrypt/src/random.c b/wolfcrypt/src/random.c index 6b0d5dafc..7718edbdc 100644 --- a/wolfcrypt/src/random.c +++ b/wolfcrypt/src/random.c @@ -311,6 +311,8 @@ static int Hash_df(DRBG* drbg, byte* out, word32 outSz, byte type, #endif #ifdef WC_ASYNC_ENABLE_SHA256 DECLARE_VAR(digest, byte, WC_SHA256_DIGEST_SIZE, drbg->heap); + if (digest == NULL) + return MEMORY_E; #else byte digest[WC_SHA256_DIGEST_SIZE]; #endif @@ -443,6 +445,8 @@ static int Hash_gen(DRBG* drbg, byte* out, word32 outSz, const byte* V) #endif #ifdef WC_ASYNC_ENABLE_SHA256 DECLARE_VAR(digest, byte, WC_SHA256_DIGEST_SIZE, drbg->heap); + if (digest == NULL) + return MEMORY_E; #else byte digest[WC_SHA256_DIGEST_SIZE]; #endif @@ -551,6 +555,8 @@ static int Hash_DRBG_Generate(DRBG* drbg, byte* out, word32 outSz) } else { #ifdef WC_ASYNC_ENABLE_SHA256 DECLARE_VAR(digest, byte, WC_SHA256_DIGEST_SIZE, drbg->heap); + if (digest == NULL) + return MEMORY_E; #else byte digest[WC_SHA256_DIGEST_SIZE]; #endif @@ -749,6 +755,8 @@ static int _InitRng(WC_RNG* rng, byte* nonce, word32 nonceSz, if (wc_RNG_HealthTestLocal(0) == 0) { #ifdef WC_ASYNC_ENABLE_SHA256 DECLARE_VAR(seed, byte, MAX_SEED_SZ, rng->heap); + if (seed == NULL) + return MEMORY_E; #else byte seed[MAX_SEED_SZ]; #endif @@ -2180,7 +2188,7 @@ int wc_GenerateSeed(OS_Seed* os, byte* output, word32 sz) word32 len = sizeof(rand); if (sz < len) len = sz; - /* Get one random 32-bit word from hw RNG */ + /* Get one random 32-bit word from hw RNG */ rand = esp_random( ); XMEMCPY(output, &rand, len); output += len; diff --git a/wolfcrypt/src/wolfmath.c b/wolfcrypt/src/wolfmath.c index 04863dc86..e3057c6a3 100644 --- a/wolfcrypt/src/wolfmath.c +++ b/wolfcrypt/src/wolfmath.c @@ -97,17 +97,13 @@ 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 ? rng->heap : NULL); + mp_digit d; if (rng == NULL) { ret = MISSING_RNG_E; goto exit; } - if (a == NULL - #ifdef WOLFSSL_ASYNC_CRYPT - || d == NULL - #endif - ) { + if (a == NULL) { ret = BAD_FUNC_ARG; goto exit; } @@ -118,13 +114,13 @@ int mp_rand(mp_int* a, int digits, WC_RNG* rng) /* first place a random non-zero digit */ do { - ret = get_rand_digit(rng, d); + ret = get_rand_digit(rng, &d); if (ret != 0) { goto exit; } - } while (*d == 0); + } while (d == 0); - if ((ret = mp_add_d(a, *d, a)) != MP_OKAY) { + if ((ret = mp_add_d(a, d, a)) != MP_OKAY) { goto exit; } @@ -132,17 +128,15 @@ int mp_rand(mp_int* a, int digits, WC_RNG* rng) if ((ret = mp_lshd(a, 1)) != MP_OKAY) { goto exit; } - if ((ret = get_rand_digit(rng, d)) != 0) { + if ((ret = get_rand_digit(rng, &d)) != 0) { goto exit; } - if ((ret = mp_add_d(a, *d, a)) != MP_OKAY) { + if ((ret = mp_add_d(a, d, a)) != MP_OKAY) { goto exit; } } exit: - FREE_VAR(d, rng ? rng->heap : NULL); - return ret; } #endif /* WC_RSA_BLINDING */ diff --git a/wolfssl/wolfcrypt/types.h b/wolfssl/wolfcrypt/types.h old mode 100755 new mode 100644 From 6aecdf59c1447dc8371d57b9043ea61bb1bbc1da Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 19 Sep 2019 12:30:05 -0700 Subject: [PATCH 2/3] Fixes for async build and tests. --- examples/client/client.c | 2 +- tests/api.c | 2 +- wolfcrypt/test/test.c | 12 ++++++++++++ wolfssl/openssl/aes.h | 3 +++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index 33f28f061..3218e4509 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -3091,7 +3091,7 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) #if defined(OPENSSL_EXTRA) && defined(HAVE_EXT_CACHE) if (flatSession) { - XFREE(flatSession, heap, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(flatSession, NULL, DYNAMIC_TYPE_TMP_BUFFER); wolfSSL_SESSION_free(session); } #endif diff --git a/tests/api.c b/tests/api.c index b6c221d0e..878285d3f 100644 --- a/tests/api.c +++ b/tests/api.c @@ -22279,7 +22279,7 @@ static void test_wolfSSL_d2i_PrivateKeys_bio(void) AssertIntEQ(wolfSSL_i2d_RSAPrivateKey(rsa, &bufPtr), sizeof_client_key_der_2048); AssertNotNull(bufPtr); - free(bufPtr); + XFREE(bufPtr, NULL, DYNAMIC_TYPE_OPENSSL); #endif /* USE_CERT_BUFFERS_2048 WOLFSSL_KEY_GEN */ RSA_free(rsa); #endif /* NO_RSA */ diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index b797388ff..acfdcad23 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -12980,21 +12980,33 @@ static int dh_test_ffdhe(WC_RNG *rng, const DhParams* params) } ret = wc_DhGenerateKeyPair(&key, rng, priv, &privSz, pub, &pubSz); +#if defined(WOLFSSL_ASYNC_CRYPT) + ret = wc_AsyncWait(ret, &key.asyncDev, WC_ASYNC_FLAG_NONE); +#endif if (ret != 0) { ERROR_OUT(-7184, done); } ret = wc_DhGenerateKeyPair(&key2, rng, priv2, &privSz2, pub2, &pubSz2); +#if defined(WOLFSSL_ASYNC_CRYPT) + ret = wc_AsyncWait(ret, &key2.asyncDev, WC_ASYNC_FLAG_NONE); +#endif if (ret != 0) { ERROR_OUT(-7185, done); } ret = wc_DhAgree(&key, agree, &agreeSz, priv, privSz, pub2, pubSz2); +#if defined(WOLFSSL_ASYNC_CRYPT) + ret = wc_AsyncWait(ret, &key.asyncDev, WC_ASYNC_FLAG_NONE); +#endif if (ret != 0) { ERROR_OUT(-7186, done); } ret = wc_DhAgree(&key2, agree2, &agreeSz2, priv2, privSz2, pub, pubSz); +#if defined(WOLFSSL_ASYNC_CRYPT) + ret = wc_AsyncWait(ret, &key2.asyncDev, WC_ASYNC_FLAG_NONE); +#endif if (ret != 0) { ERROR_OUT(-7187, done); } diff --git a/wolfssl/openssl/aes.h b/wolfssl/openssl/aes.h index 3104bb2e4..db0d8ddf5 100644 --- a/wolfssl/openssl/aes.h +++ b/wolfssl/openssl/aes.h @@ -59,6 +59,9 @@ typedef struct WOLFSSL_AES_KEY { #ifdef HAVE_PKCS11 void* pkcs11_holder[(AES_MAX_ID_LEN + sizeof(int)) / sizeof(void*)]; #endif + #ifdef WOLFSSL_ASYNC_CRYPT + void* async_holder[128 / sizeof(void*)]; + #endif } WOLFSSL_AES_KEY; typedef WOLFSSL_AES_KEY AES_KEY; From 523b1801ed79530f1700ad3cbc5f149c9b9fcb22 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 19 Sep 2019 13:33:02 -0700 Subject: [PATCH 3/3] Cleanup of the `wc_ecc_sign_hash` function to separate the async logic. This improves the ECC r/s local case to appease static analyzers. Fixes https://github.com/wolfSSL/wolfssl/issues/2342. --- wolfcrypt/src/ecc.c | 199 ++++++++++++++++++++++++++------------------ 1 file changed, 117 insertions(+), 82 deletions(-) diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 06863a4a8..38758e554 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -4507,6 +4507,83 @@ static int wc_ecc_sign_hash_hw(const byte* in, word32 inlen, } #endif /* WOLFSSL_ATECC508A || PLUTON_CRYPTO_ECC || WOLFSSL_CRYPTOCELL */ +#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_ECC) +static int wc_ecc_sign_hash_async(const byte* in, word32 inlen, byte* out, + word32 *outlen, WC_RNG* rng, ecc_key* key) +{ + int err; + mp_int *r = NULL, *s = NULL; + + if (in == NULL || out == NULL || outlen == NULL || key == NULL || + rng == NULL) { + return ECC_BAD_ARG_E; + } + + err = wc_ecc_alloc_async(key); + if (err != 0) { + return err; + } + r = key->r; + s = key->s; + + switch(key->state) { + case ECC_STATE_NONE: + case ECC_STATE_SIGN_DO: + key->state = ECC_STATE_SIGN_DO; + + if ((err = mp_init_multi(r, s, NULL, NULL, NULL, NULL)) != MP_OKAY){ + break; + } + + err = wc_ecc_sign_hash_ex(in, inlen, rng, key, r, s); + if (err < 0) { + break; + } + + FALL_THROUGH; + + case ECC_STATE_SIGN_ENCODE: + key->state = ECC_STATE_SIGN_ENCODE; + + if (key->asyncDev.marker == WOLFSSL_ASYNC_MARKER_ECC) { + #ifdef HAVE_CAVIUM_V + /* Nitrox requires r and s in sep buffer, so split it */ + NitroxEccRsSplit(key, &r->raw, &s->raw); + #endif + #ifndef WOLFSSL_ASYNC_CRYPT_TEST + /* only do this if not simulator, since it overwrites result */ + wc_bigint_to_mp(&r->raw, r); + wc_bigint_to_mp(&s->raw, s); + #endif + } + + /* encoded with DSA header */ + err = StoreECC_DSA_Sig(out, outlen, r, s); + + /* done with R/S */ + mp_clear(r); + mp_clear(s); + break; + + default: + err = BAD_STATE_E; + break; + } + + /* if async pending then return and skip done cleanup below */ + if (err == WC_PENDING_E) { + key->state++; + return err; + } + + /* cleanup */ + wc_ecc_free_async(key); + key->state = ECC_STATE_NONE; + + return err; +} +#endif /* WOLFSSL_ASYNC_CRYPT && WC_ASYNC_ENABLE_ECC */ + /** Sign a message digest in The message digest to sign @@ -4520,10 +4597,12 @@ int wc_ecc_sign_hash(const byte* in, word32 inlen, byte* out, word32 *outlen, WC_RNG* rng, ecc_key* key) { int err; +#if !defined(WOLFSSL_ASYNC_CRYPT) || !defined(WC_ASYNC_ENABLE_ECC) +#ifdef WOLFSSL_SMALL_STACK mp_int *r = NULL, *s = NULL; -#if (!defined(WOLFSSL_ASYNC_CRYPT) || !defined(WC_ASYNC_ENABLE_ECC)) && \ - !defined(WOLFSSL_SMALL_STACK) - mp_int r_lcl, s_lcl; +#else + mp_int r[1], s[1]; +#endif #endif if (in == NULL || out == NULL || outlen == NULL || key == NULL || @@ -4541,15 +4620,11 @@ int wc_ecc_sign_hash(const byte* in, word32 inlen, byte* out, word32 *outlen, #endif #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_ECC) - err = wc_ecc_alloc_async(key); - if (err != 0) - return err; - r = key->r; - s = key->s; -#elif !defined(WOLFSSL_SMALL_STACK) - r = &r_lcl; - s = &s_lcl; + /* handle async cases */ + err = wc_ecc_sign_hash_async(in, inlen, out, outlen, rng, key); #else + +#ifdef WOLFSSL_SMALL_STACK r = (mp_int*)XMALLOC(sizeof(mp_int), key->heap, DYNAMIC_TYPE_ECC); if (r == NULL) return MEMORY_E; @@ -4558,84 +4633,44 @@ int wc_ecc_sign_hash(const byte* in, word32 inlen, byte* out, word32 *outlen, XFREE(r, key->heap, DYNAMIC_TYPE_ECC); return MEMORY_E; } -#endif /* WOLFSSL_ASYNC_CRYPT && WC_ASYNC_ENABLE_ECC */ +#endif + XMEMSET(r, 0, sizeof(mp_int)); + XMEMSET(s, 0, sizeof(mp_int)); - switch(key->state) { - case ECC_STATE_NONE: - case ECC_STATE_SIGN_DO: - key->state = ECC_STATE_SIGN_DO; - - if ((err = mp_init_multi(r, s, NULL, NULL, NULL, NULL)) != MP_OKAY){ - #if !defined(WOLFSSL_ASYNC_CRYPT) && defined(WOLFSSL_SMALL_STACK) - XFREE(s, key->heap, DYNAMIC_TYPE_ECC); - XFREE(r, key->heap, DYNAMIC_TYPE_ECC); - #endif - break; - } - - /* hardware crypto */ - #if defined(WOLFSSL_ATECC508A) || defined(PLUTON_CRYPTO_ECC) || defined(WOLFSSL_CRYPTOCELL) - err = wc_ecc_sign_hash_hw(in, inlen, r, s, out, outlen, rng, key); - #else - err = wc_ecc_sign_hash_ex(in, inlen, rng, key, r, s); - #endif - if (err < 0) { - #if !defined(WOLFSSL_ASYNC_CRYPT) && defined(WOLFSSL_SMALL_STACK) - XFREE(s, key->heap, DYNAMIC_TYPE_ECC); - XFREE(r, key->heap, DYNAMIC_TYPE_ECC); - #endif - break; - } - - FALL_THROUGH; - - case ECC_STATE_SIGN_ENCODE: - key->state = ECC_STATE_SIGN_ENCODE; - - #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_ECC) - if (key->asyncDev.marker == WOLFSSL_ASYNC_MARKER_ECC) { - #ifdef HAVE_CAVIUM_V - /* Nitrox requires r and s in sep buffer, so split it */ - NitroxEccRsSplit(key, &r->raw, &s->raw); - #endif - #ifndef WOLFSSL_ASYNC_CRYPT_TEST - /* only do this if not simulator, since it overwrites result */ - wc_bigint_to_mp(&r->raw, r); - wc_bigint_to_mp(&s->raw, s); - #endif - } - #endif /* WOLFSSL_ASYNC_CRYPT */ - - /* encoded with DSA header */ - err = StoreECC_DSA_Sig(out, outlen, r, s); - - /* done with R/S */ - mp_clear(r); - mp_clear(s); - #if !defined(WOLFSSL_ASYNC_CRYPT) && defined(WOLFSSL_SMALL_STACK) - XFREE(s, key->heap, DYNAMIC_TYPE_ECC); - XFREE(r, key->heap, DYNAMIC_TYPE_ECC); - #endif - break; - - default: - err = BAD_STATE_E; - break; - } - -#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_ECC) - /* if async pending then return and skip done cleanup below */ - if (err == WC_PENDING_E) { - key->state++; + if ((err = mp_init_multi(r, s, NULL, NULL, NULL, NULL)) != MP_OKAY){ + #ifdef WOLFSSL_SMALL_STACK + XFREE(s, key->heap, DYNAMIC_TYPE_ECC); + XFREE(r, key->heap, DYNAMIC_TYPE_ECC); + #endif return err; } + +/* hardware crypto */ +#if defined(WOLFSSL_ATECC508A) || defined(PLUTON_CRYPTO_ECC) || defined(WOLFSSL_CRYPTOCELL) + err = wc_ecc_sign_hash_hw(in, inlen, r, s, out, outlen, rng, key); +#else + err = wc_ecc_sign_hash_ex(in, inlen, rng, key, r, s); #endif + if (err < 0) { + #ifdef WOLFSSL_SMALL_STACK + XFREE(s, key->heap, DYNAMIC_TYPE_ECC); + XFREE(r, key->heap, DYNAMIC_TYPE_ECC); + #endif + return err; + } + + /* encoded with DSA header */ + err = StoreECC_DSA_Sig(out, outlen, r, s); /* cleanup */ -#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_ECC) - wc_ecc_free_async(key); + mp_clear(r); + mp_clear(s); + +#ifdef WOLFSSL_SMALL_STACK + XFREE(s, key->heap, DYNAMIC_TYPE_ECC); + XFREE(r, key->heap, DYNAMIC_TYPE_ECC); #endif - key->state = ECC_STATE_NONE; +#endif /* WOLFSSL_ASYNC_CRYPT */ return err; }