From 2c531d5bb3bd5cc5b72b20c6149c3780ad465af2 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 24 Apr 2020 14:40:24 +1000 Subject: [PATCH 1/3] secure boot v2: Don't log warnings when BLK2 is empty as expected If BLK2 is empty then it's OK to continue with a warning (otherwise it may spook users into thinking something this is wrong, but this is the expected workflow.) If BLK2 is not empty and doesn't match then we need to fail because it won't be possible to trust the signature. --- .../src/esp32/secure_boot_signatures.c | 13 ++++++++++++- .../src/idf/secure_boot_signatures.c | 9 ++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/components/bootloader_support/src/esp32/secure_boot_signatures.c b/components/bootloader_support/src/esp32/secure_boot_signatures.c index 30cebbcf58..57dc55ba10 100644 --- a/components/bootloader_support/src/esp32/secure_boot_signatures.c +++ b/components/bootloader_support/src/esp32/secure_boot_signatures.c @@ -152,7 +152,18 @@ esp_err_t esp_secure_boot_verify_rsa_signature_block(const ets_secure_boot_signa bootloader_sha256_finish(sig_block_sha, (unsigned char *)sig_block_trusted_digest); if (memcmp(efuse_trusted_digest, sig_block_trusted_digest, DIGEST_LEN) != 0) { - ESP_LOGW(TAG, "Public key digest in eFuse BLK2 and the signature block don't match."); + /* Most likely explanation for this is that BLK2 is empty, and we're going to burn it + after we verify that the signature is valid. However, if BLK2 is not empty then we need to + fail here. + */ + bool all_zeroes = true; + for (int i = 0; i < DIGEST_LEN; i++) { + all_zeroes = all_zeroes && (efuse_trusted_digest[i] == 0); + } + if (!all_zeroes) { + ESP_LOGE(TAG, "Different public key digest burned to eFuse BLK2"); + return ESP_ERR_INVALID_STATE; + } } memcpy(efuse_trusted_digest, sig_block_trusted_digest, DIGEST_LEN); diff --git a/components/bootloader_support/src/idf/secure_boot_signatures.c b/components/bootloader_support/src/idf/secure_boot_signatures.c index d2021cf1a5..27b1f7af33 100644 --- a/components/bootloader_support/src/idf/secure_boot_signatures.c +++ b/components/bootloader_support/src/idf/secure_boot_signatures.c @@ -187,11 +187,14 @@ esp_err_t esp_secure_boot_verify_rsa_signature_block(const ets_secure_boot_signa bootloader_sha256_finish(sig_block_sha, (unsigned char *)sig_block_trusted_digest); if (memcmp(efuse_trusted_digest, sig_block_trusted_digest, DIGEST_LEN) != 0) { - if (esp_secure_boot_enabled()) { + const uint8_t zeroes[DIGEST_LEN] = {0}; + /* Can't continue if secure boot is enabled, OR if a different digest is already written in efuse BLK2 + + (If BLK2 is empty and Secure Boot is disabled then we assume that it will be enabled later.) + */ + if (esp_secure_boot_enabled() || memcmp(efuse_trusted_digest, zeroes, DIGEST_LEN) != 0) { ESP_LOGE(TAG, "Public key digest in eFuse BLK2 and the signature block don't match."); return ESP_FAIL; - } else { - ESP_LOGW(TAG, "Public key digest in eFuse BLK2 and the signature block don't match."); } } From 3c6b1b4c0aac8949b63391d2653a2d028b23aa07 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 24 Apr 2020 14:41:42 +1000 Subject: [PATCH 2/3] secure boot v2: Don't check efuse BLK2 if only boot-time signature verification is enabled --- .../bootloader_support/src/esp32/secure_boot_signatures.c | 6 ++++-- .../bootloader_support/src/idf/secure_boot_signatures.c | 6 +++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/components/bootloader_support/src/esp32/secure_boot_signatures.c b/components/bootloader_support/src/esp32/secure_boot_signatures.c index 57dc55ba10..7051de9e79 100644 --- a/components/bootloader_support/src/esp32/secure_boot_signatures.c +++ b/components/bootloader_support/src/esp32/secure_boot_signatures.c @@ -137,13 +137,13 @@ esp_err_t esp_secure_boot_verify_signature(uint32_t src_addr, uint32_t length) esp_err_t esp_secure_boot_verify_rsa_signature_block(const ets_secure_boot_signature_t *sig_block, const uint8_t *image_digest, uint8_t *verified_digest) { + secure_boot_v2_status_t r; uint8_t efuse_trusted_digest[DIGEST_LEN] = {0}, sig_block_trusted_digest[DIGEST_LEN] = {0}; - secure_boot_v2_status_t r; memcpy(efuse_trusted_digest, (uint8_t *)EFUSE_BLK2_RDATA0_REG, DIGEST_LEN); /* EFUSE_BLK2_RDATA0_REG - Stores the Secure Boot Public Key Digest */ if (!ets_use_secure_boot_v2()) { - ESP_LOGI(TAG, "Secure Boot EFuse bit(ABS_DONE_1) not yet programmed."); + ESP_LOGI(TAG, "Secure Boot eFuse bit(ABS_DONE_1) not yet programmed."); /* Generating the SHA of the public key components in the signature block */ bootloader_sha256_handle_t sig_block_sha; @@ -151,6 +151,7 @@ esp_err_t esp_secure_boot_verify_rsa_signature_block(const ets_secure_boot_signa bootloader_sha256_data(sig_block_sha, &sig_block->block[0].key, sizeof(sig_block->block[0].key)); bootloader_sha256_finish(sig_block_sha, (unsigned char *)sig_block_trusted_digest); +#if CONFIG_SECURE_BOOT_V2_ENABLED if (memcmp(efuse_trusted_digest, sig_block_trusted_digest, DIGEST_LEN) != 0) { /* Most likely explanation for this is that BLK2 is empty, and we're going to burn it after we verify that the signature is valid. However, if BLK2 is not empty then we need to @@ -165,6 +166,7 @@ esp_err_t esp_secure_boot_verify_rsa_signature_block(const ets_secure_boot_signa return ESP_ERR_INVALID_STATE; } } +#endif memcpy(efuse_trusted_digest, sig_block_trusted_digest, DIGEST_LEN); } diff --git a/components/bootloader_support/src/idf/secure_boot_signatures.c b/components/bootloader_support/src/idf/secure_boot_signatures.c index 27b1f7af33..ee541445bd 100644 --- a/components/bootloader_support/src/idf/secure_boot_signatures.c +++ b/components/bootloader_support/src/idf/secure_boot_signatures.c @@ -173,7 +173,10 @@ esp_err_t esp_secure_boot_verify_signature(uint32_t src_addr, uint32_t length) esp_err_t esp_secure_boot_verify_rsa_signature_block(const ets_secure_boot_signature_t *sig_block, const uint8_t *image_digest, uint8_t *verified_digest) { - uint8_t i = 0, efuse_trusted_digest[DIGEST_LEN] = {0}, sig_block_trusted_digest[DIGEST_LEN] = {0}; + int i = 0; + +#if CONFIG_SECURE_BOOT_V2_ENABLED /* Verify key against efuse block */ + uint8_t efuse_trusted_digest[DIGEST_LEN] = {0}, sig_block_trusted_digest[DIGEST_LEN] = {0}; memcpy(efuse_trusted_digest, (uint8_t *) EFUSE_BLK2_RDATA0_REG, sizeof(efuse_trusted_digest)); /* Note: in IDF verification we don't add any fault injection resistance, as we don't expect this to be called @@ -197,6 +200,7 @@ esp_err_t esp_secure_boot_verify_rsa_signature_block(const ets_secure_boot_signa return ESP_FAIL; } } +#endif ESP_LOGI(TAG, "Verifying with RSA-PSS..."); int ret = 0; From b00f38f91c22ae1e658e3a231ec5bc837b0f459f Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 24 Apr 2020 14:42:29 +1000 Subject: [PATCH 3/3] secure boot v2: Add anti-FI check that secure boot not enabled yet Prevent a fault from causing bootloader to trust the provided signature incorrectly. --- .../bootloader_support/src/esp32/secure_boot_signatures.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/bootloader_support/src/esp32/secure_boot_signatures.c b/components/bootloader_support/src/esp32/secure_boot_signatures.c index 7051de9e79..1ff4999b85 100644 --- a/components/bootloader_support/src/esp32/secure_boot_signatures.c +++ b/components/bootloader_support/src/esp32/secure_boot_signatures.c @@ -166,6 +166,8 @@ esp_err_t esp_secure_boot_verify_rsa_signature_block(const ets_secure_boot_signa return ESP_ERR_INVALID_STATE; } } + + ESP_FAULT_ASSERT(!ets_use_secure_boot_v2()); #endif memcpy(efuse_trusted_digest, sig_block_trusted_digest, DIGEST_LEN);