Skip to content

Commit

Permalink
Merge branch 'feature/micro-ecc-only-in-bootloader' into 'master'
Browse files Browse the repository at this point in the history
Use micro_ecc library only in bootloader

See merge request idf/esp-idf!4082
  • Loading branch information
projectgus committed Apr 4, 2019
2 parents d639481 + fd28549 commit 5136b76
Show file tree
Hide file tree
Showing 11 changed files with 59 additions and 16 deletions.
2 changes: 1 addition & 1 deletion .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ exclude =
__pycache__,
# submodules
components/esptool_py/esptool,
components/micro-ecc/micro-ecc,
components/bootloader/subproject/components/micro-ecc/micro-ecc,
components/nghttp/nghttp2,
components/libsodium/libsodium,
components/json/cJSON,
Expand Down
2 changes: 1 addition & 1 deletion .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ variables:
find . -name '.git' -not -path './.git' -printf '%P\n'
| sed 's|/.git||'
| xargs -I {} sh -c '
grep -q {} .gitmodules
grep -q "path = {}" .gitmodules
|| (echo "Removing {}, has .git directory but not in .gitmodules file"
&& rm -rf {});'

Expand Down
4 changes: 2 additions & 2 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
path = components/bt/lib
url = https://github.com/espressif/esp32-bt-lib.git

[submodule "components/micro-ecc/micro-ecc"]
path = components/micro-ecc/micro-ecc
[submodule "components/bootloader/subproject/components/micro-ecc/micro-ecc"]
path = components/bootloader/subproject/components/micro-ecc/micro-ecc
url = https://github.com/kmackay/micro-ecc.git

[submodule "components/coap/libcoap"]
Expand Down
7 changes: 0 additions & 7 deletions components/app_update/esp_ota_ops.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,6 @@ static esp_err_t image_validate(const esp_partition_t *partition, esp_image_load
return ESP_ERR_OTA_VALIDATE_FAILED;
}

#ifdef CONFIG_SECURE_SIGNED_ON_UPDATE
esp_err_t ret = esp_secure_boot_verify_signature(partition->address, data.image_len);
if (ret != ESP_OK) {
return ESP_ERR_OTA_VALIDATE_FAILED;
}
#endif

return ESP_OK;
}

Expand Down
1 change: 1 addition & 0 deletions components/bootloader/Kconfig.projbuild
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ menu "Security features"
config SECURE_SIGNED_ON_UPDATE
bool
default y
select MBEDTLS_ECP_DP_SECP256R1_ENABLED
depends on SECURE_BOOT_ENABLED || SECURE_SIGNED_ON_UPDATE_NO_SECURE_BOOT

config SECURE_SIGNED_APPS
Expand Down
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion components/bootloader_support/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ else()
set(COMPONENT_ADD_INCLUDEDIRS "include")
set(COMPONENT_PRIV_INCLUDEDIRS "include_bootloader")
set(COMPONENT_REQUIRES)
set(COMPONENT_PRIV_REQUIRES spi_flash mbedtls micro-ecc efuse)
set(COMPONENT_PRIV_REQUIRES spi_flash mbedtls efuse)
endif()

register_component()
55 changes: 52 additions & 3 deletions components/bootloader_support/src/secure_boot_signatures.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@
#include "esp_image_format.h"
#include "esp_secure_boot.h"

#include "uECC.h"

#ifdef BOOTLOADER_BUILD
#include "esp32/rom/sha.h"
#include "uECC.h"
typedef SHA_CTX sha_context;
#else
#include "mbedtls/sha256.h"
#include "mbedtls/x509.h"
#endif

