From 362f144f13934bc511167e35c6dc8c07b5bce277 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 23 Jun 2026 11:22:55 +0000 Subject: [PATCH] AES: reject mode operations when no key has been set (F-6151) wc_AesInit zeroes the Aes struct, leaving keylen and rounds at 0. The CTR/CBC/GCM/ECB/CFB/OFB entry points never verified that a key had been installed with wc_AesSetKey, so calling them right after wc_AesInit ran the software cipher against an all-zero key schedule and returned success instead of an error. For CTR this exposes a reconstructable keystream; for GCM the hash subkey H is also zero, making the tag forgeable. Add the keylen == 0 guard already used by wc_AesXtsEncrypt to the software mode paths. The check is placed after the crypto callback fall-through so device-managed keys are unaffected. Hardware paths that call wc_AesGetKeySize already reject this case via rounds == 0. Add aes_no_key_set_test(), run from aes_test(), which inits an Aes context and confirms every mode rejects use before a key is set. --- wolfcrypt/src/aes.c | 64 ++++++++++++++++++++ wolfcrypt/test/test.c | 133 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 197 insertions(+) diff --git a/wolfcrypt/src/aes.c b/wolfcrypt/src/aes.c index e96e4b1842..a2656ebe4f 100644 --- a/wolfcrypt/src/aes.c +++ b/wolfcrypt/src/aes.c @@ -7208,6 +7208,13 @@ int wc_AesCbcEncrypt(Aes* aes, byte* out, const byte* in, word32 sz) /* fall-through when unavailable */ } #endif + + /* Software/HW key schedule required from here on. */ + if (aes->keylen == 0) { + WOLFSSL_MSG("AES key not set"); + return BAD_FUNC_ARG; + } + #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_AES) /* if async and byte count above threshold */ if (aes->asyncDev.marker == WOLFSSL_ASYNC_MARKER_AES && @@ -7445,6 +7452,13 @@ int wc_AesCbcEncrypt(Aes* aes, byte* out, const byte* in, word32 sz) /* fall-through when unavailable */ } #endif + + /* Software/HW key schedule required from here on. */ + if (aes->keylen == 0) { + WOLFSSL_MSG("AES key not set"); + return BAD_FUNC_ARG; + } + #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_AES) /* if async and byte count above threshold */ if (aes->asyncDev.marker == WOLFSSL_ASYNC_MARKER_AES && @@ -7944,6 +7958,12 @@ int wc_AesCbcEncrypt(Aes* aes, byte* out, const byte* in, word32 sz) } #endif + /* Software/HW key schedule required from here on. */ + if (aes->keylen == 0) { + WOLFSSL_MSG("AES key not set"); + return BAD_FUNC_ARG; + } + /* consume any unused bytes left in aes->tmp */ processed = min(aes->left, sz); xorbufout(out, in, (byte*)aes->tmp + WC_AES_BLOCK_SIZE - aes->left, @@ -11374,6 +11394,12 @@ int wc_AesGcmEncrypt(Aes* aes, byte* out, const byte* in, word32 sz, } #endif + /* Software/HW key schedule (and hash subkey H) required from here on. */ + if (aes->keylen == 0) { + WOLFSSL_MSG("AES key not set"); + return BAD_FUNC_ARG; + } + #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_AES) /* if async and byte count above threshold */ /* only 12-byte IV is supported in HW */ @@ -12229,6 +12255,12 @@ int wc_AesGcmDecrypt(Aes* aes, byte* out, const byte* in, word32 sz, } #endif + /* Software/HW key schedule (and hash subkey H) required from here on. */ + if (aes->keylen == 0) { + WOLFSSL_MSG("AES key not set"); + return BAD_FUNC_ARG; + } + #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_AES) /* if async and byte count above threshold */ /* only 12-byte IV is supported in HW */ @@ -16366,6 +16398,12 @@ static WARN_UNUSED_RESULT int _AesEcbEncrypt( return DCPAesEcbEncrypt(aes, out, in, sz); #endif + /* Software key schedule required from here on. */ + if (aes->keylen == 0) { + WOLFSSL_MSG("AES key not set"); + return BAD_FUNC_ARG; + } + VECTOR_REGISTERS_PUSH; #if defined(WOLFSSL_RISCV_ASM) @@ -16476,6 +16514,12 @@ static WARN_UNUSED_RESULT int _AesEcbDecrypt( return DCPAesEcbDecrypt(aes, out, in, sz); #endif + /* Software key schedule required from here on. */ + if (aes->keylen == 0) { + WOLFSSL_MSG("AES key not set"); + return BAD_FUNC_ARG; + } + VECTOR_REGISTERS_PUSH; #if defined(WOLFSSL_RISCV_ASM) @@ -16631,6 +16675,10 @@ static WARN_UNUSED_RESULT int AesCfbEncrypt_C(Aes* aes, byte* out, if (sz == 0) { return 0; } + if (aes->keylen == 0) { + WOLFSSL_MSG("AES key not set"); + return BAD_FUNC_ARG; + } if (aes->left > 0) { /* consume any unused bytes left in aes->tmp */ @@ -16714,6 +16762,10 @@ static WARN_UNUSED_RESULT int AesCfbDecrypt_C(Aes* aes, byte* out, if (sz == 0) { return 0; } + if (aes->keylen == 0) { + WOLFSSL_MSG("AES key not set"); + return BAD_FUNC_ARG; + } if (aes->left > 0) { /* consume any unused bytes left in aes->tmp */ @@ -16942,6 +16994,10 @@ static WARN_UNUSED_RESULT int wc_AesFeedbackCFB8( if (sz == 0) { return 0; } + if (aes->keylen == 0) { + WOLFSSL_MSG("AES key not set"); + return BAD_FUNC_ARG; + } VECTOR_REGISTERS_PUSH; @@ -17002,6 +17058,10 @@ static WARN_UNUSED_RESULT int wc_AesFeedbackCFB1( if (sz == 0) { return 0; } + if (aes->keylen == 0) { + WOLFSSL_MSG("AES key not set"); + return BAD_FUNC_ARG; + } VECTOR_REGISTERS_PUSH; @@ -17160,6 +17220,10 @@ static WARN_UNUSED_RESULT int AesOfbCrypt_C(Aes* aes, byte* out, const byte* in, if (sz == 0) { return 0; } + if (aes->keylen == 0) { + WOLFSSL_MSG("AES key not set"); + return BAD_FUNC_ARG; + } if (aes->left > 0) { /* consume any unused bytes left in aes->tmp */ diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 0556fd5351..614508e64d 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -16758,12 +16758,145 @@ static wc_test_ret_t aes_ecb_direct_test(void) } #endif /* HAVE_AES_ECB || WOLFSSL_AES_DIRECT */ +#if defined(HAVE_AES_CBC) || defined(WOLFSSL_AES_COUNTER) || \ + defined(HAVE_AESGCM) || defined(HAVE_AES_ECB) || \ + defined(WOLFSSL_AES_CFB) || defined(WOLFSSL_AES_OFB) +#define WC_TEST_HAVE_AES_NO_KEY_SET +/* Ensure AES mode APIs fail when used before wc_AesSetKey installs a key, + * instead of running with the all-zero key schedule left by wc_AesInit. */ +static wc_test_ret_t aes_no_key_set_test(void) +{ + wc_test_ret_t ret = 0; +#if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC) + Aes *aes = NULL; +#else + Aes aes[1]; +#endif + byte plain[WC_AES_BLOCK_SIZE]; + byte cipher[WC_AES_BLOCK_SIZE]; +#ifdef HAVE_AESGCM + byte iv[WC_AES_BLOCK_SIZE]; + byte tag[WC_AES_BLOCK_SIZE]; +#endif + + XMEMSET(plain, 0, sizeof(plain)); + XMEMSET(cipher, 0, sizeof(cipher)); +#ifdef HAVE_AESGCM + XMEMSET(iv, 0, sizeof(iv)); + XMEMSET(tag, 0, sizeof(tag)); +#endif + +#if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC) + aes = wc_AesNew(HEAP_HINT, devId, &ret); + if (aes == NULL) + return WC_TEST_RET_ENC_EC(ret); +#else + ret = wc_AesInit(aes, HEAP_HINT, devId); + if (ret != 0) + return WC_TEST_RET_ENC_EC(ret); +#endif + + /* No wc_AesSetKey: aes->keylen is 0, so every mode must reject the call. */ +#ifdef HAVE_AES_CBC + if (wc_AesCbcEncrypt(aes, cipher, plain, WC_AES_BLOCK_SIZE) != + WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); +#ifdef HAVE_AES_DECRYPT + if (wc_AesCbcDecrypt(aes, cipher, plain, WC_AES_BLOCK_SIZE) != + WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); +#endif +#endif /* HAVE_AES_CBC */ + +#ifdef WOLFSSL_AES_COUNTER + if (wc_AesCtrEncrypt(aes, cipher, plain, WC_AES_BLOCK_SIZE) != + WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); +#endif + +#ifdef HAVE_AESGCM + if (wc_AesGcmEncrypt(aes, cipher, plain, WC_AES_BLOCK_SIZE, iv, sizeof(iv), + tag, sizeof(tag), NULL, 0) != WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); + if (wc_AesGcmDecrypt(aes, cipher, plain, WC_AES_BLOCK_SIZE, iv, sizeof(iv), + tag, sizeof(tag), NULL, 0) != WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); +#endif + +#ifdef HAVE_AES_ECB + if (wc_AesEcbEncrypt(aes, cipher, plain, WC_AES_BLOCK_SIZE) != + WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); +#ifdef HAVE_AES_DECRYPT + if (wc_AesEcbDecrypt(aes, cipher, plain, WC_AES_BLOCK_SIZE) != + WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); +#endif +#endif /* HAVE_AES_ECB */ + +#ifdef WOLFSSL_AES_CFB + if (wc_AesCfbEncrypt(aes, cipher, plain, WC_AES_BLOCK_SIZE) != + WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); +#ifdef HAVE_AES_DECRYPT + if (wc_AesCfbDecrypt(aes, cipher, plain, WC_AES_BLOCK_SIZE) != + WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); +#endif +#if !defined(WOLFSSL_NO_AES_CFB_1_8) + if (wc_AesCfb1Encrypt(aes, cipher, plain, 8) != + WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); + if (wc_AesCfb8Encrypt(aes, cipher, plain, 1) != + WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); +#ifdef HAVE_AES_DECRYPT + if (wc_AesCfb1Decrypt(aes, cipher, plain, 8) != + WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); + if (wc_AesCfb8Decrypt(aes, cipher, plain, 1) != + WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); +#endif +#endif /* !WOLFSSL_NO_AES_CFB_1_8 */ +#endif /* WOLFSSL_AES_CFB */ + +#ifdef WOLFSSL_AES_OFB + if (wc_AesOfbEncrypt(aes, cipher, plain, WC_AES_BLOCK_SIZE) != + WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); +#ifdef HAVE_AES_DECRYPT + if (wc_AesOfbDecrypt(aes, cipher, plain, WC_AES_BLOCK_SIZE) != + WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_NC, out); +#endif +#endif /* WOLFSSL_AES_OFB */ + + ret = 0; /* success */ + out: + +#if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC) + wc_AesDelete(aes, &aes); +#else + wc_AesFree(aes); +#endif + + return ret; +} +#endif /* any AES mode for aes_no_key_set_test */ + WOLFSSL_TEST_SUBROUTINE wc_test_ret_t aes_test(void) { wc_test_ret_t ret = 0; WOLFSSL_ENTER("aes_test"); +#ifdef WC_TEST_HAVE_AES_NO_KEY_SET + ret = aes_no_key_set_test(); + if (ret != 0) + return ret; +#endif + #ifndef HAVE_RENESAS_SYNC ret = aes_key_size_test(); if (ret != 0)