Skip to content

Fix integer underflow in AES-GCM key/data unwrap size calculations #296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sameehj wants to merge 1 commit into
base: main
Choose a base branch
from
+137 −9
Open

Fix integer underflow in AES-GCM key/data unwrap size calculations #296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 39 additions & 8 deletions src/wh_server_keystore.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -997,15 +997,22 @@ static int _AesGcmKeyUnwrap(whServerContext* server, uint16_t serverKeyId,
uint8_t* serverKey;
uint32_t serverKeySz;
whNvmMetadata* serverKeyMetadata;
uint8_t* encBlob = (uint8_t*)wrappedKeyIn + sizeof(iv) + sizeof(authTag);
uint16_t encBlobSz = wrappedKeySz - sizeof(iv) - sizeof(authTag);
uint8_t* encBlob;
uint16_t encBlobSz;
uint8_t plainBlob[sizeof(*metadataOut) + WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE];

if (server == NULL || wrappedKeyIn == NULL || metadataOut == NULL ||
keyOut == NULL || keySz > WOLFHSM_CFG_KEYWRAP_MAX_KEY_SIZE) {
return WH_ERROR_BADARGS;
}

if (wrappedKeySz < sizeof(iv) + sizeof(authTag)) {
return WH_ERROR_BADARGS;
}

encBlob = (uint8_t*)wrappedKeyIn + sizeof(iv) + sizeof(authTag);
encBlobSz = wrappedKeySz - sizeof(iv) - sizeof(authTag);


