From 7c5dd19c83bf8495b8a287a6b35e8b657532e5b2 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Sat, 18 May 2019 03:54:40 +0800 Subject: [PATCH 1/3] hwcrypto: Add AES fault injection check Hardware AES-CBC performance changes: Release config 11.0MB/sec -> 10.8MB/sec Debug config 9.4MB/sec -> 9.8MB/sec (Unrolling the loop to optimize the check improves performance at -Og, even with the fault check.) --- components/esp32/hwcrypto/aes.c | 39 ++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/components/esp32/hwcrypto/aes.c b/components/esp32/hwcrypto/aes.c index e51e1aefc..a3fc95910 100644 --- a/components/esp32/hwcrypto/aes.c +++ b/components/esp32/hwcrypto/aes.c @@ -28,6 +28,7 @@ #include #include "mbedtls/aes.h" #include "hwcrypto/aes.h" +#include "mbedtls/platform_util.h" #include "soc/dport_reg.h" #include "soc/hwcrypto_reg.h" #include @@ -118,19 +119,45 @@ static inline void esp_aes_setkey_hardware( esp_aes_context *ctx, int mode) * * Call only while holding esp_aes_acquire_hardware(). */ -static inline void esp_aes_block(const void *input, void *output) +static void esp_aes_block(const void *input, void *output) { const uint32_t *input_words = (const uint32_t *)input; + uint32_t i0, i1, i2, i3; uint32_t *output_words = (uint32_t *)output; - uint32_t *mem_block = (uint32_t *)AES_TEXT_BASE; - for(int i = 0; i < 4; i++) { - mem_block[i] = input_words[i]; - } + /* Storing i0,i1,i2,i3 in registers not an array + helps a lot with optimisations at -Os level */ + i0 = input_words[0]; + DPORT_REG_WRITE(AES_TEXT_BASE, i0); + + i1 = input_words[1]; + DPORT_REG_WRITE(AES_TEXT_BASE + 4, i1); + + i2 = input_words[2]; + DPORT_REG_WRITE(AES_TEXT_BASE + 8, i2); + + i3 = input_words[3]; + DPORT_REG_WRITE(AES_TEXT_BASE + 12, i3); DPORT_REG_WRITE(AES_START_REG, 1); + while (DPORT_REG_READ(AES_IDLE_REG) != 1) { } - esp_dport_access_read_buffer(output_words, (uint32_t)&mem_block[0], 4); + + esp_dport_access_read_buffer(output_words, AES_TEXT_BASE, 4); + + /* Physical security check: Verify the AES accelerator actually ran, and wasn't + skipped due to external fault injection while starting the peripheral. + + Note that i0,i1,i2,i3 are copied from input buffer in case input==output. + + Bypassing this check requires at least one additional fault. + */ + if(i0 == output_words[0] && i1 == output_words[1] && i2 == output_words[2] && i3 == output_words[3]) { + // calling two zeroing functions to narrow the + // window for a double-fault here + memset(output, 0, 16); + mbedtls_platform_zeroize(output, 16); + } } /* From 088439c634490b99b470418bd4a21404f6902ac6 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Tue, 21 May 2019 18:12:42 +1000 Subject: [PATCH 2/3] aes: Add fault injection checks when writing key to hardware Vulnerability reported by LimitedResults under Espressif Bug Bounty Program. --- components/esp32/hwcrypto/aes.c | 102 ++++++++++++++++++++---- components/esp32/include/hwcrypto/aes.h | 6 +- 2 files changed, 86 insertions(+), 22 deletions(-) diff --git a/components/esp32/hwcrypto/aes.c b/components/esp32/hwcrypto/aes.c index a3fc95910..3bf90a9bc 100644 --- a/components/esp32/hwcrypto/aes.c +++ b/components/esp32/hwcrypto/aes.c @@ -50,6 +50,11 @@ */ static portMUX_TYPE aes_spinlock = portMUX_INITIALIZER_UNLOCKED; +static inline bool valid_key_length(const esp_aes_context *ctx) +{ + return ctx->key_bytes == 128/8 || ctx->key_bytes == 192/8 || ctx->key_bytes == 256/8; +} + void esp_aes_acquire_hardware( void ) { portENTER_CRITICAL(&aes_spinlock); @@ -94,6 +99,7 @@ int esp_aes_setkey( esp_aes_context *ctx, const unsigned char *key, } ctx->key_bytes = keybits / 8; memcpy(ctx->key, key, ctx->key_bytes); + ctx->key_in_hardware = 0; return 0; } @@ -103,28 +109,47 @@ int esp_aes_setkey( esp_aes_context *ctx, const unsigned char *key, * * Call only while holding esp_aes_acquire_hardware(). */ -static inline void esp_aes_setkey_hardware( esp_aes_context *ctx, int mode) +static void esp_aes_setkey_hardware(esp_aes_context *ctx, int mode) { const uint32_t MODE_DECRYPT_BIT = 4; unsigned mode_reg_base = (mode == ESP_AES_ENCRYPT) ? 0 : MODE_DECRYPT_BIT; + ctx->key_in_hardware = 0; + for (int i = 0; i < ctx->key_bytes/4; ++i) { DPORT_REG_WRITE(AES_KEY_BASE + i * 4, *(((uint32_t *)ctx->key) + i)); + ctx->key_in_hardware += 4; } DPORT_REG_WRITE(AES_MODE_REG, mode_reg_base + ((ctx->key_bytes / 8) - 2)); + + /* Fault injection check: all words of key data should have been written to hardware */ + if (ctx->key_in_hardware < 16 + || ctx->key_in_hardware != ctx->key_bytes) { + abort(); + } } /* Run a single 16 byte block of AES, using the hardware engine. * * Call only while holding esp_aes_acquire_hardware(). */ -static void esp_aes_block(const void *input, void *output) +static int esp_aes_block(esp_aes_context *ctx, const void *input, void *output) { const uint32_t *input_words = (const uint32_t *)input; uint32_t i0, i1, i2, i3; uint32_t *output_words = (uint32_t *)output; + /* If no key is written to hardware yet, either the user hasn't called + mbedtls_aes_setkey_enc/mbedtls_aes_setkey_dec - meaning we also don't + know which mode to use - or a fault skipped the + 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); + return MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH; + } + /* Storing i0,i1,i2,i3 in registers not an array helps a lot with optimisations at -Os level */ i0 = input_words[0]; @@ -153,11 +178,14 @@ static void esp_aes_block(const void *input, void *output) Bypassing this check requires at least one additional fault. */ if(i0 == output_words[0] && i1 == output_words[1] && i2 == output_words[2] && i3 == output_words[3]) { - // calling two zeroing functions to narrow the - // window for a double-fault here + // calling zeroing functions to narrow the + // window for a double-fault of the abort step, here memset(output, 0, 16); mbedtls_platform_zeroize(output, 16); + abort(); } + + return 0; } /* @@ -167,11 +195,18 @@ int esp_internal_aes_encrypt( esp_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ) { + int r; + + if (!valid_key_length(ctx)) { + return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH; + } + esp_aes_acquire_hardware(); + ctx->key_in_hardware = 0; esp_aes_setkey_hardware(ctx, ESP_AES_ENCRYPT); - esp_aes_block(input, output); + r = esp_aes_block(ctx, input, output); esp_aes_release_hardware(); - return 0; + return r; } void esp_aes_encrypt( esp_aes_context *ctx, @@ -189,11 +224,18 @@ int esp_internal_aes_decrypt( esp_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ) { + int r; + + if (!valid_key_length(ctx)) { + return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH; + } + esp_aes_acquire_hardware(); + ctx->key_in_hardware = 0; esp_aes_setkey_hardware(ctx, ESP_AES_DECRYPT); - esp_aes_block(input, output); + r = esp_aes_block(ctx, input, output); esp_aes_release_hardware(); - return 0; + return r; } void esp_aes_decrypt( esp_aes_context *ctx, @@ -203,7 +245,6 @@ void esp_aes_decrypt( esp_aes_context *ctx, esp_internal_aes_decrypt(ctx, input, output); } - /* * AES-ECB block encryption/decryption */ @@ -212,12 +253,19 @@ int esp_aes_crypt_ecb( esp_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ) { + int r; + + if (!valid_key_length(ctx)) { + return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH; + } + esp_aes_acquire_hardware(); + ctx->key_in_hardware = 0; esp_aes_setkey_hardware(ctx, mode); - esp_aes_block(input, output); + r = esp_aes_block(ctx, input, output); esp_aes_release_hardware(); - return 0; + return r; } @@ -241,14 +289,19 @@ int esp_aes_crypt_cbc( esp_aes_context *ctx, return ( ERR_ESP_AES_INVALID_INPUT_LENGTH ); } + if (!valid_key_length(ctx)) { + return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH; + } + esp_aes_acquire_hardware(); + ctx->key_in_hardware = 0; esp_aes_setkey_hardware(ctx, mode); if ( mode == ESP_AES_DECRYPT ) { while ( length > 0 ) { memcpy(temp, input_words, 16); - esp_aes_block(input_words, output_words); + esp_aes_block(ctx, input_words, output_words); for ( i = 0; i < 4; i++ ) { output_words[i] = output_words[i] ^ iv_words[i]; @@ -267,7 +320,7 @@ int esp_aes_crypt_cbc( esp_aes_context *ctx, output_words[i] = input_words[i] ^ iv_words[i]; } - esp_aes_block(output_words, output_words); + esp_aes_block(ctx, output_words, output_words); memcpy( iv_words, output_words, 16 ); input_words += 4; @@ -295,14 +348,19 @@ int esp_aes_crypt_cfb128( esp_aes_context *ctx, int c; size_t n = *iv_off; + if (!valid_key_length(ctx)) { + return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH; + } + esp_aes_acquire_hardware(); + ctx->key_in_hardware = 0; esp_aes_setkey_hardware(ctx, ESP_AES_ENCRYPT); if ( mode == ESP_AES_DECRYPT ) { while ( length-- ) { if ( n == 0 ) { - esp_aes_block(iv, iv ); + esp_aes_block(ctx, iv, iv ); } c = *input++; @@ -314,7 +372,7 @@ int esp_aes_crypt_cfb128( esp_aes_context *ctx, } else { while ( length-- ) { if ( n == 0 ) { - esp_aes_block(iv, iv ); + esp_aes_block(ctx, iv, iv ); } iv[n] = *output++ = (unsigned char)( iv[n] ^ *input++ ); @@ -343,13 +401,18 @@ int esp_aes_crypt_cfb8( esp_aes_context *ctx, unsigned char c; unsigned char ov[17]; + if (!valid_key_length(ctx)) { + return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH; + } + esp_aes_acquire_hardware(); + ctx->key_in_hardware = 0; esp_aes_setkey_hardware(ctx, ESP_AES_ENCRYPT); while ( length-- ) { memcpy( ov, iv, 16 ); - esp_aes_block(iv, iv); + esp_aes_block(ctx, iv, iv); if ( mode == ESP_AES_DECRYPT ) { ov[16] = *input; @@ -383,13 +446,18 @@ int esp_aes_crypt_ctr( esp_aes_context *ctx, int c, i; size_t n = *nc_off; + if (!valid_key_length(ctx)) { + return MBEDTLS_ERR_AES_INVALID_KEY_LENGTH; + } + esp_aes_acquire_hardware(); + ctx->key_in_hardware = 0; esp_aes_setkey_hardware(ctx, ESP_AES_ENCRYPT); while ( length-- ) { if ( n == 0 ) { - esp_aes_block(nonce_counter, stream_block); + esp_aes_block(ctx, nonce_counter, stream_block); for ( i = 16; i > 0; i-- ) if ( ++nonce_counter[i - 1] != 0 ) { diff --git a/components/esp32/include/hwcrypto/aes.h b/components/esp32/include/hwcrypto/aes.h index 1dd5291f0..5a3fd24fe 100644 --- a/components/esp32/include/hwcrypto/aes.h +++ b/components/esp32/include/hwcrypto/aes.h @@ -41,17 +41,13 @@ extern "C" { /** * \brief AES context structure * - * \note buf is able to hold 32 extra bytes, which can be used: - * - for alignment purposes if VIA padlock is used, and/or - * - to simplify key expansion in the 256-bit case by - * generating an extra round key */ typedef struct { uint8_t key_bytes; + volatile uint8_t key_in_hardware; /* This variable is used for fault injection checks, so marked volatile to avoid optimisation */ uint8_t key[32]; } esp_aes_context; - /** * \brief The AES XTS context-type definition. */ From 3991084777cfcc26724e5c405d63e6eac6b07782 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 22 May 2019 10:18:55 +1000 Subject: [PATCH 3/3] sha: Add fault injection checks reading hash digest state Vulnerability reported by LimitedResults under Espressif Bug Bounty Program. --- components/esp32/hwcrypto/sha.c | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/components/esp32/hwcrypto/sha.c b/components/esp32/hwcrypto/sha.c index 7a064340a..3ffc43078 100644 --- a/components/esp32/hwcrypto/sha.c +++ b/components/esp32/hwcrypto/sha.c @@ -226,6 +226,9 @@ void esp_sha_wait_idle(void) void esp_sha_read_digest_state(esp_sha_type sha_type, void *digest_state) { + uint32_t *digest_state_words = NULL; + uint32_t *reg_addr_buf = NULL; + uint32_t word_len = sha_length(sha_type)/4; #ifndef NDEBUG { SemaphoreHandle_t *engine_state = sha_get_engine_state(sha_type); @@ -243,20 +246,30 @@ void esp_sha_read_digest_state(esp_sha_type sha_type, void *digest_state) DPORT_REG_WRITE(SHA_LOAD_REG(sha_type), 1); while(DPORT_REG_READ(SHA_BUSY_REG(sha_type)) == 1) { } - uint32_t *digest_state_words = (uint32_t *)digest_state; - uint32_t *reg_addr_buf = (uint32_t *)(SHA_TEXT_BASE); + digest_state_words = (uint32_t *)digest_state; + reg_addr_buf = (uint32_t *)(SHA_TEXT_BASE); if(sha_type == SHA2_384 || sha_type == SHA2_512) { /* for these ciphers using 64-bit states, swap each pair of words */ DPORT_INTERRUPT_DISABLE(); // Disable interrupt only on current CPU. - for(int i = 0; i < sha_length(sha_type)/4; i += 2) { + for(int i = 0; i < word_len; i += 2) { digest_state_words[i+1] = DPORT_SEQUENCE_REG_READ((uint32_t)®_addr_buf[i]); digest_state_words[i] = DPORT_SEQUENCE_REG_READ((uint32_t)®_addr_buf[i+1]); } DPORT_INTERRUPT_RESTORE(); // restore the previous interrupt level } else { - esp_dport_access_read_buffer(digest_state_words, (uint32_t)®_addr_buf[0], sha_length(sha_type)/4); + esp_dport_access_read_buffer(digest_state_words, (uint32_t)®_addr_buf[0], word_len); } esp_sha_unlock_memory_block(); + + /* Fault injection check: verify SHA engine actually ran, + state is not all zeroes. + */ + for (int i = 0; i < word_len; i++) { + if (digest_state_words[i] != 0) { + return; + } + } + abort(); // SHA peripheral returned all zero state, probably due to fault injection } void esp_sha_block(esp_sha_type sha_type, const void *data_block, bool is_first_block) @@ -310,6 +323,8 @@ void esp_sha(esp_sha_type sha_type, const unsigned char *input, size_t ilen, uns esp_sha_lock_engine(sha_type); + bzero(output, sha_length(sha_type)); + SHA_CTX ctx; ets_sha_init(&ctx); @@ -353,4 +368,14 @@ void esp_sha(esp_sha_type sha_type, const unsigned char *input, size_t ilen, uns } esp_sha_unlock_engine(sha_type); + + /* Fault injection check: verify SHA engine actually ran, + state is not all zeroes. + */ + for (int i = 0; i < sha_length(sha_type); i++) { + if (output[i] != 0) { + return; + } + } + abort(); // SHA peripheral returned all zero state, probably due to fault injection }