From 34f6755b34f103cafb1e7f6e9977ae51e6f7469b Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 3 Apr 2023 15:19:18 +0200 Subject: [PATCH 1/7] pkparse: add new function for deriving public key from private using PSA Instead of using the legacy mbedtls_ecp_mul() function which makes use of ECP's math, this commit adds a new function named pk_derive_public_key() which implements the same behavior using PSA functions. The flow is simple: - import the private key into PSA - export its public part Signed-off-by: Valerio Setti --- library/pkparse.c | 78 ++++++++++++++++++++++-- tests/suites/test_suite_pkparse.function | 7 +++ 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index ccca692b7..11a5b3803 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -48,6 +48,14 @@ #include "mbedtls/pkcs12.h" #endif +#if defined(MBEDTLS_PSA_CRYPTO_C) +#include "mbedtls/psa_util.h" +#endif + +#if defined(MBEDTLS_USE_PSA_CRYPTO) +#include "psa/crypto.h" +#endif + #include "mbedtls/platform.h" #if defined(MBEDTLS_FS_IO) @@ -868,6 +876,56 @@ cleanup: } #endif /* MBEDTLS_RSA_C */ +#if defined(MBEDTLS_USE_PSA_CRYPTO) +/* + * Helper function for deriving a public key from its private counterpart by + * using PSA functions + */ +static int pk_derive_public_key(mbedtls_ecp_group *grp, mbedtls_ecp_point *Q, + const mbedtls_mpi *d) +{ + psa_status_t status; + psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; + size_t curve_bits; + psa_ecc_family_t curve = mbedtls_ecc_group_to_psa(grp->id, &curve_bits); + unsigned char key_buf[MBEDTLS_PSA_MAX_EC_KEY_PAIR_LENGTH]; + size_t key_len = PSA_BITS_TO_BYTES(curve_bits); + mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; + int ret; + + 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(d, key_buf, key_len); + if (ret != 0) { + return ret; + } + + status = psa_import_key(&key_attr, key_buf, key_len, &key_id); + if (status != PSA_SUCCESS) { + ret = psa_pk_status_to_mbedtls(status); + return ret; + } + + mbedtls_platform_zeroize(key_buf, sizeof(key_buf)); + status = psa_export_public_key(key_id, key_buf, sizeof(key_buf), &key_len); + if (status != PSA_SUCCESS) { + ret = psa_pk_status_to_mbedtls(status); + status = psa_destroy_key(key_id); + return (status != PSA_SUCCESS) ? psa_pk_status_to_mbedtls(status) : ret; + } + + ret = mbedtls_ecp_point_read_binary(grp, Q, key_buf, key_len); + + status = psa_destroy_key(key_id); + if (status != PSA_SUCCESS) { + return psa_pk_status_to_mbedtls(status); + } + + return ret; +} +#endif /* MBEDTLS_USE_PSA_CRYPTO */ + #if defined(MBEDTLS_ECP_C) /* * Parse a SEC1 encoded private EC key @@ -975,11 +1033,21 @@ static int pk_parse_key_sec1_der(mbedtls_ecp_keypair *eck, } } - if (!pubkey_done && - (ret = mbedtls_ecp_mul(&eck->grp, &eck->Q, &eck->d, &eck->grp.G, - f_rng, p_rng)) != 0) { - mbedtls_ecp_keypair_free(eck); - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); + if (!pubkey_done) { +#if defined(MBEDTLS_USE_PSA_CRYPTO) + (void) f_rng; + (void) p_rng; + if ((ret = pk_derive_public_key(&eck->grp, &eck->Q, &eck->d)) != 0) { + mbedtls_ecp_keypair_free(eck); + return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); + } +#else /* MBEDTLS_USE_PSA_CRYPTO */ + if ((ret = mbedtls_ecp_mul(&eck->grp, &eck->Q, &eck->d, &eck->grp.G, + f_rng, p_rng)) != 0) { + mbedtls_ecp_keypair_free(eck); + return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); + } +#endif /* MBEDTLS_USE_PSA_CRYPTO */ } if ((ret = mbedtls_ecp_check_privkey(&eck->grp, &eck->d)) != 0) { diff --git a/tests/suites/test_suite_pkparse.function b/tests/suites/test_suite_pkparse.function index 1a6858f2e..5e4f3b770 100644 --- a/tests/suites/test_suite_pkparse.function +++ b/tests/suites/test_suite_pkparse.function @@ -101,6 +101,10 @@ void pk_parse_keyfile_ec(char *key_file, char *password, int result) mbedtls_pk_context ctx; int res; +#if defined(MBEDTLS_USE_PSA_CRYPTO) + PSA_INIT(); +#endif + mbedtls_pk_init(&ctx); res = mbedtls_pk_parse_keyfile(&ctx, key_file, password, @@ -117,6 +121,9 @@ void pk_parse_keyfile_ec(char *key_file, char *password, int result) exit: mbedtls_pk_free(&ctx); +#if defined(MBEDTLS_USE_PSA_CRYPTO) + PSA_DONE(); +#endif } /* END_CASE */ From 4bf73ad83f1878c8b8ed085773b43f483eadaca7 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Tue, 4 Apr 2023 10:48:57 +0200 Subject: [PATCH 2/7] pkparse: use proper sizing for buffer Signed-off-by: Valerio Setti --- library/pkparse.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/pkparse.c b/library/pkparse.c index 11a5b3803..87146af4f 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -888,7 +888,10 @@ static int pk_derive_public_key(mbedtls_ecp_group *grp, mbedtls_ecp_point *Q, psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; size_t curve_bits; psa_ecc_family_t curve = mbedtls_ecc_group_to_psa(grp->id, &curve_bits); - unsigned char key_buf[MBEDTLS_PSA_MAX_EC_KEY_PAIR_LENGTH]; + /* This buffer is used to store the private key at first and then the + * public one (but not at the same time). Therefore we size it for the + * latter since it's bigger. */ + unsigned char key_buf[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; size_t key_len = PSA_BITS_TO_BYTES(curve_bits); mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; int ret; From 3fddf250dc642627572e996ff21fca63cacb3fc0 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Tue, 4 Apr 2023 10:49:28 +0200 Subject: [PATCH 3/7] test: use proper macros for PSA init/done Signed-off-by: Valerio Setti --- tests/suites/test_suite_pkparse.function | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/suites/test_suite_pkparse.function b/tests/suites/test_suite_pkparse.function index 5e4f3b770..d7c2d0b59 100644 --- a/tests/suites/test_suite_pkparse.function +++ b/tests/suites/test_suite_pkparse.function @@ -101,10 +101,7 @@ void pk_parse_keyfile_ec(char *key_file, char *password, int result) mbedtls_pk_context ctx; int res; -#if defined(MBEDTLS_USE_PSA_CRYPTO) - PSA_INIT(); -#endif - + USE_PSA_INIT(); mbedtls_pk_init(&ctx); res = mbedtls_pk_parse_keyfile(&ctx, key_file, password, @@ -121,9 +118,7 @@ void pk_parse_keyfile_ec(char *key_file, char *password, int result) exit: mbedtls_pk_free(&ctx); -#if defined(MBEDTLS_USE_PSA_CRYPTO) - PSA_DONE(); -#endif + USE_PSA_DONE(); } /* END_CASE */ From aad63062123bbf44cb7c44cbb905ec4a45da735e Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Tue, 4 Apr 2023 12:58:15 +0200 Subject: [PATCH 4/7] pkparse: fix guards position Signed-off-by: Valerio Setti --- library/pkparse.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 87146af4f..73e7d8bf0 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -876,10 +876,11 @@ cleanup: } #endif /* MBEDTLS_RSA_C */ +#if defined(MBEDTLS_ECP_C) #if defined(MBEDTLS_USE_PSA_CRYPTO) /* * Helper function for deriving a public key from its private counterpart by - * using PSA functions + * using PSA functions. */ static int pk_derive_public_key(mbedtls_ecp_group *grp, mbedtls_ecp_point *Q, const mbedtls_mpi *d) @@ -929,7 +930,6 @@ static int pk_derive_public_key(mbedtls_ecp_group *grp, mbedtls_ecp_point *Q, } #endif /* MBEDTLS_USE_PSA_CRYPTO */ -#if defined(MBEDTLS_ECP_C) /* * Parse a SEC1 encoded private EC key */ From 9d65f0ef129c5f8a8b4714e02551a729cb3ae335 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Fri, 7 Apr 2023 08:53:17 +0200 Subject: [PATCH 5/7] pk_wrap: simplify prototype of eckey_check_pair_psa() Signed-off-by: Valerio Setti --- library/pk_wrap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 45d743f51..a5eb46595 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -1104,7 +1104,8 @@ cleanup: * - 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) +static int eckey_check_pair_psa(const mbedtls_ecp_keypair *pub, + const mbedtls_ecp_keypair *prv) { psa_status_t status; psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; @@ -1172,8 +1173,7 @@ static int eckey_check_pair(const void *pub, const void *prv, #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); + return eckey_check_pair_psa(pub, prv); #else /* MBEDTLS_USE_PSA_CRYPTO */ return mbedtls_ecp_check_pub_priv((const mbedtls_ecp_keypair *) pub, (const mbedtls_ecp_keypair *) prv, From 1df94f841b26e2b592ffb893afab6c95e311c5d5 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Fri, 7 Apr 2023 08:59:24 +0200 Subject: [PATCH 6/7] pk: fix return codes' precedence and code style Signed-off-by: Valerio Setti --- library/pk_wrap.c | 21 +++++++++++---------- library/pkparse.c | 22 ++++++++++------------ 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index a5eb46595..4e5293df5 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -1107,7 +1107,7 @@ cleanup: static int eckey_check_pair_psa(const mbedtls_ecp_keypair *pub, const mbedtls_ecp_keypair *prv) { - psa_status_t status; + psa_status_t status, destruction_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; @@ -1134,20 +1134,21 @@ static int eckey_check_pair_psa(const mbedtls_ecp_keypair *pub, } status = psa_import_key(&key_attr, prv_key_buf, curve_bytes, &key_id); - if (status != PSA_SUCCESS) { - ret = PSA_PK_TO_MBEDTLS_ERR(status); + ret = PSA_PK_TO_MBEDTLS_ERR(status); + if (ret != 0) { return ret; } mbedtls_platform_zeroize(prv_key_buf, sizeof(prv_key_buf)); - 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 (ret != 0 || status != PSA_SUCCESS) { - return (ret != 0) ? ret : PSA_PK_TO_MBEDTLS_ERR(status); + status = psa_export_public_key(key_id, prv_key_buf, sizeof(prv_key_buf), + &prv_key_len); + ret = PSA_PK_TO_MBEDTLS_ERR(status); + destruction_status = psa_destroy_key(key_id); + if (ret != 0) { + return ret; + } else if (destruction_status != PSA_SUCCESS) { + return PSA_PK_TO_MBEDTLS_ERR(destruction_status); } ret = mbedtls_ecp_point_write_binary(&pub_ctx->grp, &pub_ctx->Q, diff --git a/library/pkparse.c b/library/pkparse.c index 73e7d8bf0..93f435d10 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -885,7 +885,7 @@ cleanup: static int pk_derive_public_key(mbedtls_ecp_group *grp, mbedtls_ecp_point *Q, const mbedtls_mpi *d) { - psa_status_t status; + psa_status_t status, destruction_status; psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; size_t curve_bits; psa_ecc_family_t curve = mbedtls_ecc_group_to_psa(grp->id, &curve_bits); @@ -906,26 +906,24 @@ static int pk_derive_public_key(mbedtls_ecp_group *grp, mbedtls_ecp_point *Q, } status = psa_import_key(&key_attr, key_buf, key_len, &key_id); - if (status != PSA_SUCCESS) { - ret = psa_pk_status_to_mbedtls(status); + ret = psa_pk_status_to_mbedtls(status); + if (ret != 0) { return ret; } mbedtls_platform_zeroize(key_buf, sizeof(key_buf)); + status = psa_export_public_key(key_id, key_buf, sizeof(key_buf), &key_len); - if (status != PSA_SUCCESS) { - ret = psa_pk_status_to_mbedtls(status); - status = psa_destroy_key(key_id); - return (status != PSA_SUCCESS) ? psa_pk_status_to_mbedtls(status) : ret; + ret = psa_pk_status_to_mbedtls(status); + destruction_status = psa_destroy_key(key_id); + if (ret != 0) { + return ret; + } else if (destruction_status != PSA_SUCCESS) { + return psa_pk_status_to_mbedtls(destruction_status); } ret = mbedtls_ecp_point_read_binary(grp, Q, key_buf, key_len); - status = psa_destroy_key(key_id); - if (status != PSA_SUCCESS) { - return psa_pk_status_to_mbedtls(status); - } - return ret; } #endif /* MBEDTLS_USE_PSA_CRYPTO */ From 520c0384e781126380186209ca76e9c931ff8035 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Fri, 7 Apr 2023 11:38:09 +0200 Subject: [PATCH 7/7] pkparse: fix return value Signed-off-by: Valerio Setti --- library/pkparse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/pkparse.c b/library/pkparse.c index 93f435d10..fa61a0692 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1040,7 +1040,7 @@ static int pk_parse_key_sec1_der(mbedtls_ecp_keypair *eck, (void) p_rng; if ((ret = pk_derive_public_key(&eck->grp, &eck->Q, &eck->d)) != 0) { mbedtls_ecp_keypair_free(eck); - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); + return ret; } #else /* MBEDTLS_USE_PSA_CRYPTO */ if ((ret = mbedtls_ecp_mul(&eck->grp, &eck->Q, &eck->d, &eck->grp.G,