Skip to content

Commit b14aba4

Browse files
douzzerdouzzer
committed
wolfcrypt/src/cmac.c: add wc_CmacFree(), revert wc_CmacFinal(), rename wc_CmacFinal() as wc_CmacFinalNoFree() removing its deallocation clauses, and add new wc_CmacFinal() that calls wc_CmacFinalNoFree() then calls wc_CmacFree() unconditionally, for compatibility with legacy client code (some of which may have previously leaked).
tests/api.c: modify test_wc_CmacFinal() to use wc_CmacFinalNoFree() except for the final call. wolfcrypt/src/aes.c: * fix wc_AesEaxEncryptAuth() and wc_AesEaxDecryptAuth() to call wc_AesEaxFree() only if wc_AesEaxInit() succeeded. * fix wc_AesEaxInit() to free all resources on failure. * revert wc_AesEaxEncryptFinal() and wc_AesEaxDecryptFinal() changes, then change wc_CmacFinal() calls in them to wc_CmacFinalNoFree() calls. * wc_AesEaxFree(): add wc_CmacFree() calls.
1 parent 689a82a commit b14aba4

5 files changed

Lines changed: 102 additions & 101 deletions

File tree

‎src/ssl_crypto.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2106,7 +2106,7 @@ void wolfSSL_CMAC_CTX_free(WOLFSSL_CMAC_CTX *ctx)
21062106
if (ctx != NULL) {
21072107
/* Deallocate dynamically allocated fields. */
21082108
if (ctx->internal != NULL) {
2109-
wc_CmacFinal((Cmac*)ctx->internal, NULL, NULL);
2109+
wc_CmacFree((Cmac*)ctx->internal);
21102110
XFREE(ctx->internal, NULL, DYNAMIC_TYPE_CMAC);
21112111
}
21122112
if (ctx->cctx != NULL) {

‎tests/api.c‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15795,12 +15795,14 @@ static int test_wc_CmacFinal(void)
1579515795
ExpectIntEQ(wc_InitCmac(&cmac, key, keySz, type, NULL), 0);
1579615796
ExpectIntEQ(wc_CmacUpdate(&cmac, msg, msgSz), 0);
1579715797

15798-
ExpectIntEQ(wc_CmacFinal(&cmac, mac, &macSz), 0);
15798+
ExpectIntEQ(wc_CmacFinalNoFree(&cmac, mac, &macSz), 0);
1579915799
ExpectIntEQ(XMEMCMP(mac, expMac, expMacSz), 0);
1580015800

1580115801
/* Pass in bad args. */
15802-
ExpectIntEQ(wc_CmacFinal(NULL, mac, &macSz), BAD_FUNC_ARG);
15803-
ExpectIntEQ(wc_CmacFinal(&cmac, NULL, &macSz), BAD_FUNC_ARG);
15802+
ExpectIntEQ(wc_CmacFinalNoFree(NULL, mac, &macSz), BAD_FUNC_ARG);
15803+
ExpectIntEQ(wc_CmacFinalNoFree(&cmac, NULL, &macSz), BAD_FUNC_ARG);
15804+
15805+
/* For the last call, use the API with implicit wc_CmacFree(). */
1580415806
ExpectIntEQ(wc_CmacFinal(&cmac, mac, &badMacSz), BUFFER_E);
1580515807
#endif
1580615808
return EXPECT_RESULT();

‎wolfcrypt/src/aes.c‎

Lines changed: 62 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -13266,6 +13266,7 @@ int wc_AesEaxEncryptAuth(const byte* key, word32 keySz, byte* out,
1326613266
AesEax *eax = &eax_mem;
1326713267
#endif
1326813268
int ret;
13269+
int eaxInited = 0;
1326913270

1327013271
if (key == NULL || out == NULL || in == NULL || nonce == NULL
1327113272
|| authTag == NULL || authIn == NULL) {
@@ -13286,6 +13287,7 @@ int wc_AesEaxEncryptAuth(const byte* key, word32 keySz, byte* out,
1328613287
authIn, authInSz)) != 0) {
1328713288
goto cleanup;
1328813289
}
13290+
eaxInited = 1;
1328913291

1329013292
if ((ret = wc_AesEaxEncryptUpdate(eax, out, in, inSz, NULL, 0)) != 0) {
1329113293
goto cleanup;
@@ -13296,7 +13298,8 @@ int wc_AesEaxEncryptAuth(const byte* key, word32 keySz, byte* out,
1329613298
}
1329713299

1329813300
cleanup:
13299-
wc_AesEaxFree(eax);
13301+
if (eaxInited)
13302+
wc_AesEaxFree(eax);
1330013303
#if defined(WOLFSSL_SMALL_STACK)
1330113304
XFREE(eax, NULL, DYNAMIC_TYPE_AES_EAX);
1330213305
#endif
@@ -13326,6 +13329,7 @@ int wc_AesEaxDecryptAuth(const byte* key, word32 keySz, byte* out,
1332613329
AesEax *eax = &eax_mem;
1332713330
#endif
1332813331
int ret;
13332+
int eaxInited = 0;
1332913333

1333013334
if (key == NULL || out == NULL || in == NULL || nonce == NULL
1333113335
|| authTag == NULL || authIn == NULL) {
@@ -13347,6 +13351,7 @@ int wc_AesEaxDecryptAuth(const byte* key, word32 keySz, byte* out,
1334713351

1334813352
goto cleanup;
1334913353
}
13354+
eaxInited = 1;
1335013355

1335113356
if ((ret = wc_AesEaxDecryptUpdate(eax, out, in, inSz, NULL, 0)) != 0) {
1335213357
goto cleanup;
@@ -13357,7 +13362,8 @@ int wc_AesEaxDecryptAuth(const byte* key, word32 keySz, byte* out,
1335713362
}
1335813363

1335913364
cleanup:
13360-
wc_AesEaxFree(eax);
13365+
if (eaxInited)
13366+
wc_AesEaxFree(eax);
1336113367
#if defined(WOLFSSL_SMALL_STACK)
1336213368
XFREE(eax, NULL, DYNAMIC_TYPE_AES_EAX);
1336313369
#endif
@@ -13380,6 +13386,9 @@ int wc_AesEaxInit(AesEax* eax,
1338013386
{
1338113387
int ret = 0;
1338213388
word32 cmacSize;
13389+
int aesInited = 0;
13390+
int nonceCmacInited = 0;
13391+
int aadCmacInited = 0;
1338313392

1338413393
if (eax == NULL || key == NULL || nonce == NULL) {
1338513394
return BAD_FUNC_ARG;
@@ -13388,14 +13397,16 @@ int wc_AesEaxInit(AesEax* eax,
1338813397
XMEMSET(eax->prefixBuf, 0, sizeof(eax->prefixBuf));
1338913398

1339013399
if ((ret = wc_AesInit(&eax->aes, NULL, INVALID_DEVID)) != 0) {
13391-
return ret;
13400+
goto out;
1339213401
}
13402+
aesInited = 1;
13403+
1339313404
if ((ret = wc_AesSetKey(&eax->aes,
1339413405
key,
1339513406
keySz,
1339613407
NULL,
1339713408
AES_ENCRYPTION)) != 0) {
13398-
return ret;
13409+
goto out;
1339913410
}
1340013411

1340113412
/*
@@ -13409,26 +13420,27 @@ int wc_AesEaxInit(AesEax* eax,
1340913420
NULL)) != 0) {
1341013421
return ret;
1341113422
}
13423+
nonceCmacInited = 1;
1341213424

1341313425
if ((ret = wc_CmacUpdate(&eax->nonceCmac,
1341413426
eax->prefixBuf,
1341513427
sizeof(eax->prefixBuf))) != 0) {
13416-
return ret;
13428+
goto out;
1341713429
}
1341813430

1341913431
if ((ret = wc_CmacUpdate(&eax->nonceCmac, nonce, nonceSz)) != 0) {
13420-
return ret;
13432+
goto out;
1342113433
}
1342213434

1342313435
cmacSize = AES_BLOCK_SIZE;
1342413436
if ((ret = wc_CmacFinal(&eax->nonceCmac,
1342513437
eax->nonceCmacFinal,
1342613438
&cmacSize)) != 0) {
13427-
return ret;
13439+
goto out;
1342813440
}
1342913441

1343013442
if ((ret = wc_AesSetIV(&eax->aes, eax->nonceCmacFinal)) != 0) {
13431-
return ret;
13443+
goto out;
1343213444
}
1343313445

1343413446
/*
@@ -13443,18 +13455,19 @@ int wc_AesEaxInit(AesEax* eax,
1344313455
keySz,
1344413456
WC_CMAC_AES,
1344513457
NULL)) != 0) {
13446-
return ret;
13458+
goto out;
1344713459
}
13460+
aadCmacInited = 1;
1344813461

1344913462
if ((ret = wc_CmacUpdate(&eax->aadCmac,
1345013463
eax->prefixBuf,
1345113464
sizeof(eax->prefixBuf))) != 0) {
13452-
return ret;
13465+
goto out;
1345313466
}
1345413467

1345513468
if (authIn != NULL) {
1345613469
if ((ret = wc_CmacUpdate(&eax->aadCmac, authIn, authInSz)) != 0) {
13457-
return ret;
13470+
goto out;
1345813471
}
1345913472
}
1346013473

@@ -13469,13 +13482,24 @@ int wc_AesEaxInit(AesEax* eax,
1346913482
keySz,
1347013483
WC_CMAC_AES,
1347113484
NULL)) != 0) {
13472-
return ret;
13485+
goto out;
1347313486
}
1347413487

1347513488
if ((ret = wc_CmacUpdate(&eax->ciphertextCmac,
1347613489
eax->prefixBuf,
1347713490
sizeof(eax->prefixBuf))) != 0) {
13478-
return ret;
13491+
goto out;
13492+
}
13493+
13494+
out:
13495+
13496+
if (ret != 0) {
13497+
if (aesInited)
13498+
wc_AesFree(&eax->aes);
13499+
if (nonceCmacInited)
13500+
wc_CmacFree(&eax->nonceCmac);
13501+
if (aadCmacInited)
13502+
wc_CmacFree(&eax->aadCmac);
1347913503
}
1348013504

1348113505
return ret;
@@ -13599,34 +13623,25 @@ int wc_AesEaxEncryptFinal(AesEax* eax, byte* authTag, word32 authTagSz)
1359913623
word32 cmacSize;
1360013624
int ret;
1360113625
word32 i;
13602-
int ciphertextCmac_finalized = 0;
13603-
int aadCmac_finalized = 0;
1360413626

13605-
if (eax == NULL) {
13627+
if (eax == NULL || authTag == NULL || authTagSz > AES_BLOCK_SIZE) {
1360613628
return BAD_FUNC_ARG;
1360713629
}
1360813630

13609-
if (authTag == NULL || authTagSz > AES_BLOCK_SIZE) {
13610-
ret = BAD_FUNC_ARG;
13611-
goto out;
13612-
}
13613-
1361413631
/* Complete the OMAC for the ciphertext */
1361513632
cmacSize = AES_BLOCK_SIZE;
13616-
ciphertextCmac_finalized = 1;
13617-
if ((ret = wc_CmacFinal(&eax->ciphertextCmac,
13618-
eax->ciphertextCmacFinal,
13619-
&cmacSize)) != 0) {
13620-
goto out;
13633+
if ((ret = wc_CmacFinalNoFree(&eax->ciphertextCmac,
13634+
eax->ciphertextCmacFinal,
13635+
&cmacSize)) != 0) {
13636+
return ret;
1362113637
}
1362213638

1362313639
/* Complete the OMAC for auth data */
1362413640
cmacSize = AES_BLOCK_SIZE;
13625-
aadCmac_finalized = 1;
13626-
if ((ret = wc_CmacFinal(&eax->aadCmac,
13627-
eax->aadCmacFinal,
13628-
&cmacSize)) != 0) {
13629-
goto out;
13641+
if ((ret = wc_CmacFinalNoFree(&eax->aadCmac,
13642+
eax->aadCmacFinal,
13643+
&cmacSize)) != 0) {
13644+
return ret;
1363013645
}
1363113646

1363213647
/*
@@ -13640,16 +13655,7 @@ int wc_AesEaxEncryptFinal(AesEax* eax, byte* authTag, word32 authTagSz)
1364013655
^ eax->ciphertextCmacFinal[i];
1364113656
}
1364213657

13643-
ret = 0;
13644-
13645-
out:
13646-
13647-
if (! ciphertextCmac_finalized)
13648-
(void)wc_CmacFinal(&eax->ciphertextCmac, NULL, NULL);
13649-
if (! aadCmac_finalized)
13650-
(void)wc_CmacFinal(&eax->aadCmac, NULL, NULL);
13651-
13652-
return ret;
13658+
return 0;
1365313659
}
1365413660

1365513661

@@ -13668,47 +13674,37 @@ int wc_AesEaxDecryptFinal(AesEax* eax,
1366813674
int ret;
1366913675
word32 i;
1367013676
word32 cmacSize;
13671-
int ciphertextCmac_finalized = 0;
13672-
int aadCmac_finalized = 0;
1367313677

1367413678
#if defined(WOLFSSL_SMALL_STACK)
13675-
byte *authTag = NULL;
13679+
byte *authTag;
1367613680
#else
1367713681
byte authTag[AES_BLOCK_SIZE];
1367813682
#endif
1367913683

13680-
if (eax == NULL) {
13684+
if (eax == NULL || authIn == NULL || authInSz > AES_BLOCK_SIZE) {
1368113685
return BAD_FUNC_ARG;
1368213686
}
1368313687

13684-
if (authIn == NULL || authInSz > AES_BLOCK_SIZE) {
13685-
ret = BAD_FUNC_ARG;
13686-
goto out;
13687-
}
13688-
1368913688
/* Complete the OMAC for the ciphertext */
1369013689
cmacSize = AES_BLOCK_SIZE;
13691-
ciphertextCmac_finalized = 1;
13692-
if ((ret = wc_CmacFinal(&eax->ciphertextCmac,
13693-
eax->ciphertextCmacFinal,
13694-
&cmacSize)) != 0) {
13695-
goto out;
13690+
if ((ret = wc_CmacFinalNoFree(&eax->ciphertextCmac,
13691+
eax->ciphertextCmacFinal,
13692+
&cmacSize)) != 0) {
13693+
return ret;
1369613694
}
1369713695

1369813696
/* Complete the OMAC for auth data */
1369913697
cmacSize = AES_BLOCK_SIZE;
13700-
aadCmac_finalized = 1;
13701-
if ((ret = wc_CmacFinal(&eax->aadCmac,
13702-
eax->aadCmacFinal,
13703-
&cmacSize)) != 0) {
13704-
goto out;
13698+
if ((ret = wc_CmacFinalNoFree(&eax->aadCmac,
13699+
eax->aadCmacFinal,
13700+
&cmacSize)) != 0) {
13701+
return ret;
1370513702
}
1370613703

1370713704
#if defined(WOLFSSL_SMALL_STACK)
1370813705
authTag = (byte*)XMALLOC(AES_BLOCK_SIZE, NULL, DYNAMIC_TYPE_TMP_BUFFER);
1370913706
if (authTag == NULL) {
13710-
ret = MEMORY_E;
13711-
goto out;
13707+
return MEMORY_E;
1371213708
}
1371313709
#endif
1371413710

@@ -13730,13 +13726,6 @@ int wc_AesEaxDecryptFinal(AesEax* eax,
1373013726
ret = 0;
1373113727
}
1373213728

13733-
out:
13734-
13735-
if (! ciphertextCmac_finalized)
13736-
(void)wc_CmacFinal(&eax->ciphertextCmac, NULL, NULL);
13737-
if (! aadCmac_finalized)
13738-
(void)wc_CmacFinal(&eax->aadCmac, NULL, NULL);
13739-
1374013729
#if defined(WOLFSSL_SMALL_STACK)
1374113730
XFREE(authTag, NULL, DYNAMIC_TYPE_TMP_BUFFER);
1374213731
#endif
@@ -13745,8 +13734,8 @@ int wc_AesEaxDecryptFinal(AesEax* eax,
1374513734
}
1374613735

1374713736
/*
13748-
* Frees the underlying AES context. Must be called when done using the AES EAX
13749-
* context structure
13737+
* Frees the underlying CMAC and AES contexts. Must be called when done using
13738+
* the AES EAX context structure.
1375013739
*
1375113740
* Returns 0 on success
1375213741
* Returns error code on failure
@@ -13757,6 +13746,8 @@ int wc_AesEaxFree(AesEax* eax)
1375713746
return BAD_FUNC_ARG;
1375813747
}
1375913748

13749+
(void)wc_CmacFree(&eax->ciphertextCmac);
13750+
(void)wc_CmacFree(&eax->aadCmac);
1376013751
wc_AesFree(&eax->aes);
1376113752

1376213753
return 0;

0 commit comments

Comments
 (0)