mirror of
https://github.com/wolfSSL/wolfssl.git
synced 2026-10-24 04:09:07 +03:00
Wipe the old GCM H when a device claims an AES key
When a SETKEY device claimed a new key through wc_AesSetKey(), wc_AesCcmSetKey() or wc_AesSetKeyDirect(), the set-key wiped the old key schedule and devKey but left gcm.H and the GHASH table M0 of an earlier software GCM key in the context. Only wc_AesGcmSetKey() wiped them. gcmKeySet is already cleared on that path, so the stale H could not be used, but it is key material derived from the old key and goes with the rest of it. The pre-claim wipe in the generic, ARM32 and PPC set-key now clears H and M0 too, through a WC_AES_GCM_WIPE_H() macro that wc_AesGcmSetKey() uses as well. The CryptoCb AES set-key tests in test_aes.c took buffer sizes with sizeof, which is the size of a pointer under WOLFSSL_SMALL_STACK. In such builds test_wc_CryptoCb_AesSetKey and test_wc_CryptoCb_AesGcm_EncryptDecrypt failed at their first set-key with an 8-byte key, and the declined re-key check passed an 8-byte tag that wc_AesGcmEncrypt() rejects before it asks the device. Both tests now use the allocation sizes.
This commit is contained in:
12
tests/api.c
12
tests/api.c
@@ -38108,12 +38108,10 @@ static int test_wc_CryptoCb_AesGcm_SetKey(void)
|
||||
0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
|
||||
0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f
|
||||
};
|
||||
#ifdef WOLFSSL_AESGCM_STREAM
|
||||
const byte key2[AES_128_KEY_SIZE] = {
|
||||
0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7,
|
||||
0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff
|
||||
};
|
||||
#endif
|
||||
const byte iv[GCM_NONCE_MID_SZ] = {
|
||||
0xca, 0xfe, 0xba, 0xbe, 0xfa, 0xce, 0xdb, 0xad,
|
||||
0xde, 0xca, 0xf8, 0x88
|
||||
@@ -38125,8 +38123,10 @@ static int test_wc_CryptoCb_AesGcm_SetKey(void)
|
||||
byte ct[sizeof(msg)];
|
||||
byte tag[WC_AES_BLOCK_SIZE];
|
||||
byte pt[sizeof(msg)];
|
||||
byte zeroH[WC_AES_BLOCK_SIZE];
|
||||
|
||||
XMEMSET(&dev, 0, sizeof(dev));
|
||||
XMEMSET(zeroH, 0, sizeof(zeroH));
|
||||
|
||||
ExpectIntEQ(wc_AesInit(&aes, NULL, TEST_CRYPTOCB_UNREG_DEVID), 0);
|
||||
ExpectIntEQ(wc_AesGcmSetKey(&aes, key, sizeof(key)), 0);
|
||||
@@ -38193,6 +38193,14 @@ static int test_wc_CryptoCb_AesGcm_SetKey(void)
|
||||
sizeof(aad)), WC_NO_ERR_TRACE(MISSING_KEY));
|
||||
#endif
|
||||
|
||||
/* A claim through wc_AesSetKey() wipes the old software key's H. */
|
||||
dev.declineSetKey = 1;
|
||||
ExpectIntEQ(wc_AesGcmSetKey(&aes, key2, sizeof(key2)), 0);
|
||||
dev.declineSetKey = 0;
|
||||
ExpectBufNE(aes.gcm.H, zeroH, sizeof(zeroH));
|
||||
ExpectIntEQ(wc_AesSetKey(&aes, key, sizeof(key), NULL, AES_ENCRYPTION), 0);
|
||||
ExpectBufEQ(aes.gcm.H, zeroH, sizeof(zeroH));
|
||||
|
||||
if (aesInit) {
|
||||
wc_AesFree(&aes);
|
||||
}
|
||||
|
||||
@@ -12802,15 +12802,15 @@ int test_wc_CryptoCb_AesSetKey(void)
|
||||
ExpectIntEQ(aes->devId, TEST_CRYPTOCB_AES_DEVID);
|
||||
|
||||
/* Direct callback entry guardrails. */
|
||||
ExpectIntEQ(wc_CryptoCb_AesSetKey(NULL, key, sizeof(key)),
|
||||
ExpectIntEQ(wc_CryptoCb_AesSetKey(NULL, key, AES_128_KEY_SIZE),
|
||||
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
|
||||
ExpectIntEQ(wc_CryptoCb_AesSetKey(aes, NULL, sizeof(key)),
|
||||
ExpectIntEQ(wc_CryptoCb_AesSetKey(aes, NULL, AES_128_KEY_SIZE),
|
||||
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
|
||||
ExpectIntEQ(wc_CryptoCb_AesSetKey(aes, key, 0),
|
||||
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
|
||||
|
||||
/* Set key - should trigger CryptoCB and "import" to mock SE */
|
||||
ret = wc_AesGcmSetKey(aes, key, sizeof(key));
|
||||
ret = wc_AesGcmSetKey(aes, key, AES_128_KEY_SIZE);
|
||||
ExpectIntEQ(ret, 0);
|
||||
|
||||
/* Verify callback was invoked */
|
||||
@@ -12821,16 +12821,16 @@ int test_wc_CryptoCb_AesSetKey(void)
|
||||
|
||||
/* Verify key was "imported" to mock SE */
|
||||
ExpectIntEQ(mockSeKey.valid, 1);
|
||||
ExpectIntEQ(mockSeKey.keySz, (int)sizeof(key));
|
||||
ExpectIntEQ(mockSeKey.keySz, AES_128_KEY_SIZE);
|
||||
|
||||
/* Verify keylen metadata stored in Aes struct */
|
||||
ExpectIntEQ(aes->keylen, (int)sizeof(key));
|
||||
ExpectIntEQ(aes->keylen, AES_128_KEY_SIZE);
|
||||
|
||||
/* After SetKey succeeds via CryptoCB, verify key NOT in devKey */
|
||||
{
|
||||
byte zeroKey[AES_128_KEY_SIZE] = {0};
|
||||
/* Key should NOT be copied to devKey - SE owns it */
|
||||
ExpectIntEQ(XMEMCMP(aes->devKey, zeroKey, sizeof(key)), 0);
|
||||
ExpectIntEQ(XMEMCMP(aes->devKey, zeroKey, AES_128_KEY_SIZE), 0);
|
||||
}
|
||||
|
||||
#if defined(WOLFSSL_AESGCM_STREAM) && !defined(HAVE_FIPS) && \
|
||||
@@ -12850,29 +12850,29 @@ int test_wc_CryptoCb_AesSetKey(void)
|
||||
/* Missing device context should fail in the callback instead of falling
|
||||
* through as a successful offload path. */
|
||||
aes->devCtx = NULL;
|
||||
ExpectIntEQ(wc_AesGcmEncrypt(aes, cipher, plain, sizeof(plain),
|
||||
iv, sizeof(iv), authTag, sizeof(authTag), NULL, 0),
|
||||
ExpectIntEQ(wc_AesGcmEncrypt(aes, cipher, plain, 16,
|
||||
iv, GCM_NONCE_MID_SZ, authTag, AES_BLOCK_SIZE, NULL, 0),
|
||||
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
|
||||
|
||||
/* Restore callback-owned state for the rest of the test. */
|
||||
ret = wc_AesGcmSetKey(aes, key, sizeof(key));
|
||||
ret = wc_AesGcmSetKey(aes, key, AES_128_KEY_SIZE);
|
||||
ExpectIntEQ(ret, 0);
|
||||
ExpectPtrEq(aes->devCtx, cryptoCbAesMockHandle);
|
||||
|
||||
/* Test encrypt - callback performs crypto using stored key */
|
||||
ret = wc_AesGcmEncrypt(aes, cipher, plain, sizeof(plain),
|
||||
iv, sizeof(iv), authTag, sizeof(authTag),
|
||||
ret = wc_AesGcmEncrypt(aes, cipher, plain, 16,
|
||||
iv, GCM_NONCE_MID_SZ, authTag, AES_BLOCK_SIZE,
|
||||
NULL, 0);
|
||||
ExpectIntEQ(ret, 0);
|
||||
|
||||
/* Test decrypt - callback performs crypto using stored key */
|
||||
ret = wc_AesGcmDecrypt(aes, decrypted, cipher, sizeof(cipher),
|
||||
iv, sizeof(iv), authTag, sizeof(authTag),
|
||||
ret = wc_AesGcmDecrypt(aes, decrypted, cipher, 16,
|
||||
iv, GCM_NONCE_MID_SZ, authTag, AES_BLOCK_SIZE,
|
||||
NULL, 0);
|
||||
ExpectIntEQ(ret, 0);
|
||||
|
||||
/* Verify round-trip */
|
||||
ExpectIntEQ(XMEMCMP(plain, decrypted, sizeof(plain)), 0);
|
||||
ExpectIntEQ(XMEMCMP(plain, decrypted, 16), 0);
|
||||
|
||||
#if defined(WOLFSSL_AESGCM_STREAM) && !defined(HAVE_FIPS) && \
|
||||
!defined(HAVE_SELFTEST)
|
||||
@@ -12882,8 +12882,8 @@ int test_wc_CryptoCb_AesSetKey(void)
|
||||
ExpectIntEQ(wc_AesGcmSetKey(aes, key, AES_128_KEY_SIZE), 0);
|
||||
cryptoCbAesDeclineSetKey = 0;
|
||||
ExpectNull(aes->devCtx);
|
||||
ExpectIntEQ(wc_AesGcmEncrypt(aes, cipher, plain, sizeof(plain),
|
||||
iv, sizeof(iv), authTag, sizeof(authTag), NULL, 0),
|
||||
ExpectIntEQ(wc_AesGcmEncrypt(aes, cipher, plain, 16,
|
||||
iv, GCM_NONCE_MID_SZ, authTag, AES_BLOCK_SIZE, NULL, 0),
|
||||
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
|
||||
ExpectIntEQ(wc_AesGcmInit(aes, NULL, 0, iv, GCM_NONCE_MID_SZ), 0);
|
||||
ExpectIntEQ(wc_AesGcmEncryptUpdate(aes, cipher, plain, 16, NULL, 0), 0);
|
||||
@@ -12911,7 +12911,7 @@ int test_wc_CryptoCb_AesSetKey(void)
|
||||
|
||||
/* Cleanup */
|
||||
wc_CryptoCb_UnRegisterDevice(TEST_CRYPTOCB_AES_DEVID);
|
||||
ExpectIntEQ(wc_CryptoCb_AesSetKey(aes, key, sizeof(key)),
|
||||
ExpectIntEQ(wc_CryptoCb_AesSetKey(aes, key, AES_128_KEY_SIZE),
|
||||
WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE));
|
||||
|
||||
/* Test software path (no devId) still works */
|
||||
@@ -12921,7 +12921,7 @@ int test_wc_CryptoCb_AesSetKey(void)
|
||||
ret = wc_AesInit(aes, NULL, INVALID_DEVID);
|
||||
ExpectIntEQ(ret, 0);
|
||||
|
||||
ret = wc_AesGcmSetKey(aes, key, sizeof(key));
|
||||
ret = wc_AesGcmSetKey(aes, key, AES_128_KEY_SIZE);
|
||||
ExpectIntEQ(ret, 0);
|
||||
|
||||
/* Callback should NOT have been invoked */
|
||||
@@ -13250,7 +13250,7 @@ int test_wc_CryptoCb_AesGcm_EncryptDecrypt(void)
|
||||
ExpectIntEQ(aes->devId, TEST_CRYPTOCB_AESGCM_OFFLOAD_DEVID);
|
||||
|
||||
/* Set key - should trigger CryptoCB and "import" to mock SE */
|
||||
ret = wc_AesGcmSetKey(aes, key, sizeof(key));
|
||||
ret = wc_AesGcmSetKey(aes, key, AES_128_KEY_SIZE);
|
||||
ExpectIntEQ(ret, 0);
|
||||
|
||||
/* Verify SetKey callback was invoked */
|
||||
@@ -13261,14 +13261,14 @@ int test_wc_CryptoCb_AesGcm_EncryptDecrypt(void)
|
||||
|
||||
/* Verify key was "imported" to mock SE */
|
||||
ExpectIntEQ(mockSeKeyOffload.valid, 1);
|
||||
ExpectIntEQ(mockSeKeyOffload.keySz, (int)sizeof(key));
|
||||
ExpectIntEQ(mockSeKeyOffload.keySz, AES_128_KEY_SIZE);
|
||||
|
||||
/* Verify keylen metadata stored in Aes struct */
|
||||
ExpectIntEQ(aes->keylen, (int)sizeof(key));
|
||||
ExpectIntEQ(aes->keylen, AES_128_KEY_SIZE);
|
||||
|
||||
/* Encrypt via wolfCrypt API - should route through CryptoCb */
|
||||
ret = wc_AesGcmEncrypt(aes, ciphertext, plaintext, 32,
|
||||
iv, sizeof(iv), authTag, sizeof(authTag),
|
||||
iv, GCM_NONCE_MID_SZ, authTag, AES_BLOCK_SIZE,
|
||||
aad, 16);
|
||||
ExpectIntEQ(ret, 0);
|
||||
|
||||
@@ -13280,7 +13280,7 @@ int test_wc_CryptoCb_AesGcm_EncryptDecrypt(void)
|
||||
|
||||
/* Assert: Auth tag is non-zero */
|
||||
hasNonZero = 0;
|
||||
for (i = 0; i < (int)sizeof(authTag); i++) {
|
||||
for (i = 0; i < AES_BLOCK_SIZE; i++) {
|
||||
if (authTag[i] != 0) {
|
||||
hasNonZero = 1;
|
||||
break;
|
||||
@@ -13290,7 +13290,7 @@ int test_wc_CryptoCb_AesGcm_EncryptDecrypt(void)
|
||||
|
||||
/* Decrypt via wolfCrypt API - should route through CryptoCb */
|
||||
ret = wc_AesGcmDecrypt(aes, decrypted, ciphertext, 32,
|
||||
iv, sizeof(iv), authTag, sizeof(authTag),
|
||||
iv, GCM_NONCE_MID_SZ, authTag, AES_BLOCK_SIZE,
|
||||
aad, 16);
|
||||
ExpectIntEQ(ret, 0);
|
||||
|
||||
|
||||
@@ -160,6 +160,18 @@ block cipher mechanism that uses n-bit binary string parameter key with 128-bits
|
||||
#define WC_AES_GCM_UNKEY(aes) WC_DO_NOTHING
|
||||
#endif
|
||||
|
||||
#if defined(HAVE_AESGCM) && (defined(GCM_TABLE) || defined(GCM_TABLE_4BIT))
|
||||
#define WC_AES_GCM_WIPE_H(aes) do { \
|
||||
ForceZero((aes)->gcm.H, sizeof((aes)->gcm.H)); \
|
||||
ForceZero((aes)->gcm.M0, sizeof((aes)->gcm.M0)); \
|
||||
} while (0)
|
||||
#elif defined(HAVE_AESGCM)
|
||||
#define WC_AES_GCM_WIPE_H(aes) \
|
||||
ForceZero((aes)->gcm.H, sizeof((aes)->gcm.H))
|
||||
#else
|
||||
#define WC_AES_GCM_WIPE_H(aes) WC_DO_NOTHING
|
||||
#endif
|
||||
|
||||
#if defined(__aarch64__) && defined(WOLFSSL_ARMASM)
|
||||
#define WC_AES_ARM64_SVR_BEGIN() SAVE_VECTOR_REGISTERS(return _svr_ret;)
|
||||
#define WC_AES_ARM64_SVR_END() RESTORE_VECTOR_REGISTERS()
|
||||
@@ -5454,6 +5466,7 @@ static WARN_UNUSED_RESULT int wc_AesDecrypt(Aes* aes, const byte* inBlock,
|
||||
aes->rounds = 0;
|
||||
aes->keylen = 0;
|
||||
WC_AES_GCM_UNKEY(aes);
|
||||
WC_AES_GCM_WIPE_H(aes);
|
||||
ForceZero(aes->key, sizeof(aes->key));
|
||||
ForceZero(aes->devKey, sizeof(aes->devKey));
|
||||
#endif
|
||||
@@ -5588,6 +5601,7 @@ static WARN_UNUSED_RESULT int wc_AesDecrypt(Aes* aes, const byte* inBlock,
|
||||
aes->rounds = 0;
|
||||
aes->keylen = 0;
|
||||
WC_AES_GCM_UNKEY(aes);
|
||||
WC_AES_GCM_WIPE_H(aes);
|
||||
ForceZero(aes->key, sizeof(aes->key));
|
||||
ForceZero(aes->devKey, sizeof(aes->devKey));
|
||||
#endif
|
||||
@@ -6122,6 +6136,7 @@ static void AesSetKey_C(Aes* aes, const byte* key, word32 keySz, int dir)
|
||||
aes->rounds = 0;
|
||||
aes->keylen = 0;
|
||||
WC_AES_GCM_UNKEY(aes);
|
||||
WC_AES_GCM_WIPE_H(aes);
|
||||
ForceZero(aes->key, sizeof(aes->key));
|
||||
ForceZero(aes->devKey, sizeof(aes->devKey));
|
||||
#ifdef WC_C_DYNAMIC_FALLBACK
|
||||
@@ -9336,10 +9351,7 @@ int wc_AesGcmSetKey(Aes* aes, const byte* key, word32 len)
|
||||
{
|
||||
/* No schedule: a device claimed the key or the set-key failed. */
|
||||
if ((ret != 0) || (aes->rounds == 0)) {
|
||||
ForceZero(aes->gcm.H, sizeof(aes->gcm.H));
|
||||
#if defined(GCM_TABLE) || defined(GCM_TABLE_4BIT)
|
||||
ForceZero(aes->gcm.M0, sizeof(aes->gcm.M0));
|
||||
#endif
|
||||
WC_AES_GCM_WIPE_H(aes);
|
||||
#ifdef WOLFSSL_IMX6_CAAM_BLOB
|
||||
ForceZero(local, sizeof(local));
|
||||
#endif
|
||||
|
||||
Reference in New Issue
Block a user