From 1d8e1c4ce4766494aaccdb8aa078c55ebffaeb87 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 8 Mar 2019 16:14:15 +1100 Subject: [PATCH] esp32 hwcrypto: Prevent esp_sha() from disabling interrupts for extended period * Closes https://github.com/espressif/esp-idf/issues/3127 * Closes IDFGH-681 Also reported at https://esp32.com/viewtopic.php?f=13&t=9506 --- components/esp32/hwcrypto/sha.c | 32 +++++++++++++----- components/esp32/test/test_sha.c | 57 ++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 8 deletions(-) create mode 100644 components/esp32/test/test_sha.c diff --git a/components/esp32/hwcrypto/sha.c b/components/esp32/hwcrypto/sha.c index fe42997a1..ac0c8c5e0 100644 --- a/components/esp32/hwcrypto/sha.c +++ b/components/esp32/hwcrypto/sha.c @@ -309,26 +309,42 @@ void esp_sha(esp_sha_type sha_type, const unsigned char *input, size_t ilen, uns SHA_CTX ctx; ets_sha_init(&ctx); - esp_sha_lock_memory_block(); + while(ilen > 0) { size_t chunk_len = (ilen > block_len) ? block_len : ilen; + + // Wait for idle before entering critical section + // (to reduce time spent in it), then check after esp_sha_wait_idle(); + esp_sha_lock_memory_block(); + esp_sha_wait_idle(); + DPORT_STALL_OTHER_CPU_START(); { // This SHA ROM function reads DPORT regs ets_sha_update(&ctx, sha_type, input, chunk_len * 8); } DPORT_STALL_OTHER_CPU_END(); + input += chunk_len; ilen -= chunk_len; + + if (ilen == 0) { + /* Finish the last block before releasing the memory + block lock, as ets_sha_update() may have written data to + the memory block already (partial last block) and hardware + hasn't yet processed it. + */ + DPORT_STALL_OTHER_CPU_START(); + { + // This SHA ROM function also reads DPORT regs + ets_sha_finish(&ctx, sha_type, output); + } + DPORT_STALL_OTHER_CPU_END(); + } + + esp_sha_unlock_memory_block(); } - esp_sha_wait_idle(); - DPORT_STALL_OTHER_CPU_START(); - { - ets_sha_finish(&ctx, sha_type, output); - } - DPORT_STALL_OTHER_CPU_END(); - esp_sha_unlock_memory_block(); esp_sha_unlock_engine(sha_type); } diff --git a/components/esp32/test/test_sha.c b/components/esp32/test/test_sha.c new file mode 100644 index 000000000..5f85c74fd --- /dev/null +++ b/components/esp32/test/test_sha.c @@ -0,0 +1,57 @@ +#include +#include +#include +#include "esp_types.h" +#include "esp_clk.h" + +#include "unity.h" +#include "test_utils.h" +#include "mbedtls/sha1.h" +#include "mbedtls/sha256.h" +#include "mbedtls/sha512.h" +#include "hwcrypto/sha.h" + +/* Note: Most of the SHA functions are called as part of mbedTLS, so +are tested as part of mbedTLS tests. Only esp_sha() is different. +*/ + +TEST_CASE("Test esp_sha() function", "[hw_crypto]") +{ + const void* ptr; + spi_flash_mmap_handle_t handle; + uint8_t sha1_espsha[20] = { 0 }; + uint8_t sha1_mbedtls[20] = { 0 }; + uint8_t sha256_espsha[32] = { 0 }; + uint8_t sha256_mbedtls[32] = { 0 }; + uint8_t sha512_espsha[64] = { 0 }; + uint8_t sha512_mbedtls[64] = { 0 }; + + const size_t LEN = 1024 * 1024; + + /* mmap() 1MB of flash, we don't care what it is really */ + esp_err_t err = spi_flash_mmap(0x0, LEN, SPI_FLASH_MMAP_DATA, &ptr, &handle); + + TEST_ASSERT_EQUAL_HEX32(ESP_OK, err); + TEST_ASSERT_NOT_NULL(ptr); + + /* Compare esp_sha() result to the mbedTLS result, should always be the same */ + + esp_sha(SHA1, ptr, LEN, sha1_espsha); + int r = mbedtls_sha1_ret(ptr, LEN, sha1_mbedtls); + TEST_ASSERT_EQUAL(0, r); + + esp_sha(SHA2_256, ptr, LEN, sha256_espsha); + r = mbedtls_sha256_ret(ptr, LEN, sha256_mbedtls, 0); + TEST_ASSERT_EQUAL(0, r); + + esp_sha(SHA2_512, ptr, LEN, sha512_espsha); + r = mbedtls_sha512_ret(ptr, LEN, sha512_mbedtls, 0); + TEST_ASSERT_EQUAL(0, r); + + TEST_ASSERT_EQUAL_MEMORY_MESSAGE(sha1_espsha, sha1_mbedtls, sizeof(sha1_espsha), "SHA1 results should match"); + + TEST_ASSERT_EQUAL_MEMORY_MESSAGE(sha256_espsha, sha256_mbedtls, sizeof(sha256_espsha), "SHA256 results should match"); + + TEST_ASSERT_EQUAL_MEMORY_MESSAGE(sha512_espsha, sha512_mbedtls, sizeof(sha512_espsha), "SHA512 results should match"); +} +