Skip to content

Commit

Permalink
Merge branch 'fix/hardware_ecc_port' into 'master'
Browse files Browse the repository at this point in the history 
mbedtls/ecp: Fix incorrect ECP parameter value

Closes IDF-6706

See merge request espressif/esp-idf!22046
  • Loading branch information
@mahavirj
mahavirj committed Jan 19, 2023
2 parents ac1c550 + d9ac693 commit 91c25b5
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 14 deletions.
25 changes: 17 additions & 8 deletions components/mbedtls/port/ecc/ecc_alt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 );
Expand Down
42 changes: 41 additions & 1 deletion components/mbedtls/test_apps/main/test_ecp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 0 additions & 5 deletions components/wpa_supplicant/test/test_crypto.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)

0 comments on commit 91c25b5

Please sign in to comment.