From 4b0e8f0e2c67f88816f7ebfc314e725986fe6343 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 6 Jul 2023 12:25:12 +0200 Subject: [PATCH 01/27] Remove redundant include MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's also included later, guarded by support for ECC keys, and actually that's the only case where we need it. Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 1 - 1 file changed, 1 deletion(-) diff --git a/library/pkparse.c b/library/pkparse.c index e1422df771..687c2c0828 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -26,7 +26,6 @@ #include "mbedtls/oid.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" -#include "pk_internal.h" #include From da88c380bd9674efef64b91b9ff34057c6b537d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 6 Jul 2023 12:31:43 +0200 Subject: [PATCH 02/27] Minimize key-type-related includes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - we don't use any ECDSA function here - we only need to include ecp.h when supporting ECC keys Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 687c2c0828..1dd5653e24 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -29,16 +29,16 @@ #include +/* Key types */ #if defined(MBEDTLS_RSA_C) #include "mbedtls/rsa.h" #endif -#include "mbedtls/ecp.h" #if defined(MBEDTLS_PK_HAVE_ECC_KEYS) +#include "mbedtls/ecp.h" #include "pk_internal.h" #endif -#if defined(MBEDTLS_ECDSA_C) -#include "mbedtls/ecdsa.h" -#endif + +/* Extended formats */ #if defined(MBEDTLS_PEM_PARSE_C) #include "mbedtls/pem.h" #endif From 5fcbe4c1f802b5532a198f91d7cfa33f211c7cbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 6 Jul 2023 13:02:51 +0200 Subject: [PATCH 03/27] Further rationalize includes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - only include psa_util when we use PSA Crypto - re-order includes Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 1dd5653e24..3fca2c44f7 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -25,10 +25,16 @@ #include "mbedtls/asn1.h" #include "mbedtls/oid.h" #include "mbedtls/platform_util.h" +#include "mbedtls/platform.h" #include "mbedtls/error.h" #include +#if defined(MBEDTLS_USE_PSA_CRYPTO) +#include "mbedtls/psa_util.h" +#include "psa/crypto.h" +#endif + /* Key types */ #if defined(MBEDTLS_RSA_C) #include "mbedtls/rsa.h" @@ -49,16 +55,6 @@ #include "mbedtls/pkcs12.h" #endif -#if defined(MBEDTLS_PSA_CRYPTO_C) -#include "psa_util_internal.h" -#endif - -#if defined(MBEDTLS_USE_PSA_CRYPTO) -#include "psa/crypto.h" -#endif - -#include "mbedtls/platform.h" - /* Helper for Montgomery curves */ #if defined(MBEDTLS_PK_HAVE_ECC_KEYS) && defined(MBEDTLS_PK_HAVE_RFC8410_CURVES) #define MBEDTLS_PK_IS_RFC8410_GROUP_ID(id) \ From 2585852231fb4e895dc9471f547c0e81bd97f308 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 24 Jul 2023 11:44:55 +0200 Subject: [PATCH 04/27] Factor common code into a function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There were two places that were calling either pk_update_ecparams() or mbedtls_ecp_group_load() depending on the same guard. Factor this into a single function, that works in both configs, so that callers don't have to worry about guards. Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 67 ++++++++++++++++++----------------------------- 1 file changed, 26 insertions(+), 41 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 3fca2c44f7..d5ffc862d8 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -453,28 +453,39 @@ cleanup: } #endif /* MBEDTLS_PK_PARSE_EC_EXTENDED */ -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) -/* Functions pk_use_ecparams() and pk_use_ecparams_rfc8410() update the - * ecp_keypair structure with proper group ID. The purpose of this helper - * function is to update ec_family and ec_bits accordingly. */ -static int pk_update_psa_ecparams(mbedtls_pk_context *pk, - mbedtls_ecp_group_id grp_id) +/* + * Set the group used by this key. + */ +static int pk_ecc_set_group(mbedtls_pk_context *pk, mbedtls_ecp_group_id grp_id) { - psa_ecc_family_t ec_family; - size_t bits; +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + size_t ec_bits; + psa_ecc_family_t ec_family = mbedtls_ecc_group_to_psa(grp_id, &ec_bits); - ec_family = mbedtls_ecc_group_to_psa(grp_id, &bits); - - if ((pk->ec_family != 0) && (pk->ec_family != ec_family)) { + /* group may already be initialized; if so, make sure IDs match */ + if ((pk->ec_family != 0 && pk->ec_family != ec_family) || + (pk->ec_bits != 0 && pk->ec_bits != ec_bits)) { return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; } + /* set group */ pk->ec_family = ec_family; - pk->ec_bits = bits; + pk->ec_bits = ec_bits; return 0; -} +#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ + mbedtls_ecp_keypair *ecp = mbedtls_pk_ec_rw(*pk); + + /* grp may already be initialized; if so, make sure IDs match */ + if (mbedtls_pk_ec_ro(*pk)->grp.id != MBEDTLS_ECP_DP_NONE && + mbedtls_pk_ec_ro(*pk)->grp.id != grp_id) { + return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; + } + + /* set group */ + return mbedtls_ecp_group_load(&(ecp->grp), grp_id); #endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ +} /* * Use EC parameters to initialise an EC group @@ -503,22 +514,7 @@ static int pk_use_ecparams(const mbedtls_asn1_buf *params, mbedtls_pk_context *p #endif } -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) - ret = pk_update_psa_ecparams(pk, grp_id); -#else - /* grp may already be initialized; if so, make sure IDs match */ - if (mbedtls_pk_ec_ro(*pk)->grp.id != MBEDTLS_ECP_DP_NONE && - mbedtls_pk_ec_ro(*pk)->grp.id != grp_id) { - return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; - } - - if ((ret = mbedtls_ecp_group_load(&(mbedtls_pk_ec_rw(*pk)->grp), - grp_id)) != 0) { - return ret; - } -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ - - return ret; + return pk_ecc_set_group(pk, grp_id); } /* @@ -588,22 +584,11 @@ static int pk_use_ecparams_rfc8410(const mbedtls_asn1_buf *params, mbedtls_ecp_group_id grp_id, mbedtls_pk_context *pk) { - int ret; - if (params->tag != 0 || params->len != 0) { return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; } -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) - ret = pk_update_psa_ecparams(pk, grp_id); -#else - mbedtls_ecp_keypair *ecp = mbedtls_pk_ec_rw(*pk); - ret = mbedtls_ecp_group_load(&(ecp->grp), grp_id); - if (ret != 0) { - return ret; - } -#endif - return ret; + return pk_ecc_set_group(pk, grp_id); } /* From d5b43720121916237f4474f13e27e288c62cb8f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 24 Jul 2023 12:06:22 +0200 Subject: [PATCH 05/27] Slightly simplify pk_derive_public_key() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add a comment explain potentially surprising parameters - avoid nesting #if guards: I find the linear structure #if #elif #else makes the three cases clearer. Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index d5ffc862d8..78155ba322 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -519,24 +519,32 @@ static int pk_use_ecparams(const mbedtls_asn1_buf *params, mbedtls_pk_context *p /* * Helper function for deriving a public key from its private counterpart. + * + * Note: the private key information is always available from pk, + * however for convenience the serialized version is also passed, + * as it's available at each calling site, and useful in some configs + * (as otherwise we're have to re-serialize it from the pk context). */ static int pk_derive_public_key(mbedtls_pk_context *pk, const unsigned char *d, size_t d_len, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) { - int ret; -#if defined(MBEDTLS_USE_PSA_CRYPTO) +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) psa_status_t status; (void) f_rng; (void) p_rng; -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) (void) d; (void) d_len; status = psa_export_public_key(pk->priv_id, pk->pub_raw, sizeof(pk->pub_raw), &pk->pub_raw_len); - ret = psa_pk_status_to_mbedtls(status); -#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ + return psa_pk_status_to_mbedtls(status); +#elif defined(MBEDTLS_USE_PSA_CRYPTO) /* && !MBEDTLS_PK_USE_PSA_EC_DATA */ + int ret; + psa_status_t status; + (void) f_rng; + (void) p_rng; + mbedtls_ecp_keypair *eck = (mbedtls_ecp_keypair *) pk->pk_ctx; unsigned char key_buf[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; size_t key_len; @@ -563,16 +571,14 @@ static int pk_derive_public_key(mbedtls_pk_context *pk, } else if (destruction_status != PSA_SUCCESS) { return psa_pk_status_to_mbedtls(destruction_status); } - ret = mbedtls_ecp_point_read_binary(&eck->grp, &eck->Q, key_buf, key_len); -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ + return mbedtls_ecp_point_read_binary(&eck->grp, &eck->Q, key_buf, key_len); #else /* MBEDTLS_USE_PSA_CRYPTO */ mbedtls_ecp_keypair *eck = (mbedtls_ecp_keypair *) pk->pk_ctx; (void) d; (void) d_len; - ret = mbedtls_ecp_mul(&eck->grp, &eck->Q, &eck->d, &eck->grp.G, f_rng, p_rng); + return mbedtls_ecp_mul(&eck->grp, &eck->Q, &eck->d, &eck->grp.G, f_rng, p_rng); #endif /* MBEDTLS_USE_PSA_CRYPTO */ - return ret; } #if defined(MBEDTLS_PK_HAVE_RFC8410_CURVES) From 6db11d5068d73c56ca8bfb2a7ca68b52cc258173 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 25 Jul 2023 11:20:48 +0200 Subject: [PATCH 06/27] Group two versions of the same code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Just moving code around. The two blocks do morally the same thing: load the key, and grouping them makes the #if #else structure clearer. Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 78155ba322..edcc2e2912 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1214,11 +1214,30 @@ static int pk_parse_key_sec1_der(mbedtls_pk_context *pk, } -#if !defined(MBEDTLS_PK_USE_PSA_EC_DATA) +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + psa_set_key_type(&attributes, PSA_KEY_TYPE_ECC_KEY_PAIR(pk->ec_family)); + /* Setting largest masks for usage and key algorithms */ + psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_SIGN_HASH | + PSA_KEY_USAGE_SIGN_MESSAGE | + PSA_KEY_USAGE_EXPORT | PSA_KEY_USAGE_DERIVE); +#if defined(MBEDTLS_ECDSA_DETERMINISTIC) + psa_set_key_algorithm(&attributes, + PSA_ALG_DETERMINISTIC_ECDSA(PSA_ALG_ANY_HASH)); +#else + psa_set_key_algorithm(&attributes, PSA_ALG_ECDSA(PSA_ALG_ANY_HASH)); +#endif + psa_set_key_enrollment_algorithm(&attributes, PSA_ALG_ECDH); + + status = psa_import_key(&attributes, d, d_len, &pk->priv_id); + if (status != PSA_SUCCESS) { + ret = psa_pk_status_to_mbedtls(status); + return ret; + } +#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ if ((ret = mbedtls_ecp_read_key(eck->grp.id, eck, d, d_len)) != 0) { return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); } -#endif +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ if (p != end) { /* @@ -1255,27 +1274,6 @@ static int pk_parse_key_sec1_der(mbedtls_pk_context *pk, } } -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) - psa_set_key_type(&attributes, PSA_KEY_TYPE_ECC_KEY_PAIR(pk->ec_family)); - /* Setting largest masks for usage and key algorithms */ - psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_SIGN_HASH | - PSA_KEY_USAGE_SIGN_MESSAGE | - PSA_KEY_USAGE_EXPORT | PSA_KEY_USAGE_DERIVE); -#if defined(MBEDTLS_ECDSA_DETERMINISTIC) - psa_set_key_algorithm(&attributes, - PSA_ALG_DETERMINISTIC_ECDSA(PSA_ALG_ANY_HASH)); -#else - psa_set_key_algorithm(&attributes, PSA_ALG_ECDSA(PSA_ALG_ANY_HASH)); -#endif - psa_set_key_enrollment_algorithm(&attributes, PSA_ALG_ECDH); - - status = psa_import_key(&attributes, d, d_len, &pk->priv_id); - if (status != PSA_SUCCESS) { - ret = psa_pk_status_to_mbedtls(status); - return ret; - } -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ - if (!pubkey_done) { if ((ret = pk_derive_public_key(pk, d, d_len, f_rng, p_rng)) != 0) { return ret; From dcd98fffabeedfbff4dae473b8be368b728960af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 25 Jul 2023 11:58:31 +0200 Subject: [PATCH 07/27] Factor similar code into pk_ecc_set_key() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 100 ++++++++++++++++++++++++---------------------- 1 file changed, 52 insertions(+), 48 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index edcc2e2912..114a563895 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -487,6 +487,48 @@ static int pk_ecc_set_group(mbedtls_pk_context *pk, mbedtls_ecp_group_id grp_id) #endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ } +/* + * Set the private key material + * + * Must have already set the group with pk_ecc_set_group(). + * + * The 'key' argument points to the raw private key (no ASN.1 wrapping). + */ +static int pk_ecc_set_key(mbedtls_pk_context *pk, + unsigned char *key, size_t len) +{ +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_status_t status; + + psa_set_key_type(&attributes, PSA_KEY_TYPE_ECC_KEY_PAIR(pk->ec_family)); + psa_set_key_algorithm(&attributes, PSA_ALG_ECDH); + psa_key_usage_t flags = PSA_KEY_USAGE_EXPORT | PSA_KEY_USAGE_DERIVE; + if (pk->ec_family != PSA_ECC_FAMILY_MONTGOMERY) { + flags |= PSA_KEY_USAGE_SIGN_HASH | PSA_KEY_USAGE_SIGN_MESSAGE; +#if defined(MBEDTLS_ECDSA_DETERMINISTIC) + psa_set_key_enrollment_algorithm(&attributes, + PSA_ALG_DETERMINISTIC_ECDSA(PSA_ALG_ANY_HASH)); +#else + psa_set_key_enrollment_algorithm(&attributes, PSA_ALG_ECDSA(PSA_ALG_ANY_HASH)); +#endif + } + psa_set_key_usage_flags(&attributes, flags); + + status = psa_import_key(&attributes, key, len, &pk->priv_id); + return psa_pk_status_to_mbedtls(status); + +#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ + + mbedtls_ecp_keypair *eck = mbedtls_pk_ec_rw(*pk); + int ret = mbedtls_ecp_read_key(eck->grp.id, eck, key, len); + if (ret != 0) { + return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); + } + return 0; +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ +} + /* * Use EC parameters to initialise an EC group * @@ -617,27 +659,13 @@ static int pk_parse_key_rfc8410_der(mbedtls_pk_context *pk, return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; } -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) - psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - psa_status_t status; - - psa_set_key_type(&attributes, PSA_KEY_TYPE_ECC_KEY_PAIR(pk->ec_family)); - psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_EXPORT | - PSA_KEY_USAGE_DERIVE); - psa_set_key_algorithm(&attributes, PSA_ALG_ECDH); - - status = psa_import_key(&attributes, key, len, &pk->priv_id); - if (status != PSA_SUCCESS) { - ret = psa_pk_status_to_mbedtls(status); + /* + * Load the private key + */ + ret = pk_ecc_set_key(pk, key, len); + if (ret != 0) { return ret; } -#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ - mbedtls_ecp_keypair *eck = mbedtls_pk_ec_rw(*pk); - - if ((ret = mbedtls_ecp_read_key(eck->grp.id, eck, key, len)) != 0) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); - } -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ /* pk_parse_key_pkcs8_unencrypted_der() only supports version 1 PKCS8 keys, * which never contain a public key. As such, derive the public key @@ -1153,12 +1181,6 @@ static int pk_parse_key_sec1_der(mbedtls_pk_context *pk, unsigned char *d; unsigned char *end = p + keylen; unsigned char *end2; -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) - psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - psa_status_t status; -#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ - mbedtls_ecp_keypair *eck = mbedtls_pk_ec_rw(*pk); -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ /* * RFC 5915, or SEC1 Appendix C.4 @@ -1213,31 +1235,13 @@ static int pk_parse_key_sec1_der(mbedtls_pk_context *pk, } } - -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) - psa_set_key_type(&attributes, PSA_KEY_TYPE_ECC_KEY_PAIR(pk->ec_family)); - /* Setting largest masks for usage and key algorithms */ - psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_SIGN_HASH | - PSA_KEY_USAGE_SIGN_MESSAGE | - PSA_KEY_USAGE_EXPORT | PSA_KEY_USAGE_DERIVE); -#if defined(MBEDTLS_ECDSA_DETERMINISTIC) - psa_set_key_algorithm(&attributes, - PSA_ALG_DETERMINISTIC_ECDSA(PSA_ALG_ANY_HASH)); -#else - psa_set_key_algorithm(&attributes, PSA_ALG_ECDSA(PSA_ALG_ANY_HASH)); -#endif - psa_set_key_enrollment_algorithm(&attributes, PSA_ALG_ECDH); - - status = psa_import_key(&attributes, d, d_len, &pk->priv_id); - if (status != PSA_SUCCESS) { - ret = psa_pk_status_to_mbedtls(status); + /* + * Load the private key + */ + ret = pk_ecc_set_key(pk, d, d_len); + if (ret != 0) { return ret; } -#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ - if ((ret = mbedtls_ecp_read_key(eck->grp.id, eck, d, d_len)) != 0) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); - } -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ if (p != end) { /* From 116175c5d7f2b44f5d3186e845b9881d519d53d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 25 Jul 2023 12:06:55 +0200 Subject: [PATCH 08/27] Use helper macro for (deterministic) ECDSA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - centralizes decision making about which version to use when - avoids nested #ifs in pk_ecc_set_key() Signed-off-by: Manuel Pégourié-Gonnard --- library/pk_internal.h | 9 +++++++-- library/pk_wrap.c | 9 ++------- library/pkparse.c | 7 ++----- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/library/pk_internal.h b/library/pk_internal.h index 004660e094..9becbecd41 100644 --- a/library/pk_internal.h +++ b/library/pk_internal.h @@ -117,14 +117,19 @@ static inline mbedtls_ecp_group_id mbedtls_pk_get_group_id(const mbedtls_pk_cont #endif /* MBEDTLS_ECP_HAVE_CURVE25519 || MBEDTLS_ECP_DP_CURVE448 */ #endif /* MBEDTLS_PK_HAVE_ECC_KEYS */ -#if defined(MBEDTLS_TEST_HOOKS) +/* Helper for (deterministic) ECDSA */ +#if defined(MBEDTLS_ECDSA_DETERMINISTIC) +#define MBEDTLS_PK_PSA_ALG_ECDSA_MAYBE_DET PSA_ALG_DETERMINISTIC_ECDSA +#else +#define MBEDTLS_PK_PSA_ALG_ECDSA_MAYBE_DET PSA_ALG_ECDSA +#endif +#if defined(MBEDTLS_TEST_HOOKS) MBEDTLS_STATIC_TESTABLE int mbedtls_pk_parse_key_pkcs8_encrypted_der( mbedtls_pk_context *pk, unsigned char *key, size_t keylen, const unsigned char *pwd, size_t pwdlen, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng); - #endif #endif /* MBEDTLS_PK_INTERNAL_H */ diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 436876a5d5..53e11d5cb9 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -1037,13 +1037,8 @@ static int ecdsa_sign_wrap(mbedtls_pk_context *pk, mbedtls_md_type_t md_alg, psa_ecc_family_t curve = mbedtls_ecc_group_to_psa(ctx->grp.id, &curve_bits); size_t key_len = PSA_BITS_TO_BYTES(curve_bits); -#if defined(MBEDTLS_ECDSA_DETERMINISTIC) - psa_algorithm_t psa_sig_md = - PSA_ALG_DETERMINISTIC_ECDSA(mbedtls_md_psa_alg_from_type(md_alg)); -#else - psa_algorithm_t psa_sig_md = - PSA_ALG_ECDSA(mbedtls_md_psa_alg_from_type(md_alg)); -#endif + psa_algorithm_t psa_hash = mbedtls_md_psa_alg_from_type(md_alg); + psa_algorithm_t psa_sig_md = MBEDTLS_PK_PSA_ALG_ECDSA_MAYBE_DET(psa_hash); ((void) f_rng); ((void) p_rng); diff --git a/library/pkparse.c b/library/pkparse.c index 114a563895..3f67786e44 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -504,14 +504,11 @@ static int pk_ecc_set_key(mbedtls_pk_context *pk, psa_set_key_type(&attributes, PSA_KEY_TYPE_ECC_KEY_PAIR(pk->ec_family)); psa_set_key_algorithm(&attributes, PSA_ALG_ECDH); psa_key_usage_t flags = PSA_KEY_USAGE_EXPORT | PSA_KEY_USAGE_DERIVE; + /* Montgomery allows only ECDH, others ECDSA too */ if (pk->ec_family != PSA_ECC_FAMILY_MONTGOMERY) { flags |= PSA_KEY_USAGE_SIGN_HASH | PSA_KEY_USAGE_SIGN_MESSAGE; -#if defined(MBEDTLS_ECDSA_DETERMINISTIC) psa_set_key_enrollment_algorithm(&attributes, - PSA_ALG_DETERMINISTIC_ECDSA(PSA_ALG_ANY_HASH)); -#else - psa_set_key_enrollment_algorithm(&attributes, PSA_ALG_ECDSA(PSA_ALG_ANY_HASH)); -#endif + MBEDTLS_PK_PSA_ALG_ECDSA_MAYBE_DET(PSA_ALG_ANY_HASH)); } psa_set_key_usage_flags(&attributes, flags); From e82fcd9c9e01cd3b63c76e792b9136834cc38f34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 26 Jul 2023 10:53:25 +0200 Subject: [PATCH 09/27] Avoid nested #ifs in body of pk_get_ecpubkey() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 3f67786e44..72eed097f1 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -675,7 +675,7 @@ static int pk_parse_key_rfc8410_der(mbedtls_pk_context *pk, } #endif /* MBEDTLS_PK_HAVE_RFC8410_CURVES */ -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) && defined(MBEDTLS_PK_PARSE_EC_COMPRESSED) +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) /* * Create a temporary ecp_keypair for converting an EC point in compressed * format to an uncompressed one @@ -685,6 +685,7 @@ static int pk_convert_compressed_ec(mbedtls_pk_context *pk, size_t *out_buf_len, unsigned char *out_buf, size_t out_buf_size) { +#if defined(MBEDTLS_PK_PARSE_EC_COMPRESSED) mbedtls_ecp_keypair ecp_key; mbedtls_ecp_group_id ecp_group_id; int ret; @@ -708,8 +709,11 @@ static int pk_convert_compressed_ec(mbedtls_pk_context *pk, exit: mbedtls_ecp_keypair_free(&ecp_key); return ret; +#else /* MBEDTLS_PK_PARSE_EC_COMPRESSED */ + return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; +#endif /* MBEDTLS_PK_PARSE_EC_COMPRESSED */ } -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA && MBEDTLS_PK_PARSE_EC_COMPRESSED */ +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ /* * EC public key is an EC point @@ -732,20 +736,15 @@ static int pk_get_ecpubkey(unsigned char **p, const unsigned char *end, return MBEDTLS_ERR_PK_BAD_INPUT_DATA; } - /* Compressed point format are not supported yet by PSA crypto. As a - * consequence ecp functions are used to "convert" the point to - * uncompressed format */ if ((**p == 0x02) || (**p == 0x03)) { -#if defined(MBEDTLS_PK_PARSE_EC_COMPRESSED) + /* Compressed format, not supported by PSA Crypto. + * Try converting using functions from ECP_LIGHT. */ ret = pk_convert_compressed_ec(pk, *p, len, &(pk->pub_raw_len), pk->pub_raw, PSA_EXPORT_PUBLIC_KEY_MAX_SIZE); if (ret != 0) { return ret; } -#else /* MBEDTLS_PK_PARSE_EC_COMPRESSED */ - return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; -#endif /* MBEDTLS_PK_PARSE_EC_COMPRESSED */ } else { /* Uncompressed format */ if ((size_t) (end - *p) > MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN) { From df151bbc37c47c9da99ecf7eca8e127e521038db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 26 Jul 2023 11:06:46 +0200 Subject: [PATCH 10/27] Minor improvements to pk_ecc_read_compressed() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - new name starting with pk_ecc for consistency - re-order params to match the PSA convention: buf, len, &size - add comment about input consumption Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 72eed097f1..3fa1a84227 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -679,11 +679,14 @@ static int pk_parse_key_rfc8410_der(mbedtls_pk_context *pk, /* * Create a temporary ecp_keypair for converting an EC point in compressed * format to an uncompressed one + * + * Consumes everything or fails - inherited from + * mbedtls_ecp_point_read_binary(). */ -static int pk_convert_compressed_ec(mbedtls_pk_context *pk, - const unsigned char *in_start, size_t in_len, - size_t *out_buf_len, unsigned char *out_buf, - size_t out_buf_size) +static int pk_ecc_read_compressed(mbedtls_pk_context *pk, + const unsigned char *in_start, size_t in_len, + unsigned char *out_buf, size_t out_buf_size, + size_t *out_buf_len) { #if defined(MBEDTLS_PK_PARSE_EC_COMPRESSED) mbedtls_ecp_keypair ecp_key; @@ -730,7 +733,7 @@ static int pk_get_ecpubkey(unsigned char **p, const unsigned char *end, #if defined(MBEDTLS_PK_USE_PSA_EC_DATA) mbedtls_svc_key_id_t key; psa_key_attributes_t key_attrs = PSA_KEY_ATTRIBUTES_INIT; - size_t len = (end - *p); + size_t len = (size_t) (end - *p); if (len > PSA_EXPORT_PUBLIC_KEY_MAX_SIZE) { return MBEDTLS_ERR_PK_BAD_INPUT_DATA; @@ -739,19 +742,20 @@ static int pk_get_ecpubkey(unsigned char **p, const unsigned char *end, if ((**p == 0x02) || (**p == 0x03)) { /* Compressed format, not supported by PSA Crypto. * Try converting using functions from ECP_LIGHT. */ - ret = pk_convert_compressed_ec(pk, *p, len, - &(pk->pub_raw_len), pk->pub_raw, - PSA_EXPORT_PUBLIC_KEY_MAX_SIZE); + ret = pk_ecc_read_compressed(pk, *p, len, + pk->pub_raw, + PSA_EXPORT_PUBLIC_KEY_MAX_SIZE, + &pk->pub_raw_len); if (ret != 0) { return ret; } } else { /* Uncompressed format */ - if ((size_t) (end - *p) > MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN) { + if (len > MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN) { return MBEDTLS_ERR_PK_BUFFER_TOO_SMALL; } - memcpy(pk->pub_raw, *p, (end - *p)); - pk->pub_raw_len = end - *p; + memcpy(pk->pub_raw, *p, len); + pk->pub_raw_len = len; } /* Validate the key by trying to importing it */ @@ -778,7 +782,8 @@ static int pk_get_ecpubkey(unsigned char **p, const unsigned char *end, #endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ /* - * We know mbedtls_ecp_point_read_binary consumed all bytes or failed + * We know mbedtls_ecp_point_read_binary and pk_ecc_read_compressed either + * consumed all bytes or failed, and memcpy consumed all bytes too. */ *p = (unsigned char *) end; From 212517b87d72fb96145e066535f3a895884ea701 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 26 Jul 2023 12:05:38 +0200 Subject: [PATCH 11/27] Start re-ordering functions in pkparse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The general order is low-level first, top-level last, for the sake of static function and avoiding forward declarations. The obvious exception was functions that parse files that were at the beginning; move them to the end. Also start defining sections in the file; this is incomplete as I don't have a clear view of the beginning of the file yet. Will continue in further commits. Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 222 +++++++++++++++++++++++++--------------------- 1 file changed, 120 insertions(+), 102 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 3fa1a84227..fbf8cbfc11 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -61,108 +61,6 @@ ((id == MBEDTLS_ECP_DP_CURVE25519) || (id == MBEDTLS_ECP_DP_CURVE448)) #endif /* MBEDTLS_PK_HAVE_ECC_KEYS && MBEDTLS_PK_HAVE_RFC8410_CURVES */ -#if defined(MBEDTLS_FS_IO) -/* - * Load all data from a file into a given buffer. - * - * The file is expected to contain either PEM or DER encoded data. - * A terminating null byte is always appended. It is included in the announced - * length only if the data looks like it is PEM encoded. - */ -int mbedtls_pk_load_file(const char *path, unsigned char **buf, size_t *n) -{ - FILE *f; - long size; - - if ((f = fopen(path, "rb")) == NULL) { - return MBEDTLS_ERR_PK_FILE_IO_ERROR; - } - - /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ - mbedtls_setbuf(f, NULL); - - fseek(f, 0, SEEK_END); - if ((size = ftell(f)) == -1) { - fclose(f); - return MBEDTLS_ERR_PK_FILE_IO_ERROR; - } - fseek(f, 0, SEEK_SET); - - *n = (size_t) size; - - if (*n + 1 == 0 || - (*buf = mbedtls_calloc(1, *n + 1)) == NULL) { - fclose(f); - return MBEDTLS_ERR_PK_ALLOC_FAILED; - } - - if (fread(*buf, 1, *n, f) != *n) { - fclose(f); - - mbedtls_zeroize_and_free(*buf, *n); - - return MBEDTLS_ERR_PK_FILE_IO_ERROR; - } - - fclose(f); - - (*buf)[*n] = '\0'; - - if (strstr((const char *) *buf, "-----BEGIN ") != NULL) { - ++*n; - } - - return 0; -} - -/* - * Load and parse a private key - */ -int mbedtls_pk_parse_keyfile(mbedtls_pk_context *ctx, - const char *path, const char *pwd, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t n; - unsigned char *buf; - - if ((ret = mbedtls_pk_load_file(path, &buf, &n)) != 0) { - return ret; - } - - if (pwd == NULL) { - ret = mbedtls_pk_parse_key(ctx, buf, n, NULL, 0, f_rng, p_rng); - } else { - ret = mbedtls_pk_parse_key(ctx, buf, n, - (const unsigned char *) pwd, strlen(pwd), f_rng, p_rng); - } - - mbedtls_zeroize_and_free(buf, n); - - return ret; -} - -/* - * Load and parse a public key - */ -int mbedtls_pk_parse_public_keyfile(mbedtls_pk_context *ctx, const char *path) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t n; - unsigned char *buf; - - if ((ret = mbedtls_pk_load_file(path, &buf, &n)) != 0) { - return ret; - } - - ret = mbedtls_pk_parse_public_key(ctx, buf, n); - - mbedtls_zeroize_and_free(buf, n); - - return ret; -} -#endif /* MBEDTLS_FS_IO */ - #if defined(MBEDTLS_PK_HAVE_ECC_KEYS) /* Minimally parse an ECParameters buffer to and mbedtls_asn1_buf * @@ -1289,6 +1187,12 @@ static int pk_parse_key_sec1_der(mbedtls_pk_context *pk, } #endif /* MBEDTLS_PK_HAVE_ECC_KEYS */ +/*********************************************************************** + * + * PKCS#8 parsing functions + * + **********************************************************************/ + /* * Parse an unencrypted PKCS#8 encoded private key * @@ -1524,6 +1428,12 @@ MBEDTLS_STATIC_TESTABLE int mbedtls_pk_parse_key_pkcs8_encrypted_der( } #endif /* MBEDTLS_PKCS12_C || MBEDTLS_PKCS5_C */ +/*********************************************************************** + * + * Top-level functions, with format auto-discovery + * + **********************************************************************/ + /* * Parse a private key */ @@ -1843,4 +1753,112 @@ int mbedtls_pk_parse_public_key(mbedtls_pk_context *ctx, return ret; } +/*********************************************************************** + * + * Top-level functions, with filesystem support + * + **********************************************************************/ + +#if defined(MBEDTLS_FS_IO) +/* + * Load all data from a file into a given buffer. + * + * The file is expected to contain either PEM or DER encoded data. + * A terminating null byte is always appended. It is included in the announced + * length only if the data looks like it is PEM encoded. + */ +int mbedtls_pk_load_file(const char *path, unsigned char **buf, size_t *n) +{ + FILE *f; + long size; + + if ((f = fopen(path, "rb")) == NULL) { + return MBEDTLS_ERR_PK_FILE_IO_ERROR; + } + + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf(f, NULL); + + fseek(f, 0, SEEK_END); + if ((size = ftell(f)) == -1) { + fclose(f); + return MBEDTLS_ERR_PK_FILE_IO_ERROR; + } + fseek(f, 0, SEEK_SET); + + *n = (size_t) size; + + if (*n + 1 == 0 || + (*buf = mbedtls_calloc(1, *n + 1)) == NULL) { + fclose(f); + return MBEDTLS_ERR_PK_ALLOC_FAILED; + } + + if (fread(*buf, 1, *n, f) != *n) { + fclose(f); + + mbedtls_zeroize_and_free(*buf, *n); + + return MBEDTLS_ERR_PK_FILE_IO_ERROR; + } + + fclose(f); + + (*buf)[*n] = '\0'; + + if (strstr((const char *) *buf, "-----BEGIN ") != NULL) { + ++*n; + } + + return 0; +} + +/* + * Load and parse a private key + */ +int mbedtls_pk_parse_keyfile(mbedtls_pk_context *ctx, + const char *path, const char *pwd, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + size_t n; + unsigned char *buf; + + if ((ret = mbedtls_pk_load_file(path, &buf, &n)) != 0) { + return ret; + } + + if (pwd == NULL) { + ret = mbedtls_pk_parse_key(ctx, buf, n, NULL, 0, f_rng, p_rng); + } else { + ret = mbedtls_pk_parse_key(ctx, buf, n, + (const unsigned char *) pwd, strlen(pwd), f_rng, p_rng); + } + + mbedtls_zeroize_and_free(buf, n); + + return ret; +} + +/* + * Load and parse a public key + */ +int mbedtls_pk_parse_public_keyfile(mbedtls_pk_context *ctx, const char *path) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + size_t n; + unsigned char *buf; + + if ((ret = mbedtls_pk_load_file(path, &buf, &n)) != 0) { + return ret; + } + + ret = mbedtls_pk_parse_public_key(ctx, buf, n); + + mbedtls_zeroize_and_free(buf, n); + + return ret; +} +#endif /* MBEDTLS_FS_IO */ + #endif /* MBEDTLS_PK_PARSE_C */ From 997a95e5926d4ef64836e8702fe2d00c955b9878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 26 Jul 2023 15:18:30 +0200 Subject: [PATCH 12/27] Merge two consecutive #ifs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index fbf8cbfc11..98094a36fe 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -55,13 +55,14 @@ #include "mbedtls/pkcs12.h" #endif +#if defined(MBEDTLS_PK_HAVE_ECC_KEYS) + /* Helper for Montgomery curves */ -#if defined(MBEDTLS_PK_HAVE_ECC_KEYS) && defined(MBEDTLS_PK_HAVE_RFC8410_CURVES) +#if defined(MBEDTLS_PK_HAVE_RFC8410_CURVES) #define MBEDTLS_PK_IS_RFC8410_GROUP_ID(id) \ ((id == MBEDTLS_ECP_DP_CURVE25519) || (id == MBEDTLS_ECP_DP_CURVE448)) -#endif /* MBEDTLS_PK_HAVE_ECC_KEYS && MBEDTLS_PK_HAVE_RFC8410_CURVES */ +#endif /* MBEDTLS_PK_HAVE_RFC8410_CURVES */ -#if defined(MBEDTLS_PK_HAVE_ECC_KEYS) /* Minimally parse an ECParameters buffer to and mbedtls_asn1_buf * * ECParameters ::= CHOICE { From 5470898e37875e2cf29019c4e66322f8a4ba5e69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 26 Jul 2023 15:38:36 +0200 Subject: [PATCH 13/27] Move code around again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 566 ++++++++++++++++++++++++---------------------- 1 file changed, 292 insertions(+), 274 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 98094a36fe..5a35c43fbb 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -57,56 +57,274 @@ #if defined(MBEDTLS_PK_HAVE_ECC_KEYS) -/* Helper for Montgomery curves */ -#if defined(MBEDTLS_PK_HAVE_RFC8410_CURVES) -#define MBEDTLS_PK_IS_RFC8410_GROUP_ID(id) \ - ((id == MBEDTLS_ECP_DP_CURVE25519) || (id == MBEDTLS_ECP_DP_CURVE448)) -#endif /* MBEDTLS_PK_HAVE_RFC8410_CURVES */ - -/* Minimally parse an ECParameters buffer to and mbedtls_asn1_buf +/*********************************************************************** * - * ECParameters ::= CHOICE { - * namedCurve OBJECT IDENTIFIER - * specifiedCurve SpecifiedECDomain -- = SEQUENCE { ... } - * -- implicitCurve NULL - * } + * ECC setters + * + * 1. This is an abstraction layer around MBEDTLS_PK_USE_PSA_EC_DATA: + * this macro will not appear outside this section. + * 2. All inputs are raw: no metadata, no ASN.1 until the next section. + * + **********************************************************************/ + +/* + * Set the group used by this key. */ -static int pk_get_ecparams(unsigned char **p, const unsigned char *end, - mbedtls_asn1_buf *params) +static int pk_ecc_set_group(mbedtls_pk_context *pk, mbedtls_ecp_group_id grp_id) +{ +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + size_t ec_bits; + psa_ecc_family_t ec_family = mbedtls_ecc_group_to_psa(grp_id, &ec_bits); + + /* group may already be initialized; if so, make sure IDs match */ + if ((pk->ec_family != 0 && pk->ec_family != ec_family) || + (pk->ec_bits != 0 && pk->ec_bits != ec_bits)) { + return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; + } + + /* set group */ + pk->ec_family = ec_family; + pk->ec_bits = ec_bits; + + return 0; +#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ + mbedtls_ecp_keypair *ecp = mbedtls_pk_ec_rw(*pk); + + /* grp may already be initialized; if so, make sure IDs match */ + if (mbedtls_pk_ec_ro(*pk)->grp.id != MBEDTLS_ECP_DP_NONE && + mbedtls_pk_ec_ro(*pk)->grp.id != grp_id) { + return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; + } + + /* set group */ + return mbedtls_ecp_group_load(&(ecp->grp), grp_id); +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ +} + +/* + * Set the private key material + * + * Must have already set the group with pk_ecc_set_group(). + * + * The 'key' argument points to the raw private key (no ASN.1 wrapping). + */ +static int pk_ecc_set_key(mbedtls_pk_context *pk, + unsigned char *key, size_t len) +{ +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_status_t status; + + psa_set_key_type(&attributes, PSA_KEY_TYPE_ECC_KEY_PAIR(pk->ec_family)); + psa_set_key_algorithm(&attributes, PSA_ALG_ECDH); + psa_key_usage_t flags = PSA_KEY_USAGE_EXPORT | PSA_KEY_USAGE_DERIVE; + /* Montgomery allows only ECDH, others ECDSA too */ + if (pk->ec_family != PSA_ECC_FAMILY_MONTGOMERY) { + flags |= PSA_KEY_USAGE_SIGN_HASH | PSA_KEY_USAGE_SIGN_MESSAGE; + psa_set_key_enrollment_algorithm(&attributes, + MBEDTLS_PK_PSA_ALG_ECDSA_MAYBE_DET(PSA_ALG_ANY_HASH)); + } + psa_set_key_usage_flags(&attributes, flags); + + status = psa_import_key(&attributes, key, len, &pk->priv_id); + return psa_pk_status_to_mbedtls(status); + +#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ + + mbedtls_ecp_keypair *eck = mbedtls_pk_ec_rw(*pk); + int ret = mbedtls_ecp_read_key(eck->grp.id, eck, key, len); + if (ret != 0) { + return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); + } + return 0; +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ +} + +/* + * Helper function for deriving a public key from its private counterpart. + * + * Note: the private key information is always available from pk, + * however for convenience the serialized version is also passed, + * as it's available at each calling site, and useful in some configs + * (as otherwise we're have to re-serialize it from the pk context). + */ +static int pk_derive_public_key(mbedtls_pk_context *pk, + const unsigned char *d, size_t d_len, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) +{ +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + psa_status_t status; + (void) f_rng; + (void) p_rng; + (void) d; + (void) d_len; + + status = psa_export_public_key(pk->priv_id, pk->pub_raw, sizeof(pk->pub_raw), + &pk->pub_raw_len); + return psa_pk_status_to_mbedtls(status); +#elif defined(MBEDTLS_USE_PSA_CRYPTO) /* && !MBEDTLS_PK_USE_PSA_EC_DATA */ + int ret; + psa_status_t status; + (void) f_rng; + (void) p_rng; + + mbedtls_ecp_keypair *eck = (mbedtls_ecp_keypair *) pk->pk_ctx; + unsigned char key_buf[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; + size_t key_len; + mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; + psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; + size_t curve_bits; + psa_ecc_family_t curve = mbedtls_ecc_group_to_psa(eck->grp.id, &curve_bits); + psa_status_t destruction_status; + + psa_set_key_type(&key_attr, PSA_KEY_TYPE_ECC_KEY_PAIR(curve)); + psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_EXPORT); + + status = psa_import_key(&key_attr, d, d_len, &key_id); + ret = psa_pk_status_to_mbedtls(status); + if (ret != 0) { + return ret; + } + + status = psa_export_public_key(key_id, key_buf, sizeof(key_buf), &key_len); + 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); + } + return mbedtls_ecp_point_read_binary(&eck->grp, &eck->Q, key_buf, key_len); +#else /* MBEDTLS_USE_PSA_CRYPTO */ + mbedtls_ecp_keypair *eck = (mbedtls_ecp_keypair *) pk->pk_ctx; + (void) d; + (void) d_len; + + return mbedtls_ecp_mul(&eck->grp, &eck->Q, &eck->d, &eck->grp.G, f_rng, p_rng); +#endif /* MBEDTLS_USE_PSA_CRYPTO */ +} + +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) +/* + * Create a temporary ecp_keypair for converting an EC point in compressed + * format to an uncompressed one + * + * Consumes everything or fails - inherited from + * mbedtls_ecp_point_read_binary(). + */ +static int pk_ecc_read_compressed(mbedtls_pk_context *pk, + const unsigned char *in_start, size_t in_len, + unsigned char *out_buf, size_t out_buf_size, + size_t *out_buf_len) +{ +#if defined(MBEDTLS_PK_PARSE_EC_COMPRESSED) + mbedtls_ecp_keypair ecp_key; + mbedtls_ecp_group_id ecp_group_id; + int ret; + + ecp_group_id = mbedtls_ecc_group_of_psa(pk->ec_family, pk->ec_bits, 0); + + mbedtls_ecp_keypair_init(&ecp_key); + ret = mbedtls_ecp_group_load(&(ecp_key.grp), ecp_group_id); + if (ret != 0) { + return ret; + } + ret = mbedtls_ecp_point_read_binary(&(ecp_key.grp), &ecp_key.Q, + in_start, in_len); + if (ret != 0) { + goto exit; + } + ret = mbedtls_ecp_point_write_binary(&(ecp_key.grp), &ecp_key.Q, + MBEDTLS_ECP_PF_UNCOMPRESSED, + out_buf_len, out_buf, out_buf_size); + +exit: + mbedtls_ecp_keypair_free(&ecp_key); + return ret; +#else /* MBEDTLS_PK_PARSE_EC_COMPRESSED */ + return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; +#endif /* MBEDTLS_PK_PARSE_EC_COMPRESSED */ +} +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ + +/* + * EC public key is an EC point + * + * The caller is responsible for clearing the structure upon failure if + * desired. Take care to pass along the possible ECP_FEATURE_UNAVAILABLE + * return code of mbedtls_ecp_point_read_binary() and leave p in a usable state. + */ +static int pk_get_ecpubkey(unsigned char **p, const unsigned char *end, + mbedtls_pk_context *pk) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - if (end - *p < 1) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, - MBEDTLS_ERR_ASN1_OUT_OF_DATA); +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + mbedtls_svc_key_id_t key; + psa_key_attributes_t key_attrs = PSA_KEY_ATTRIBUTES_INIT; + size_t len = (size_t) (end - *p); + + if (len > PSA_EXPORT_PUBLIC_KEY_MAX_SIZE) { + return MBEDTLS_ERR_PK_BAD_INPUT_DATA; } - /* Tag may be either OID or SEQUENCE */ - params->tag = **p; - if (params->tag != MBEDTLS_ASN1_OID -#if defined(MBEDTLS_PK_PARSE_EC_EXTENDED) - && params->tag != (MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE) -#endif - ) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, - MBEDTLS_ERR_ASN1_UNEXPECTED_TAG); + if ((**p == 0x02) || (**p == 0x03)) { + /* Compressed format, not supported by PSA Crypto. + * Try converting using functions from ECP_LIGHT. */ + ret = pk_ecc_read_compressed(pk, *p, len, + pk->pub_raw, + PSA_EXPORT_PUBLIC_KEY_MAX_SIZE, + &pk->pub_raw_len); + if (ret != 0) { + return ret; + } + } else { + /* Uncompressed format */ + if (len > MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN) { + return MBEDTLS_ERR_PK_BUFFER_TOO_SMALL; + } + memcpy(pk->pub_raw, *p, len); + pk->pub_raw_len = len; } - if ((ret = mbedtls_asn1_get_tag(p, end, ¶ms->len, params->tag)) != 0) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); + /* Validate the key by trying to importing it */ + psa_set_key_usage_flags(&key_attrs, 0); + psa_set_key_algorithm(&key_attrs, PSA_ALG_ECDSA_ANY); + psa_set_key_type(&key_attrs, PSA_KEY_TYPE_ECC_PUBLIC_KEY(pk->ec_family)); + psa_set_key_bits(&key_attrs, pk->ec_bits); + + if ((psa_import_key(&key_attrs, pk->pub_raw, pk->pub_raw_len, + &key) != PSA_SUCCESS) || + (psa_destroy_key(key) != PSA_SUCCESS)) { + mbedtls_platform_zeroize(pk->pub_raw, MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN); + pk->pub_raw_len = 0; + return MBEDTLS_ERR_PK_BAD_INPUT_DATA; } - - params->p = *p; - *p += params->len; - - if (*p != end) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, - MBEDTLS_ERR_ASN1_LENGTH_MISMATCH); + ret = 0; +#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ + mbedtls_ecp_keypair *ec_key = (mbedtls_ecp_keypair *) pk->pk_ctx; + if ((ret = mbedtls_ecp_point_read_binary(&ec_key->grp, &ec_key->Q, + (const unsigned char *) *p, + end - *p)) == 0) { + ret = mbedtls_ecp_check_pubkey(&ec_key->grp, &ec_key->Q); } +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ - return 0; + /* + * We know mbedtls_ecp_point_read_binary and pk_ecc_read_compressed either + * consumed all bytes or failed, and memcpy consumed all bytes too. + */ + *p = (unsigned char *) end; + + return ret; } +/*********************************************************************** + * + * Unsorted (yet!) from this point on until the next section header + * + **********************************************************************/ + #if defined(MBEDTLS_PK_PARSE_EC_EXTENDED) /* * Parse a SpecifiedECDomain (SEC 1 C.2) and (mostly) fill the group with it. @@ -352,77 +570,49 @@ cleanup: } #endif /* MBEDTLS_PK_PARSE_EC_EXTENDED */ -/* - * Set the group used by this key. - */ -static int pk_ecc_set_group(mbedtls_pk_context *pk, mbedtls_ecp_group_id grp_id) -{ -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) - size_t ec_bits; - psa_ecc_family_t ec_family = mbedtls_ecc_group_to_psa(grp_id, &ec_bits); - /* group may already be initialized; if so, make sure IDs match */ - if ((pk->ec_family != 0 && pk->ec_family != ec_family) || - (pk->ec_bits != 0 && pk->ec_bits != ec_bits)) { - return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; - } - - /* set group */ - pk->ec_family = ec_family; - pk->ec_bits = ec_bits; - - return 0; -#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ - mbedtls_ecp_keypair *ecp = mbedtls_pk_ec_rw(*pk); - - /* grp may already be initialized; if so, make sure IDs match */ - if (mbedtls_pk_ec_ro(*pk)->grp.id != MBEDTLS_ECP_DP_NONE && - mbedtls_pk_ec_ro(*pk)->grp.id != grp_id) { - return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; - } - - /* set group */ - return mbedtls_ecp_group_load(&(ecp->grp), grp_id); -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ -} - -/* - * Set the private key material +/* Minimally parse an ECParameters buffer to and mbedtls_asn1_buf * - * Must have already set the group with pk_ecc_set_group(). - * - * The 'key' argument points to the raw private key (no ASN.1 wrapping). + * ECParameters ::= CHOICE { + * namedCurve OBJECT IDENTIFIER + * specifiedCurve SpecifiedECDomain -- = SEQUENCE { ... } + * -- implicitCurve NULL + * } */ -static int pk_ecc_set_key(mbedtls_pk_context *pk, - unsigned char *key, size_t len) +static int pk_get_ecparams(unsigned char **p, const unsigned char *end, + mbedtls_asn1_buf *params) { -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) - psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - psa_status_t status; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - psa_set_key_type(&attributes, PSA_KEY_TYPE_ECC_KEY_PAIR(pk->ec_family)); - psa_set_key_algorithm(&attributes, PSA_ALG_ECDH); - psa_key_usage_t flags = PSA_KEY_USAGE_EXPORT | PSA_KEY_USAGE_DERIVE; - /* Montgomery allows only ECDH, others ECDSA too */ - if (pk->ec_family != PSA_ECC_FAMILY_MONTGOMERY) { - flags |= PSA_KEY_USAGE_SIGN_HASH | PSA_KEY_USAGE_SIGN_MESSAGE; - psa_set_key_enrollment_algorithm(&attributes, - MBEDTLS_PK_PSA_ALG_ECDSA_MAYBE_DET(PSA_ALG_ANY_HASH)); + if (end - *p < 1) { + return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, + MBEDTLS_ERR_ASN1_OUT_OF_DATA); } - psa_set_key_usage_flags(&attributes, flags); - status = psa_import_key(&attributes, key, len, &pk->priv_id); - return psa_pk_status_to_mbedtls(status); + /* Tag may be either OID or SEQUENCE */ + params->tag = **p; + if (params->tag != MBEDTLS_ASN1_OID +#if defined(MBEDTLS_PK_PARSE_EC_EXTENDED) + && params->tag != (MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE) +#endif + ) { + return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG); + } -#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ - - mbedtls_ecp_keypair *eck = mbedtls_pk_ec_rw(*pk); - int ret = mbedtls_ecp_read_key(eck->grp.id, eck, key, len); - if (ret != 0) { + if ((ret = mbedtls_asn1_get_tag(p, end, ¶ms->len, params->tag)) != 0) { return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); } + + params->p = *p; + *p += params->len; + + if (*p != end) { + return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH); + } + return 0; -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ } /* @@ -455,70 +645,6 @@ static int pk_use_ecparams(const mbedtls_asn1_buf *params, mbedtls_pk_context *p return pk_ecc_set_group(pk, grp_id); } -/* - * Helper function for deriving a public key from its private counterpart. - * - * Note: the private key information is always available from pk, - * however for convenience the serialized version is also passed, - * as it's available at each calling site, and useful in some configs - * (as otherwise we're have to re-serialize it from the pk context). - */ -static int pk_derive_public_key(mbedtls_pk_context *pk, - const unsigned char *d, size_t d_len, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) -{ -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) - psa_status_t status; - (void) f_rng; - (void) p_rng; - (void) d; - (void) d_len; - - status = psa_export_public_key(pk->priv_id, pk->pub_raw, sizeof(pk->pub_raw), - &pk->pub_raw_len); - return psa_pk_status_to_mbedtls(status); -#elif defined(MBEDTLS_USE_PSA_CRYPTO) /* && !MBEDTLS_PK_USE_PSA_EC_DATA */ - int ret; - psa_status_t status; - (void) f_rng; - (void) p_rng; - - mbedtls_ecp_keypair *eck = (mbedtls_ecp_keypair *) pk->pk_ctx; - unsigned char key_buf[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; - size_t key_len; - mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; - psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; - size_t curve_bits; - psa_ecc_family_t curve = mbedtls_ecc_group_to_psa(eck->grp.id, &curve_bits); - psa_status_t destruction_status; - - psa_set_key_type(&key_attr, PSA_KEY_TYPE_ECC_KEY_PAIR(curve)); - psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_EXPORT); - - status = psa_import_key(&key_attr, d, d_len, &key_id); - ret = psa_pk_status_to_mbedtls(status); - if (ret != 0) { - return ret; - } - - status = psa_export_public_key(key_id, key_buf, sizeof(key_buf), &key_len); - 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); - } - return mbedtls_ecp_point_read_binary(&eck->grp, &eck->Q, key_buf, key_len); -#else /* MBEDTLS_USE_PSA_CRYPTO */ - mbedtls_ecp_keypair *eck = (mbedtls_ecp_keypair *) pk->pk_ctx; - (void) d; - (void) d_len; - - return mbedtls_ecp_mul(&eck->grp, &eck->Q, &eck->d, &eck->grp.G, f_rng, p_rng); -#endif /* MBEDTLS_USE_PSA_CRYPTO */ -} - #if defined(MBEDTLS_PK_HAVE_RFC8410_CURVES) /* @@ -574,120 +700,6 @@ static int pk_parse_key_rfc8410_der(mbedtls_pk_context *pk, } #endif /* MBEDTLS_PK_HAVE_RFC8410_CURVES */ -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) -/* - * Create a temporary ecp_keypair for converting an EC point in compressed - * format to an uncompressed one - * - * Consumes everything or fails - inherited from - * mbedtls_ecp_point_read_binary(). - */ -static int pk_ecc_read_compressed(mbedtls_pk_context *pk, - const unsigned char *in_start, size_t in_len, - unsigned char *out_buf, size_t out_buf_size, - size_t *out_buf_len) -{ -#if defined(MBEDTLS_PK_PARSE_EC_COMPRESSED) - mbedtls_ecp_keypair ecp_key; - mbedtls_ecp_group_id ecp_group_id; - int ret; - - ecp_group_id = mbedtls_ecc_group_of_psa(pk->ec_family, pk->ec_bits, 0); - - mbedtls_ecp_keypair_init(&ecp_key); - ret = mbedtls_ecp_group_load(&(ecp_key.grp), ecp_group_id); - if (ret != 0) { - return ret; - } - ret = mbedtls_ecp_point_read_binary(&(ecp_key.grp), &ecp_key.Q, - in_start, in_len); - if (ret != 0) { - goto exit; - } - ret = mbedtls_ecp_point_write_binary(&(ecp_key.grp), &ecp_key.Q, - MBEDTLS_ECP_PF_UNCOMPRESSED, - out_buf_len, out_buf, out_buf_size); - -exit: - mbedtls_ecp_keypair_free(&ecp_key); - return ret; -#else /* MBEDTLS_PK_PARSE_EC_COMPRESSED */ - return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; -#endif /* MBEDTLS_PK_PARSE_EC_COMPRESSED */ -} -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ - -/* - * EC public key is an EC point - * - * The caller is responsible for clearing the structure upon failure if - * desired. Take care to pass along the possible ECP_FEATURE_UNAVAILABLE - * return code of mbedtls_ecp_point_read_binary() and leave p in a usable state. - */ -static int pk_get_ecpubkey(unsigned char **p, const unsigned char *end, - mbedtls_pk_context *pk) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) - mbedtls_svc_key_id_t key; - psa_key_attributes_t key_attrs = PSA_KEY_ATTRIBUTES_INIT; - size_t len = (size_t) (end - *p); - - if (len > PSA_EXPORT_PUBLIC_KEY_MAX_SIZE) { - return MBEDTLS_ERR_PK_BAD_INPUT_DATA; - } - - if ((**p == 0x02) || (**p == 0x03)) { - /* Compressed format, not supported by PSA Crypto. - * Try converting using functions from ECP_LIGHT. */ - ret = pk_ecc_read_compressed(pk, *p, len, - pk->pub_raw, - PSA_EXPORT_PUBLIC_KEY_MAX_SIZE, - &pk->pub_raw_len); - if (ret != 0) { - return ret; - } - } else { - /* Uncompressed format */ - if (len > MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN) { - return MBEDTLS_ERR_PK_BUFFER_TOO_SMALL; - } - memcpy(pk->pub_raw, *p, len); - pk->pub_raw_len = len; - } - - /* Validate the key by trying to importing it */ - psa_set_key_usage_flags(&key_attrs, 0); - psa_set_key_algorithm(&key_attrs, PSA_ALG_ECDSA_ANY); - psa_set_key_type(&key_attrs, PSA_KEY_TYPE_ECC_PUBLIC_KEY(pk->ec_family)); - psa_set_key_bits(&key_attrs, pk->ec_bits); - - if ((psa_import_key(&key_attrs, pk->pub_raw, pk->pub_raw_len, - &key) != PSA_SUCCESS) || - (psa_destroy_key(key) != PSA_SUCCESS)) { - mbedtls_platform_zeroize(pk->pub_raw, MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN); - pk->pub_raw_len = 0; - return MBEDTLS_ERR_PK_BAD_INPUT_DATA; - } - ret = 0; -#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ - mbedtls_ecp_keypair *ec_key = (mbedtls_ecp_keypair *) pk->pk_ctx; - if ((ret = mbedtls_ecp_point_read_binary(&ec_key->grp, &ec_key->Q, - (const unsigned char *) *p, - end - *p)) == 0) { - ret = mbedtls_ecp_check_pubkey(&ec_key->grp, &ec_key->Q); - } -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ - - /* - * We know mbedtls_ecp_point_read_binary and pk_ecc_read_compressed either - * consumed all bytes or failed, and memcpy consumed all bytes too. - */ - *p = (unsigned char *) end; - - return ret; -} #endif /* MBEDTLS_PK_HAVE_ECC_KEYS */ #if defined(MBEDTLS_RSA_C) @@ -799,6 +811,12 @@ static int pk_get_pk_alg(unsigned char **p, return 0; } +/* Helper for Montgomery curves */ +#if defined(MBEDTLS_PK_HAVE_RFC8410_CURVES) +#define MBEDTLS_PK_IS_RFC8410_GROUP_ID(id) \ + ((id == MBEDTLS_ECP_DP_CURVE25519) || (id == MBEDTLS_ECP_DP_CURVE448)) +#endif /* MBEDTLS_PK_HAVE_RFC8410_CURVES */ + /* * SubjectPublicKeyInfo ::= SEQUENCE { * algorithm AlgorithmIdentifier, From d1aa64239443ec4de20c0571f2dc56b50afa2586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 26 Jul 2023 22:24:23 +0200 Subject: [PATCH 14/27] Document pk_ecc_set_group() and pk_ecc_set_key() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 5a35c43fbb..a123743582 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -69,6 +69,10 @@ /* * Set the group used by this key. + * + * [in/out] pk: in: must have been pk_setup() to an ECC type + * out: will have group (curve) information set + * [in] grp_in: a supported group ID (not NONE) */ static int pk_ecc_set_group(mbedtls_pk_context *pk, mbedtls_ecp_group_id grp_id) { @@ -104,12 +108,12 @@ static int pk_ecc_set_group(mbedtls_pk_context *pk, mbedtls_ecp_group_id grp_id) /* * Set the private key material * - * Must have already set the group with pk_ecc_set_group(). - * - * The 'key' argument points to the raw private key (no ASN.1 wrapping). + * [in/out] pk: in: must have the group set already, see pk_ecc_set_group(). + * out: will have the private key set. + * [in] key, key_len: the raw private key (no ASN.1 wrapping). */ static int pk_ecc_set_key(mbedtls_pk_context *pk, - unsigned char *key, size_t len) + unsigned char *key, size_t key_len) { #if defined(MBEDTLS_PK_USE_PSA_EC_DATA) psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; @@ -126,13 +130,13 @@ static int pk_ecc_set_key(mbedtls_pk_context *pk, } psa_set_key_usage_flags(&attributes, flags); - status = psa_import_key(&attributes, key, len, &pk->priv_id); + status = psa_import_key(&attributes, key, key_len, &pk->priv_id); return psa_pk_status_to_mbedtls(status); #else /* MBEDTLS_PK_USE_PSA_EC_DATA */ mbedtls_ecp_keypair *eck = mbedtls_pk_ec_rw(*pk); - int ret = mbedtls_ecp_read_key(eck->grp.id, eck, key, len); + int ret = mbedtls_ecp_read_key(eck->grp.id, eck, key, key_len); if (ret != 0) { return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); } From de25194a20a978a253f8804f47c7cbcf7c402ef8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 26 Jul 2023 22:33:58 +0200 Subject: [PATCH 15/27] Rename and document pk_ecc_set_pubkey_from_prv() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index a123743582..007e1fc03f 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -145,23 +145,34 @@ static int pk_ecc_set_key(mbedtls_pk_context *pk, } /* - * Helper function for deriving a public key from its private counterpart. + * Derive a public key from its private counterpart. + * Computationally intensive, only use when public key is not available. + * + * [in/out] pk: in: must have the private key set, see pk_ecc_set_key(). + * out: will have the public key set. + * [in] prv, prv_len: the raw private key (see note below). + * [in] f_rng, p_rng: RNG function and context. * * Note: the private key information is always available from pk, * however for convenience the serialized version is also passed, * as it's available at each calling site, and useful in some configs * (as otherwise we're have to re-serialize it from the pk context). + * + * There are three implementations of this function: + * 1. MBEDTLS_PK_USE_PSA_EC_DATA, + * 2. MBEDTLS_USE_PSA_CRYPTO but not MBEDTLS_PK_USE_PSA_EC_DATA, + * 3. not MBEDTLS_USE_PSA_CRYPTO. */ -static int pk_derive_public_key(mbedtls_pk_context *pk, - const unsigned char *d, size_t d_len, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) +static int pk_ecc_set_pubkey_from_prv(mbedtls_pk_context *pk, + const unsigned char *prv, size_t prv_len, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) { #if defined(MBEDTLS_PK_USE_PSA_EC_DATA) psa_status_t status; (void) f_rng; (void) p_rng; - (void) d; - (void) d_len; + (void) prv; + (void) prv_len; status = psa_export_public_key(pk->priv_id, pk->pub_raw, sizeof(pk->pub_raw), &pk->pub_raw_len); @@ -184,7 +195,7 @@ static int pk_derive_public_key(mbedtls_pk_context *pk, psa_set_key_type(&key_attr, PSA_KEY_TYPE_ECC_KEY_PAIR(curve)); psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_EXPORT); - status = psa_import_key(&key_attr, d, d_len, &key_id); + status = psa_import_key(&key_attr, prv, prv_len, &key_id); ret = psa_pk_status_to_mbedtls(status); if (ret != 0) { return ret; @@ -201,8 +212,8 @@ static int pk_derive_public_key(mbedtls_pk_context *pk, return mbedtls_ecp_point_read_binary(&eck->grp, &eck->Q, key_buf, key_len); #else /* MBEDTLS_USE_PSA_CRYPTO */ mbedtls_ecp_keypair *eck = (mbedtls_ecp_keypair *) pk->pk_ctx; - (void) d; - (void) d_len; + (void) prv; + (void) prv_len; return mbedtls_ecp_mul(&eck->grp, &eck->Q, &eck->d, &eck->grp.G, f_rng, p_rng); #endif /* MBEDTLS_USE_PSA_CRYPTO */ @@ -696,7 +707,7 @@ static int pk_parse_key_rfc8410_der(mbedtls_pk_context *pk, /* pk_parse_key_pkcs8_unencrypted_der() only supports version 1 PKCS8 keys, * which never contain a public key. As such, derive the public key * unconditionally. */ - if ((ret = pk_derive_public_key(pk, key, len, f_rng, p_rng)) != 0) { + if ((ret = pk_ecc_set_pubkey_from_prv(pk, key, len, f_rng, p_rng)) != 0) { return ret; } @@ -1201,7 +1212,7 @@ static int pk_parse_key_sec1_der(mbedtls_pk_context *pk, } if (!pubkey_done) { - if ((ret = pk_derive_public_key(pk, d, d_len, f_rng, p_rng)) != 0) { + if ((ret = pk_ecc_set_pubkey_from_prv(pk, d, d_len, f_rng, p_rng)) != 0) { return ret; } } From 0b8e45650f0b8d322a823b61ad9a90a2e7cbbbfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 26 Jul 2023 22:43:25 +0200 Subject: [PATCH 16/27] Tune body of pk_ecc_set_pubkey_from_prv() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - avoid useless use of ret in PSA code, keep only status - improve variable names - keep declarations closer to use - a few internal comments Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 007e1fc03f..826412a54e 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -168,54 +168,59 @@ static int pk_ecc_set_pubkey_from_prv(mbedtls_pk_context *pk, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) { #if defined(MBEDTLS_PK_USE_PSA_EC_DATA) - psa_status_t status; + (void) f_rng; (void) p_rng; (void) prv; (void) prv_len; + psa_status_t status; status = psa_export_public_key(pk->priv_id, pk->pub_raw, sizeof(pk->pub_raw), &pk->pub_raw_len); return psa_pk_status_to_mbedtls(status); + #elif defined(MBEDTLS_USE_PSA_CRYPTO) /* && !MBEDTLS_PK_USE_PSA_EC_DATA */ - int ret; - psa_status_t status; + (void) f_rng; (void) p_rng; + psa_status_t status; mbedtls_ecp_keypair *eck = (mbedtls_ecp_keypair *) pk->pk_ctx; - unsigned char key_buf[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; - size_t key_len; - mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; - psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; size_t curve_bits; psa_ecc_family_t curve = mbedtls_ecc_group_to_psa(eck->grp.id, &curve_bits); - psa_status_t destruction_status; + /* Import private key into PSA, from serialized input */ + mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; + psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; psa_set_key_type(&key_attr, PSA_KEY_TYPE_ECC_KEY_PAIR(curve)); psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_EXPORT); - status = psa_import_key(&key_attr, prv, prv_len, &key_id); - ret = psa_pk_status_to_mbedtls(status); - if (ret != 0) { - return ret; + if (status != PSA_SUCCESS) { + return psa_pk_status_to_mbedtls(status); } - status = psa_export_public_key(key_id, key_buf, sizeof(key_buf), &key_len); - ret = psa_pk_status_to_mbedtls(status); - destruction_status = psa_destroy_key(key_id); - if (ret != 0) { - return ret; + /* Export public key from PSA */ + unsigned char pub[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; + size_t pub_len; + status = psa_export_public_key(key_id, pub, sizeof(pub), &pub_len); + psa_status_t destruction_status = psa_destroy_key(key_id); + if (status != PSA_SUCCESS) { + return psa_pk_status_to_mbedtls(status); } else if (destruction_status != PSA_SUCCESS) { return psa_pk_status_to_mbedtls(destruction_status); } - return mbedtls_ecp_point_read_binary(&eck->grp, &eck->Q, key_buf, key_len); + + /* Load serialized public key into ecp_keypair structure */ + return mbedtls_ecp_point_read_binary(&eck->grp, &eck->Q, pub, pub_len); + #else /* MBEDTLS_USE_PSA_CRYPTO */ - mbedtls_ecp_keypair *eck = (mbedtls_ecp_keypair *) pk->pk_ctx; + (void) prv; (void) prv_len; + mbedtls_ecp_keypair *eck = (mbedtls_ecp_keypair *) pk->pk_ctx; return mbedtls_ecp_mul(&eck->grp, &eck->Q, &eck->d, &eck->grp.G, f_rng, p_rng); + #endif /* MBEDTLS_USE_PSA_CRYPTO */ } From 681e30b7275c408549db58a20c8a999c98ab4fce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 26 Jul 2023 23:03:35 +0200 Subject: [PATCH 17/27] Rework pk_ecc_set_pubkey_psa_ecp_fallback() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - new semantic: sets the pubkey directly in the PK context - new name to reflect that semantic and obey the naming scheme - trivial case first - documentation and better parameter names Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 826412a54e..91f56062d3 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -226,18 +226,26 @@ static int pk_ecc_set_pubkey_from_prv(mbedtls_pk_context *pk, #if defined(MBEDTLS_PK_USE_PSA_EC_DATA) /* - * Create a temporary ecp_keypair for converting an EC point in compressed - * format to an uncompressed one + * Set the public key: fallback using ECP_LIGHT in the USE_PSA_EC_DATA case. * - * Consumes everything or fails - inherited from - * mbedtls_ecp_point_read_binary(). + * Normally, when MBEDTLS_PK_USE_PSA_EC_DATA is enabled, we only use PSA + * functions to handle keys. However, currently psa_import_key() does not + * support compressed points. In case that support was explicitly requested, + * this fallback uses ECP functions to get the job done. This is the reason + * why MBEDTLS_PK_PARSE_EC_COMPRESSED auto-enables MBEDTLS_ECP_LIGHT. + * + * [in/out] pk: in: must have the group set, see pk_ecc_set_group(). + * out: will have the public key set. + * [in] pub, pub_len: the public key as an ECPoint, + * in any format supported by ECP. */ -static int pk_ecc_read_compressed(mbedtls_pk_context *pk, - const unsigned char *in_start, size_t in_len, - unsigned char *out_buf, size_t out_buf_size, - size_t *out_buf_len) +static int pk_ecc_set_pubkey_psa_ecp_fallback(mbedtls_pk_context *pk, + const unsigned char *pub, + size_t pub_len) { -#if defined(MBEDTLS_PK_PARSE_EC_COMPRESSED) +#if !defined(MBEDTLS_PK_PARSE_EC_COMPRESSED) + return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; +#else /* MBEDTLS_PK_PARSE_EC_COMPRESSED */ mbedtls_ecp_keypair ecp_key; mbedtls_ecp_group_id ecp_group_id; int ret; @@ -250,19 +258,18 @@ static int pk_ecc_read_compressed(mbedtls_pk_context *pk, return ret; } ret = mbedtls_ecp_point_read_binary(&(ecp_key.grp), &ecp_key.Q, - in_start, in_len); + pub, pub_len); if (ret != 0) { goto exit; } ret = mbedtls_ecp_point_write_binary(&(ecp_key.grp), &ecp_key.Q, MBEDTLS_ECP_PF_UNCOMPRESSED, - out_buf_len, out_buf, out_buf_size); + &pk->pub_raw_len, pk->pub_raw, + sizeof(pk->pub_raw)); exit: mbedtls_ecp_keypair_free(&ecp_key); return ret; -#else /* MBEDTLS_PK_PARSE_EC_COMPRESSED */ - return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; #endif /* MBEDTLS_PK_PARSE_EC_COMPRESSED */ } #endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ @@ -291,10 +298,7 @@ static int pk_get_ecpubkey(unsigned char **p, const unsigned char *end, if ((**p == 0x02) || (**p == 0x03)) { /* Compressed format, not supported by PSA Crypto. * Try converting using functions from ECP_LIGHT. */ - ret = pk_ecc_read_compressed(pk, *p, len, - pk->pub_raw, - PSA_EXPORT_PUBLIC_KEY_MAX_SIZE, - &pk->pub_raw_len); + ret = pk_ecc_set_pubkey_psa_ecp_fallback(pk, *p, len); if (ret != 0) { return ret; } From e4c883bc8cc64141b764b30ad0bc5c820e6831d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 26 Jul 2023 23:31:01 +0200 Subject: [PATCH 18/27] New signature for pk_ecc_set_pubkey() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also new name, for consistency, and documentation. The signature **p, *end is mostly for parsing functions that may not consume everything, and need to update the "current" pointer to reflect what has been consumed. This is not the case here. Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 42 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 91f56062d3..f897c1e8c1 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -275,40 +275,39 @@ exit: #endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ /* - * EC public key is an EC point + * Set the public key. * - * The caller is responsible for clearing the structure upon failure if - * desired. Take care to pass along the possible ECP_FEATURE_UNAVAILABLE - * return code of mbedtls_ecp_point_read_binary() and leave p in a usable state. + * [in/out] pk: in: must have its group set, see pk_ecc_set_group(). + * out: will have the public key set. + * [in] pub, pub_len: the raw public key (an ECPoint). */ -static int pk_get_ecpubkey(unsigned char **p, const unsigned char *end, - mbedtls_pk_context *pk) +static int pk_ecc_set_pubkey(mbedtls_pk_context *pk, + const unsigned char *pub, size_t pub_len) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; #if defined(MBEDTLS_PK_USE_PSA_EC_DATA) mbedtls_svc_key_id_t key; psa_key_attributes_t key_attrs = PSA_KEY_ATTRIBUTES_INIT; - size_t len = (size_t) (end - *p); - if (len > PSA_EXPORT_PUBLIC_KEY_MAX_SIZE) { + if (pub_len > PSA_EXPORT_PUBLIC_KEY_MAX_SIZE) { return MBEDTLS_ERR_PK_BAD_INPUT_DATA; } - if ((**p == 0x02) || (**p == 0x03)) { + if ((*pub == 0x02) || (*pub == 0x03)) { /* Compressed format, not supported by PSA Crypto. * Try converting using functions from ECP_LIGHT. */ - ret = pk_ecc_set_pubkey_psa_ecp_fallback(pk, *p, len); + ret = pk_ecc_set_pubkey_psa_ecp_fallback(pk, pub, pub_len); if (ret != 0) { return ret; } } else { /* Uncompressed format */ - if (len > MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN) { + if (pub_len > MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN) { return MBEDTLS_ERR_PK_BUFFER_TOO_SMALL; } - memcpy(pk->pub_raw, *p, len); - pk->pub_raw_len = len; + memcpy(pk->pub_raw, pub, pub_len); + pk->pub_raw_len = pub_len; } /* Validate the key by trying to importing it */ @@ -328,18 +327,10 @@ static int pk_get_ecpubkey(unsigned char **p, const unsigned char *end, #else /* MBEDTLS_PK_USE_PSA_EC_DATA */ mbedtls_ecp_keypair *ec_key = (mbedtls_ecp_keypair *) pk->pk_ctx; if ((ret = mbedtls_ecp_point_read_binary(&ec_key->grp, &ec_key->Q, - (const unsigned char *) *p, - end - *p)) == 0) { + pub, pub_len)) == 0) { ret = mbedtls_ecp_check_pubkey(&ec_key->grp, &ec_key->Q); } #endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ - - /* - * We know mbedtls_ecp_point_read_binary and pk_ecc_read_compressed either - * consumed all bytes or failed, and memcpy consumed all bytes too. - */ - *p = (unsigned char *) end; - return ret; } @@ -900,7 +891,8 @@ int mbedtls_pk_parse_subpubkey(unsigned char **p, const unsigned char *end, ret = pk_use_ecparams(&alg_params, pk); } if (ret == 0) { - ret = pk_get_ecpubkey(p, end, pk); + ret = pk_ecc_set_pubkey(pk, *p, end - *p); + *p += end - *p; } } else #endif /* MBEDTLS_PK_HAVE_ECC_KEYS */ @@ -1204,11 +1196,11 @@ static int pk_parse_key_sec1_der(mbedtls_pk_context *pk, MBEDTLS_ERR_ASN1_LENGTH_MISMATCH); } - if ((ret = pk_get_ecpubkey(&p, end2, pk)) == 0) { + if ((ret = pk_ecc_set_pubkey(pk, p, end2 - p)) == 0) { pubkey_done = 1; } else { /* - * The only acceptable failure mode of pk_get_ecpubkey() above + * The only acceptable failure mode of pk_ecc_set_pubkey() above * is if the point format is not recognized. */ if (ret != MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE) { From ff72ea9d518b5cfda03288adb586e0c6055d6103 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 26 Jul 2023 23:56:05 +0200 Subject: [PATCH 19/27] Rework pk_ecc_set_pubkey() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix the logic around format: we were just assuming that if the format was not compressed, it was uncompressed, but it could also have been just invalid. - Remove redundant length check: the fallback does its own checks. - Remove set_algorithm() that's not needed and introduced a depencency on ECDSA. - Some style / naming / scope reduction. Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 57 ++++++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index f897c1e8c1..e8678ed348 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -284,54 +284,51 @@ exit: static int pk_ecc_set_pubkey(mbedtls_pk_context *pk, const unsigned char *pub, size_t pub_len) { - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - #if defined(MBEDTLS_PK_USE_PSA_EC_DATA) - mbedtls_svc_key_id_t key; - psa_key_attributes_t key_attrs = PSA_KEY_ATTRIBUTES_INIT; - if (pub_len > PSA_EXPORT_PUBLIC_KEY_MAX_SIZE) { - return MBEDTLS_ERR_PK_BAD_INPUT_DATA; - } - - if ((*pub == 0x02) || (*pub == 0x03)) { - /* Compressed format, not supported by PSA Crypto. - * Try converting using functions from ECP_LIGHT. */ - ret = pk_ecc_set_pubkey_psa_ecp_fallback(pk, pub, pub_len); - if (ret != 0) { - return ret; - } - } else { - /* Uncompressed format */ - if (pub_len > MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN) { + /* Load the key */ + if (*pub == 0x04) { + /* Uncompressed format, directly supported by PSA */ + if (pub_len > sizeof(pk->pub_raw)) { return MBEDTLS_ERR_PK_BUFFER_TOO_SMALL; } memcpy(pk->pub_raw, pub, pub_len); pk->pub_raw_len = pub_len; + } else { + /* Other format, try the fallback */ + int ret = pk_ecc_set_pubkey_psa_ecp_fallback(pk, pub, pub_len); + if (ret != 0) { + return ret; + } } - /* Validate the key by trying to importing it */ + /* Validate the key by trying to import it */ + mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; + psa_key_attributes_t key_attrs = PSA_KEY_ATTRIBUTES_INIT; + psa_set_key_usage_flags(&key_attrs, 0); - psa_set_key_algorithm(&key_attrs, PSA_ALG_ECDSA_ANY); psa_set_key_type(&key_attrs, PSA_KEY_TYPE_ECC_PUBLIC_KEY(pk->ec_family)); psa_set_key_bits(&key_attrs, pk->ec_bits); if ((psa_import_key(&key_attrs, pk->pub_raw, pk->pub_raw_len, - &key) != PSA_SUCCESS) || - (psa_destroy_key(key) != PSA_SUCCESS)) { - mbedtls_platform_zeroize(pk->pub_raw, MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN); - pk->pub_raw_len = 0; - return MBEDTLS_ERR_PK_BAD_INPUT_DATA; + &key_id) != PSA_SUCCESS) || + (psa_destroy_key(key_id) != PSA_SUCCESS)) { + return MBEDTLS_ERR_PK_INVALID_PUBKEY; } - ret = 0; + + return 0; + #else /* MBEDTLS_PK_USE_PSA_EC_DATA */ + + int ret; mbedtls_ecp_keypair *ec_key = (mbedtls_ecp_keypair *) pk->pk_ctx; - if ((ret = mbedtls_ecp_point_read_binary(&ec_key->grp, &ec_key->Q, - pub, pub_len)) == 0) { - ret = mbedtls_ecp_check_pubkey(&ec_key->grp, &ec_key->Q); + ret = mbedtls_ecp_point_read_binary(&ec_key->grp, &ec_key->Q, pub, pub_len); + if (ret != 0) { + return ret; } + return mbedtls_ecp_check_pubkey(&ec_key->grp, &ec_key->Q); + #endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ - return ret; } /*********************************************************************** From fac9819edcbc4b647dc44b99909972cac066970c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 27 Jul 2023 09:19:42 +0200 Subject: [PATCH 20/27] Fix and document return of pk_ecc_set_pubkey() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One of the calling site needs to distinguish between "the format is potentially valid but not supported" vs "other errors", and it uses MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE for that. Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/library/pkparse.c b/library/pkparse.c index e8678ed348..d12984fb07 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -238,13 +238,19 @@ static int pk_ecc_set_pubkey_from_prv(mbedtls_pk_context *pk, * out: will have the public key set. * [in] pub, pub_len: the public key as an ECPoint, * in any format supported by ECP. + * + * Return: + * - 0 on success; + * - MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE if the format is potentially valid + * but not supported; + * - another error code otherwise. */ static int pk_ecc_set_pubkey_psa_ecp_fallback(mbedtls_pk_context *pk, const unsigned char *pub, size_t pub_len) { #if !defined(MBEDTLS_PK_PARSE_EC_COMPRESSED) - return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; + return MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE; #else /* MBEDTLS_PK_PARSE_EC_COMPRESSED */ mbedtls_ecp_keypair ecp_key; mbedtls_ecp_group_id ecp_group_id; @@ -280,6 +286,12 @@ exit: * [in/out] pk: in: must have its group set, see pk_ecc_set_group(). * out: will have the public key set. * [in] pub, pub_len: the raw public key (an ECPoint). + * + * Return: + * - 0 on success; + * - MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE if the format is potentially valid + * but not supported; + * - another error code otherwise. */ static int pk_ecc_set_pubkey(mbedtls_pk_context *pk, const unsigned char *pub, size_t pub_len) From 12ea63a5f7b96b2169331a40221b0f5779111201 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 27 Jul 2023 12:20:16 +0200 Subject: [PATCH 21/27] Abstract away MBEDTLS_PK_PARSE_EC_EXTENDED MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 69 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 17 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index d12984fb07..9d87a71506 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -345,15 +345,51 @@ static int pk_ecc_set_pubkey(mbedtls_pk_context *pk, /*********************************************************************** * - * Unsorted (yet!) from this point on until the next section header + * Low-level ECC parsing: optional support for SpecifiedECDomain + * + * There are two functions here that are used by the rest of the code: + * - pk_ecc_tag_may_be_speficied_ec_domain() + * - pk_ecc_group_id_from_specified() + * + * All the other functions are internal to this section. + * + * The two "public" functions have a dummy variant provided + * in configs without MBEDTLS_PK_PARSE_EC_EXTENDED. This acts as an + * abstraction layer for this macro, which should not appear outside + * this section. * **********************************************************************/ -#if defined(MBEDTLS_PK_PARSE_EC_EXTENDED) +#if !defined(MBEDTLS_PK_PARSE_EC_EXTENDED) +/* See the "real" version for documentation */ +static int pk_ecc_tag_may_be_specified_ec_domain(int tag) +{ + (void) tag; + return 0; +} + +/* See the "real" version for documentation */ +static int pk_ecc_group_id_from_specified(const mbedtls_asn1_buf *params, + mbedtls_ecp_group_id *grp_id) +{ + (void) params; + (void) grp_id; + return MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE; +} +#else /* MBEDTLS_PK_PARSE_EC_EXTENDED */ +/* + * Tell if the passed tag might be the start of SpecifiedECDomain + * (that is, a sequence). + */ +static int pk_ecc_tag_may_be_specified_ec_domain(int tag) +{ + return tag == (MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE); +} + /* * Parse a SpecifiedECDomain (SEC 1 C.2) and (mostly) fill the group with it. * WARNING: the resulting group should only be used with - * pk_group_id_from_specified(), since its base point may not be set correctly + * pk_ecc_group_id_from_specified(), since its base point may not be set correctly * if it was encoded compressed. * * SpecifiedECDomain ::= SEQUENCE { @@ -562,8 +598,8 @@ cleanup: /* * Parse a SpecifiedECDomain (SEC 1 C.2) and find the associated group ID */ -static int pk_group_id_from_specified(const mbedtls_asn1_buf *params, - mbedtls_ecp_group_id *grp_id) +static int pk_ecc_group_id_from_specified(const mbedtls_asn1_buf *params, + mbedtls_ecp_group_id *grp_id) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_ecp_group grp; @@ -578,7 +614,7 @@ static int pk_group_id_from_specified(const mbedtls_asn1_buf *params, cleanup: /* The API respecting lifecycle for mbedtls_ecp_group struct is - * _init(), _load() and _free(). In pk_group_id_from_specified() the + * _init(), _load() and _free(). In pk_ecc_group_id_from_specified() the * temporary grp breaks that flow and it's members are populated * by pk_group_id_from_group(). As such mbedtls_ecp_group_free() * which is assuming a group populated by _setup() may not clean-up @@ -594,6 +630,11 @@ cleanup: } #endif /* MBEDTLS_PK_PARSE_EC_EXTENDED */ +/*********************************************************************** + * + * Unsorted (yet!) from this point on until the next section header + * + **********************************************************************/ /* Minimally parse an ECParameters buffer to and mbedtls_asn1_buf * @@ -613,13 +654,10 @@ static int pk_get_ecparams(unsigned char **p, const unsigned char *end, MBEDTLS_ERR_ASN1_OUT_OF_DATA); } - /* Tag may be either OID or SEQUENCE */ + /* Acceptable tags: OID for namedCurve, or specifiedECDomain */ params->tag = **p; - if (params->tag != MBEDTLS_ASN1_OID -#if defined(MBEDTLS_PK_PARSE_EC_EXTENDED) - && params->tag != (MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE) -#endif - ) { + if (params->tag != MBEDTLS_ASN1_OID && + !pk_ecc_tag_may_be_specified_ec_domain(params->tag)) { return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, MBEDTLS_ERR_ASN1_UNEXPECTED_TAG); } @@ -657,13 +695,10 @@ static int pk_use_ecparams(const mbedtls_asn1_buf *params, mbedtls_pk_context *p return MBEDTLS_ERR_PK_UNKNOWN_NAMED_CURVE; } } else { -#if defined(MBEDTLS_PK_PARSE_EC_EXTENDED) - if ((ret = pk_group_id_from_specified(params, &grp_id)) != 0) { + ret = pk_ecc_group_id_from_specified(params, &grp_id); + if (ret != 0) { return ret; } -#else - return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; -#endif } return pk_ecc_set_group(pk, grp_id); From 53d3e40a21c9c56a053c25ec8fe801ccb796a18e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 1 Aug 2023 11:19:24 +0200 Subject: [PATCH 22/27] Fix unused warnings in dummy definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/pkparse.c b/library/pkparse.c index 9d87a71506..ffa91d30d6 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -250,6 +250,9 @@ static int pk_ecc_set_pubkey_psa_ecp_fallback(mbedtls_pk_context *pk, size_t pub_len) { #if !defined(MBEDTLS_PK_PARSE_EC_COMPRESSED) + (void) pk; + (void) pub; + (void) pub_len; return MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE; #else /* MBEDTLS_PK_PARSE_EC_COMPRESSED */ mbedtls_ecp_keypair ecp_key; From 564bc1bb960cd4d67a3d58488fff53565cf74ef2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 2 Aug 2023 12:05:16 +0200 Subject: [PATCH 23/27] Fix limitation in checking supported alg in pk_sign MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recent changes in pkparse made it so ECDSA (deterministic or not) is set as the secondary alg and ECDH the first one. This broke the wrapper in pk_wrap as it was only checking the first alg when deciding whether to use deterministic or not. The wrapper should not have unnecessary requirements on how algs are set up, so make the check more flexible. Signed-off-by: Manuel Pégourié-Gonnard --- library/pk_wrap.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 53e11d5cb9..2c67836747 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -976,16 +976,17 @@ static int ecdsa_sign_psa(mbedtls_svc_key_id_t key_id, mbedtls_md_type_t md_alg, psa_status_t status; psa_algorithm_t psa_sig_md; psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; - psa_algorithm_t alg; + psa_algorithm_t alg, alg2; status = psa_get_key_attributes(key_id, &key_attr); if (status != PSA_SUCCESS) { return PSA_PK_ECDSA_TO_MBEDTLS_ERR(status); } alg = psa_get_key_algorithm(&key_attr); + alg2 = psa_get_key_enrollment_algorithm(&key_attr); psa_reset_key_attributes(&key_attr); - if (PSA_ALG_IS_DETERMINISTIC_ECDSA(alg)) { + if (PSA_ALG_IS_DETERMINISTIC_ECDSA(alg) || PSA_ALG_IS_DETERMINISTIC_ECDSA(alg2)) { psa_sig_md = PSA_ALG_DETERMINISTIC_ECDSA(mbedtls_md_psa_alg_from_type(md_alg)); } else { psa_sig_md = PSA_ALG_ECDSA(mbedtls_md_psa_alg_from_type(md_alg)); From 94cf1f82ad73ea2dcf68367d9daf18a13e3b3db5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 2 Aug 2023 12:09:24 +0200 Subject: [PATCH 24/27] Fix a typo in a comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/pkparse.c b/library/pkparse.c index ffa91d30d6..bb6db0824c 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -156,7 +156,7 @@ static int pk_ecc_set_key(mbedtls_pk_context *pk, * Note: the private key information is always available from pk, * however for convenience the serialized version is also passed, * as it's available at each calling site, and useful in some configs - * (as otherwise we're have to re-serialize it from the pk context). + * (as otherwise we would have to re-serialize it from the pk context). * * There are three implementations of this function: * 1. MBEDTLS_PK_USE_PSA_EC_DATA, From 842ffc5085a12998407de61d465c3d2fbb910e04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 2 Aug 2023 12:10:51 +0200 Subject: [PATCH 25/27] Make code more robust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using return here is only correct if we know that group_load() is atomic (either succeeds, or allocates no ressources). I'm not sure it is, and even if it were, goto exit is more obviously correct, so let's use that. Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/pkparse.c b/library/pkparse.c index bb6db0824c..b0eef95a32 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -264,7 +264,7 @@ static int pk_ecc_set_pubkey_psa_ecp_fallback(mbedtls_pk_context *pk, mbedtls_ecp_keypair_init(&ecp_key); ret = mbedtls_ecp_group_load(&(ecp_key.grp), ecp_group_id); if (ret != 0) { - return ret; + goto exit; } ret = mbedtls_ecp_point_read_binary(&(ecp_key.grp), &ecp_key.Q, pub, pub_len); From f1b7633443ef3c4709e594817fd7e57645404472 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 2 Aug 2023 12:14:19 +0200 Subject: [PATCH 26/27] Use clearer function name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I went for "may be" as I was thinking just checking the tag technically does not guarantee that what follows is correct, but I was wrong: according to ASN.1, when there are variants, the tag does distinguish unambiguously between variants, so we can be more positive here. (Whether the thing inside that variant is correct is a different question.) As a welcome side effect, this makes the name more standard hence more readable. Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index b0eef95a32..820c8d1cf4 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -351,7 +351,7 @@ static int pk_ecc_set_pubkey(mbedtls_pk_context *pk, * Low-level ECC parsing: optional support for SpecifiedECDomain * * There are two functions here that are used by the rest of the code: - * - pk_ecc_tag_may_be_speficied_ec_domain() + * - pk_ecc_tag_is_speficied_ec_domain() * - pk_ecc_group_id_from_specified() * * All the other functions are internal to this section. @@ -365,7 +365,7 @@ static int pk_ecc_set_pubkey(mbedtls_pk_context *pk, #if !defined(MBEDTLS_PK_PARSE_EC_EXTENDED) /* See the "real" version for documentation */ -static int pk_ecc_tag_may_be_specified_ec_domain(int tag) +static int pk_ecc_tag_is_specified_ec_domain(int tag) { (void) tag; return 0; @@ -384,7 +384,7 @@ static int pk_ecc_group_id_from_specified(const mbedtls_asn1_buf *params, * Tell if the passed tag might be the start of SpecifiedECDomain * (that is, a sequence). */ -static int pk_ecc_tag_may_be_specified_ec_domain(int tag) +static int pk_ecc_tag_is_specified_ec_domain(int tag) { return tag == (MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE); } @@ -660,7 +660,7 @@ static int pk_get_ecparams(unsigned char **p, const unsigned char *end, /* Acceptable tags: OID for namedCurve, or specifiedECDomain */ params->tag = **p; if (params->tag != MBEDTLS_ASN1_OID && - !pk_ecc_tag_may_be_specified_ec_domain(params->tag)) { + !pk_ecc_tag_is_specified_ec_domain(params->tag)) { return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, MBEDTLS_ERR_ASN1_UNEXPECTED_TAG); } From 52e9548c227d659fb889d3a73657244608cf8d82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 3 Aug 2023 10:22:41 +0200 Subject: [PATCH 27/27] Fix check for format supported by PSA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For non-Weierstrass curves there's only one format and it's supported. Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 820c8d1cf4..b4299518fd 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -302,8 +302,10 @@ static int pk_ecc_set_pubkey(mbedtls_pk_context *pk, #if defined(MBEDTLS_PK_USE_PSA_EC_DATA) /* Load the key */ - if (*pub == 0x04) { - /* Uncompressed format, directly supported by PSA */ + if (!PSA_ECC_FAMILY_IS_WEIERSTRASS(pk->ec_family) || *pub == 0x04) { + /* Format directly supported by PSA: + * - non-Weierstrass curves that only have one format; + * - uncompressed format for Weierstrass curves. */ if (pub_len > sizeof(pk->pub_raw)) { return MBEDTLS_ERR_PK_BUFFER_TOO_SMALL; }