From 0fe1ee27e57ce851d6a45db6c1a28f84e13edc83 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 3 Apr 2023 14:42:22 +0200 Subject: [PATCH 1/5] pk: add an alternative function for checking private/public key pairs Instead of using the legacy mbedtls_ecp_check_pub_priv() function which was based on ECP math, we add a new option named eckey_check_pair_psa() which takes advantage of PSA. Of course, this is available when MBEDTLS_USE_PSA_CRYPTO in enabled. Tests were also fixed accordingly. Signed-off-by: Valerio Setti --- include/mbedtls/ecp.h | 3 ++ library/ecp.c | 3 +- library/pk_wrap.c | 79 ++++++++++++++++++++++++++++ tests/suites/test_suite_ecp.function | 2 +- tests/suites/test_suite_pk.function | 9 ++++ 5 files changed, 94 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index b6144d9ae..0b72a837a 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1296,9 +1296,12 @@ int mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, * \return An \c MBEDTLS_ERR_ECP_XXX or an \c MBEDTLS_ERR_MPI_XXX * error code on calculation failure. */ + +#if !defined(MBEDTLS_USE_PSA_CRYPTO) int mbedtls_ecp_check_pub_priv( const mbedtls_ecp_keypair *pub, const mbedtls_ecp_keypair *prv, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng); +#endif /* MBEDTLS_USE_PSA_CRYPTO */ /** * \brief This function exports generic key-pair parameters. diff --git a/library/ecp.c b/library/ecp.c index 08fbe86c7..a794b3b66 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -3316,7 +3316,7 @@ cleanup: return ret; } - +#if !defined(MBEDTLS_USE_PSA_CRYPTO) /* * Check a public-private key pair */ @@ -3357,6 +3357,7 @@ cleanup: return ret; } +#endif /* !MBEDTLS_USE_PSA_CRYPTO */ /* * Export generic key-pair parameters. diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 4d91f22b2..2d5a0b727 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -1095,13 +1095,92 @@ cleanup: } #endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ +#if defined(MBEDTLS_USE_PSA_CRYPTO) +/* + * Alternative function used to verify that the EC private/public key pair + * is valid using PSA functions instead of ECP ones. + * The flow is: + * - import the private key "prv" to PSA and export its public part + * - write the raw content of public key "pub" to a local buffer + * - compare the two buffers + */ +static int eckey_check_pair_psa(const void *pub, const void *prv) +{ + psa_status_t status; + psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; + mbedtls_ecp_keypair *prv_ctx = (mbedtls_ecp_keypair *) prv; + mbedtls_ecp_keypair *pub_ctx = (mbedtls_ecp_keypair *) pub; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + uint8_t prv_key_buf[MBEDTLS_PSA_MAX_EC_KEY_PAIR_LENGTH]; + size_t prv_key_len; + uint8_t pub_key_buf[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; + size_t pub_key_len; + mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; + size_t curve_bits; + psa_ecc_family_t curve = + mbedtls_ecc_group_to_psa(prv_ctx->grp.id, &curve_bits); + size_t curve_bytes = PSA_BITS_TO_BYTES(curve_bits); + + psa_set_key_type(&key_attr, PSA_KEY_TYPE_ECC_KEY_PAIR(curve)); + psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_EXPORT); + + ret = mbedtls_mpi_write_binary(&prv_ctx->d, prv_key_buf, curve_bytes); + if (ret != 0) { + return ret; + } + + status = psa_import_key(&key_attr, prv_key_buf, curve_bytes, &key_id); + if (status != PSA_SUCCESS) { + ret = PSA_PK_TO_MBEDTLS_ERR(status); + return ret; + } + + mbedtls_platform_zeroize(prv_key_buf, sizeof(prv_key_buf)); + + status = psa_export_public_key(key_id, prv_key_buf, sizeof(prv_key_buf), + &prv_key_len); + if (status != PSA_SUCCESS) { + ret = PSA_PK_TO_MBEDTLS_ERR(status); + status = psa_destroy_key(key_id); + return (status != PSA_SUCCESS) ? PSA_PK_TO_MBEDTLS_ERR(status) : ret; + } + + status = psa_destroy_key(key_id); + if (status != PSA_SUCCESS) { + return PSA_PK_TO_MBEDTLS_ERR(status); + } + + ret = mbedtls_ecp_point_write_binary(&pub_ctx->grp, &pub_ctx->Q, + MBEDTLS_ECP_PF_UNCOMPRESSED, + &pub_key_len, pub_key_buf, + sizeof(pub_key_buf)); + if (ret != 0) { + return ret; + } + + if (memcmp(prv_key_buf, pub_key_buf, curve_bytes) != 0) { + return MBEDTLS_ERR_PK_BAD_INPUT_DATA; + } + + return 0; +} +#endif /* MBEDTLS_USE_PSA_CRYPTO */ + static int eckey_check_pair(const void *pub, const void *prv, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) { +#if defined(MBEDTLS_USE_PSA_CRYPTO) + (void) f_rng; + (void) p_rng; + return eckey_check_pair_psa((const mbedtls_ecp_keypair *) pub, + (const mbedtls_ecp_keypair *) prv); +#else /* MBEDTLS_USE_PSA_CRYPTO */ return mbedtls_ecp_check_pub_priv((const mbedtls_ecp_keypair *) pub, (const mbedtls_ecp_keypair *) prv, f_rng, p_rng); +#endif /* MBEDTLS_USE_PSA_CRYPTO */ + return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; } static void *eckey_alloc_wrap(void) diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index 408fe5dff..a77260897 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -955,7 +955,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:!MBEDTLS_USE_PSA_CRYPTO */ void mbedtls_ecp_check_pub_priv(int id_pub, char *Qx_pub, char *Qy_pub, int id, char *d, char *Qx, char *Qy, int ret) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 20f61fc3b..de531d32e 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -489,6 +489,15 @@ void mbedtls_pk_check_pair(char *pub_file, char *prv_file, int ret) mbedtls_pk_init(&prv); mbedtls_pk_init(&alt); +#if defined(MBEDTLS_USE_PSA_CRYPTO) + /* mbedtls_pk_check_pair() returns either PK or ECP error codes depending + on MBEDTLS_USE_PSA_CRYPTO so here we dynamically translate between the + two */ + if (ret == MBEDTLS_ERR_ECP_BAD_INPUT_DATA) { + ret = MBEDTLS_ERR_PK_BAD_INPUT_DATA; + } +#endif + TEST_ASSERT(mbedtls_pk_parse_public_keyfile(&pub, pub_file) == 0); TEST_ASSERT(mbedtls_pk_parse_keyfile(&prv, prv_file, NULL, mbedtls_test_rnd_std_rand, NULL) From f3dc4a1a21f62a2eed01dbbfe2c95ed41f48c53a Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 3 Apr 2023 15:37:53 +0200 Subject: [PATCH 2/5] fixed guard position for doxygen Signed-off-by: Valerio Setti --- include/mbedtls/ecp.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 0b72a837a..8bddbae7b 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1276,6 +1276,7 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, int mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, unsigned char *buf, size_t buflen); +#if !defined(MBEDTLS_USE_PSA_CRYPTO) /** * \brief This function checks that the keypair objects * \p pub and \p prv have the same group and the @@ -1296,8 +1297,6 @@ int mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, * \return An \c MBEDTLS_ERR_ECP_XXX or an \c MBEDTLS_ERR_MPI_XXX * error code on calculation failure. */ - -#if !defined(MBEDTLS_USE_PSA_CRYPTO) int mbedtls_ecp_check_pub_priv( const mbedtls_ecp_keypair *pub, const mbedtls_ecp_keypair *prv, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng); From 8eb552647fc23e89f79eca43371380aca3966e46 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Tue, 4 Apr 2023 10:20:53 +0200 Subject: [PATCH 3/5] pk_wrap: fix sizing for private key buffer Signed-off-by: Valerio Setti --- library/pk_wrap.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 2d5a0b727..f4b2d486f 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -1111,7 +1111,10 @@ static int eckey_check_pair_psa(const void *pub, const void *prv) mbedtls_ecp_keypair *prv_ctx = (mbedtls_ecp_keypair *) prv; mbedtls_ecp_keypair *pub_ctx = (mbedtls_ecp_keypair *) pub; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - uint8_t prv_key_buf[MBEDTLS_PSA_MAX_EC_KEY_PAIR_LENGTH]; + /* We are using MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH for the size of this + * buffer because it will be used to hold the private key at first and + * then its public part (but not at the same time). */ + uint8_t prv_key_buf[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; size_t prv_key_len; uint8_t pub_key_buf[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; size_t pub_key_len; From 98680fc2edc25978800a1b6aedbf6bbf7a9f2022 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Tue, 4 Apr 2023 10:22:59 +0200 Subject: [PATCH 4/5] ecp: revert changes to ECP module and test suite Signed-off-by: Valerio Setti --- include/mbedtls/ecp.h | 2 -- library/ecp.c | 3 +-- tests/suites/test_suite_ecp.function | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 8bddbae7b..b6144d9ae 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1276,7 +1276,6 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, int mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, unsigned char *buf, size_t buflen); -#if !defined(MBEDTLS_USE_PSA_CRYPTO) /** * \brief This function checks that the keypair objects * \p pub and \p prv have the same group and the @@ -1300,7 +1299,6 @@ int mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, int mbedtls_ecp_check_pub_priv( const mbedtls_ecp_keypair *pub, const mbedtls_ecp_keypair *prv, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng); -#endif /* MBEDTLS_USE_PSA_CRYPTO */ /** * \brief This function exports generic key-pair parameters. diff --git a/library/ecp.c b/library/ecp.c index a794b3b66..08fbe86c7 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -3316,7 +3316,7 @@ cleanup: return ret; } -#if !defined(MBEDTLS_USE_PSA_CRYPTO) + /* * Check a public-private key pair */ @@ -3357,7 +3357,6 @@ cleanup: return ret; } -#endif /* !MBEDTLS_USE_PSA_CRYPTO */ /* * Export generic key-pair parameters. diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index a77260897..408fe5dff 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -955,7 +955,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:!MBEDTLS_USE_PSA_CRYPTO */ +/* BEGIN_CASE */ void mbedtls_ecp_check_pub_priv(int id_pub, char *Qx_pub, char *Qy_pub, int id, char *d, char *Qx, char *Qy, int ret) From f2866640696aad6e4be6f5124386a78e01fdd066 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 6 Apr 2023 16:49:54 +0200 Subject: [PATCH 5/5] pk_wrap: minor code optimizations Signed-off-by: Valerio Setti --- library/pk_wrap.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index f4b2d486f..45d743f51 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -1120,9 +1120,9 @@ static int eckey_check_pair_psa(const void *pub, const void *prv) size_t pub_key_len; mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; size_t curve_bits; - psa_ecc_family_t curve = + const psa_ecc_family_t curve = mbedtls_ecc_group_to_psa(prv_ctx->grp.id, &curve_bits); - size_t curve_bytes = PSA_BITS_TO_BYTES(curve_bits); + const size_t curve_bytes = PSA_BITS_TO_BYTES(curve_bits); psa_set_key_type(&key_attr, PSA_KEY_TYPE_ECC_KEY_PAIR(curve)); psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_EXPORT); @@ -1140,17 +1140,13 @@ static int eckey_check_pair_psa(const void *pub, const void *prv) mbedtls_platform_zeroize(prv_key_buf, sizeof(prv_key_buf)); - status = psa_export_public_key(key_id, prv_key_buf, sizeof(prv_key_buf), - &prv_key_len); - if (status != PSA_SUCCESS) { - ret = PSA_PK_TO_MBEDTLS_ERR(status); - status = psa_destroy_key(key_id); - return (status != PSA_SUCCESS) ? PSA_PK_TO_MBEDTLS_ERR(status) : ret; - } - + ret = PSA_PK_TO_MBEDTLS_ERR(psa_export_public_key(key_id, + prv_key_buf, + sizeof(prv_key_buf), + &prv_key_len)); status = psa_destroy_key(key_id); - if (status != PSA_SUCCESS) { - return PSA_PK_TO_MBEDTLS_ERR(status); + if (ret != 0 || status != PSA_SUCCESS) { + return (ret != 0) ? ret : PSA_PK_TO_MBEDTLS_ERR(status); } ret = mbedtls_ecp_point_write_binary(&pub_ctx->grp, &pub_ctx->Q,