Merge branch 'fix/esp_aes_return_values_v4.4' into 'release/v4.4'

fix(mbedtls): fix return values of esp-aes APIs (v4.4)

See merge request espressif/esp-idf!24807

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

@@ -73,6 +73,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)
 {
@@ -86,7 +90,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];
@@ -190,7 +194,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;
 }
 
@@ -205,6 +208,7 @@ int esp_aes_crypt_cbc(esp_aes_context *ctx,
                       const unsigned char *input,
                       unsigned char *output )
 {
+    int ret = -1;
     uint32_t *output_words = (uint32_t *)output;
     const uint32_t *input_words = (const uint32_t *)input;
     uint32_t *iv_words = (uint32_t *)iv;
@@ -226,7 +230,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];
@@ -247,7 +254,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;
@@ -255,10 +266,11 @@ int esp_aes_crypt_cbc(esp_aes_context *ctx,
             length -= 16;
         }
     }
+    ret = 0;
 
+cleanup:
     esp_aes_release_hardware();
-
+    return ret;
-    return 0;
 }
 
 /*
@@ -272,7 +284,7 @@ int esp_aes_crypt_cfb128(esp_aes_context *ctx,
                          const unsigned char *input,
                          unsigned char *output )
 {
-    int c;
+    int c, ret = -1;
     size_t n = *iv_off;
 
     if (!valid_key_length(ctx)) {
@@ -286,7 +298,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++;
@@ -298,7 +313,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++ );
@@ -308,10 +326,11 @@ int esp_aes_crypt_cfb128(esp_aes_context *ctx,
     }
 
     *iv_off = n;
+    ret = 0;
 
+cleanup:
     esp_aes_release_hardware();
-
+    return ret;
-    return 0;
 }
 
 /*
@@ -324,6 +343,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];
 
@@ -338,7 +358,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;
@@ -352,10 +375,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;
 }
 
 /*
@@ -370,6 +394,7 @@ int esp_aes_crypt_ctr(esp_aes_context *ctx,
                       unsigned char *output )
 {
     int c, i;
+    int ret = -1;
     size_t n = *nc_off;
 
     if (!valid_key_length(ctx)) {
@@ -383,7 +408,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 ) {
@@ -398,10 +426,11 @@ int esp_aes_crypt_ctr(esp_aes_context *ctx,
     }
 
     *nc_off = n;
+    ret = 0;
 
+cleanup:
     esp_aes_release_hardware();
-
+    return ret;
-    return 0;
 }
 
 /*
@@ -414,7 +443,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 (ctx == NULL || iv_off == NULL || iv == NULL ||
@@ -439,7 +468,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];
 
@@ -447,7 +479,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

@@ -181,7 +181,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 */
@@ -235,6 +238,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)
@@ -251,9 +256,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;
         }
     }
 
@@ -261,9 +266,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;
@@ -307,7 +312,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;
@@ -331,7 +342,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;
     }
 
@@ -368,9 +379,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;
@@ -416,6 +427,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;
         }
@@ -449,6 +461,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;
 }
@@ -456,7 +471,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;
@@ -480,7 +501,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;
     }
 
@@ -490,9 +511,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;
@@ -539,6 +560,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;
         }
@@ -566,6 +588,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;
 }
@@ -779,13 +804,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

@@ -389,7 +389,6 @@ int esp_aes_gcm_starts( esp_gcm_context *ctx,
         aes_hal_gcm_calc_hash(ctx->H);
 
         esp_aes_release_hardware();
-
         gcm_gen_table(ctx);
     }
 
@@ -454,7 +453,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, length, &nc_off, nonce_counter, stream, input, output);
+    int ret = esp_aes_crypt_ctr(&ctx->aes_ctx, 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);
@@ -489,9 +491,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;
 }
 
 /* Due to restrictions in the hardware (e.g. need to do the whole conversion in one go),
@@ -657,6 +657,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);