fix resource leak (missing calls to wc_AesFree()) in wolfSSL_EVP_CIPHER_CTX_cleanup();

fix file descriptor leaks in AF_ALG code, and fix return codes (WC_AFALG_SOCK_E, not -1) in afalg_aes.c;

fixes for sanitizer-detected forbidden null pointer args in AfalgHashUpdate() and AfalgHashCopy();

fixes for resource leaks in api.c test_wolfSSL_AES_cbc_encrypt() (missing wc_AesFree()s);

fixes for resource leaks in test.c openssl_test() (missing wolfSSL_EVP_CIPHER_CTX_cleanup());

also some local fixes for bugprone-signed-char-misuse, readability-redundant-preprocessor, and clang-diagnostic-strict-prototypes, in src/pk.c and src/ssl.c.
This commit is contained in:
Daniel Pouzzner
2023-02-01 00:49:34 -06:00
parent 462f76c1ff
commit 38c057a084
8 changed files with 145 additions and 44 deletions

View File

@ -12521,7 +12521,7 @@ point_conversion_form_t wolfSSL_EC_KEY_get_conv_form(const WOLFSSL_EC_KEY* key)
int ret = -1;
if (key != NULL) {
ret = key->form;
ret = (int)(unsigned char)key->form;
}
return ret;

View File

