Merge branch 'fix/mbedtls_port_sanity_checks_and_return_values' into 'master'

mbedtls/port: refactor sanity checks and their return values

Closes IDF-3810

See merge request espressif/esp-idf!21987

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

@@ -28,6 +28,7 @@
 #include <string.h>
 #include "mbedtls/aes.h"
 #include "mbedtls/platform_util.h"
+#include "esp_log.h"
 #include "aes/esp_aes.h"
 #include "soc/hwcrypto_periph.h"
 #include <sys/lock.h>
@@ -40,6 +41,7 @@
 #include "esp_private/periph_ctrl.h"
 
 
+static const char *TAG = "esp-aes";
 /* AES uses a spinlock mux not a lock as the underlying block operation
    only takes 208 cycles (to write key & compute block), +600 cycles
    for DPORT protection but +3400 cycles again if you use a full sized lock.
@@ -113,6 +115,26 @@ static int esp_aes_block(esp_aes_context *ctx, const void *input, void *output)
     return 0;
 }
 
+static int esp_aes_validate_input(esp_aes_context *ctx, const unsigned char *input,
+                                  const unsigned char *output )
+{
+    if (!ctx) {
+        ESP_LOGD(TAG, "No AES context supplied");
+        return -1;
+    }
+    if (!input) {
+        ESP_LOGD(TAG, "No input supplied");
+        return -1;
+    }
+    if (!output) {
+        ESP_LOGD(TAG, "No output supplied");
+        return -1;
+    }
+
+    return 0;
+}
+
+
 void esp_aes_encrypt(esp_aes_context *ctx,
                      const unsigned char input[16],
                      unsigned char output[16] )
@@ -129,6 +151,10 @@ int esp_internal_aes_encrypt(esp_aes_context *ctx,
 {
     int r;
 
+    if (esp_aes_validate_input(ctx, input, output)) {
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
     if (!valid_key_length(ctx)) {
         return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH;
     }
@@ -158,6 +184,10 @@ int esp_internal_aes_decrypt(esp_aes_context *ctx,
 {
     int r;
 
+    if (esp_aes_validate_input(ctx, input, output)) {
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
     if (!valid_key_length(ctx)) {
         return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH;
     }
@@ -180,6 +210,10 @@ int esp_aes_crypt_ecb(esp_aes_context *ctx,
 {
     int r;
 
+    if (esp_aes_validate_input(ctx, input, output)) {
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
     if (!valid_key_length(ctx)) {
         return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH;
     }
@@ -204,6 +238,15 @@ int esp_aes_crypt_cbc(esp_aes_context *ctx,
                       const unsigned char *input,
                       unsigned char *output )
 {
+    if (esp_aes_validate_input(ctx, input, output)) {
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
+    if (!iv) {
+        ESP_LOGD(TAG, "No IV supplied");
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
     uint32_t *output_words = (uint32_t *)output;
     const uint32_t *input_words = (const uint32_t *)input;
     uint32_t *iv_words = (uint32_t *)iv;
@@ -274,6 +317,20 @@ int esp_aes_crypt_cfb128(esp_aes_context *ctx,
     int c;
     size_t n = *iv_off;
 
+    if (esp_aes_validate_input(ctx, input, output)) {
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
+    if (!iv) {
+        ESP_LOGE(TAG, "No IV supplied");
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
+    if (!iv_off) {
+        ESP_LOGE(TAG, "No IV offset supplied");
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
     if (!valid_key_length(ctx)) {
         return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH;
     }
@@ -326,6 +383,15 @@ int esp_aes_crypt_cfb8(esp_aes_context *ctx,
     unsigned char c;
     unsigned char ov[17];
 
+    if (esp_aes_validate_input(ctx, input, output)) {
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
+    if (!iv) {
+        ESP_LOGE(TAG, "No IV supplied");
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
     if (!valid_key_length(ctx)) {
         return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH;
     }
@@ -369,8 +435,27 @@ int esp_aes_crypt_ctr(esp_aes_context *ctx,
                       unsigned char *output )
 {
     int c, i;
-    size_t n = *nc_off;
 
+    if (esp_aes_validate_input(ctx, input, output)) {
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
+    if (!stream_block) {
+        ESP_LOGE(TAG, "No stream supplied");
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
+    if (!nonce_counter) {
+        ESP_LOGE(TAG, "No nonce supplied");
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
+    if (!nc_off) {
+        ESP_LOGE(TAG, "No nonce offset supplied");
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
+    size_t n = *nc_off;
     if (!valid_key_length(ctx)) {
         return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH;
     }
@@ -416,8 +501,17 @@ int esp_aes_crypt_ofb(esp_aes_context *ctx,
     int ret = 0;
     size_t n;
 
-    if (ctx == NULL || iv_off == NULL || iv == NULL ||
+    if (esp_aes_validate_input(ctx, input, output)) {
-            input == NULL || output == NULL ) {
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
+    if (!iv) {
+        ESP_LOGE(TAG, "No IV supplied");
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
+    if (!iv_off) {
+        ESP_LOGE(TAG, "No IV offset supplied");
         return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 

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

@@ -606,7 +606,7 @@ int esp_internal_aes_encrypt(esp_aes_context *ctx,
     int r;
 
     if (esp_aes_validate_input(ctx, input, output)) {
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
     if (!valid_key_length(ctx)) {
@@ -640,7 +640,7 @@ int esp_internal_aes_decrypt(esp_aes_context *ctx,
     int r;
 
     if (esp_aes_validate_input(ctx, input, output)) {
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
     if (!valid_key_length(ctx)) {
@@ -676,7 +676,7 @@ int esp_aes_crypt_ecb(esp_aes_context *ctx,
     int r;
 
     if (esp_aes_validate_input(ctx, input, output)) {
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
     if (!valid_key_length(ctx)) {
@@ -705,12 +705,12 @@ int esp_aes_crypt_cbc(esp_aes_context *ctx,
 {
     int r = 0;
     if (esp_aes_validate_input(ctx, input, output)) {
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
     if (!iv) {
         ESP_LOGE(TAG, "No IV supplied");
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
     /* For CBC input length should be multiple of
@@ -758,12 +758,12 @@ int esp_aes_crypt_cfb8(esp_aes_context *ctx,
     size_t block_bytes = length - (length % AES_BLOCK_BYTES);
 
     if (esp_aes_validate_input(ctx, input, output)) {
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
     if (!iv) {
         ESP_LOGE(TAG, "No IV supplied");
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
 
@@ -846,17 +846,17 @@ int esp_aes_crypt_cfb128(esp_aes_context *ctx,
     size_t n;
 
     if (esp_aes_validate_input(ctx, input, output)) {
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
     if (!iv) {
         ESP_LOGE(TAG, "No IV supplied");
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
     if (!iv_off) {
         ESP_LOGE(TAG, "No IV offset supplied");
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
     if (!valid_key_length(ctx)) {
@@ -931,17 +931,17 @@ int esp_aes_crypt_ofb(esp_aes_context *ctx,
     size_t stream_bytes = 0;
 
     if (esp_aes_validate_input(ctx, input, output)) {
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
     if (!iv) {
         ESP_LOGE(TAG, "No IV supplied");
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
     if (!iv_off) {
         ESP_LOGE(TAG, "No IV offset supplied");
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
     n = *iv_off;
@@ -992,17 +992,22 @@ int esp_aes_crypt_ctr(esp_aes_context *ctx,
     size_t n;
 
     if (esp_aes_validate_input(ctx, input, output)) {
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
+    }
+
+    if (!stream_block) {
+        ESP_LOGE(TAG, "No stream supplied");
         return -1;
     }
 
     if (!nonce_counter) {
         ESP_LOGE(TAG, "No nonce supplied");
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
     if (!nc_off) {
         ESP_LOGE(TAG, "No nonce offset supplied");
-        return -1;
+        return MBEDTLS_ERR_AES_BAD_INPUT_DATA;
     }
 
     n = *nc_off;

components/mbedtls/port/aes/esp_aes_gcm.c

@@ -24,6 +24,7 @@
 
 #include "esp_log.h"
 #include "mbedtls/aes.h"
+#include "mbedtls/gcm.h"
 #include "esp_heap_caps.h"
 #include "soc/soc_memory_layout.h"
 
@@ -340,12 +341,12 @@ int esp_aes_gcm_starts( esp_gcm_context *ctx,
 
     if (!ctx) {
         ESP_LOGE(TAG, "No AES context supplied");
-        return -1;
+        return MBEDTLS_ERR_GCM_BAD_INPUT;
     }
 
     if (!iv) {
         ESP_LOGE(TAG, "No IV supplied");
-        return -1;
+        return MBEDTLS_ERR_GCM_BAD_INPUT;
     }
 
     /* Initialize AES-GCM context */
