bootutil: Refactor signature verification
The current ECDSA-P256 implementation code contains
a lot of code that is tied to a specific condition being met.
The aim of this commit is to cleanup the main verification
logic to be unified between crypto backends and move the
conditional code where it is relevant.
Signed-off-by: Roland Mikhel <roland.mikhel@arm.com>
Change-Id: I06b050a263b2b88b08708defb6aa1001a08ba2ae
diff --git a/boot/bootutil/include/bootutil/crypto/ecdsa_p256.h b/boot/bootutil/include/bootutil/crypto/ecdsa_p256.h
index 6b5b315..4b45b98 100644
--- a/boot/bootutil/include/bootutil/crypto/ecdsa_p256.h
+++ b/boot/bootutil/include/bootutil/crypto/ecdsa_p256.h
@@ -31,14 +31,125 @@
#if defined(MCUBOOT_USE_MBED_TLS)
#include <mbedtls/ecdsa.h>
- #include "bootutil/crypto/common.h"
#define BOOTUTIL_CRYPTO_ECDSA_P256_HASH_SIZE (4 * 8)
+ /* Indicate to the caller that the verify function needs the raw ASN.1
+ * signature, not a decoded one. */
+ #define MCUBOOT_ECDSA_NEED_ASN1_SIG
+#endif /* MCUBOOT_USE_MBED_TLS */
+
+/*TODO: remove this after cypress port mbedtls to abstract crypto api */
+#if defined(MCUBOOT_USE_CC310) || defined(MCUBOOT_USE_MBED_TLS)
+#define NUM_ECC_BYTES (256 / 8)
#endif
+#include "mbedtls/oid.h"
+#include "mbedtls/asn1.h"
+#include "bootutil/sign_key.h"
+#include "common.h"
+
#ifdef __cplusplus
extern "C" {
#endif
+/*
+ * Declaring these like this adds NULL termination.
+ */
+static const uint8_t ec_pubkey_oid[] = MBEDTLS_OID_EC_ALG_UNRESTRICTED;
+static const uint8_t ec_secp256r1_oid[] = MBEDTLS_OID_EC_GRP_SECP256R1;
+
+#ifndef MCUBOOT_ECDSA_NEED_ASN1_SIG
+/*
+ * cp points to ASN1 string containing an integer.
+ * Verify the tag, and that the length is 32 bytes.
+ */
+static int bootutil_read_bigint(uint8_t i[NUM_ECC_BYTES], uint8_t **cp, uint8_t *end)
+{
+ size_t len;
+
+ if (mbedtls_asn1_get_tag(cp, end, &len, MBEDTLS_ASN1_INTEGER)) {
+ return -3;
+ }
+
+ if (len >= NUM_ECC_BYTES) {
+ memcpy(i, *cp + len - NUM_ECC_BYTES, NUM_ECC_BYTES);
+ } else {
+ memset(i, 0, NUM_ECC_BYTES - len);
+ memcpy(i + NUM_ECC_BYTES - len, *cp, len);
+ }
+ *cp += len;
+ return 0;
+}
+
+/*
+ * Read in signature. Signature has r and s encoded as integers.
+ */
+static int bootutil_decode_sig(uint8_t signature[NUM_ECC_BYTES * 2], uint8_t *cp, uint8_t *end)
+{
+ int rc;
+ size_t len;
+
+ rc = mbedtls_asn1_get_tag(&cp, end, &len,
+ MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE);
+ if (rc) {
+ return -1;
+ }
+ if (cp + len > end) {
+ return -2;
+ }
+
+ rc = bootutil_read_bigint(signature, &cp, end);
+ if (rc) {
+ return -3;
+ }
+ rc = bootutil_read_bigint(signature + NUM_ECC_BYTES, &cp, end);
+ if (rc) {
+ return -4;
+ }
+ return 0;
+}
+#endif /* not MCUBOOT_ECDSA_NEED_ASN1_SIG */
+
+static int bootutil_import_key(uint8_t **cp, uint8_t *end)
+{
+ size_t len;
+ mbedtls_asn1_buf alg;
+ mbedtls_asn1_buf param;
+
+ if (mbedtls_asn1_get_tag(cp, end, &len,
+ MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE)) {
+ return -1;
+ }
+ end = *cp + len;
+
+ /* ECParameters (RFC5480) */
+ if (mbedtls_asn1_get_alg(cp, end, &alg, ¶m)) {
+ return -2;
+ }
+ /* id-ecPublicKey (RFC5480) */
+ if (alg.MBEDTLS_CONTEXT_MEMBER(len) != sizeof(ec_pubkey_oid) - 1 ||
+ memcmp(alg.MBEDTLS_CONTEXT_MEMBER(p), ec_pubkey_oid, sizeof(ec_pubkey_oid) - 1)) {
+ return -3;
+ }
+ /* namedCurve (RFC5480) */
+ if (param.MBEDTLS_CONTEXT_MEMBER(len) != sizeof(ec_secp256r1_oid) - 1 ||
+ memcmp(param.MBEDTLS_CONTEXT_MEMBER(p), ec_secp256r1_oid, sizeof(ec_secp256r1_oid) - 1)) {
+ return -4;
+ }
+ /* ECPoint (RFC5480) */
+ if (mbedtls_asn1_get_bitstring_null(cp, end, &len)) {
+ return -6;
+ }
+ if (*cp + len != end) {
+ return -7;
+ }
+
+ if (len != 2 * NUM_ECC_BYTES + 1) {
+ return -8;
+ }
+
+ return 0;
+}
+
#if defined(MCUBOOT_USE_TINYCRYPT)
typedef uintptr_t bootutil_ecdsa_p256_context;
static inline void bootutil_ecdsa_p256_init(bootutil_ecdsa_p256_context *ctx)
@@ -52,14 +163,21 @@
}
static inline int bootutil_ecdsa_p256_verify(bootutil_ecdsa_p256_context *ctx,
- const uint8_t *pk, size_t pk_len,
- const uint8_t *hash,
- const uint8_t *sig, size_t sig_len)
+ uint8_t *pk, size_t pk_len,
+ uint8_t *hash, size_t hash_len,
+ uint8_t *sig, size_t sig_len)
{
int rc;
(void)ctx;
(void)pk_len;
(void)sig_len;
+ (void)hash_len;
+
+ uint8_t signature[2 * NUM_ECC_BYTES];
+ rc = bootutil_decode_sig(signature, sig, sig + sig_len);
+ if (rc) {
+ return -1;
+ }
/* Only support uncompressed keys. */
if (pk[0] != 0x04) {
@@ -67,12 +185,19 @@
}
pk++;
- rc = uECC_verify(pk, hash, BOOTUTIL_CRYPTO_ECDSA_P256_HASH_SIZE, sig, uECC_secp256r1());
+ rc = uECC_verify(pk, hash, BOOTUTIL_CRYPTO_ECDSA_P256_HASH_SIZE, signature, uECC_secp256r1());
if (rc != TC_CRYPTO_SUCCESS) {
return -1;
}
return 0;
}
+
+static inline int bootutil_ecdsa_p256_parse_public_key(bootutil_ecdsa_p256_context *ctx,
+ uint8_t **cp,uint8_t *end)
+{
+ (void)ctx;
+ return bootutil_import_key(cp, end);
+}
#endif /* MCUBOOT_USE_TINYCRYPT */
#if defined(MCUBOOT_USE_CC310)
@@ -89,12 +214,13 @@
static inline int bootutil_ecdsa_p256_verify(bootutil_ecdsa_p256_context *ctx,
uint8_t *pk, size_t pk_len,
- uint8_t *hash,
+ uint8_t *hash, size_t hash_len,
uint8_t *sig, size_t sig_len)
{
(void)ctx;
(void)pk_len;
(void)sig_len;
+ (void)hash_len;
/* Only support uncompressed keys. */
if (pk[0] != 0x04) {
@@ -104,14 +230,17 @@
return cc310_ecdsa_verify_secp256r1(hash, pk, sig, BOOTUTIL_CRYPTO_ECDSA_P256_HASH_SIZE);
}
+
+static inline int bootutil_ecdsa_p256_parse_public_key(bootutil_ecdsa_p256_context *ctx,
+ uint8_t **cp,uint8_t *end)
+{
+ (void)ctx;
+ return bootutil_import_key(cp, end);
+}
#endif /* MCUBOOT_USE_CC310 */
#if defined(MCUBOOT_USE_MBED_TLS)
-/* Indicate to the caller that the verify function needs the raw ASN.1
- * signature, not a decoded one. */
-#define MCUBOOT_ECDSA_NEED_ASN1_SIG
-
typedef mbedtls_ecdsa_context bootutil_ecdsa_p256_context;
static inline void bootutil_ecdsa_p256_init(bootutil_ecdsa_p256_context *ctx)
{
@@ -123,15 +252,84 @@
mbedtls_ecdsa_free(ctx);
}
+#ifdef CY_MBEDTLS_HW_ACCELERATION
+/*
+ * Parse the public key used for signing.
+ */
+static int bootutil_parse_eckey(bootutil_ecdsa_p256_context *ctx, uint8_t **p, uint8_t *end)
+{
+ size_t len;
+ mbedtls_asn1_buf alg;
+ mbedtls_asn1_buf param;
+
+ if (mbedtls_asn1_get_tag(p, end, &len,
+ MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE)) {
+ return -1;
+ }
+ end = *p + len;
+
+ if (mbedtls_asn1_get_alg(p, end, &alg, ¶m)) {
+ return -2;
+ }
+ if (alg.MBEDTLS_CONTEXT_MEMBER(len) != sizeof(ec_pubkey_oid) - 1 ||
+ memcmp(alg.MBEDTLS_CONTEXT_MEMBER(p), ec_pubkey_oid, sizeof(ec_pubkey_oid) - 1)) {
+ return -3;
+ }
+ if (param.MBEDTLS_CONTEXT_MEMBER(len) != sizeof(ec_secp256r1_oid) - 1||
+ memcmp(param.MBEDTLS_CONTEXT_MEMBER(p), ec_secp256r1_oid, sizeof(ec_secp256r1_oid) - 1)) {
+ return -4;
+ }
+
+ if (mbedtls_ecp_group_load(&ctx->grp, MBEDTLS_ECP_DP_SECP256R1)) {
+ return -5;
+ }
+
+ if (mbedtls_asn1_get_bitstring_null(p, end, &len)) {
+ return -6;
+ }
+ if (*p + len != end) {
+ return -7;
+ }
+
+ if (mbedtls_ecp_point_read_binary(&ctx->grp, &ctx->Q, *p, end - *p)) {
+ return -8;
+ }
+
+ if (mbedtls_ecp_check_pubkey(&ctx->grp, &ctx->Q)) {
+ return -9;
+ }
+ return 0;
+}
+
static inline int bootutil_ecdsa_p256_verify(bootutil_ecdsa_p256_context *ctx,
uint8_t *pk, size_t pk_len,
- uint8_t *hash,
+ uint8_t *hash, size_t hash_len,
+ uint8_t *sig, size_t sig_len)
+{
+ (void)pk;
+ (void)pk_len;
+
+ /*
+ * This is simplified, as the hash length is also 32 bytes.
+ */
+ while (sig[sig_len - 1] == '\0') {
+ sig_len--;
+ }
+ return mbedtls_ecdsa_read_signature(&ctx, hash, hash_len, sig, sig_len);
+}
+
+#else /* CY_MBEDTLS_HW_ACCELERATION */
+
+static inline int bootutil_ecdsa_p256_verify(bootutil_ecdsa_p256_context *ctx,
+ uint8_t *pk, size_t pk_len,
+ uint8_t *hash, size_t hash_len,
uint8_t *sig, size_t sig_len)
{
int rc;
(void)sig;
(void)hash;
+ (void)hash_len;
rc = mbedtls_ecp_group_load(&ctx->MBEDTLS_CONTEXT_MEMBER(grp), MBEDTLS_ECP_DP_SECP256R1);
if (rc) {
@@ -156,6 +354,22 @@
return 0;
}
+
+#endif /* CY_MBEDTLS_HW_ACCELERATION */
+
+static inline int bootutil_ecdsa_p256_parse_public_key(bootutil_ecdsa_p256_context *ctx,
+ uint8_t **cp,uint8_t *end)
+{
+ int rc;
+#ifdef CY_MBEDTLS_HW_ACCELERATION
+ rc = bootutil_parse_eckey(&ctx, cp, end);
+#else
+ (void)ctx;
+ rc = bootutil_import_key(cp, end);
+#endif
+ return rc;
+}
+
#endif /* MCUBOOT_USE_MBED_TLS */
#ifdef __cplusplus
diff --git a/boot/bootutil/src/image_ec256.c b/boot/bootutil/src/image_ec256.c
index 196d593..4858d9a 100644
--- a/boot/bootutil/src/image_ec256.c
+++ b/boot/bootutil/src/image_ec256.c
@@ -3,7 +3,7 @@
*
* Copyright (c) 2016-2019 JUUL Labs
* Copyright (c) 2017 Linaro LTD
- * Copyright (C) 2021 Arm Limited
+ * Copyright (C) 2021-2023 Arm Limited
*
* Original license:
*
@@ -30,232 +30,35 @@
#include "mcuboot_config/mcuboot_config.h"
#ifdef MCUBOOT_SIGN_EC256
-/*TODO: remove this after cypress port mbedtls to abstract crypto api */
-#if defined(MCUBOOT_USE_CC310) || defined(MCUBOOT_USE_MBED_TLS)
-#define NUM_ECC_BYTES (256 / 8)
-#endif
-#if defined(MCUBOOT_USE_TINYCRYPT) || defined(MCUBOOT_USE_CC310) || \
- defined(MCUBOOT_USE_MBED_TLS)
-#include "bootutil/sign_key.h"
-#include "mbedtls/oid.h"
-#include "mbedtls/asn1.h"
-#include "bootutil/crypto/ecdsa_p256.h"
-#include "bootutil/crypto/common.h"
#include "bootutil_priv.h"
+#include "bootutil/fault_injection_hardening.h"
+#include "bootutil/crypto/ecdsa_p256.h"
-/*
- * Declaring these like this adds NULL termination.
- */
-static const uint8_t ec_pubkey_oid[] = MBEDTLS_OID_EC_ALG_UNRESTRICTED;
-static const uint8_t ec_secp256r1_oid[] = MBEDTLS_OID_EC_GRP_SECP256R1;
-
-/*
- * Parse the public key used for signing.
- */
-#ifdef CY_MBEDTLS_HW_ACCELERATION
-static int
-bootutil_parse_eckey(mbedtls_ecdsa_context *ctx, uint8_t **p, uint8_t *end)
-{
- size_t len;
- mbedtls_asn1_buf alg;
- mbedtls_asn1_buf param;
-
- if (mbedtls_asn1_get_tag(p, end, &len,
- MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE)) {
- return -1;
- }
- end = *p + len;
-
- if (mbedtls_asn1_get_alg(p, end, &alg, ¶m)) {
- return -2;
- }
- if (alg.MBEDTLS_CONTEXT_MEMBER(len) != sizeof(ec_pubkey_oid) - 1 ||
- memcmp(alg.MBEDTLS_CONTEXT_MEMBER(p), ec_pubkey_oid, sizeof(ec_pubkey_oid) - 1)) {
- return -3;
- }
- if (param.MBEDTLS_CONTEXT_MEMBER(len) != sizeof(ec_secp256r1_oid) - 1||
- memcmp(param.MBEDTLS_CONTEXT_MEMBER(p), ec_secp256r1_oid, sizeof(ec_secp256r1_oid) - 1)) {
- return -4;
- }
-
- if (mbedtls_ecp_group_load(&ctx->grp, MBEDTLS_ECP_DP_SECP256R1)) {
- return -5;
- }
-
- if (mbedtls_asn1_get_bitstring_null(p, end, &len)) {
- return -6;
- }
- if (*p + len != end) {
- return -7;
- }
-
- if (mbedtls_ecp_point_read_binary(&ctx->grp, &ctx->Q, *p, end - *p)) {
- return -8;
- }
-
- if (mbedtls_ecp_check_pubkey(&ctx->grp, &ctx->Q)) {
- return -9;
- }
- return 0;
-}
-#endif /* CY_MBEDTLS_HW_ACCELERATION */
-static int
-bootutil_import_key(uint8_t **cp, uint8_t *end)
-{
- size_t len;
- mbedtls_asn1_buf alg;
- mbedtls_asn1_buf param;
-
- if (mbedtls_asn1_get_tag(cp, end, &len,
- MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE)) {
- return -1;
- }
- end = *cp + len;
-
- /* ECParameters (RFC5480) */
- if (mbedtls_asn1_get_alg(cp, end, &alg, ¶m)) {
- return -2;
- }
- /* id-ecPublicKey (RFC5480) */
- if (alg.MBEDTLS_CONTEXT_MEMBER(len) != sizeof(ec_pubkey_oid) - 1 ||
- memcmp(alg.MBEDTLS_CONTEXT_MEMBER(p), ec_pubkey_oid, sizeof(ec_pubkey_oid) - 1)) {
- return -3;
- }
- /* namedCurve (RFC5480) */
- if (param.MBEDTLS_CONTEXT_MEMBER(len) != sizeof(ec_secp256r1_oid) - 1 ||
- memcmp(param.MBEDTLS_CONTEXT_MEMBER(p), ec_secp256r1_oid, sizeof(ec_secp256r1_oid) - 1)) {
- return -4;
- }
- /* ECPoint (RFC5480) */
- if (mbedtls_asn1_get_bitstring_null(cp, end, &len)) {
- return -6;
- }
- if (*cp + len != end) {
- return -7;
- }
-
- if (len != 2 * NUM_ECC_BYTES + 1) {
- return -8;
- }
-
- return 0;
-}
-
-#ifndef MCUBOOT_ECDSA_NEED_ASN1_SIG
-/*
- * cp points to ASN1 string containing an integer.
- * Verify the tag, and that the length is 32 bytes.
- */
-static int
-bootutil_read_bigint(uint8_t i[NUM_ECC_BYTES], uint8_t **cp, uint8_t *end)
-{
- size_t len;
-
- if (mbedtls_asn1_get_tag(cp, end, &len, MBEDTLS_ASN1_INTEGER)) {
- return -3;
- }
-
- if (len >= NUM_ECC_BYTES) {
- memcpy(i, *cp + len - NUM_ECC_BYTES, NUM_ECC_BYTES);
- } else {
- memset(i, 0, NUM_ECC_BYTES - len);
- memcpy(i + NUM_ECC_BYTES - len, *cp, len);
- }
- *cp += len;
- return 0;
-}
-
-/*
- * Read in signature. Signature has r and s encoded as integers.
- */
-static int
-bootutil_decode_sig(uint8_t signature[NUM_ECC_BYTES * 2], uint8_t *cp, uint8_t *end)
-{
- int rc;
- size_t len;
-
- rc = mbedtls_asn1_get_tag(&cp, end, &len,
- MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE);
- if (rc) {
- return -1;
- }
- if (cp + len > end) {
- return -2;
- }
-
- rc = bootutil_read_bigint(signature, &cp, end);
- if (rc) {
- return -3;
- }
- rc = bootutil_read_bigint(signature + NUM_ECC_BYTES, &cp, end);
- if (rc) {
- return -4;
- }
- return 0;
-}
-#endif /* not MCUBOOT_ECDSA_NEED_ASN1_SIG */
-
-int
+fih_int
bootutil_verify_sig(uint8_t *hash, uint32_t hlen, uint8_t *sig, size_t slen,
- uint8_t key_id)
+ uint8_t key_id)
{
int rc;
bootutil_ecdsa_p256_context ctx;
+ FIH_DECLARE(fih_rc, FIH_FAILURE);
uint8_t *pubkey;
uint8_t *end;
-#ifndef MCUBOOT_ECDSA_NEED_ASN1_SIG
- uint8_t signature[2 * NUM_ECC_BYTES];
-#endif
-
pubkey = (uint8_t *)bootutil_keys[key_id].key;
end = pubkey + *bootutil_keys[key_id].len;
-
-#ifdef CY_MBEDTLS_HW_ACCELERATION
- mbedtls_ecdsa_init(&ctx);
- rc = bootutil_parse_eckey(&ctx, &pubkey, end);
-#else
- rc = bootutil_import_key(&pubkey, end);
-#endif
- if (rc) {
- return -1;
- }
-
-#ifndef MCUBOOT_ECDSA_NEED_ASN1_SIG
- rc = bootutil_decode_sig(signature, sig, sig + slen);
- if (rc) {
- return -1;
- }
-#endif
-
- /*
- * This is simplified, as the hash length is also 32 bytes.
- */
-#ifdef CY_MBEDTLS_HW_ACCELERATION
- while (sig[slen - 1] == '\0') {
- slen--;
- }
- rc = mbedtls_ecdsa_read_signature(&ctx, hash, hlen, sig, slen);
-
-#else /* CY_MBEDTLS_HW_ACCELERATION */
- if (hlen != NUM_ECC_BYTES) {
- return -1;
- }
-
bootutil_ecdsa_p256_init(&ctx);
-#ifdef MCUBOOT_ECDSA_NEED_ASN1_SIG
- rc = bootutil_ecdsa_p256_verify(&ctx, pubkey, end - pubkey, hash, sig, slen);
-#else
- rc = bootutil_ecdsa_p256_verify(&ctx, pubkey, end - pubkey, hash, signature,
- 2 * NUM_ECC_BYTES);
-#endif
-#endif /* CY_MBEDTLS_HW_ACCELERATION */
+ rc = bootutil_ecdsa_p256_parse_public_key(&ctx, &pubkey, end);
+ if (rc) {
+ FIH_SET(fih_rc, FIH_FAILURE);
+ FIH_RET(fih_rc);
+ }
+
+ FIH_CALL(bootutil_ecdsa_p256_verify, fih_rc, &ctx, pubkey, end-pubkey, hash, hlen, sig, slen);
bootutil_ecdsa_p256_drop(&ctx);
- return rc;
+ FIH_RET(fih_rc);
}
-#endif /* MCUBOOT_USE_TINYCRYPT || defined MCUBOOT_USE_CC310 */
#endif /* MCUBOOT_SIGN_EC256 */