@ -295,14 +295,13 @@ int wc_OBJ_sn2nid(const char *sn)
#define HAVE_GLOBAL_RNG /* consolidate flags for using globalRNG */
static WC_RNG globalRNG;
static int initGlobalRNG = 0;
#ifndef WOLFCRYPT_ONLY
static wolfSSL_Mutex globalRNGMutex;
static int globalRNGMutex_valid = 0;
#if defined(OPENSSL_EXTRA) && defined(HAVE_HASHDRBG)
static WOLFSSL_DRBG_CTX* gDrbgDefCtx = NULL;
#endif
#endif
WC_RNG* wolfssl_get_global_rng(void)
{
@ -321,7 +320,7 @@ WC_RNG* wolfssl_get_global_rng(void)
* @return Global RNG on success.
* @return NULL on error.
*/
WC_RNG* wolfssl_make_global_rng()
WC_RNG* wolfssl_make_global_rng(void)
{
WC_RNG* ret;

View File

@ -44173,6 +44173,7 @@ static int test_wolfSSL_AES_cbc_encrypt(void)
AssertIntEQ(wolfSSL_AES_set_encrypt_key(key128, sizeof(key128)*8, &aes), 0);
wolfSSL_AES_cbc_encrypt(pt128, out, len, &aes, iv128tmp, AES_ENCRYPT);
AssertIntEQ(XMEMCMP(out, ct128, AES_BLOCK_SIZE), 0);
wc_AesFree((Aes*)&aes);
#ifdef HAVE_AES_DECRYPT
@ -44184,6 +44185,7 @@ static int test_wolfSSL_AES_cbc_encrypt(void)
AssertIntEQ(wolfSSL_AES_set_decrypt_key(key128, sizeof(key128)*8, &aes), 0);
wolfSSL_AES_cbc_encrypt(ct128, out, len, &aes, iv128tmp, AES_DECRYPT);
AssertIntEQ(XMEMCMP(out, pt128, AES_BLOCK_SIZE), 0);
wc_AesFree((Aes*)&aes);
#endif
@ -44217,6 +44219,7 @@ static int test_wolfSSL_AES_cbc_encrypt(void)
AssertIntEQ(wolfSSL_AES_set_encrypt_key(key192, sizeof(key192)*8, &aes), 0);
wolfSSL_AES_cbc_encrypt(pt192, out, len, &aes, iv192tmp, AES_ENCRYPT);
AssertIntEQ(XMEMCMP(out, ct192, AES_BLOCK_SIZE), 0);
wc_AesFree((Aes*)&aes);
#ifdef HAVE_AES_DECRYPT
@ -44228,6 +44231,7 @@ static int test_wolfSSL_AES_cbc_encrypt(void)
AssertIntEQ(wolfSSL_AES_set_decrypt_key(key192, sizeof(key192)*8, &aes), 0);
wolfSSL_AES_cbc_encrypt(ct192, out, len, &aes, iv192tmp, AES_DECRYPT);
AssertIntEQ(XMEMCMP(out, pt192, AES_BLOCK_SIZE), 0);
wc_AesFree((Aes*)&aes);
#endif
}
@ -44262,6 +44266,7 @@ static int test_wolfSSL_AES_cbc_encrypt(void)
AssertIntEQ(wolfSSL_AES_set_encrypt_key(key256, sizeof(key256)*8, &aes), 0);
wolfSSL_AES_cbc_encrypt(pt256, out, len, &aes, iv256tmp, AES_ENCRYPT);
AssertIntEQ(XMEMCMP(out, ct256, AES_BLOCK_SIZE), 0);
wc_AesFree((Aes*)&aes);
#ifdef HAVE_AES_DECRYPT
@ -44273,6 +44278,7 @@ static int test_wolfSSL_AES_cbc_encrypt(void)
AssertIntEQ(wolfSSL_AES_set_decrypt_key(key256, sizeof(key256)*8, &aes), 0);
wolfSSL_AES_cbc_encrypt(ct256, out, len, &aes, iv256tmp, AES_DECRYPT);
AssertIntEQ(XMEMCMP(out, pt256, AES_BLOCK_SIZE), 0);
wc_AesFree((Aes*)&aes);
#endif
@ -44288,6 +44294,7 @@ static int test_wolfSSL_AES_cbc_encrypt(void)
15), WOLFSSL_FAILURE);
AssertIntEQ(wolfSSL_AES_wrap_key(&aes, NULL, wrapCipher, key256,
sizeof(key256)), sizeof(wrapCipher));
wc_AesFree((Aes*)&aes);
/* wolfSSL_AES_unwrap_key() 256-bit NULL iv */
AssertIntEQ(wolfSSL_AES_set_decrypt_key(key256, sizeof(key256)*8, &aes), 0);
@ -44298,17 +44305,20 @@ static int test_wolfSSL_AES_cbc_encrypt(void)
AssertIntEQ(XMEMCMP(wrapPlain, key256, sizeof(key256)), 0);
XMEMSET(wrapCipher, 0, sizeof(wrapCipher));
XMEMSET(wrapPlain, 0, sizeof(wrapPlain));
wc_AesFree((Aes*)&aes);
/* wolfSSL_AES_wrap_key() 256-bit custom iv */
AssertIntEQ(wolfSSL_AES_set_encrypt_key(key256, sizeof(key256)*8, &aes), 0);
AssertIntEQ(wolfSSL_AES_wrap_key(&aes, wrapIV, wrapCipher, key256,
sizeof(key256)), sizeof(wrapCipher));
wc_AesFree((Aes*)&aes);
/* wolfSSL_AES_unwrap_key() 256-bit custom iv */
AssertIntEQ(wolfSSL_AES_set_decrypt_key(key256, sizeof(key256)*8, &aes), 0);
AssertIntEQ(wolfSSL_AES_unwrap_key(&aes, wrapIV, wrapPlain, wrapCipher,
sizeof(wrapCipher)), sizeof(wrapPlain));
AssertIntEQ(XMEMCMP(wrapPlain, key256, sizeof(key256)), 0);
wc_AesFree((Aes*)&aes);
}
#endif /* HAVE_AES_KEYWRAP */
}

View File

@ -10664,8 +10664,8 @@ int wc_AesInit(Aes* aes, void* heap, int devId)
#endif /* WOLFSSL_ASYNC_CRYPT */
#ifdef WOLFSSL_AFALG
aes->alFd = -1;
aes->rdFd = -1;
aes->alFd = WC_SOCK_NOTSET;
aes->rdFd = WC_SOCK_NOTSET;
#endif
#ifdef WOLFSSL_KCAPI_AES
aes->handle = NULL;
@ -10769,9 +10769,11 @@ void wc_AesFree(Aes* aes)
#if defined(WOLFSSL_AFALG) || defined(WOLFSSL_AFALG_XILINX_AES)
if (aes->rdFd > 0) { /* negative is error case */
close(aes->rdFd);
aes->rdFd = WC_SOCK_NOTSET;
}
if (aes->alFd > 0) {
close(aes->alFd);
aes->alFd = WC_SOCK_NOTSET;
}
#endif /* WOLFSSL_AFALG */
#ifdef WOLFSSL_KCAPI_AES

