fix(mbedtls): Fixed the transmission of return values of the esp-aes APIs

- Earlier, some intermediate return values were not stored and returned,
thus incorrect return values used to get transmitted to the upper layer of APIs.

- Also, zeroised the output buffer in case of error condition.

components/mbedtls/port/aes/block/esp_aes.c

@@ -74,6 +74,10 @@ void esp_aes_release_hardware( void )
 /* Run a single 16 byte block of AES, using the hardware engine.
  *
  * Call only while holding esp_aes_acquire_hardware().
+ *
+ * The function esp_aes_block zeroises the output buffer in the case of following conditions:
+ * 1. If key is not written in the hardware
+ * 2. If the fault injection check failed
  */
 static int esp_aes_block(esp_aes_context *ctx, const void *input, void *output)
 {
@@ -87,7 +91,7 @@ static int esp_aes_block(esp_aes_context *ctx, const void *input, void *output)
        key write to hardware. Treat this as a fatal error and zero the output block.
     */
     if (ctx->key_in_hardware != ctx->key_bytes) {
-        bzero(output, 16);
+        mbedtls_platform_zeroize(output, 16);
         return MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH;
     }
     i0 = input_words[0];
@@ -223,7 +227,6 @@ int esp_aes_crypt_ecb(esp_aes_context *ctx,
     ctx->key_in_hardware = aes_hal_setkey(ctx->key, ctx->key_bytes, mode);
     r = esp_aes_block(ctx, input, output);
     esp_aes_release_hardware();
-
     return r;
 }
 
@@ -238,6 +241,7 @@ int esp_aes_crypt_cbc(esp_aes_context *ctx,
                       const unsigned char *input,
                       unsigned char *output )
 {
+    int ret = -1;
     if (esp_aes_validate_input(ctx, input, output)) {
         return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
@@ -268,7 +272,10 @@ int esp_aes_crypt_cbc(esp_aes_context *ctx,
     if ( mode == ESP_AES_DECRYPT ) {
         while ( length > 0 ) {
             memcpy(temp, input_words, 16);
-            esp_aes_block(ctx, input_words, output_words);
+            ret = esp_aes_block(ctx, input_words, output_words);
+            if (ret != 0) {
+                goto cleanup;
+            }
 
             output_words[0] = output_words[0] ^ iv_words[0];
             output_words[1] = output_words[1] ^ iv_words[1];
@@ -289,7 +296,11 @@ int esp_aes_crypt_cbc(esp_aes_context *ctx,
             output_words[2] = input_words[2] ^ iv_words[2];
             output_words[3] = input_words[3] ^ iv_words[3];
 
-            esp_aes_block(ctx, output_words, output_words);
+            ret = esp_aes_block(ctx, output_words, output_words);
+            if (ret != 0) {
+                goto cleanup;
+            }
+
             memcpy( iv_words, output_words, 16 );
 
             input_words  += 4;
@@ -297,10 +308,11 @@ int esp_aes_crypt_cbc(esp_aes_context *ctx,
             length -= 16;
         }
     }
+    ret = 0;
 
+cleanup:
     esp_aes_release_hardware();
-
+    return ret;
-    return 0;
 }
 
 /*
@@ -314,6 +326,7 @@ int esp_aes_crypt_cfb128(esp_aes_context *ctx,
                          const unsigned char *input,
                          unsigned char *output )
 {
+    int ret = -1;
     if (esp_aes_validate_input(ctx, input, output)) {
         return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
@@ -341,7 +354,10 @@ int esp_aes_crypt_cfb128(esp_aes_context *ctx,
     if ( mode == ESP_AES_DECRYPT ) {
         while ( length-- ) {
             if ( n == 0 ) {
-                esp_aes_block(ctx, iv, iv);
+                ret = esp_aes_block(ctx, iv, iv);
+                if (ret != 0) {
+                    goto cleanup;
+                }
             }
 
             c = *input++;
@@ -353,7 +369,10 @@ int esp_aes_crypt_cfb128(esp_aes_context *ctx,
     } else {
         while ( length-- ) {
             if ( n == 0 ) {
-                esp_aes_block(ctx, iv, iv);
+                ret = esp_aes_block(ctx, iv, iv);
+                if (ret != 0) {
+                    goto cleanup;
+                }
             }
 
             iv[n] = *output++ = (unsigned char)( iv[n] ^ *input++ );
@@ -363,10 +382,11 @@ int esp_aes_crypt_cfb128(esp_aes_context *ctx,
     }
 
     *iv_off = n;
+    ret = 0;
 
+cleanup:
     esp_aes_release_hardware();
-
+    return ret;
-    return 0;
 }
 
 /*
@@ -379,6 +399,7 @@ int esp_aes_crypt_cfb8(esp_aes_context *ctx,
                        const unsigned char *input,
                        unsigned char *output )
 {
+    int ret = -1;
     unsigned char c;
     unsigned char ov[17];
 
@@ -402,7 +423,10 @@ int esp_aes_crypt_cfb8(esp_aes_context *ctx,
 
     while ( length-- ) {
         memcpy( ov, iv, 16 );
-        esp_aes_block(ctx, iv, iv);
+        ret = esp_aes_block(ctx, iv, iv);
+        if (ret != 0) {
+            goto cleanup;
+        }
 
         if ( mode == ESP_AES_DECRYPT ) {
             ov[16] = *input;
@@ -416,10 +440,11 @@ int esp_aes_crypt_cfb8(esp_aes_context *ctx,
 
         memcpy( iv, ov + 1, 16 );
     }
+    ret = 0;
 
+cleanup:
     esp_aes_release_hardware();
-
+    return ret;
-    return 0;
 }
 
 /*
@@ -433,7 +458,7 @@ int esp_aes_crypt_ctr(esp_aes_context *ctx,
                       const unsigned char *input,
                       unsigned char *output )
 {
-    int c, i;
+    int c, i, ret = -1;
 
     if (esp_aes_validate_input(ctx, input, output)) {
         return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
@@ -466,7 +491,10 @@ int esp_aes_crypt_ctr(esp_aes_context *ctx,
 
     while ( length-- ) {
         if ( n == 0 ) {
-            esp_aes_block(ctx, nonce_counter, stream_block);
+            ret = esp_aes_block(ctx, nonce_counter, stream_block);
+            if (ret != 0) {
+                goto cleanup;
+            }
 
             for ( i = 16; i > 0; i-- ) {
                 if ( ++nonce_counter[i - 1] != 0 ) {
@@ -481,10 +509,11 @@ int esp_aes_crypt_ctr(esp_aes_context *ctx,
     }
 
     *nc_off = n;
+    ret = 0;
 
+cleanup:
     esp_aes_release_hardware();
-
+    return ret;
-    return 0;
 }
 
 /*
@@ -497,7 +526,7 @@ int esp_aes_crypt_ofb(esp_aes_context *ctx,
                       const unsigned char *input,
                       unsigned char *output )
 {
-    int ret = 0;
+    int ret = -1;
     size_t n;
 
     if (esp_aes_validate_input(ctx, input, output)) {
@@ -531,7 +560,10 @@ int esp_aes_crypt_ofb(esp_aes_context *ctx,
 
     while (length--) {
         if ( n == 0 ) {
-            esp_aes_block(ctx, iv, iv);
+            ret = esp_aes_block(ctx, iv, iv);
+            if (ret != 0) {
+                goto cleanup;
+            }
         }
         *output++ =  *input++ ^ iv[n];
 
@@ -539,7 +571,9 @@ int esp_aes_crypt_ofb(esp_aes_context *ctx,
     }
 
     *iv_off = n;
+    ret = 0;
 
+cleanup:
     esp_aes_release_hardware();
 
     return ( ret );

components/mbedtls/port/aes/dma/esp_aes.c

@@ -185,7 +185,10 @@ static esp_err_t esp_aes_isr_initialise( void )
             return ESP_FAIL;
         }
 
-        esp_intr_alloc(ETS_AES_INTR_SOURCE, 0, esp_aes_complete_isr, NULL, NULL);
+        esp_err_t ret = esp_intr_alloc(ETS_AES_INTR_SOURCE, 0, esp_aes_complete_isr, NULL, NULL);
+        if (ret != ESP_OK) {
+            return ret;
+        }
     }
 
     /* AES is clocked proportionally to CPU clock, take power management lock */
@@ -239,6 +242,8 @@ static int esp_aes_process_dma(esp_aes_context *ctx, const unsigned char *input,
 /* Output buffers in external ram needs to be 16-byte aligned and DMA cant access input in the iCache mem range,
    reallocate them into internal memory and encrypt in chunks to avoid
    having to malloc too big of a buffer
+
+  The function esp_aes_process_dma_ext_ram zeroises the output buffer in the case of memory allocation failure.
 */
 
 static int esp_aes_process_dma_ext_ram(esp_aes_context *ctx, const unsigned char *input, unsigned char *output, size_t len, uint8_t *stream_out, bool realloc_input, bool realloc_output)
@@ -255,9 +260,9 @@ static int esp_aes_process_dma_ext_ram(esp_aes_context *ctx, const unsigned char
         input_buf = heap_caps_malloc(chunk_len, MALLOC_CAP_DMA);
 
         if (input_buf == NULL) {
+            mbedtls_platform_zeroize(output, len);
             ESP_LOGE(TAG, "Failed to allocate memory");
-            ret = -1;
+            return -1;
-            goto cleanup;
         }
     }
 
@@ -265,9 +270,9 @@ static int esp_aes_process_dma_ext_ram(esp_aes_context *ctx, const unsigned char
         output_buf = heap_caps_malloc(chunk_len, MALLOC_CAP_DMA);
 
         if (output_buf == NULL) {
+            mbedtls_platform_zeroize(output, len);
             ESP_LOGE(TAG, "Failed to allocate memory");
-            ret = -1;
+            return -1;
-            goto cleanup;
         }
     } else {
         output_buf = output;
@@ -311,7 +316,13 @@ cleanup:
     return ret;
 }
 
-/* Encrypt/decrypt the input using DMA */
+/* Encrypt/decrypt the input using DMA
+ * The function esp_aes_process_dma zeroises the output buffer in the case of following conditions:
+ * 1. If key is not written in the hardware
+ * 2. Memory allocation failures
+ * 3. If AES interrupt is enabled and ISR initialisation fails
+ * 4. Failure in any of the AES operations
+ */
 static int esp_aes_process_dma(esp_aes_context *ctx, const unsigned char *input, unsigned char *output, size_t len, uint8_t *stream_out)
 {
     lldesc_t *in_desc_head = NULL, *out_desc_head = NULL;
@@ -335,7 +346,7 @@ static int esp_aes_process_dma(esp_aes_context *ctx, const unsigned char *input,
        key write to hardware. Treat this as a fatal error and zero the output block.
     */
     if (ctx->key_in_hardware != ctx->key_bytes) {
-        bzero(output, len);
+        mbedtls_platform_zeroize(output, len);
         return MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH;
     }
 
@@ -372,9 +383,9 @@ static int esp_aes_process_dma(esp_aes_context *ctx, const unsigned char *input,
         /* Allocate both in and out descriptors to save a malloc/free per function call */
         block_desc = heap_caps_calloc(lldesc_num * 2, sizeof(lldesc_t), MALLOC_CAP_DMA);
         if (block_desc == NULL) {
+            mbedtls_platform_zeroize(output, len);
             ESP_LOGE(TAG, "Failed to allocate memory");
-            ret = -1;
+            return -1;
-            goto cleanup;
         }
 
         block_in_desc = block_desc;
@@ -420,6 +431,7 @@ static int esp_aes_process_dma(esp_aes_context *ctx, const unsigned char *input,
     if (len > AES_DMA_INTR_TRIG_LEN) {
         use_intr = true;
         if (esp_aes_isr_initialise() == ESP_FAIL) {
+            ESP_LOGE(TAG, "ESP-AES ISR initialisation failed");
             ret = -1;
             goto cleanup;
         }
@@ -453,6 +465,9 @@ static int esp_aes_process_dma(esp_aes_context *ctx, const unsigned char *input,
     }
 
 cleanup:
+    if (ret != 0) {
+        mbedtls_platform_zeroize(output, len);
+    }
     free(block_desc);
     return ret;
 }
@@ -460,7 +475,13 @@ cleanup:
 
 #if SOC_AES_SUPPORT_GCM
 
-/* Encrypt/decrypt with AES-GCM the input using DMA */
+/* Encrypt/decrypt with AES-GCM the input using DMA
+ * The function esp_aes_process_dma_gcm zeroises the output buffer in the case of following conditions:
+ * 1. If key is not written in the hardware
+ * 2. Memory allocation failures
+ * 3. If AES interrupt is enabled and ISR initialisation fails
+ * 4. Failure in any of the AES operations
+ */
 int esp_aes_process_dma_gcm(esp_aes_context *ctx, const unsigned char *input, unsigned char *output, size_t len, lldesc_t *aad_desc, size_t aad_len)
 {
     lldesc_t *in_desc_head = NULL, *out_desc_head = NULL, *len_desc = NULL;
@@ -484,7 +505,7 @@ int esp_aes_process_dma_gcm(esp_aes_context *ctx, const unsigned char *input, un
        key write to hardware. Treat this as a fatal error and zero the output block.
     */
     if (ctx->key_in_hardware != ctx->key_bytes) {
-        bzero(output, len);
+        mbedtls_platform_zeroize(output, len);
         return MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH;
     }
 
@@ -494,9 +515,9 @@ int esp_aes_process_dma_gcm(esp_aes_context *ctx, const unsigned char *input, un
     /* Allocate both in and out descriptors to save a malloc/free per function call, add 1 for length descriptor */
     block_desc = heap_caps_calloc( (lldesc_num * 2) + 1, sizeof(lldesc_t), MALLOC_CAP_DMA);
     if (block_desc == NULL) {
+        mbedtls_platform_zeroize(output, len);
         ESP_LOGE(TAG, "Failed to allocate memory");
-        ret = -1;
+        return -1;
-        goto cleanup;
     }
 
     block_in_desc = block_desc;
@@ -543,6 +564,7 @@ int esp_aes_process_dma_gcm(esp_aes_context *ctx, const unsigned char *input, un
     if (len > AES_DMA_INTR_TRIG_LEN) {
         use_intr = true;
         if (esp_aes_isr_initialise() == ESP_FAIL) {
+            ESP_LOGE(TAG, "ESP-AES ISR initialisation failed");
             ret = -1;
             goto cleanup;
         }
@@ -570,6 +592,9 @@ int esp_aes_process_dma_gcm(esp_aes_context *ctx, const unsigned char *input, un
     }
 
 cleanup:
+    if (ret != 0) {
+        mbedtls_platform_zeroize(output, len);
+    }
     free(block_desc);
     return ret;
 }
@@ -783,13 +808,13 @@ int esp_aes_crypt_cfb8(esp_aes_context *ctx,
         aes_hal_mode_init(ESP_AES_BLOCK_MODE_CFB8);
         aes_hal_set_iv(iv);
         r = esp_aes_process_dma(ctx, input, output, block_bytes, NULL);
-        aes_hal_read_iv(iv);
-
         if (r != 0) {
             esp_aes_release_hardware();
             return r;
         }
 
+        aes_hal_read_iv(iv);
+
         length -= block_bytes;
         input += block_bytes;
         output += block_bytes;

components/mbedtls/port/aes/esp_aes_gcm.c

@@ -6,7 +6,7 @@
  *
  * SPDX-License-Identifier: Apache-2.0
  *
- * SPDX-FileContributor: 2016-2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileContributor: 2016-2023 Espressif Systems (Shanghai) CO LTD
  */
 /*
  *  The AES block cipher was designed by Vincent Rijmen and Joan Daemen.
@@ -372,7 +372,10 @@ int esp_aes_gcm_starts( esp_gcm_context *ctx,
         esp_aes_release_hardware();
 #else
         memset(ctx->H, 0, sizeof(ctx->H));
-        esp_aes_crypt_ecb(&ctx->aes_ctx, MBEDTLS_AES_ENCRYPT, ctx->H, ctx->H);
+        int ret = esp_aes_crypt_ecb(&ctx->aes_ctx, MBEDTLS_AES_ENCRYPT, ctx->H, ctx->H);
+        if (ret != 0) {
+            return ret;
+        }
 #endif
         gcm_gen_table(ctx);
     }
@@ -475,7 +478,10 @@ int esp_aes_gcm_update( esp_gcm_context *ctx,
     }
 
     /* Output = GCTR(J0, Input): Encrypt/Decrypt the input */
-    esp_aes_crypt_ctr(&ctx->aes_ctx, input_length, &nc_off, nonce_counter, stream, input, output);
+    int ret = esp_aes_crypt_ctr(&ctx->aes_ctx, input_length, &nc_off, nonce_counter, stream, input, output);
+    if (ret != 0) {
+        return ret;
+    }
 
     /* ICB gets auto incremented after GCTR operation here so update the context */
     memcpy(ctx->J0, nonce_counter, AES_BLOCK_BYTES);
@@ -511,9 +517,7 @@ int esp_aes_gcm_finish( esp_gcm_context *ctx,
     esp_gcm_ghash(ctx, len_block, AES_BLOCK_BYTES, ctx->ghash);
 
     /* Tag T = GCTR(J0, ) where T is truncated to tag_len */
-    esp_aes_crypt_ctr(&ctx->aes_ctx, tag_len, &nc_off, ctx->ori_j0, stream, ctx->ghash, tag);
+    return esp_aes_crypt_ctr(&ctx->aes_ctx, tag_len, &nc_off, ctx->ori_j0, stream, ctx->ghash, tag);
-
-    return 0;
 }
 
 #if SOC_AES_SUPPORT_GCM
@@ -687,6 +691,10 @@ int esp_aes_gcm_crypt_and_tag( esp_gcm_context *ctx,
     aes_hal_gcm_set_j0(ctx->J0);
 
     ret = esp_aes_process_dma_gcm(&ctx->aes_ctx, input, output, length, aad_head_desc, aad_len);
+    if (ret != 0) {
+        esp_aes_release_hardware();
+        return ret;
+    }
 
     aes_hal_gcm_read_tag(tag, tag_len);