From b06017940069fc30eaeace5c67241860a910bb75 Mon Sep 17 00:00:00 2001 From: Sachin Parekh Date: Fri, 13 Jan 2023 13:15:38 +0530 Subject: [PATCH 1/2] mbedtls/ecp: Fix incorrect ECP parameter value - Add sanity checks in mbedtls port - Add ECP test cases covering shorter scalar values --- components/mbedtls/port/ecc/ecc_alt.c | 25 ++++++++---- components/mbedtls/test_apps/main/test_ecp.c | 42 +++++++++++++++++++- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/components/mbedtls/port/ecc/ecc_alt.c b/components/mbedtls/port/ecc/ecc_alt.c index e78babc4046..ad3c1d63e9c 100644 --- a/components/mbedtls/port/ecc/ecc_alt.c +++ b/components/mbedtls/port/ecc/ecc_alt.c @@ -24,28 +24,33 @@ static int esp_mbedtls_ecp_point_multiply(const mbedtls_ecp_group *grp, mbedtls_ const mbedtls_mpi *m, const mbedtls_ecp_point *P) { int ret = MBEDTLS_ERR_ECP_BAD_INPUT_DATA; - uint8_t x_tmp[MAX_SIZE]; - uint8_t y_tmp[MAX_SIZE]; + uint8_t x_tmp[MAX_SIZE] = {0}; + uint8_t y_tmp[MAX_SIZE] = {0}; + uint8_t m_le[MAX_SIZE] = {0}; ecc_point_t p_pt = {0}; ecc_point_t r_pt = {0}; p_pt.len = grp->pbits / 8; - memcpy(&p_pt.x, P->MBEDTLS_PRIVATE(X).MBEDTLS_PRIVATE(p), mbedtls_mpi_size(&P->MBEDTLS_PRIVATE(X))); - memcpy(&p_pt.y, P->MBEDTLS_PRIVATE(Y).MBEDTLS_PRIVATE(p), mbedtls_mpi_size(&P->MBEDTLS_PRIVATE(Y))); + MBEDTLS_MPI_CHK(mbedtls_mpi_write_binary_le(&P->MBEDTLS_PRIVATE(X), p_pt.x, MAX_SIZE)); + MBEDTLS_MPI_CHK(mbedtls_mpi_write_binary_le(&P->MBEDTLS_PRIVATE(Y), p_pt.y, MAX_SIZE)); + MBEDTLS_MPI_CHK(mbedtls_mpi_write_binary_le(m, m_le, MAX_SIZE)); - ret = esp_ecc_point_multiply(&p_pt, (uint8_t *)m->MBEDTLS_PRIVATE(p), &r_pt, false); + ret = esp_ecc_point_multiply(&p_pt, m_le, &r_pt, false); for (int i = 0; i < MAX_SIZE; i++) { x_tmp[MAX_SIZE - i - 1] = r_pt.x[i]; y_tmp[MAX_SIZE - i - 1] = r_pt.y[i]; } - mbedtls_mpi_read_binary(&R->MBEDTLS_PRIVATE(X), x_tmp, MAX_SIZE); - mbedtls_mpi_read_binary(&R->MBEDTLS_PRIVATE(Y), y_tmp, MAX_SIZE); - mbedtls_mpi_lset(&R->MBEDTLS_PRIVATE(Z), 1); + MBEDTLS_MPI_CHK(mbedtls_mpi_read_binary(&R->MBEDTLS_PRIVATE(X), x_tmp, MAX_SIZE)); + MBEDTLS_MPI_CHK(mbedtls_mpi_read_binary(&R->MBEDTLS_PRIVATE(Y), y_tmp, MAX_SIZE)); + MBEDTLS_MPI_CHK(mbedtls_mpi_lset(&R->MBEDTLS_PRIVATE(Z), 1)); return ret; + +cleanup: + return MBEDTLS_ERR_ECP_BAD_INPUT_DATA; } int ecp_mul_restartable_internal( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, @@ -62,6 +67,10 @@ int ecp_mul_restartable_internal( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, #endif } + /* Common sanity checks to conform with mbedTLS return values */ + MBEDTLS_MPI_CHK( mbedtls_ecp_check_privkey(grp, m) ); + MBEDTLS_MPI_CHK( mbedtls_ecp_check_pubkey(grp, P) ); + MBEDTLS_MPI_CHK( esp_mbedtls_ecp_point_multiply(grp, R, m, P) ); cleanup: return( ret ); diff --git a/components/mbedtls/test_apps/main/test_ecp.c b/components/mbedtls/test_apps/main/test_ecp.c index 14b8b00044a..37cd5fbb4c5 100644 --- a/components/mbedtls/test_apps/main/test_ecp.c +++ b/components/mbedtls/test_apps/main/test_ecp.c @@ -96,6 +96,9 @@ TEST_CASE("mbedtls ECP mul w/ koblitz", "[mbedtls]") } #if CONFIG_MBEDTLS_HARDWARE_ECC + +#define SMALL_SCALAR 127 + /* * Coordinates and integers stored in big endian format */ @@ -129,6 +132,18 @@ const uint8_t ecc_p192_mul_res_y[] = { 0xE8, 0x29, 0x5E, 0xD9, 0x46, 0x54, 0xC3, 0xE1 }; +const uint8_t ecc_p192_small_mul_res_x[] = { + 0x62, 0xBF, 0x33, 0xC1, 0x75, 0xB5, 0xEB, 0x1D, + 0xBE, 0xC7, 0x15, 0x04, 0x03, 0xA7, 0xDD, 0x9D, + 0x0B, 0x17, 0x9D, 0x3B, 0x06, 0x63, 0xFE, 0xD3 +}; + +const uint8_t ecc_p192_small_mul_res_y[] = { + 0xD4, 0xE9, 0x4E, 0x4D, 0x89, 0x4D, 0xB5, 0x99, + 0x8A, 0xE1, 0x85, 0x81, 0x27, 0x38, 0x23, 0x32, + 0x92, 0xCF, 0xE8, 0x38, 0xCA, 0x39, 0xF2, 0xE1 +}; + const uint8_t ecc_p256_point_x[] = { 0x6B, 0x17, 0xD1, 0xF2, 0xE1, 0x2C, 0x42, 0x47, 0xF8, 0xBC, 0xE6, 0xE5, 0x63, 0xA4, 0x40, 0xF2, @@ -164,6 +179,21 @@ const uint8_t ecc_p256_mul_res_y[] = { 0xC7, 0xD4, 0x0C, 0x90, 0xA1, 0xC9, 0xD3, 0x3A }; +const uint8_t ecc_p256_small_mul_res_x[] = { + 0x53, 0x4D, 0x45, 0xDB, 0x6B, 0xAC, 0xA8, 0xE2, + 0xD2, 0xA5, 0xD0, 0xA7, 0x65, 0xF1, 0x60, 0x13, + 0xA8, 0xD4, 0xEB, 0x58, 0xC6, 0xAA, 0xAD, 0x35, + 0x67, 0xCE, 0xBD, 0xFA, 0xC4, 0x2D, 0x62, 0x3C +}; + +const uint8_t ecc_p256_small_mul_res_y[] = { + 0xFA, 0xD6, 0x69, 0xC8, 0x9A, 0x2A, 0x54, 0xE4, + 0x41, 0x54, 0x35, 0x7F, 0x99, 0x2C, 0xCE, 0xC8, + 0xEE, 0xF0, 0x93, 0xE0, 0xF2, 0x3A, 0x63, 0x1D, + 0x17, 0xFD, 0xF6, 0x64, 0x41, 0x9E, 0x50, 0x0C +}; + + static int rng_wrapper(void *ctx, unsigned char *buf, size_t len) { esp_fill_random(buf, len); @@ -193,7 +223,11 @@ static void test_ecp_mul(mbedtls_ecp_group_id id, const uint8_t *x_coord, const size = grp.pbits / 8; - mbedtls_mpi_read_binary(&m, scalar, size); + if (!scalar) { + mbedtls_mpi_lset(&m, SMALL_SCALAR); + } else { + mbedtls_mpi_read_binary(&m, scalar, size); + } mbedtls_mpi_read_binary(&P.MBEDTLS_PRIVATE(X), x_coord, size); mbedtls_mpi_read_binary(&P.MBEDTLS_PRIVATE(Y), y_coord, size); @@ -228,12 +262,18 @@ TEST_CASE("mbedtls ECP point multiply with SECP192R1", "[mbedtls]") { test_ecp_mul(MBEDTLS_ECP_DP_SECP192R1, ecc_p192_point_x, ecc_p192_point_y, ecc_p192_scalar, ecc_p192_mul_res_x, ecc_p192_mul_res_y); + + test_ecp_mul(MBEDTLS_ECP_DP_SECP192R1, ecc_p192_point_x, ecc_p192_point_y, NULL, + ecc_p192_small_mul_res_x, ecc_p192_small_mul_res_y); } TEST_CASE("mbedtls ECP point multiply with SECP256R1", "[mbedtls]") { test_ecp_mul(MBEDTLS_ECP_DP_SECP256R1, ecc_p256_point_x, ecc_p256_point_y, ecc_p256_scalar, ecc_p256_mul_res_x, ecc_p256_mul_res_y); + + test_ecp_mul(MBEDTLS_ECP_DP_SECP256R1, ecc_p256_point_x, ecc_p256_point_y, NULL, + ecc_p256_small_mul_res_x, ecc_p256_small_mul_res_y); } static void test_ecp_verify(mbedtls_ecp_group_id id, const uint8_t *x_coord, const uint8_t *y_coord) From d9ac69362a5dd8c35382f23a26d2f433e75a0c82 Mon Sep 17 00:00:00 2001 From: Sachin Parekh Date: Fri, 13 Jan 2023 13:21:05 +0530 Subject: [PATCH 2/2] wpa_supplicant: Enable ECC test case --- components/wpa_supplicant/test/test_crypto.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/components/wpa_supplicant/test/test_crypto.c b/components/wpa_supplicant/test/test_crypto.c index 32ccb05d549..d0460d5293e 100644 --- a/components/wpa_supplicant/test/test_crypto.c +++ b/components/wpa_supplicant/test/test_crypto.c @@ -315,9 +315,6 @@ TEST_CASE("Test crypto lib bignum apis", "[wpa_crypto]") #endif /* bits in mbedtls_mpi_uint */ -#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32C6, ESP32H2) -#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32C2) -//IDF-5046 /* * Create an MPI from embedded constants * (assumes len is an exact multiple of sizeof mbedtls_mpi_uint) @@ -541,5 +538,3 @@ TEST_CASE("Test crypto lib ECC apis", "[wpa_crypto]") } } -#endif //!TEMPORARY_DISABLED_FOR_TARGETS(ESP32C6, ESP32H2) -#endif //!TEMPORARY_DISABLED_FOR_TARGETS(ESP32C2)