diff --git a/src/wh_server_crypto.c b/src/wh_server_crypto.c index 788f91f53..aa7d7109f 100644 --- a/src/wh_server_crypto.c +++ b/src/wh_server_crypto.c @@ -2534,22 +2534,33 @@ static int _HandleAesCtr(whServerContext* ctx, uint16_t magic, int devId, ret = wc_AesSetKeyDirect(aes, (byte*)key, (word32)key_len, (byte*)iv, enc != 0 ? AES_ENCRYPTION : AES_DECRYPTION); if (ret == WH_ERROR_OK) { - /* do the crypto operation; also restore previous left */ - aes->left = left; - memcpy(aes->tmp, tmp, sizeof(aes->tmp)); - if (enc != 0) { - ret = wc_AesCtrEncrypt(aes, (byte*)out, (byte*)in, (word32)len); - if (ret == WH_ERROR_OK) { - WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Encrypted output", - out, len); - } + /* Reject client-supplied left values outside the valid range. + * wc_AesCtrEncrypt indexes aes->tmp via AES_BLOCK_SIZE - aes->left; + * an out-of-range value causes an out-of-bounds read that could + * disclose server-side memory across the HSM trust boundary. */ + if (left > AES_BLOCK_SIZE) { + ret = WH_ERROR_BADARGS; } else { - /* CTR uses the same function for encrypt and decrypt */ - ret = wc_AesCtrEncrypt(aes, (byte*)out, (byte*)in, (word32)len); - if (ret == WH_ERROR_OK) { - WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Decrypted output", - out, len); + /* Restore streaming CTR context from the previous call. */ + aes->left = left; + memcpy(aes->tmp, tmp, sizeof(aes->tmp)); + if (enc != 0) { + ret = wc_AesCtrEncrypt(aes, (byte*)out, (byte*)in, + (word32)len); + if (ret == WH_ERROR_OK) { + WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Encrypted output", + out, len); + } + } + else { + /* CTR uses the same function for encrypt and decrypt */ + ret = wc_AesCtrEncrypt(aes, (byte*)out, (byte*)in, + (word32)len); + if (ret == WH_ERROR_OK) { + WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Decrypted output", + out, len); + } } } } @@ -2695,33 +2706,41 @@ static int _HandleAesCtrDma(whServerContext* ctx, uint16_t magic, int devId, ret = wc_AesSetKeyDirect(aes, (byte*)key, (word32)keyLen, (byte*)iv, enc != 0 ? AES_ENCRYPTION : AES_DECRYPTION); if (ret == WH_ERROR_OK) { - /* do the crypto operation */ - /* restore previous left */ - aes->left = left; - memcpy(aes->tmp, tmp, sizeof(aes->tmp)); - if (enc != 0) { - ret = wc_AesCtrEncrypt(aes, (byte*)outAddr, (byte*)inAddr, - (word32)len); - if (ret == WH_ERROR_OK) { - WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Encrypted output", - outAddr, len); - } + /* Reject client-supplied left values outside the valid range. + * wc_AesCtrEncrypt indexes aes->tmp via AES_BLOCK_SIZE - aes->left; + * an out-of-range value causes an out-of-bounds read that could + * disclose server-side memory across the HSM trust boundary. */ + if (left > AES_BLOCK_SIZE) { + ret = WH_ERROR_BADARGS; } else { - /* CTR uses the same function for encrypt and decrypt */ - ret = wc_AesCtrEncrypt(aes, (byte*)outAddr, (byte*)inAddr, - (word32)len); + /* Restore streaming CTR context from the previous call. */ + aes->left = left; + memcpy(aes->tmp, tmp, sizeof(aes->tmp)); + if (enc != 0) { + ret = wc_AesCtrEncrypt(aes, (byte*)outAddr, (byte*)inAddr, + (word32)len); + if (ret == WH_ERROR_OK) { + WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Encrypted output", + outAddr, len); + } + } + else { + /* CTR uses the same function for encrypt and decrypt */ + ret = wc_AesCtrEncrypt(aes, (byte*)outAddr, (byte*)inAddr, + (word32)len); + if (ret == WH_ERROR_OK) { + WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Decrypted output", + outAddr, len); + } + } if (ret == WH_ERROR_OK) { - WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Decrypted output", - outAddr, len); + left = aes->left; + outSz = len; + memcpy(out_tmp, aes->tmp, AES_BLOCK_SIZE); + memcpy(out_iv, aes->reg, AES_IV_SIZE); } } - if (ret == WH_ERROR_OK) { - left = aes->left; - outSz = len; - memcpy(out_tmp, aes->tmp, AES_BLOCK_SIZE); - memcpy(out_iv, aes->reg, AES_IV_SIZE); - } } } diff --git a/test-refactor/client-server/wh_test_crypto_aes.c b/test-refactor/client-server/wh_test_crypto_aes.c index 12a1e4d06..78c7edece 100644 --- a/test-refactor/client-server/wh_test_crypto_aes.c +++ b/test-refactor/client-server/wh_test_crypto_aes.c @@ -442,6 +442,91 @@ static int whTest_CryptoAesCbcStreaming(whClientContext* ctx) } #endif /* HAVE_AES_CBC */ +#ifdef WOLFSSL_AES_COUNTER +/* Verifies that wh_Client_AesCtr (and wh_Client_AesCtrDma when DMA is + * enabled) reject aes->left > AES_BLOCK_SIZE with WH_ERROR_BADARGS. + * wc_AesCtrEncrypt indexes aes->tmp via AES_BLOCK_SIZE - aes->left; an + * oversized value would cause an out-of-bounds read disclosing server-side + * memory across the HSM trust boundary. */ +static int whTest_CryptoAesCtrLeftOob(whClientContext* ctx) +{ + int devId = WH_DEV_ID; + int ret = 0; + int tmp; + Aes aes[1]; + uint8_t cipher[AES_BLOCK_SIZE] = {0}; + const uint8_t key[] = {0x2b, 0x7e, 0x15, 0x16, + 0x28, 0xae, 0xd2, 0xa6, + 0xab, 0xf7, 0x15, 0x88, + 0x09, 0xcf, 0x4f, 0x3c}; + const uint8_t iv[AES_BLOCK_SIZE] = {0x00, 0x01, 0x02, 0x03, + 0x04, 0x05, 0x06, 0x07, + 0x08, 0x09, 0x0a, 0x0b, + 0x0c, 0x0d, 0x0e, 0x0f}; + const uint8_t plainIn[AES_BLOCK_SIZE] = {0x6b, 0xc1, 0xbe, 0xe2, + 0x2e, 0x40, 0x9f, 0x96, + 0xe9, 0x3d, 0x7e, 0x11, + 0x73, 0x93, 0x17, 0x2a}; + + ret = wc_AesInit(aes, NULL, devId); + if (ret != 0) { + WH_ERROR_PRINT("Failed to wc_AesInit %d\n", ret); + } + else { + ret = wc_AesSetKeyDirect(aes, key, sizeof(key), iv, AES_ENCRYPTION); + if (ret != 0) { + WH_ERROR_PRINT("Failed to wc_AesSetKeyDirect %d\n", ret); + } + else { + aes->left = AES_BLOCK_SIZE + 1; + tmp = wh_Client_AesCtr(ctx, aes, 1, plainIn, sizeof(plainIn), + cipher); + if (tmp != WH_ERROR_BADARGS) { + WH_ERROR_PRINT( + "AES-CTR: left > AES_BLOCK_SIZE should be BADARGS, " + "got %d\n", + tmp); + ret = -1; + } + } + (void)wc_AesFree(aes); + } + +#ifdef WOLFHSM_CFG_DMA + if (ret == 0) { + ret = wc_AesInit(aes, NULL, devId); + if (ret != 0) { + WH_ERROR_PRINT("Failed to wc_AesInit %d\n", ret); + } + else { + ret = wc_AesSetKeyDirect(aes, key, sizeof(key), iv, AES_ENCRYPTION); + if (ret != 0) { + WH_ERROR_PRINT("Failed to wc_AesSetKeyDirect %d\n", ret); + } + else { + aes->left = AES_BLOCK_SIZE + 1; + tmp = wh_Client_AesCtrDma(ctx, aes, 1, plainIn, + sizeof(plainIn), cipher); + if (tmp != WH_ERROR_BADARGS) { + WH_ERROR_PRINT( + "AES-CTR DMA: left > AES_BLOCK_SIZE should be " + "BADARGS, got %d\n", + tmp); + ret = -1; + } + } + (void)wc_AesFree(aes); + } + } +#endif /* WOLFHSM_CFG_DMA */ + + if (ret == 0) { + WH_TEST_PRINT("AES CTR left OOB DEVID=0x%X SUCCESS\n", devId); + } + return ret; +} +#endif /* WOLFSSL_AES_COUNTER */ + int whTest_CryptoAesKeyUsagePolicies(whClientContext* ctx) { int ret = 0; @@ -1025,6 +1110,9 @@ int whTest_Crypto_Aes(whClientContext* ctx) /* CBC streaming drives the request/response API directly (INVALID_DEVID), * so it is not devId-routed -- run it once. */ WH_TEST_RETURN_ON_FAIL(whTest_CryptoAesCbcStreaming(ctx)); +#endif +#ifdef WOLFSSL_AES_COUNTER + WH_TEST_RETURN_ON_FAIL(whTest_CryptoAesCtrLeftOob(ctx)); #endif /* TODO: port legacy AES async + DMA-async coverage (comm-buffer & DMA, round-trip & KAT) -- the only remaining legacy crypto parity gap; deferred to follow-up PR. */ return 0; diff --git a/test/wh_test_crypto.c b/test/wh_test_crypto.c index b5fe773b8..f70a7e3b6 100644 --- a/test/wh_test_crypto.c +++ b/test/wh_test_crypto.c @@ -8447,6 +8447,31 @@ static int whTest_CryptoAesAsync(whClientContext* ctx, int devId, WC_RNG* rng) } (void)wc_AesFree(aes); } + + /* CTR: server must reject left > AES_BLOCK_SIZE. + * wc_AesCtrEncrypt indexes aes->tmp via AES_BLOCK_SIZE - aes->left; + * an oversized client-supplied value would cause an out-of-bounds read + * disclosing server-side memory across the HSM trust boundary. */ + if (ret == 0) { + int tmp; + ret = wc_AesInit(aes, NULL, devId); + if (ret == 0) { + ret = wc_AesSetKeyDirect(aes, key, sizeof(key), iv, AES_ENCRYPTION); + } + if (ret == 0) { + aes->left = AES_BLOCK_SIZE + 1; + tmp = wh_Client_AesCtr(ctx, aes, 1, plainIn, sizeof(plainIn), + cipher); + if (tmp != WH_ERROR_BADARGS) { + WH_ERROR_PRINT( + "AES-CTR async: left > AES_BLOCK_SIZE should be " + "BADARGS, got %d\n", + tmp); + ret = -1; + } + } + (void)wc_AesFree(aes); + } if (ret == 0) { WH_TEST_PRINT("AES CTR ASYNC DEVID=0x%X SUCCESS\n", devId); } @@ -9446,6 +9471,28 @@ static int whTest_CryptoAesDmaAsync(whClientContext* ctx, int devId, memset(cipher, 0, sizeof(cipher)); memset(plainOut, 0, sizeof(plainOut)); } + + /* CTR DMA: same left > AES_BLOCK_SIZE rejection as the non-DMA path. */ + if (ret == 0) { + int tmp; + ret = wc_AesInit(aes, NULL, devId); + if (ret == 0) { + ret = wc_AesSetKeyDirect(aes, key, sizeof(key), iv, AES_ENCRYPTION); + } + if (ret == 0) { + aes->left = AES_BLOCK_SIZE + 1; + tmp = wh_Client_AesCtrDma(ctx, aes, 1, plainIn, sizeof(plainIn), + cipher); + if (tmp != WH_ERROR_BADARGS) { + WH_ERROR_PRINT( + "AES-CTR DMA async: left > AES_BLOCK_SIZE should be " + "BADARGS, got %d\n", + tmp); + ret = -1; + } + } + (void)wc_AesFree(aes); + } if (ret == 0) { WH_TEST_PRINT("AES CTR DMA ASYNC DEVID=0x%X SUCCESS\n", devId); }