static const char* TAG = "secure_boot";
Expand Down Expand Up @@ -71,7 +71,6 @@ esp_err_t esp_secure_boot_verify_signature(uint32_t src_addr, uint32_t length)
esp_err_t esp_secure_boot_verify_signature_block(const esp_secure_boot_sig_block_t *sig_block, const uint8_t *image_digest)
{
ptrdiff_t keylen;
bool is_valid;

keylen = signature_verification_key_end - signature_verification_key_start;
if(keylen != SIGNATURE_VERIFICATION_KEYLEN) {
Expand All @@ -86,11 +85,61 @@ esp_err_t esp_secure_boot_verify_signature_block(const esp_secure_boot_sig_block

ESP_LOGD(TAG, "Verifying secure boot signature");

#ifdef BOOTLOADER_BUILD
bool is_valid;
is_valid = uECC_verify(signature_verification_key_start,
image_digest,
DIGEST_LEN,
sig_block->signature,
uECC_secp256r1());
ESP_LOGD(TAG, "Verification result %d", is_valid);
return is_valid ? ESP_OK : ESP_ERR_IMAGE_INVALID;
#else /* BOOTLOADER_BUILD */
int ret;
mbedtls_mpi r, s;

mbedtls_mpi_init(&r);
mbedtls_mpi_init(&s);

/* Extract r and s components from RAW ECDSA signature of 64 bytes */
#define ECDSA_INTEGER_LEN 32
ret = mbedtls_mpi_read_binary(&r, &sig_block->signature[0], ECDSA_INTEGER_LEN);
if (ret != 0) {
ESP_LOGE(TAG, "Failed mbedtls_mpi_read_binary(1), err:%d", ret);
return ESP_FAIL;
}

ret = mbedtls_mpi_read_binary(&s, &sig_block->signature[ECDSA_INTEGER_LEN], ECDSA_INTEGER_LEN);
if (ret != 0) {
ESP_LOGE(TAG, "Failed mbedtls_mpi_read_binary(2), err:%d", ret);
mbedtls_mpi_free(&r);
return ESP_FAIL;
}

/* Initialise ECDSA context */
mbedtls_ecdsa_context ecdsa_context;
mbedtls_ecdsa_init(&ecdsa_context);

mbedtls_ecp_group_load(&ecdsa_context.grp, MBEDTLS_ECP_DP_SECP256R1);
size_t plen = mbedtls_mpi_size(&ecdsa_context.grp.P);
if (keylen != 2*plen) {
ESP_LOGE(TAG, "Incorrect ECDSA key length %d", keylen);
ret = ESP_FAIL;
goto cleanup;
}

/* Extract X and Y components from ECDSA public key */
MBEDTLS_MPI_CHK(mbedtls_mpi_read_binary(&ecdsa_context.Q.X, signature_verification_key_start, plen));
MBEDTLS_MPI_CHK(mbedtls_mpi_read_binary(&ecdsa_context.Q.Y, signature_verification_key_start + plen, plen));
MBEDTLS_MPI_CHK(mbedtls_mpi_lset(&ecdsa_context.Q.Z, 1));

ret = mbedtls_ecdsa_verify(&ecdsa_context.grp, image_digest, DIGEST_LEN, &ecdsa_context.Q, &r, &s);
ESP_LOGD(TAG, "Verification result %d", ret);

cleanup:
mbedtls_mpi_free(&r);
mbedtls_mpi_free(&s);
mbedtls_ecdsa_free(&ecdsa_context);
return ret == 0 ? ESP_OK : ESP_ERR_IMAGE_INVALID;
#endif /* !BOOTLOADER_BUILD */
}
2 changes: 1 addition & 1 deletion tools/ci/mirror-list.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ components/json/cJSON @GENERAL_MIRROR_SERV
components/libsodium/libsodium @GENERAL_MIRROR_SERVER@/idf/libsodium.git ALLOW_TO_SYNC_FROM_PUBLIC
components/mbedtls/mbedtls @GENERAL_MIRROR_SERVER@/idf/mbedtls.git ALLOW_TO_SYNC_FROM_PUBLIC
components/expat/expat @GENERAL_MIRROR_SERVER@/idf/libexpat.git ALLOW_TO_SYNC_FROM_PUBLIC
components/micro-ecc/micro-ecc @GENERAL_MIRROR_SERVER@/idf/micro-ecc.git ALLOW_TO_SYNC_FROM_PUBLIC
components/bootloader/subproject/components/micro-ecc/micro-ecc @GENERAL_MIRROR_SERVER@/idf/micro-ecc.git ALLOW_TO_SYNC_FROM_PUBLIC
components/nghttp/nghttp2 @GENERAL_MIRROR_SERVER@/idf/nghttp2.git ALLOW_TO_SYNC_FROM_PUBLIC
components/spiffs/spiffs @GENERAL_MIRROR_SERVER@/idf/spiffs.git ALLOW_TO_SYNC_FROM_PUBLIC
components/asio/asio @GENERAL_MIRROR_SERVER@/idf/asio.git
Expand Down

3 comments on commit 5136b76

@pedrominatel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi!

The btstack (https://github.com/bluekitchen/btstack) component stopped working after this change.

When adding on components.mk: COMPONENT_DEPENDS += micro-ecc
The error message appears: make: *** No rule to make target 'component-micro-ecc-build', needed by 'component-btstack-build'. Stop.

The workaround for this was creating a symbolic link:

ln -s ~/esp/esp-idf/components/bootloader/subproject/components/micro-ecc/ ~/esp/esp-idf/components/micro-ecc

Is there any other solution for this error?

Thanks!

@mahavirj
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pedrominatel It seems like btstack had dependency on micro-ecc library. In that case, I would suggest that you use copy from https://github.com/bluekitchen/btstack/tree/master/3rd-party/micro-ecc, rather than from IDF. This MR essentially moves micro-ecc to bootloader project, since that is where dependency lies as far as IDF is concerned.

@pedrominatel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mahavirj !

Please sign in to comment.