/* Get the server side key */
ret = wh_Server_KeystoreFreshenKey(server, serverKeyId,
Expand Down Expand Up @@ -1133,14 +1140,21 @@ static int _AesGcmDataUnwrap(whServerContext* server, uint16_t serverKeyId,
uint8_t* serverKey;
uint32_t serverKeySz;
whNvmMetadata* serverKeyMetadata;
uint8_t* encBlob = (uint8_t*)wrappedDataIn + sizeof(iv) + sizeof(authTag);
uint16_t encBlobSz = wrappedDataSz - sizeof(iv) - sizeof(authTag);
uint8_t* encBlob;
uint16_t encBlobSz;

if (server == NULL || wrappedDataIn == NULL || dataOut == NULL ||
dataSz > WOLFHSM_CFG_KEYWRAP_MAX_DATA_SIZE) {
return WH_ERROR_BADARGS;
}

if (wrappedDataSz < sizeof(iv) + sizeof(authTag)) {
return WH_ERROR_BADARGS;
}

encBlob = (uint8_t*)wrappedDataIn + sizeof(iv) + sizeof(authTag);
encBlobSz = wrappedDataSz - sizeof(iv) - sizeof(authTag);

/* Get the server side key */
ret = wh_Server_KeystoreFreshenKey(server, serverKeyId,
&serverKey, &serverKeyMetadata);
Expand Down Expand Up @@ -1306,8 +1320,15 @@ static int _HandleKeyUnwrapAndExportRequest(
#ifndef NO_AES
#ifdef HAVE_AESGCM
case WC_CIPHER_AES_GCM: {
uint16_t keySz = req->wrappedKeySz -
WH_KEYWRAP_AES_GCM_HEADER_SIZE - sizeof(*metadata);
uint16_t keySz;

if (req->wrappedKeySz < WH_KEYWRAP_AES_GCM_HEADER_SIZE +
sizeof(*metadata)) {
return WH_ERROR_BADARGS;
}

keySz = req->wrappedKeySz -
WH_KEYWRAP_AES_GCM_HEADER_SIZE - sizeof(*metadata);

/* Check if the response data can fit the metadata + key */
if (respDataSz < sizeof(*metadata) + keySz) {
Expand Down Expand Up @@ -1415,6 +1436,11 @@ static int _HandleKeyUnwrapAndCacheRequest(
#ifndef NO_AES
#ifdef HAVE_AESGCM
case WC_CIPHER_AES_GCM: {
if (req->wrappedKeySz < WH_KEYWRAP_AES_GCM_HEADER_SIZE +
sizeof(metadata)) {
return WH_ERROR_BADARGS;
}

keySz = req->wrappedKeySz - WH_KEYWRAP_AES_GCM_HEADER_SIZE -
sizeof(metadata);
resp->cipherType = WC_CIPHER_AES_GCM;
Expand Down Expand Up @@ -1595,8 +1621,13 @@ static int _HandleDataUnwrapRequest(whServerContext* server,
#ifndef NO_AES
#ifdef HAVE_AESGCM
case WC_CIPHER_AES_GCM: {
uint16_t dataSz =
req->wrappedDataSz - WH_KEYWRAP_AES_GCM_HEADER_SIZE;
uint16_t dataSz;

if (req->wrappedDataSz < WH_KEYWRAP_AES_GCM_HEADER_SIZE) {
return WH_ERROR_BADARGS;
}

dataSz = req->wrappedDataSz - WH_KEYWRAP_AES_GCM_HEADER_SIZE;

/* Check if the response data can fit the unwrapped data */
if (respDataSz < dataSz) {
Expand Down
4 changes: 3 additions & 1 deletion test/wh_test_crypto.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5614,9 +5614,11 @@ int whTest_CryptoClientConfig(whClientConfig* config)

#ifdef WOLFHSM_CFG_KEYWRAP
if (ret == 0) {
/* Test keywrap functionality */
ret = whTest_Client_KeyWrap(client);
}
if (ret == 0) {
ret = whTest_Client_DataWrap(client);
}
#endif

#ifndef NO_AES
Expand Down
94 changes: 94 additions & 0 deletions test/wh_test_keywrap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,84 @@ static int _AesGcm_TestDataWrap(whClientContext* client)
return ret;
}

static int _AesGcm_TestKeyUnwrapUnderflow(whClientContext* client)
{
int ret;
uint8_t dummyBuf[1] = {0};
whNvmMetadata tmpMetadata = {0};
uint8_t tmpKey[WH_TEST_AES_KEYSIZE] = {0};
uint16_t tmpKeySz = sizeof(tmpKey);
whKeyId wrappedKeyId = WH_KEYID_ERASED;

/* wrappedKeySz=0: must return WH_ERROR_BADARGS, not underflow */
ret = wh_Client_KeyUnwrapAndExport(client, WC_CIPHER_AES_GCM, WH_TEST_KEKID,
dummyBuf, 0, &tmpMetadata, tmpKey,
&tmpKeySz);
if (ret != WH_ERROR_BADARGS) {
WH_ERROR_PRINT("KeyUnwrapAndExport(sz=0) expected BADARGS, got %d\n",
ret);
return WH_TEST_FAIL;
}

/* wrappedKeySz=1: must return WH_ERROR_BADARGS, not underflow */
tmpKeySz = sizeof(tmpKey);
ret = wh_Client_KeyUnwrapAndExport(client, WC_CIPHER_AES_GCM, WH_TEST_KEKID,
dummyBuf, 1, &tmpMetadata, tmpKey,
&tmpKeySz);
if (ret != WH_ERROR_BADARGS) {
WH_ERROR_PRINT("KeyUnwrapAndExport(sz=1) expected BADARGS, got %d\n",
ret);
return WH_TEST_FAIL;
}

/* wrappedKeySz=0: test KeyUnwrapAndCache path */
ret = wh_Client_KeyUnwrapAndCache(client, WC_CIPHER_AES_GCM, WH_TEST_KEKID,
dummyBuf, 0, &wrappedKeyId);
if (ret != WH_ERROR_BADARGS) {
WH_ERROR_PRINT("KeyUnwrapAndCache(sz=0) expected BADARGS, got %d\n",
ret);
return WH_TEST_FAIL;
}

/* wrappedKeySz=1: test KeyUnwrapAndCache path */
ret = wh_Client_KeyUnwrapAndCache(client, WC_CIPHER_AES_GCM, WH_TEST_KEKID,
dummyBuf, 1, &wrappedKeyId);
if (ret != WH_ERROR_BADARGS) {
WH_ERROR_PRINT("KeyUnwrapAndCache(sz=1) expected BADARGS, got %d\n",
ret);
return WH_TEST_FAIL;
}

return WH_ERROR_OK;
}

static int _AesGcm_TestDataUnwrapUnderflow(whClientContext* client)
{
int ret;
uint8_t dummyBuf[1] = {0};
uint8_t outBuf[32] = {0};
uint32_t outSz = sizeof(outBuf);

/* wrappedDataSz=0: must return WH_ERROR_BADARGS, not underflow */
ret = wh_Client_DataUnwrap(client, WC_CIPHER_AES_GCM, WH_TEST_KEKID,
dummyBuf, 0, outBuf, &outSz);
if (ret != WH_ERROR_BADARGS) {
WH_ERROR_PRINT("DataUnwrap(sz=0) expected BADARGS, got %d\n", ret);
return WH_TEST_FAIL;
}

/* wrappedDataSz=1: must return WH_ERROR_BADARGS, not underflow */
outSz = sizeof(outBuf);
ret = wh_Client_DataUnwrap(client, WC_CIPHER_AES_GCM, WH_TEST_KEKID,
dummyBuf, 1, outBuf, &outSz);
if (ret != WH_ERROR_BADARGS) {
WH_ERROR_PRINT("DataUnwrap(sz=1) expected BADARGS, got %d\n", ret);
return WH_TEST_FAIL;
}

return WH_ERROR_OK;
}

#endif /* HAVE_AESGCM */

int whTest_Client_KeyWrap(whClientContext* client)
Expand All @@ -281,6 +359,14 @@ int whTest_Client_KeyWrap(whClientContext* client)
if (ret != WH_ERROR_OK) {
WH_ERROR_PRINT("Failed to _AesGcm_TestKeyWrap %d\n", ret);
}

if (ret == WH_ERROR_OK) {
ret = _AesGcm_TestKeyUnwrapUnderflow(client);
if (ret != WH_ERROR_OK) {
WH_ERROR_PRINT("Failed to _AesGcm_TestKeyUnwrapUnderflow %d\n",
ret);
}
}
#endif

_CleanupServerKek(client);
Expand All @@ -304,6 +390,14 @@ int whTest_Client_DataWrap(whClientContext* client)
if (ret != WH_ERROR_OK) {
WH_ERROR_PRINT("Failed to _AesGcm_TestDataWrap %d\n", ret);
}

if (ret == WH_ERROR_OK) {
ret = _AesGcm_TestDataUnwrapUnderflow(client);
if (ret != WH_ERROR_OK) {
WH_ERROR_PRINT("Failed to _AesGcm_TestDataUnwrapUnderflow %d\n",
ret);
}
}
#endif

_CleanupServerKek(client);
Expand Down
1 change: 1 addition & 0 deletions test/wh_test_keywrap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "wolfhsm/wh_client.h"

int whTest_Client_KeyWrap(whClientContext* ctx);
int whTest_Client_DataWrap(whClientContext* ctx);
int whTest_KeyWrapClientConfig(whClientConfig* cf);

#endif /* WH_TEST_COMM_H_ */
Loading