Skip to content

Commit 3e4549e

Browse files
dgarskedgarske
authored
Merge pull request #296 from sameehj/fix
Fix integer underflow in AES-GCM key/data unwrap size calculations
2 parents 20e70a4 + 07d8db8 commit 3e4549e

4 files changed

Lines changed: 137 additions & 9 deletions

File tree

‎src/wh_server_keystore.c‎

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -997,15 +997,22 @@ static int _AesGcmKeyUnwrap(whServerContext* server, uint16_t serverKeyId,
997997
uint8_t* serverKey;
998998
uint32_t serverKeySz;
999999
whNvmMetadata* serverKeyMetadata;
1000-
uint8_t* encBlob = (uint8_t*)wrappedKeyIn + sizeof(iv) + sizeof(authTag);
1001-
uint16_t encBlobSz = wrappedKeySz - sizeof(iv) - sizeof(authTag);
1000+
uint8_t* encBlob;
1001+
uint16_t encBlobSz;
10021002
uint8_t plainBlob[sizeof(*metadataOut) + WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE];
10031003

10041004
if (server == NULL || wrappedKeyIn == NULL || metadataOut == NULL ||
10051005
keyOut == NULL || keySz > WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE) {
10061006
return WH_ERROR_BADARGS;
10071007
}
10081008

1009+
if (wrappedKeySz < sizeof(iv) + sizeof(authTag)) {
1010+
return WH_ERROR_BADARGS;
1011+
}
1012+
1013+
encBlob = (uint8_t*)wrappedKeyIn + sizeof(iv) + sizeof(authTag);
1014+
encBlobSz = wrappedKeySz - sizeof(iv) - sizeof(authTag);
1015+
10091016

10101017
/* Get the server side key */
10111018
ret = wh_Server_KeystoreFreshenKey(server, serverKeyId,
@@ -1133,14 +1140,21 @@ static int _AesGcmDataUnwrap(whServerContext* server, uint16_t serverKeyId,
11331140
uint8_t* serverKey;
11341141
uint32_t serverKeySz;
11351142
whNvmMetadata* serverKeyMetadata;
1136-
uint8_t* encBlob = (uint8_t*)wrappedDataIn + sizeof(iv) + sizeof(authTag);
1137-
uint16_t encBlobSz = wrappedDataSz - sizeof(iv) - sizeof(authTag);
1143+
uint8_t* encBlob;
1144+
uint16_t encBlobSz;
11381145

11391146
if (server == NULL || wrappedDataIn == NULL || dataOut == NULL ||
11401147
dataSz > WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE) {
11411148
return WH_ERROR_BADARGS;
11421149
}
11431150

1151+
if (wrappedDataSz < sizeof(iv) + sizeof(authTag)) {
1152+
return WH_ERROR_BADARGS;
1153+
}
1154+
1155+
encBlob = (uint8_t*)wrappedDataIn + sizeof(iv) + sizeof(authTag);
1156+
encBlobSz = wrappedDataSz - sizeof(iv) - sizeof(authTag);
1157+
11441158
/* Get the server side key */
11451159
ret = wh_Server_KeystoreFreshenKey(server, serverKeyId,
11461160
&serverKey, &serverKeyMetadata);
@@ -1306,8 +1320,15 @@ static int _HandleKeyUnwrapAndExportRequest(
13061320
#ifndef NO_AES
13071321
#ifdef HAVE_AESGCM
13081322
case WC_CIPHER_AES_GCM: {
1309-
uint16_t keySz = req->wrappedKeySz -
1310-
WH_KEYWRAP_AES_GCM_HEADER_SIZE - sizeof(*metadata);
1323+
uint16_t keySz;
1324+
1325+
if (req->wrappedKeySz < WH_KEYWRAP_AES_GCM_HEADER_SIZE +
1326+
sizeof(*metadata)) {
1327+
return WH_ERROR_BADARGS;
1328+
}
1329+
1330+
keySz = req->wrappedKeySz -
1331+
WH_KEYWRAP_AES_GCM_HEADER_SIZE - sizeof(*metadata);
13111332

13121333
/* Check if the response data can fit the metadata + key */
13131334
if (respDataSz < sizeof(*metadata) + keySz) {
@@ -1415,6 +1436,11 @@ static int _HandleKeyUnwrapAndCacheRequest(
14151436
#ifndef NO_AES
14161437
#ifdef HAVE_AESGCM
14171438
case WC_CIPHER_AES_GCM: {
1439+
if (req->wrappedKeySz < WH_KEYWRAP_AES_GCM_HEADER_SIZE +
1440+
sizeof(metadata)) {
1441+
return WH_ERROR_BADARGS;
1442+
}
1443+
14181444
keySz = req->wrappedKeySz - WH_KEYWRAP_AES_GCM_HEADER_SIZE -
14191445
sizeof(metadata);
14201446
resp->cipherType = WC_CIPHER_AES_GCM;
@@ -1595,8 +1621,13 @@ static int _HandleDataUnwrapRequest(whServerContext* server,
15951621
#ifndef NO_AES
15961622
#ifdef HAVE_AESGCM
15971623
case WC_CIPHER_AES_GCM: {
1598-
uint16_t dataSz =
1599-
req->wrappedDataSz - WH_KEYWRAP_AES_GCM_HEADER_SIZE;
1624+
uint16_t dataSz;
1625+
1626+
if (req->wrappedDataSz < WH_KEYWRAP_AES_GCM_HEADER_SIZE) {
1627+
return WH_ERROR_BADARGS;
1628+
}
1629+
1630+
dataSz = req->wrappedDataSz - WH_KEYWRAP_AES_GCM_HEADER_SIZE;
16001631

16011632
/* Check if the response data can fit the unwrapped data */
16021633
if (respDataSz < dataSz) {

‎test/wh_test_crypto.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5791,9 +5791,11 @@ int whTest_CryptoClientConfig(whClientConfig* config)
57915791

57925792
#ifdef WOLFHSM_CFG_KEYWRAP
57935793
if (ret == 0) {
5794-
/* Test keywrap functionality */
57955794
ret = whTest_Client_KeyWrap(client);
57965795
}
5796+
if (ret == 0) {
5797+
ret = whTest_Client_DataWrap(client);
5798+
}
57975799
#endif
57985800

57995801
#ifndef NO_AES

‎test/wh_test_keywrap.c‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,84 @@ static int _AesGcm_TestDataWrap(whClientContext* client)
257257
return ret;
258258
}
259259

260+
static int _AesGcm_TestKeyUnwrapUnderflow(whClientContext* client)
261+
{
262+
int ret;
263+
uint8_t dummyBuf[1] = {0};
264+
whNvmMetadata tmpMetadata = {0};
265+
uint8_t tmpKey[WH_TEST_AES_KEYSIZE] = {0};
266+
uint16_t tmpKeySz = sizeof(tmpKey);
267+
whKeyId wrappedKeyId = WH_KEYID_ERASED;
268+
269+
/* wrappedKeySz=0: must return WH_ERROR_BADARGS, not underflow */
270+
ret = wh_Client_KeyUnwrapAndExport(client, WC_CIPHER_AES_GCM, WH_TEST_KEKID,
271+
dummyBuf, 0, &tmpMetadata, tmpKey,
272+
&tmpKeySz);
273+
if (ret != WH_ERROR_BADARGS) {
274+
WH_ERROR_PRINT("KeyUnwrapAndExport(sz=0) expected BADARGS, got %d\n",
275+
ret);
276+
return WH_TEST_FAIL;
277+
}
278+
279+
/* wrappedKeySz=1: must return WH_ERROR_BADARGS, not underflow */
280+
tmpKeySz = sizeof(tmpKey);
281+
ret = wh_Client_KeyUnwrapAndExport(client, WC_CIPHER_AES_GCM, WH_TEST_KEKID,
282+
dummyBuf, 1, &tmpMetadata, tmpKey,
283+
&tmpKeySz);
284+
if (ret != WH_ERROR_BADARGS) {
285+
WH_ERROR_PRINT("KeyUnwrapAndExport(sz=1) expected BADARGS, got %d\n",
286+
ret);
287+
return WH_TEST_FAIL;
288+
}
289+
290+
/* wrappedKeySz=0: test KeyUnwrapAndCache path */
291+
ret = wh_Client_KeyUnwrapAndCache(client, WC_CIPHER_AES_GCM, WH_TEST_KEKID,
292+
dummyBuf, 0, &wrappedKeyId);
293+
if (ret != WH_ERROR_BADARGS) {
294+
WH_ERROR_PRINT("KeyUnwrapAndCache(sz=0) expected BADARGS, got %d\n",
295+
ret);
296+
return WH_TEST_FAIL;
297+
}
298+
299+
/* wrappedKeySz=1: test KeyUnwrapAndCache path */
300+
ret = wh_Client_KeyUnwrapAndCache(client, WC_CIPHER_AES_GCM, WH_TEST_KEKID,
301+
dummyBuf, 1, &wrappedKeyId);
302+
if (ret != WH_ERROR_BADARGS) {
303+
WH_ERROR_PRINT("KeyUnwrapAndCache(sz=1) expected BADARGS, got %d\n",
304+
ret);
305+
return WH_TEST_FAIL;
306+
}
307+
308+
return WH_ERROR_OK;
309+
}
310+
311+
static int _AesGcm_TestDataUnwrapUnderflow(whClientContext* client)
312+
{
313+
int ret;
314+
uint8_t dummyBuf[1] = {0};
315+
uint8_t outBuf[32] = {0};
316+
uint32_t outSz = sizeof(outBuf);
317+
318+
/* wrappedDataSz=0: must return WH_ERROR_BADARGS, not underflow */
319+
ret = wh_Client_DataUnwrap(client, WC_CIPHER_AES_GCM, WH_TEST_KEKID,
320+
dummyBuf, 0, outBuf, &outSz);
321+
if (ret != WH_ERROR_BADARGS) {
322+
WH_ERROR_PRINT("DataUnwrap(sz=0) expected BADARGS, got %d\n", ret);
323+
return WH_TEST_FAIL;
324+
}
325+
326+
/* wrappedDataSz=1: must return WH_ERROR_BADARGS, not underflow */
327+
outSz = sizeof(outBuf);
328+
ret = wh_Client_DataUnwrap(client, WC_CIPHER_AES_GCM, WH_TEST_KEKID,
329+
dummyBuf, 1, outBuf, &outSz);
330+
if (ret != WH_ERROR_BADARGS) {
331+
WH_ERROR_PRINT("DataUnwrap(sz=1) expected BADARGS, got %d\n", ret);
332+
return WH_TEST_FAIL;
333+
}
334+
335+
return WH_ERROR_OK;
336+
}
337+
260338
#endif /* HAVE_AESGCM */
261339

262340
int whTest_Client_KeyWrap(whClientContext* client)
@@ -281,6 +359,14 @@ int whTest_Client_KeyWrap(whClientContext* client)
281359
if (ret != WH_ERROR_OK) {
282360
WH_ERROR_PRINT("Failed to _AesGcm_TestKeyWrap %d\n", ret);
283361
}
362+
363+
if (ret == WH_ERROR_OK) {
364+
ret = _AesGcm_TestKeyUnwrapUnderflow(client);
365+
if (ret != WH_ERROR_OK) {
366+
WH_ERROR_PRINT("Failed to _AesGcm_TestKeyUnwrapUnderflow %d\n",
367+
ret);
368+
}
369+
}
284370
#endif
285371

286372
_CleanupServerKek(client);
@@ -304,6 +390,14 @@ int whTest_Client_DataWrap(whClientContext* client)
304390
if (ret != WH_ERROR_OK) {
305391
WH_ERROR_PRINT("Failed to _AesGcm_TestDataWrap %d\n", ret);
306392
}
393+
394+
if (ret == WH_ERROR_OK) {
395+
ret = _AesGcm_TestDataUnwrapUnderflow(client);
396+
if (ret != WH_ERROR_OK) {
397+
WH_ERROR_PRINT("Failed to _AesGcm_TestDataUnwrapUnderflow %d\n",
398+
ret);
399+
}
400+
}
307401
#endif
308402

309403
_CleanupServerKek(client);

‎test/wh_test_keywrap.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "wolfhsm/wh_client.h"
2424

2525
int whTest_Client_KeyWrap(whClientContext* ctx);
26+
int whTest_Client_DataWrap(whClientContext* ctx);
2627
int whTest_KeyWrapClientConfig(whClientConfig* cf);
2728

2829
#endif /* WH_TEST_COMM_H_ */

0 commit comments

Comments
 (0)