From d7cf1896b55e81ada571f2915617e9168f7b30f7 Mon Sep 17 00:00:00 2001 From: Ashish Sharma Date: Fri, 22 May 2026 13:11:37 +0800 Subject: [PATCH 1/2] fix(mbedtls): fixes missing check before ecdsa verify --- .../esp_ecdsa/psa_crypto_driver_esp_ecdsa.c | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/components/mbedtls/port/psa_driver/esp_ecdsa/psa_crypto_driver_esp_ecdsa.c b/components/mbedtls/port/psa_driver/esp_ecdsa/psa_crypto_driver_esp_ecdsa.c index 01613cd84c4..911e8d241c9 100644 --- a/components/mbedtls/port/psa_driver/esp_ecdsa/psa_crypto_driver_esp_ecdsa.c +++ b/components/mbedtls/port/psa_driver/esp_ecdsa/psa_crypto_driver_esp_ecdsa.c @@ -14,6 +14,9 @@ #include "soc/soc_caps.h" #include "esp_log.h" +#include "mbedtls/ecp.h" +#include "mbedtls/bignum.h" + #include "esp_assert.h" #include "esp_crypto_lock.h" #include "esp_crypto_periph_clk.h" @@ -312,6 +315,61 @@ static psa_status_t validate_ecdsa_sha_alg(psa_algorithm_t alg, const esp_ecdsa_ } #if SOC_ECDSA_SUPPORTED + +static mbedtls_ecp_group_id ecdsa_curve_to_mbedtls_group(esp_ecdsa_curve_t curve) +{ + switch (curve) { + case ESP_ECDSA_CURVE_SECP256R1: return MBEDTLS_ECP_DP_SECP256R1; +#if SOC_ECDSA_SUPPORT_CURVE_P384 + case ESP_ECDSA_CURVE_SECP384R1: return MBEDTLS_ECP_DP_SECP384R1; +#endif + default: return MBEDTLS_ECP_DP_NONE; + } +} + +static psa_status_t check_ecdsa_signature_range(const uint8_t *signature, size_t key_len, + esp_ecdsa_curve_t curve) +{ + mbedtls_ecp_group_id grp_id = ecdsa_curve_to_mbedtls_group(curve); + if (grp_id == MBEDTLS_ECP_DP_NONE) { + return PSA_ERROR_NOT_SUPPORTED; + } + + mbedtls_ecp_group grp; + mbedtls_mpi r, s; + mbedtls_ecp_group_init(&grp); + mbedtls_mpi_init(&r); + mbedtls_mpi_init(&s); + + psa_status_t status = PSA_ERROR_INVALID_SIGNATURE; + + if (mbedtls_ecp_group_load(&grp, grp_id) != 0) { + status = PSA_ERROR_GENERIC_ERROR; + goto cleanup; + } + if (mbedtls_mpi_read_binary(&r, signature, key_len) != 0 || + mbedtls_mpi_read_binary(&s, signature + key_len, key_len) != 0) { + status = PSA_ERROR_GENERIC_ERROR; + goto cleanup; + } + + /* 1 <= scalar <= n-1: equivalently scalar > 0 and scalar < n. */ + if (mbedtls_mpi_cmp_int(&r, 0) <= 0 || + mbedtls_mpi_cmp_mpi(&r, &grp.N) >= 0 || + mbedtls_mpi_cmp_int(&s, 0) <= 0 || + mbedtls_mpi_cmp_mpi(&s, &grp.N) >= 0) { + goto cleanup; + } + + status = PSA_SUCCESS; + +cleanup: + mbedtls_mpi_free(&r); + mbedtls_mpi_free(&s); + mbedtls_ecp_group_free(&grp); + return status; +} + static void esp_ecdsa_acquire_hardware(void) { esp_crypto_ecdsa_lock_acquire(); @@ -409,6 +467,11 @@ psa_status_t esp_ecdsa_transparent_verify_hash_start( return PSA_ERROR_INVALID_SIGNATURE; } + status = check_ecdsa_signature_range(signature, key_len, curve); + if (status != PSA_SUCCESS) { + return status; + } + const uint8_t *public_key_buffer = NULL; size_t public_key_buffer_size = 0; uint8_t public_key[2 * MAX_ECDSA_COMPONENT_LEN + 1]; From a24bd0bf1017f129fb585775bc2e889aa08c31af Mon Sep 17 00:00:00 2001 From: "harshal.patil" Date: Sat, 23 May 2026 01:06:43 +0530 Subject: [PATCH 2/2] test(mbedtls): Add out-of-bounds test for the ECDSA hardware driver --- .../mbedtls_ut/main/test_psa_ecdsa.c | 116 +++++++++++++++--- 1 file changed, 102 insertions(+), 14 deletions(-) diff --git a/components/mbedtls/test_apps/mbedtls_ut/main/test_psa_ecdsa.c b/components/mbedtls/test_apps/mbedtls_ut/main/test_psa_ecdsa.c index 0f370419b93..08794629ba5 100644 --- a/components/mbedtls/test_apps/mbedtls_ut/main/test_psa_ecdsa.c +++ b/components/mbedtls/test_apps/mbedtls_ut/main/test_psa_ecdsa.c @@ -216,7 +216,7 @@ const uint8_t ecdsa384_pub_y_km[] = { #endif /* SOC_KEY_MANAGER_SUPPORTED */ void test_ecdsa_verify(esp_ecdsa_curve_t curve, const uint8_t *hash, const uint8_t *r_comp, const uint8_t *s_comp, - const uint8_t *pub_x, const uint8_t *pub_y) + const uint8_t *pub_x, const uint8_t *pub_y, psa_status_t expected_status) { size_t hash_len = 0; int64_t elapsed_time; @@ -265,19 +265,25 @@ void test_ecdsa_verify(esp_ecdsa_curve_t curve, const uint8_t *hash, const uint8 memcpy(signature, r_comp, plen_bytes); memcpy(signature + plen_bytes, s_comp, plen_bytes); - ccomp_timer_start(); - status = psa_verify_hash(key_id, PSA_ALG_ECDSA(sha_alg), hash, hash_len, signature, 2 * plen_bytes); - TEST_ASSERT_EQUAL(PSA_SUCCESS, status); - elapsed_time = ccomp_timer_stop(); + if (expected_status == PSA_SUCCESS) { + ccomp_timer_start(); + status = psa_verify_hash(key_id, PSA_ALG_ECDSA(sha_alg), hash, hash_len, signature, 2 * plen_bytes); + TEST_ASSERT_EQUAL(expected_status, status); + elapsed_time = ccomp_timer_stop(); - if (curve == ESP_ECDSA_CURVE_SECP256R1) { - TEST_PERFORMANCE_CCOMP_LESS_THAN(ECDSA_P256_VERIFY_OP, "%" NEWLIB_NANO_COMPAT_FORMAT" us", NEWLIB_NANO_COMPAT_CAST(elapsed_time)); - } + if (curve == ESP_ECDSA_CURVE_SECP256R1) { + TEST_PERFORMANCE_CCOMP_LESS_THAN(ECDSA_P256_VERIFY_OP, "%" NEWLIB_NANO_COMPAT_FORMAT" us", NEWLIB_NANO_COMPAT_CAST(elapsed_time)); + } #if SOC_ECDSA_SUPPORT_CURVE_P384 - else if (curve == ESP_ECDSA_CURVE_SECP384R1) { - TEST_PERFORMANCE_CCOMP_LESS_THAN(ECDSA_P384_VERIFY_OP, "%" NEWLIB_NANO_COMPAT_FORMAT" us", NEWLIB_NANO_COMPAT_CAST(elapsed_time)); - } + else if (curve == ESP_ECDSA_CURVE_SECP384R1) { + TEST_PERFORMANCE_CCOMP_LESS_THAN(ECDSA_P384_VERIFY_OP, "%" NEWLIB_NANO_COMPAT_FORMAT" us", NEWLIB_NANO_COMPAT_CAST(elapsed_time)); + } #endif + } else { + status = psa_verify_hash(key_id, PSA_ALG_ECDSA(sha_alg), hash, hash_len, signature, 2 * plen_bytes); + TEST_ASSERT_EQUAL(expected_status, status); + } + psa_destroy_key(key_id); psa_reset_key_attributes(&key_attr); } @@ -289,7 +295,7 @@ TEST_CASE("mbedtls ECDSA signature verification performance on SECP256R1", "[mbe TEST_IGNORE_MESSAGE("ECDSA is not supported"); } #endif - test_ecdsa_verify(ESP_ECDSA_CURVE_SECP256R1, sha, ecdsa256_r, ecdsa256_s, ecdsa256_pub_x, ecdsa256_pub_y); + test_ecdsa_verify(ESP_ECDSA_CURVE_SECP256R1, sha, ecdsa256_r, ecdsa256_s, ecdsa256_pub_x, ecdsa256_pub_y, PSA_SUCCESS); } #ifdef SOC_ECDSA_SUPPORT_CURVE_P384 @@ -300,7 +306,89 @@ TEST_CASE("mbedtls ECDSA signature verification performance on SECP384R1", "[mbe TEST_IGNORE_MESSAGE("ECDSA is not supported"); } #endif - test_ecdsa_verify(ESP_ECDSA_CURVE_SECP384R1, sha, ecdsa384_r, ecdsa384_s, ecdsa384_pub_x, ecdsa384_pub_y); + test_ecdsa_verify(ESP_ECDSA_CURVE_SECP384R1, sha, ecdsa384_r, ecdsa384_s, ecdsa384_pub_x, ecdsa384_pub_y, PSA_SUCCESS); +} +#endif /* SOC_ECDSA_SUPPORT_CURVE_P384 */ + +/* + * Range-check regression test for the esp_ecdsa PSA driver. + * + * The two cases below exercise both branches of the r,s range check: + * - r = 0, s = 0 (lower bound: r > 0 / s > 0) + * - r = N, s = valid (upper bound: r < N) + * Both must be rejected by the verifier. + * + * ROM mbedtls (e.g. ESP32-C2 with CONFIG_MBEDTLS_USE_CRYPTO_ROM_IMPL=y): + * The ROM was built against an older mbedtls where + * MBEDTLS_ERR_ECP_VERIFY_FAILED was the legacy high-level value + * -0x4E80 (-20096). The current mbedtls_to_psa_error() no longer + * has a case for that number (the macro name now resolves to + * -149 in the new tree), and -20096 falls outside the PSA + * pass-through window (-0x1000, -0x80) in psa_crypto.c, so it + * hits the default branch and is returned as + * PSA_ERROR_GENERIC_ERROR (-132). + */ +#if !SOC_ECDSA_SUPPORTED && CONFIG_MBEDTLS_USE_CRYPTO_ROM_IMPL +#define ECDSA_RANGE_CHECK_REJECT_STATUS PSA_ERROR_GENERIC_ERROR +#else +#define ECDSA_RANGE_CHECK_REJECT_STATUS PSA_ERROR_INVALID_SIGNATURE +#endif + +TEST_CASE("mbedtls ECDSA signature verification rejects out-of-range r, s on SECP256R1", "[mbedtls]") +{ +#if SOC_ECDSA_SUPPORTED + if (!ecdsa_ll_is_supported()) { + TEST_IGNORE_MESSAGE("ECDSA is not supported"); + } +#endif + /* Case A: r = 0, s = 0 -- caught by 'r > 0' / 's > 0' check. */ + static const uint8_t zero32[32] = { 0 }; + test_ecdsa_verify(ESP_ECDSA_CURVE_SECP256R1, sha, + zero32, zero32, + ecdsa256_pub_x, ecdsa256_pub_y, + ECDSA_RANGE_CHECK_REJECT_STATUS); + + /* Case B: r = N (SECP256R1 curve order), s = valid -- caught by 'r < N' check. */ + static const uint8_t p256_n_be[32] = { + 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x00, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xbc, 0xe6, 0xfa, 0xad, 0xa7, 0x17, 0x9e, 0x84, + 0xf3, 0xb9, 0xca, 0xc2, 0xfc, 0x63, 0x25, 0x51, + }; + test_ecdsa_verify(ESP_ECDSA_CURVE_SECP256R1, sha, + p256_n_be, ecdsa256_s, + ecdsa256_pub_x, ecdsa256_pub_y, + ECDSA_RANGE_CHECK_REJECT_STATUS); +} + +#ifdef SOC_ECDSA_SUPPORT_CURVE_P384 +TEST_CASE("mbedtls ECDSA signature verification rejects out-of-range r, s on SECP384R1", "[mbedtls]") +{ +#if SOC_ECDSA_SUPPORTED + if (!ecdsa_ll_is_supported()) { + TEST_IGNORE_MESSAGE("ECDSA is not supported"); + } +#endif + /* Case A: r = 0, s = 0 */ + static const uint8_t zero48[48] = { 0 }; + test_ecdsa_verify(ESP_ECDSA_CURVE_SECP384R1, sha, + zero48, zero48, + ecdsa384_pub_x, ecdsa384_pub_y, + ECDSA_RANGE_CHECK_REJECT_STATUS); + + /* Case B: r = N (SECP384R1 curve order), s = valid */ + static const uint8_t p384_n_be[48] = { + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xc7, 0x63, 0x4d, 0x81, 0xf4, 0x37, 0x2d, 0xdf, + 0x58, 0x1a, 0x0d, 0xb2, 0x48, 0xb0, 0xa7, 0x7a, + 0xec, 0xec, 0x19, 0x6a, 0xcc, 0xc5, 0x29, 0x73, + }; + test_ecdsa_verify(ESP_ECDSA_CURVE_SECP384R1, sha, + p384_n_be, ecdsa384_s, + ecdsa384_pub_x, ecdsa384_pub_y, + ECDSA_RANGE_CHECK_REJECT_STATUS); } #endif /* SOC_ECDSA_SUPPORT_CURVE_P384 */ @@ -403,7 +491,7 @@ void test_ecdsa_sign(esp_ecdsa_curve_t curve, const uint8_t *hash, const uint8_t TEST_ASSERT_EQUAL_HEX32(PSA_SUCCESS, status); TEST_ASSERT_TRUE(signature_len == 2 * plen_bytes); - test_ecdsa_verify(curve, sha, signature, signature + plen_bytes, pub_x, pub_y); + test_ecdsa_verify(curve, sha, signature, signature + plen_bytes, pub_x, pub_y, PSA_SUCCESS); psa_destroy_key(priv_key_id); psa_reset_key_attributes(&priv_attr); }