View File

@ -5991,20 +5991,57 @@ int wolfSSL_EVP_MD_type(const WOLFSSL_EVP_MD* type)
if (ctx) {
#if (!defined(HAVE_FIPS) && !defined(HAVE_SELFTEST)) || \
(defined(HAVE_FIPS_VERSION) && (HAVE_FIPS_VERSION >= 2))
#ifndef NO_AES
switch (ctx->cipherType) {
#if defined(HAVE_AESGCM) && defined(WOLFSSL_AESGCM_STREAM)
if ((ctx->cipherType == AES_128_GCM_TYPE) ||
(ctx->cipherType == AES_192_GCM_TYPE) ||
(ctx->cipherType == AES_256_GCM_TYPE)) {
wc_AesFree(&ctx->cipher.aes);
}
case AES_128_GCM_TYPE:
case AES_192_GCM_TYPE:
case AES_256_GCM_TYPE:
#endif /* HAVE_AESGCM && WOLFSSL_AESGCM_STREAM */
#if defined(HAVE_AESCCM)
if ((ctx->cipherType == AES_128_CCM_TYPE) ||
(ctx->cipherType == AES_192_CCM_TYPE) ||
(ctx->cipherType == AES_256_CCM_TYPE)) {
wc_AesFree(&ctx->cipher.aes);
}
case AES_128_CCM_TYPE:
case AES_192_CCM_TYPE:
case AES_256_CCM_TYPE:
#endif /* HAVE_AESCCM */
#ifdef HAVE_AESCBC
case AES_128_CBC_TYPE:
case AES_192_CBC_TYPE:
case AES_256_CBC_TYPE:
#endif
#ifdef WOLFSSL_AES_COUNTER
case AES_128_CTR_TYPE:
case AES_192_CTR_TYPE:
case AES_256_CTR_TYPE:
#endif
#ifdef HAVE_AES_ECB
case AES_128_ECB_TYPE:
case AES_192_ECB_TYPE:
case AES_256_ECB_TYPE:
#endif
#ifdef HAVE_AES_CFB
case AES_128_CFB1_TYPE:
case AES_192_CFB1_TYPE:
case AES_256_CFB1_TYPE:
case AES_128_CFB8_TYPE:
case AES_192_CFB8_TYPE:
case AES_256_CFB8_TYPE:
case AES_128_CFB128_TYPE:
case AES_192_CFB128_TYPE:
case AES_256_CFB128_TYPE:
#endif
#ifdef HAVE_AES_OFB
case AES_128_OFB_TYPE:
case AES_192_OFB_TYPE:
case AES_256_OFB_TYPE:
#endif
#ifdef WOLFSSL_AES_XTS
case AES_128_XTS_TYPE:
case AES_256_XTS_TYPE:
#endif
wc_AesFree(&ctx->cipher.aes);
}
#endif /* !NO_AES */
#endif /* not FIPS or FIPS v2+ */
ctx->cipherType = WOLFSSL_EVP_CIPH_TYPE_INIT; /* not yet initialized */
#if defined(HAVE_CHACHA) && defined(HAVE_POLY1305)

View File

@ -62,11 +62,12 @@ static int wc_AesSetup(Aes* aes, const char* type, const char* name, int ivSz, i
if (aes->rdFd < 0) {
WOLFSSL_MSG("Unable to accept and get AF_ALG read socket");
aes->rdFd = WC_SOCK_NOTSET;
return aes->rdFd;
return WC_AFALG_SOCK_E;
}
if (setsockopt(aes->alFd, SOL_ALG, ALG_SET_KEY, key, aes->keylen) != 0) {
WOLFSSL_MSG("Unable to set AF_ALG key");
(void)close(aes->rdFd);
aes->rdFd = WC_SOCK_NOTSET;
return WC_AFALG_SOCK_E;
}
@ -93,8 +94,9 @@ static int wc_AesSetup(Aes* aes, const char* type, const char* name, int ivSz, i
if (wc_Afalg_SetOp(CMSG_FIRSTHDR(&(aes->msg)), aes->dir) < 0) {
WOLFSSL_MSG("Error with setting AF_ALG operation");
(void)close(aes->rdFd);
aes->rdFd = WC_SOCK_NOTSET;
return -1;
return WC_AFALG_SOCK_E;
}
return 0;
@ -127,8 +129,14 @@ int wc_AesSetKey(Aes* aes, const byte* userKey, word32 keylen,
aes->left = 0;
#endif
if (aes->rdFd > 0) {
(void)close(aes->rdFd);
}
aes->rdFd = WC_SOCK_NOTSET;
aes->alFd = wc_Afalg_Socket();
if (aes->alFd <= 0) {
aes->alFd = wc_Afalg_Socket();
}
if (aes->alFd < 0) {
WOLFSSL_MSG("Unable to open an AF_ALG socket");
return WC_AFALG_SOCK_E;
@ -190,11 +198,11 @@ int wc_AesSetKey(Aes* aes, const byte* userKey, word32 keylen,
ret = (int)sendmsg(aes->rdFd, &(aes->msg), 0);
if (ret < 0) {
return ret;
return WC_AFALG_SOCK_E;
}
ret = (int)read(aes->rdFd, out, sz);
if (ret < 0) {
return ret;
return WC_AFALG_SOCK_E;
}
/* set IV for next CBC call */
@ -251,11 +259,11 @@ int wc_AesSetKey(Aes* aes, const byte* userKey, word32 keylen,
ret = (int)sendmsg(aes->rdFd, &(aes->msg), 0);
if (ret < 0) {
return ret;
return WC_AFALG_SOCK_E;
}
ret = (int)read(aes->rdFd, out, sz);
if (ret < 0) {
return ret;
return WC_AFALG_SOCK_E;
}
}
@ -301,11 +309,11 @@ static int wc_Afalg_AesDirect(Aes* aes, byte* out, const byte* in, word32 sz)
ret = (int)sendmsg(aes->rdFd, &(aes->msg), 0);
if (ret < 0) {
return ret;
return WC_AFALG_SOCK_E;
}
ret = (int)read(aes->rdFd, out, sz);
if (ret < 0) {
return ret;
return WC_AFALG_SOCK_E;
}
return 0;
@ -410,7 +418,7 @@ int wc_AesSetKeyDirect(Aes* aes, const byte* userKey, word32 keylen,
ret = (int)sendmsg(aes->rdFd, &(aes->msg), 0);
if (ret < 0) {
return ret;
return WC_AFALG_SOCK_E;
}
@ -428,7 +436,7 @@ int wc_AesSetKeyDirect(Aes* aes, const byte* userKey, word32 keylen,
ret = (int)readv(aes->rdFd, iov, 2);
if (ret < 0) {
return ret;
return WC_AFALG_SOCK_E;
}
if (aes->left > 0) {
@ -503,8 +511,14 @@ int wc_AesGcmSetKey(Aes* aes, const byte* key, word32 len)
aes->keylen = len;
aes->rounds = len/4 + 6;
if (aes->rdFd > 0) {
(void)close(aes->rdFd);
}
aes->rdFd = WC_SOCK_NOTSET;
aes->alFd = wc_Afalg_Socket();
if (aes->alFd <= 0) {
aes->alFd = wc_Afalg_Socket();
}
if (aes->alFd < 0) {
WOLFSSL_MSG("Unable to open an AF_ALG socket");
return WC_AFALG_SOCK_E;
@ -564,6 +578,11 @@ int wc_AesGcmEncrypt(Aes* aes, byte* out, const byte* in, word32 sz,
return BAD_FUNC_ARG;
}
if (aes->alFd <= 0) {
WOLFSSL_MSG("AF_ALG GcmEncrypt called with alFd unset");
return BAD_FUNC_ARG;
}
if (aes->rdFd == WC_SOCK_NOTSET) {
aes->dir = AES_ENCRYPTION;
if ((ret = wc_AesSetup(aes, WC_TYPE_AEAD, WC_NAME_AESGCM, ivSz,
@ -634,12 +653,12 @@ int wc_AesGcmEncrypt(Aes* aes, byte* out, const byte* in, word32 sz,
XFREE(tmp, aes->heap, DYNAMIC_TYPE_TMP_BUFFER);
#endif
if (ret < 0) {
return ret;
return WC_AFALG_SOCK_E;
}
ret = read(aes->rdFd, out, sz + AES_BLOCK_SIZE);
if (ret < 0) {
return ret;
return WC_AFALG_SOCK_E;
}
XMEMCPY(authTag, out + sz, authTagSz);
}
@ -679,7 +698,7 @@ int wc_AesGcmEncrypt(Aes* aes, byte* out, const byte* in, word32 sz,
ret = (int)sendmsg(aes->rdFd, msg, 0);
if (ret < 0) {
return ret;
return WC_AFALG_SOCK_E;
}
{
@ -702,7 +721,7 @@ int wc_AesGcmEncrypt(Aes* aes, byte* out, const byte* in, word32 sz,
XFREE(tmp, aes->heap, DYNAMIC_TYPE_TMP_BUFFER);
}
if (ret < 0) {
return ret;
return WC_AFALG_SOCK_E;
}
#endif
@ -845,7 +864,7 @@ int wc_AesGcmDecrypt(Aes* aes, byte* out, const byte* in, word32 sz,
XFREE(tmp, aes->heap, DYNAMIC_TYPE_TMP_BUFFER);
#endif
if (ret < 0) {
return ret;
return WC_AFALG_SOCK_E;
}
ret = read(aes->rdFd, out, sz + AES_BLOCK_SIZE);
@ -886,7 +905,7 @@ int wc_AesGcmDecrypt(Aes* aes, byte* out, const byte* in, word32 sz,
msg->msg_iovlen = 3; /* # of iov structures */
ret = (int)sendmsg(aes->rdFd, &(aes->msg), 0);
if (ret < 0) {
return ret;
return WC_AFALG_SOCK_E;
}
{

View File

@ -121,8 +121,10 @@ static int AfalgHashUpdate(wolfssl_AFALG_Hash* hash, const byte* in, word32 sz)
}
hash->len = hash->used + sz;
}
XMEMCPY(hash->msg + hash->used, in, sz);
hash->used += sz;
if (sz > 0) {
XMEMCPY(hash->msg + hash->used, in, sz);
hash->used += sz;
}
#else
int ret;
@ -139,32 +141,41 @@ static int AfalgHashFinal(wolfssl_AFALG_Hash* hash, byte* out, word32 outSz,
const char* type)
{
int ret;
void* heap;
if (hash == NULL || out == NULL) {
return BAD_FUNC_ARG;
}
heap = hash->heap; /* keep because AfalgHashInit clears the pointer */
#ifdef WOLFSSL_AFALG_HASH_KEEP
/* keep full message to out at end instead of incremental updates */
if ((ret = (int)send(hash->rdFd, hash->msg, hash->used, 0)) < 0) {
return ret;
ret = WC_AFALG_SOCK_E;
goto out;
}
XFREE(hash->msg, heap, DYNAMIC_TYPE_TMP_BUFFER);
XFREE(hash->msg, hash->heap, DYNAMIC_TYPE_TMP_BUFFER);
hash->msg = NULL;
#else
if ((ret = (int)send(hash->rdFd, NULL, 0, 0)) < 0) {
return ret;
ret = WC_AFALG_SOCK_E;
goto out;
}
#endif
if ((ret = (int)read(hash->rdFd, out, outSz)) != (int)outSz) {
return ret;
ret = WC_AFALG_SOCK_E;
goto out;
}
ret = 0;
out:
AfalgHashFree(hash);
return AfalgHashInit(hash, heap, 0, type);
if (ret != 0)
return ret;
else
return AfalgHashInit(hash, hash->heap, 0, type);
}
@ -212,7 +223,8 @@ static int AfalgHashCopy(wolfssl_AFALG_Hash* src, wolfssl_AFALG_Hash* dst)
if (dst->msg == NULL) {
return MEMORY_E;
}
XMEMCPY(dst->msg, src->msg, src->len);
if (src->len > 0)
XMEMCPY(dst->msg, src->msg, src->len);
#endif
dst->rdFd = accept(src->rdFd, NULL, 0);

View File

@ -20327,6 +20327,8 @@ WOLFSSL_TEST_SUBROUTINE int openssl_test(void)
(byte*)cbcPlain, 0) != 1)
return -8666;
if (wolfSSL_EVP_CIPHER_CTX_cleanup(en) != WOLFSSL_SUCCESS)
return -8724;
EVP_CIPHER_CTX_init(en);
if (EVP_CipherInit(en, EVP_aes_128_cbc(),
(unsigned char*)key, (unsigned char*)iv, 1) == 0)
@ -20391,6 +20393,8 @@ WOLFSSL_TEST_SUBROUTINE int openssl_test(void)
return -8684;
total = 0;
if (wolfSSL_EVP_CIPHER_CTX_cleanup(en) != WOLFSSL_SUCCESS)
return -8725;
EVP_CIPHER_CTX_init(en);
if (EVP_EncryptInit(en, EVP_aes_128_cbc(),
(unsigned char*)key, (unsigned char*)iv) == 0)
@ -20416,6 +20420,8 @@ WOLFSSL_TEST_SUBROUTINE int openssl_test(void)
return 3438;
total = 0;
if (wolfSSL_EVP_CIPHER_CTX_cleanup(de) != WOLFSSL_SUCCESS)
return -8726;
EVP_CIPHER_CTX_init(de);
if (EVP_DecryptInit(de, EVP_aes_128_cbc(),
(unsigned char*)key, (unsigned char*)iv) == 0)
@ -20462,11 +20468,15 @@ WOLFSSL_TEST_SUBROUTINE int openssl_test(void)
if (EVP_CIPHER_CTX_mode(en) != (en->flags & WOLFSSL_EVP_CIPH_MODE))
return -8704;
if (wolfSSL_EVP_CIPHER_CTX_cleanup(en) != WOLFSSL_SUCCESS)
return -8727;
EVP_CIPHER_CTX_init(en);
if (EVP_CipherInit_ex(en, EVP_aes_128_cbc(), NULL,
(unsigned char*)key, (unsigned char*)iv, 0) == 0)
return -8705;
if (wolfSSL_EVP_CIPHER_CTX_cleanup(en) != WOLFSSL_SUCCESS)
return -8728;
EVP_CIPHER_CTX_init(en);
if (EVP_EncryptInit_ex(en, EVP_aes_128_cbc(), NULL,
(unsigned char*)key, (unsigned char*)iv) == 0)
@ -20478,6 +20488,11 @@ WOLFSSL_TEST_SUBROUTINE int openssl_test(void)
if (wolfSSL_EVP_EncryptFinal(NULL, NULL, NULL) != WOLFSSL_FAILURE)
return -8708;
if (wolfSSL_EVP_CIPHER_CTX_cleanup(de) != WOLFSSL_SUCCESS)
return -8729;
if (wolfSSL_EVP_CIPHER_CTX_cleanup(de) != WOLFSSL_SUCCESS)
return -8730;
EVP_CIPHER_CTX_init(de);
if (EVP_DecryptInit_ex(de, EVP_aes_128_cbc(), NULL,
(unsigned char*)key, (unsigned char*)iv) == 0)
@ -20492,6 +20507,8 @@ WOLFSSL_TEST_SUBROUTINE int openssl_test(void)
if (EVP_CIPHER_CTX_block_size(NULL) != BAD_FUNC_ARG)
return -8712;
if (wolfSSL_EVP_CIPHER_CTX_cleanup(en) != WOLFSSL_SUCCESS)
return -8731;
EVP_CIPHER_CTX_init(en);
EVP_EncryptInit_ex(en, EVP_aes_128_cbc(), NULL,
(unsigned char*)key, (unsigned char*)iv);
@ -20522,6 +20539,11 @@ WOLFSSL_TEST_SUBROUTINE int openssl_test(void)
if (EVP_CIPHER_CTX_set_padding(en, 1) != WOLFSSL_SUCCESS)
return -8721;
if (wolfSSL_EVP_CIPHER_CTX_cleanup(en) != WOLFSSL_SUCCESS)
return -8732;
if (wolfSSL_EVP_CIPHER_CTX_cleanup(de) != WOLFSSL_SUCCESS)
return -8733;
#if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC)
wolfSSL_EVP_CIPHER_CTX_free(en);
wolfSSL_EVP_CIPHER_CTX_free(de);