From efbabbfb291ec4400d51a4da1471514b378c8830 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 25 Jul 2018 15:04:26 -0700 Subject: [PATCH] Further improvements to hashing code to make sure wc_*Free is always called including wc_HashFree. Added new defines to disable PIC32MZ hardware features using `NO_PIC32MZ_HASH`, `NO_PIC32MZ_RNG` and `NO_PIC32MZ_CRYPT`. --- src/tls.c | 83 ++++----------------- tests/api.c | 9 +++ wolfcrypt/src/ecc.c | 24 ++---- wolfcrypt/src/hash.c | 73 ++++++++++++++++-- wolfcrypt/src/hmac.c | 5 ++ wolfcrypt/src/pkcs7.c | 138 +++++++++++++---------------------- wolfcrypt/src/pwdbased.c | 4 + wolfcrypt/test/test.c | 2 + wolfssl/wolfcrypt/hash.h | 2 +- wolfssl/wolfcrypt/settings.h | 12 ++- 10 files changed, 167 insertions(+), 185 deletions(-) diff --git a/src/tls.c b/src/tls.c index 2c94127b2..d95501b3d 100644 --- a/src/tls.c +++ b/src/tls.c @@ -935,74 +935,21 @@ static int Hmac_HashFinalRaw(Hmac* hmac, unsigned char* hash) static int Hmac_OuterHash(Hmac* hmac, unsigned char* mac) { int ret = BAD_FUNC_ARG; + wc_HashAlg hash; + enum wc_HashType hashType = (enum wc_HashType)hmac->macType; + int digestSz = wc_HashGetDigestSize(hashType); + int blockSz = wc_HashGetBlockSize(hashType); - switch (hmac->macType) { - #ifndef NO_SHA - case WC_SHA: - ret = wc_InitSha(&hmac->hash.sha); - if (ret == 0) { - ret = wc_ShaUpdate(&hmac->hash.sha, (byte*)hmac->opad, - WC_SHA_BLOCK_SIZE); - if (ret == 0) - ret = wc_ShaUpdate(&hmac->hash.sha, (byte*)hmac->innerHash, - WC_SHA_DIGEST_SIZE); - if (ret == 0) - ret = wc_ShaFinal(&hmac->hash.sha, mac); - wc_ShaFree(&hmac->hash.sha); - } - break; - #endif /* !NO_SHA */ - - #ifndef NO_SHA256 - case WC_SHA256: - ret = wc_InitSha256(&hmac->hash.sha256); - if (ret == 0) { - ret = wc_Sha256Update(&hmac->hash.sha256, (byte*)hmac->opad, - WC_SHA256_BLOCK_SIZE); - if (ret == 0) - ret = wc_Sha256Update(&hmac->hash.sha256, - (byte*)hmac->innerHash, - WC_SHA256_DIGEST_SIZE); - if (ret == 0) - ret = wc_Sha256Final(&hmac->hash.sha256, mac); - wc_Sha256Free(&hmac->hash.sha256); - } - break; - #endif /* !NO_SHA256 */ - - #ifdef WOLFSSL_SHA384 - case WC_SHA384: - ret = wc_InitSha384(&hmac->hash.sha384); - if (ret == 0) { - ret = wc_Sha384Update(&hmac->hash.sha384, (byte*)hmac->opad, - WC_SHA384_BLOCK_SIZE); - if (ret == 0) - ret = wc_Sha384Update(&hmac->hash.sha384, - (byte*)hmac->innerHash, - WC_SHA384_DIGEST_SIZE); - if (ret == 0) - ret = wc_Sha384Final(&hmac->hash.sha384, mac); - wc_Sha384Free(&hmac->hash.sha384); - } - break; - #endif /* WOLFSSL_SHA384 */ - - #ifdef WOLFSSL_SHA512 - case WC_SHA512: - ret = wc_InitSha512(&hmac->hash.sha512); - if (ret == 0) { - ret = wc_Sha512Update(&hmac->hash.sha512,(byte*)hmac->opad, - WC_SHA512_BLOCK_SIZE); - if (ret == 0) - ret = wc_Sha512Update(&hmac->hash.sha512, - (byte*)hmac->innerHash, - WC_SHA512_DIGEST_SIZE); - if (ret == 0) - ret = wc_Sha512Final(&hmac->hash.sha512, mac); - wc_Sha512Free(&hmac->hash.sha512); - } - break; - #endif /* WOLFSSL_SHA512 */ + ret = wc_HashInit(&hash, hashType); + if (ret == 0) { + ret = wc_HashUpdate(&hash, hashType, (byte*)hmac->opad, + blockSz); + if (ret == 0) + ret = wc_HashUpdate(&hash, hashType, (byte*)hmac->innerHash, + digestSz); + if (ret == 0) + ret = wc_HashFinal(&hash, hashType, mac); + wc_HashFree(&hash, hashType); } return ret; @@ -10175,7 +10122,7 @@ int TLSX_Parse(WOLFSSL* ssl, byte* input, word16 length, byte msgType, return method; } #endif /* WOLFSSL_ALLOW_TLSV10 */ - + #if defined(OPENSSL_EXTRA) || defined(OPENSSL_ALL) /* Gets a WOLFSL_METHOD type that is not set as client or server * diff --git a/tests/api.c b/tests/api.c index 84e0b6795..4345024d8 100644 --- a/tests/api.c +++ b/tests/api.c @@ -5277,6 +5277,7 @@ static int test_wc_Sha3_224_Final (void) ret = WOLFSSL_FATAL_ERROR; } } + wc_Sha3_224_Free(&sha3); printf(resultFmt, ret == 0 ? passed : failed); if (ret == 0) { @@ -5370,7 +5371,9 @@ static int test_wc_Sha3_256_Final (void) ret = WOLFSSL_FATAL_ERROR; } } + wc_Sha3_256_Free(&sha3); printf(resultFmt, ret == 0 ? passed : failed); + if (ret == 0) { printf(testingFmt, "wc_Sha3_256_GetHash()"); @@ -5461,7 +5464,9 @@ static int test_wc_Sha3_384_Final (void) ret = WOLFSSL_FATAL_ERROR; } } + wc_Sha3_384_Free(&sha3); printf(resultFmt, ret == 0 ? passed : failed); + if (ret == 0) { printf(testingFmt, "wc_Sha3_384_GetHash()"); @@ -5554,7 +5559,9 @@ static int test_wc_Sha3_512_Final (void) ret = WOLFSSL_FATAL_ERROR; } } + wc_Sha3_512_Free(&sha3); printf(resultFmt, ret == 0 ? passed : failed); + if (ret == 0) { printf(testingFmt, "wc_Sha3_512_GetHash()"); @@ -15390,6 +15397,8 @@ static int test_wc_HashInit(void) ret = 1; break; } + wc_HashFree(&hash, enumArray[i]); + /* check for null ptr */ if (wc_HashInit(NULL, enumArray[i]) != BAD_FUNC_ARG) { ret = 1; diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 871ed166e..dbd686475 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -9539,36 +9539,24 @@ int wc_X963_KDF(enum wc_HashType type, const byte* secret, word32 secretSz, ret = wc_HashUpdate(hash, type, secret, secretSz); if (ret != 0) { -#ifdef WOLFSSL_SMALL_STACK - XFREE(hash, NULL, DYNAMIC_TYPE_HASHES); -#endif - return ret; + break; } ret = wc_HashUpdate(hash, type, counter, sizeof(counter)); if (ret != 0) { -#ifdef WOLFSSL_SMALL_STACK - XFREE(hash, NULL, DYNAMIC_TYPE_HASHES); -#endif - return ret; + break; } if (sinfo) { ret = wc_HashUpdate(hash, type, sinfo, sinfoSz); if (ret != 0) { -#ifdef WOLFSSL_SMALL_STACK - XFREE(hash, NULL, DYNAMIC_TYPE_HASHES); -#endif - return ret; + break; } } ret = wc_HashFinal(hash, type, tmp); if (ret != 0) { -#ifdef WOLFSSL_SMALL_STACK - XFREE(hash, NULL, DYNAMIC_TYPE_HASHES); -#endif - return ret; + break; } copySz = min(remaining, digestSz); @@ -9578,11 +9566,13 @@ int wc_X963_KDF(enum wc_HashType type, const byte* secret, word32 secretSz, outIdx += copySz; } + wc_HashFree(hash, type); + #ifdef WOLFSSL_SMALL_STACK XFREE(hash, NULL, DYNAMIC_TYPE_HASHES); #endif - return 0; + return ret; } #endif /* HAVE_X963_KDF */ diff --git a/wolfcrypt/src/hash.c b/wolfcrypt/src/hash.c index 60ae8aa48..78fdaa189 100644 --- a/wolfcrypt/src/hash.c +++ b/wolfcrypt/src/hash.c @@ -475,8 +475,7 @@ int wc_HashInit(wc_HashAlg* hash, enum wc_HashType type) switch (type) { case WC_HASH_TYPE_MD5: #ifndef NO_MD5 - wc_InitMd5(&hash->md5); - ret = 0; + ret = wc_InitMd5(&hash->md5); #endif break; case WC_HASH_TYPE_SHA: @@ -533,15 +532,12 @@ int wc_HashUpdate(wc_HashAlg* hash, enum wc_HashType type, const byte* data, switch (type) { case WC_HASH_TYPE_MD5: #ifndef NO_MD5 - wc_Md5Update(&hash->md5, data, dataSz); - ret = 0; + ret = wc_Md5Update(&hash->md5, data, dataSz); #endif break; case WC_HASH_TYPE_SHA: #ifndef NO_SHA ret = wc_ShaUpdate(&hash->sha, data, dataSz); - if (ret != 0) - return ret; #endif break; case WC_HASH_TYPE_SHA224: @@ -592,8 +588,7 @@ int wc_HashFinal(wc_HashAlg* hash, enum wc_HashType type, byte* out) switch (type) { case WC_HASH_TYPE_MD5: #ifndef NO_MD5 - wc_Md5Final(&hash->md5, out); - ret = 0; + ret = wc_Md5Final(&hash->md5, out); #endif break; case WC_HASH_TYPE_SHA: @@ -639,6 +634,68 @@ int wc_HashFinal(wc_HashAlg* hash, enum wc_HashType type, byte* out) return ret; } +int wc_HashFree(wc_HashAlg* hash, enum wc_HashType type) +{ + int ret = HASH_TYPE_E; /* Default to hash type error */ + + if (hash == NULL) + return BAD_FUNC_ARG; + + switch (type) { + case WC_HASH_TYPE_MD5: +#ifndef NO_MD5 + wc_Md5Free(&hash->md5); + ret = 0; +#endif + break; + case WC_HASH_TYPE_SHA: +#ifndef NO_SHA + wc_ShaFree(&hash->sha); + ret = 0; +#endif + break; + case WC_HASH_TYPE_SHA224: +#ifdef WOLFSSL_SHA224 + wc_Sha224Free(&hash->sha224); + ret = 0; +#endif + break; + case WC_HASH_TYPE_SHA256: +#ifndef NO_SHA256 + wc_Sha256Free(&hash->sha256); + ret = 0; +#endif + break; + case WC_HASH_TYPE_SHA384: +#ifdef WOLFSSL_SHA384 + wc_Sha384Free(&hash->sha384); + ret = 0; +#endif + break; + case WC_HASH_TYPE_SHA512: +#ifdef WOLFSSL_SHA512 + wc_Sha512Free(&hash->sha512); + ret = 0; +#endif + break; + + /* not supported */ + case WC_HASH_TYPE_MD5_SHA: + case WC_HASH_TYPE_MD2: + case WC_HASH_TYPE_MD4: + case WC_HASH_TYPE_SHA3_224: + case WC_HASH_TYPE_SHA3_256: + case WC_HASH_TYPE_SHA3_384: + case WC_HASH_TYPE_SHA3_512: + case WC_HASH_TYPE_BLAKE2B: + case WC_HASH_TYPE_NONE: + default: + ret = BAD_FUNC_ARG; + }; + + return ret; +} + #if !defined(WOLFSSL_TI_HASH) diff --git a/wolfcrypt/src/hmac.c b/wolfcrypt/src/hmac.c index 9f2d24bee..af9d38fd4 100644 --- a/wolfcrypt/src/hmac.c +++ b/wolfcrypt/src/hmac.c @@ -296,6 +296,11 @@ int wc_HmacSetKey(Hmac* hmac, int type, const byte* key, word32 length) return BAD_FUNC_ARG; } + /* if set key has already been run then make sure and free existing */ + if (hmac->macType != 0) { + wc_HmacFree(hmac); + } + hmac->innerHashKeyed = 0; hmac->macType = (byte)type; diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 40a2910a8..7f7293193 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -897,16 +897,14 @@ static int wc_PKCS7_BuildDigestInfo(PKCS7* pkcs7, byte* flatSignedAttribs, ret = wc_HashUpdate(&esd->hash, esd->hashType, attribSet, attribSetSz); - if (ret < 0) - return ret; + if (ret == 0) + ret = wc_HashUpdate(&esd->hash, esd->hashType, + flatSignedAttribs, flatSignedAttribsSz); + if (ret == 0) + ret = wc_HashFinal(&esd->hash, esd->hashType, + esd->contentAttribsDigest); + wc_HashFree(&esd->hash, esd->hashType); - ret = wc_HashUpdate(&esd->hash, esd->hashType, - flatSignedAttribs, flatSignedAttribsSz); - if (ret < 0) - return ret; - - ret = wc_HashFinal(&esd->hash, esd->hashType, - esd->contentAttribsDigest); if (ret < 0) return ret; @@ -1089,33 +1087,26 @@ int wc_PKCS7_EncodeSignedData(PKCS7* pkcs7, byte* output, word32 outputSz) } hashSz = ret; - ret = wc_HashInit(&esd->hash, esd->hashType); - if (ret != 0) { -#ifdef WOLFSSL_SMALL_STACK - XFREE(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); -#endif - return ret; - } - if (pkcs7->contentSz != 0) { - ret = wc_HashUpdate(&esd->hash, esd->hashType, - pkcs7->content, pkcs7->contentSz); - if (ret < 0) { -#ifdef WOLFSSL_SMALL_STACK - XFREE(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); -#endif - return ret; + ret = wc_HashInit(&esd->hash, esd->hashType); + if (ret == 0) { + ret = wc_HashUpdate(&esd->hash, esd->hashType, + pkcs7->content, pkcs7->contentSz); + if (ret == 0) { + esd->contentDigest[0] = ASN_OCTET_STRING; + esd->contentDigest[1] = (byte)hashSz; + ret = wc_HashFinal(&esd->hash, esd->hashType, + &esd->contentDigest[2]); + } + wc_HashFree(&esd->hash, esd->hashType); } - esd->contentDigest[0] = ASN_OCTET_STRING; - esd->contentDigest[1] = (byte)hashSz; - ret = wc_HashFinal(&esd->hash, esd->hashType, - &esd->contentDigest[2]); + if (ret < 0) { #ifdef WOLFSSL_SMALL_STACK XFREE(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); #endif - return ret; + return ret; } } @@ -1492,11 +1483,27 @@ static int wc_PKCS7_BuildSignedDataDigest(PKCS7* pkcs7, byte* signedAttrib, wc_HashAlg hash; enum wc_HashType hashType; + /* check arguments */ if (pkcs7 == NULL || pkcs7Digest == NULL || pkcs7DigestSz == NULL || plainDigest == NULL) { return BAD_FUNC_ARG; } + hashType = wc_OidGetHash(pkcs7->hashOID); + ret = wc_HashGetDigestSize(hashType); + if (ret < 0) + return ret; + hashSz = ret; + + if (signedAttribSz > 0) { + if (signedAttrib == NULL) + return BAD_FUNC_ARG; + } + else { + if (pkcs7->content == NULL) + return BAD_FUNC_ARG; + } + #ifdef WOLFSSL_SMALL_STACK digestInfo = (byte*)XMALLOC(MAX_PKCS7_DIGEST_SZ, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); @@ -1508,15 +1515,6 @@ static int wc_PKCS7_BuildSignedDataDigest(PKCS7* pkcs7, byte* signedAttrib, XMEMSET(digest, 0, WC_MAX_DIGEST_SIZE); XMEMSET(digestInfo, 0, MAX_PKCS7_DIGEST_SZ); - hashType = wc_OidGetHash(pkcs7->hashOID); - ret = wc_HashGetDigestSize(hashType); - if (ret < 0) { -#ifdef WOLFSSL_SMALL_STACK - XFREE(digestInfo, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); -#endif - return ret; - } - hashSz = ret; /* calculate digest */ ret = wc_HashInit(&hash, hashType); @@ -1528,63 +1526,27 @@ static int wc_PKCS7_BuildSignedDataDigest(PKCS7* pkcs7, byte* signedAttrib, } if (signedAttribSz > 0) { - - if (signedAttrib == NULL) { -#ifdef WOLFSSL_SMALL_STACK - XFREE(digestInfo, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); -#endif - return BAD_FUNC_ARG; - } - attribSetSz = SetSet(signedAttribSz, attribSet); + + /* calculate digest */ ret = wc_HashUpdate(&hash, hashType, attribSet, attribSetSz); - if (ret < 0) { -#ifdef WOLFSSL_SMALL_STACK - XFREE(digestInfo, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); -#endif - return ret; - } - - ret = wc_HashUpdate(&hash, hashType, signedAttrib, signedAttribSz); - if (ret < 0) { -#ifdef WOLFSSL_SMALL_STACK - XFREE(digestInfo, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); -#endif - return ret; - } - - ret = wc_HashFinal(&hash, hashType, digest); - if (ret < 0) { -#ifdef WOLFSSL_SMALL_STACK - XFREE(digestInfo, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); -#endif - return ret; - } - + if (ret == 0) + ret = wc_HashUpdate(&hash, hashType, signedAttrib, signedAttribSz); + if (ret == 0) + ret = wc_HashFinal(&hash, hashType, digest); } else { - - if (pkcs7->content == NULL) { -#ifdef WOLFSSL_SMALL_STACK - XFREE(digestInfo, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); -#endif - return BAD_FUNC_ARG; - } - ret = wc_HashUpdate(&hash, hashType, pkcs7->content, pkcs7->contentSz); - if (ret < 0) { -#ifdef WOLFSSL_SMALL_STACK - XFREE(digestInfo, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); -#endif - return ret; - } + if (ret == 0) + ret = wc_HashFinal(&hash, hashType, digest); + } - ret = wc_HashFinal(&hash, hashType, digest); - if (ret < 0) { + wc_HashFree(&hash, hashType); + + if (ret < 0) { #ifdef WOLFSSL_SMALL_STACK - XFREE(digestInfo, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(digestInfo, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); #endif - return ret; - } + return ret; } /* Set algoID, with NULL attributes */ diff --git a/wolfcrypt/src/pwdbased.c b/wolfcrypt/src/pwdbased.c index cadd1c892..57d46e9a0 100644 --- a/wolfcrypt/src/pwdbased.c +++ b/wolfcrypt/src/pwdbased.c @@ -142,6 +142,8 @@ int wc_PBKDF1_ex(byte* key, int keyLen, byte* iv, int ivLen, } } + wc_HashFree(hash, hashT); + #ifdef WOLFSSL_SMALL_STACK XFREE(hash, heap, DYNAMIC_TYPE_HASHCTX); #endif @@ -308,6 +310,8 @@ static int DoPKCS12Hash(int hashType, byte* buffer, word32 totalLen, ret = wc_HashFinal(hash, hashT, Ai); } + wc_HashFree(hash, hashT); + #ifdef WOLFSSL_SMALL_STACK XFREE(hash, NULL, DYNAMIC_TYPE_HASHCTX); #endif diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 5fdeda243..a61ac1456 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -2764,6 +2764,7 @@ int hash_test(void) ret = wc_HashFinal(&hash, typesBad[i], out); if (ret != BAD_FUNC_ARG) return -2927 - i; + wc_HashFree(&hash, typesBad[i]); } /* Try valid hash algorithms. */ @@ -2783,6 +2784,7 @@ int hash_test(void) ret = wc_HashFinal(&hash, typesGood[i], out); if (ret != exp_ret) return -2957 - i; + wc_HashFree(&hash, typesGood[i]); digestSz = wc_HashGetDigestSize(typesGood[i]); if (exp_ret < 0 && digestSz != exp_ret) diff --git a/wolfssl/wolfcrypt/hash.h b/wolfssl/wolfcrypt/hash.h index 698fa51e3..db16d41d1 100644 --- a/wolfssl/wolfcrypt/hash.h +++ b/wolfssl/wolfcrypt/hash.h @@ -134,7 +134,7 @@ WOLFSSL_API int wc_HashUpdate(wc_HashAlg* hash, enum wc_HashType type, const byte* data, word32 dataSz); WOLFSSL_API int wc_HashFinal(wc_HashAlg* hash, enum wc_HashType type, byte* out); - +WOLFSSL_API int wc_HashFree(wc_HashAlg* hash, enum wc_HashType type); #ifndef NO_MD5 #include diff --git a/wolfssl/wolfcrypt/settings.h b/wolfssl/wolfcrypt/settings.h index 9309c0e54..1027f3398 100644 --- a/wolfssl/wolfcrypt/settings.h +++ b/wolfssl/wolfcrypt/settings.h @@ -247,9 +247,15 @@ #endif #ifdef WOLFSSL_MICROCHIP_PIC32MZ - #define WOLFSSL_PIC32MZ_CRYPT - #define WOLFSSL_PIC32MZ_RNG - #define WOLFSSL_PIC32MZ_HASH + #ifndef NO_PIC32MZ_CRYPT + #define WOLFSSL_PIC32MZ_CRYPT + #endif + #ifndef NO_PIC32MZ_RNG + #define WOLFSSL_PIC32MZ_RNG + #endif + #ifndef NO_PIC32MZ_HASH + #define WOLFSSL_PIC32MZ_HASH + #endif #endif #ifdef MICROCHIP_TCPIP_V5