@@ -401,12 +402,12 @@ int esp_aes_gcm_update_ad( esp_gcm_context *ctx,
 
     if (!ctx) {
         ESP_LOGE(TAG, "No AES context supplied");
-        return -1;
+        return MBEDTLS_ERR_GCM_BAD_INPUT;
     }
 
     if ( (aad_len > 0) && !aad) {
         ESP_LOGE(TAG, "No aad supplied");
-        return -1;
+        return MBEDTLS_ERR_GCM_BAD_INPUT;
     }
 
     if (ctx->gcm_state != ESP_AES_GCM_STATE_START) {
@@ -435,21 +436,21 @@ int esp_aes_gcm_update( esp_gcm_context *ctx,
 
     if (!output_length) {
         ESP_LOGE(TAG, "No output length supplied");
-        return -1;
+        return MBEDTLS_ERR_GCM_BAD_INPUT;
     }
     *output_length = input_length;
 
     if (!ctx) {
         ESP_LOGE(TAG, "No GCM context supplied");
-        return -1;
+        return MBEDTLS_ERR_GCM_BAD_INPUT;
     }
     if (!input) {
         ESP_LOGE(TAG, "No input supplied");
-        return -1;
+        return MBEDTLS_ERR_GCM_BAD_INPUT;
     }
     if (!output) {
         ESP_LOGE(TAG, "No output supplied");
-        return -1;
+        return MBEDTLS_ERR_GCM_BAD_INPUT;
     }
 
     if ( output > input && (size_t) ( output - input ) < input_length ) {
@@ -611,7 +612,7 @@ int esp_aes_gcm_crypt_and_tag( esp_gcm_context *ctx,
        In practice, e.g. with mbedtls the length of aad will always be short
     */
     if (aad_len > LLDESC_MAX_NUM_PER_DESC) {
-        return -1;
+        return MBEDTLS_ERR_GCM_BAD_INPUT;
     }
     /* IV and AD are limited to 2^32 bits, so 2^29 bytes */
     /* IV is not allowed to be zero length */
@@ -623,17 +624,17 @@ int esp_aes_gcm_crypt_and_tag( esp_gcm_context *ctx,
 
     if (!ctx) {
         ESP_LOGE(TAG, "No AES context supplied");
-        return -1;
+        return MBEDTLS_ERR_GCM_BAD_INPUT;
     }
 
     if (!iv) {
         ESP_LOGE(TAG, "No IV supplied");
-        return -1;
+        return MBEDTLS_ERR_GCM_BAD_INPUT;
     }
 
     if ( (aad_len > 0) && !aad) {
         ESP_LOGE(TAG, "No aad supplied");
-        return -1;
+        return MBEDTLS_ERR_GCM_BAD_INPUT;
     }
 
     /* Initialize AES-GCM context */

components/mbedtls/port/include/aes/esp_aes_gcm.h

@@ -18,10 +18,6 @@
 extern "C" {
 #endif
 
-
-#define MBEDTLS_ERR_GCM_AUTH_FAILED      -0x0012  /**< Authenticated decryption failed. */
-#define MBEDTLS_ERR_GCM_BAD_INPUT        -0x0014  /**< Bad input parameters to function.*/
-
 typedef enum {
     ESP_AES_GCM_STATE_INIT,
     ESP_AES_GCM_STATE_START,

components/mbedtls/test_apps/main/test_aes_gcm.c

@@ -830,4 +830,56 @@ TEST_CASE("mbedtls AES GCM - Combine different IV/Key/Plaintext/AAD lengths", "[
     }
 }
 
+TEST_CASE("mbedtls AES GCM - Different Authentication Tag lengths", "[aes-gcm]")
+{
+    const unsigned CALL_SZ = 160;
+    uint8_t iv[16];
+    uint8_t key[16];
+    uint8_t aad[16];
+    uint8_t *input = heap_caps_malloc(CALL_SZ, MALLOC_CAP_DMA | MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL);
+    TEST_ASSERT_NOT_NULL(input);
+
+    memset(input, 0x67, CALL_SZ);
+    memset(iv, 0xA2, sizeof(iv));
+    memset(key, 0x48, sizeof(key));
+    memset(aad, 0x12, sizeof(aad));
+
+    size_t tag_len[] = {4, 8, 11, 16};
+
+    const uint8_t expected_last_block[] = {
+        0xcd, 0xb9, 0xad, 0x6f, 0xc9, 0x35, 0x21, 0x0d,
+        0xc9, 0x5d, 0xea, 0xd9, 0xf7, 0x1d, 0x43, 0xed
+    };
+
+    const uint8_t expected_tag[16] = {
+        0x57, 0x10, 0x22, 0x91, 0x65, 0xfa, 0x89, 0xba,
+        0x0a, 0x3e, 0xc1, 0x7c, 0x93, 0x6e, 0x35, 0xac
+    };
+
+    aes_gcm_test_cfg_t cfg = {
+        .plaintext = input,
+        .plaintext_length = CALL_SZ,
+        .output_caps = MALLOC_CAP_DMA | MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL,
+        .add_buf = aad,
+        .add_length = sizeof(aad),
+        .iv = iv,
+        .iv_length = sizeof(iv),
+        .key = key,
+        .key_bits = 8 * sizeof(key),
+    };
+
+    aes_gcm_test_expected_res_t res = {
+        .expected_tag = expected_tag,
+        .ciphertext_last_block = expected_last_block,
+    };
+
+    for (int i = 0; i < sizeof(tag_len) / sizeof(tag_len[0]); i++) {
+        printf("Test AES-GCM with tag length = %d\n", tag_len[i]);
+        cfg.tag_len = tag_len[i];
+        aes_gcm_test(&cfg, &res, AES_GCM_TEST_CRYPT_N_TAG);
+        aes_gcm_test(&cfg, &res, AES_GCM_TEST_START_UPDATE_FINISH);
+    }
+    free(input);
+}
+
 #endif //CONFIG_MBEDTLS_HARDWARE_AES