From 4ac9d44d8381aa73e421a44adb7d14bbc6519e5a Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 17 May 2023 12:32:13 +0200 Subject: [PATCH 01/12] pk: fix typos in description of mbedtls_pk_ec_[ro/rw] Signed-off-by: Valerio Setti --- library/pk_internal.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/pk_internal.h b/library/pk_internal.h index 7c4f285719f2..402cb65353be 100644 --- a/library/pk_internal.h +++ b/library/pk_internal.h @@ -30,11 +30,11 @@ #if defined(MBEDTLS_ECP_LIGHT) /** * Public function mbedtls_pk_ec() can be used to get direct access to the - * wrapped ecp_keypair strucure pointed to the pk_ctx. However this is not - * ideal because it bypasses the PK module on the control of its internal's + * wrapped ecp_keypair structure pointed to the pk_ctx. However this is not + * ideal because it bypasses the PK module on the control of its internal * structure (pk_context) fields. * For backward compatibility we keep mbedtls_pk_ec() when ECP_C is defined, but - * we provide 2 very similar function when only ECP_LIGHT is enabled and not + * we provide 2 very similar functions when only ECP_LIGHT is enabled and not * ECP_C. * These variants embed the "ro" or "rw" keywords in their name to make the * usage of the returned pointer explicit. Of course the returned value is From 722f8f7472c3a7dd8bbc593041fb7a584dc37eb4 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 17 May 2023 15:31:21 +0200 Subject: [PATCH 02/12] pk: adding a new field to store the public key in raw format Signed-off-by: Valerio Setti --- include/mbedtls/pk.h | 53 ++++++++++++++++++++++++++++++++++++++++---- library/pk.c | 6 +++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index ec2a2513ec06..9b2cab8fbf1b 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -202,6 +202,21 @@ typedef struct mbedtls_pk_rsassa_pss_options { #define MBEDTLS_PK_CAN_ECDH #endif +/* Helper to define which fields in the pk_context structure below should be + * used for EC keys: legacy ecp_keypair or the raw (PSA friendly) format. + * It should be noticed that this only affect how data is stored, not which + * functions are used for various operations. The overall picture looks like + * this: + * - if ECP_C is defined then use legacy functions + * - if USE_PSA is defined and + * - if ECP_C then use ecp_keypair structure, convert data to a PSA friendly + * format and use PSA functions + * - if !ECP_C then use new raw data and PSA functions directly. + */ +#if defined(MBEDTLS_USE_PSA_CRYPTO) && !defined(MBEDTLS_ECP_C) +#define MBEDTLS_PK_USE_PSA_EC_DATA +#endif /* MBEDTLS_USE_PSA_CRYPTO && !MBEDTLS_ECP_C */ + /** * \brief Types for interfacing with the debug module */ @@ -232,19 +247,49 @@ typedef struct mbedtls_pk_debug_item { */ typedef struct mbedtls_pk_info_t mbedtls_pk_info_t; +#define MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN \ + PSA_KEY_EXPORT_ECC_PUBLIC_KEY_MAX_SIZE(PSA_VENDOR_ECC_MAX_CURVE_BITS) /** * \brief Public key container - * - * \note The priv_id is guarded by MBEDTLS_PSA_CRYPTO_C and not - * by MBEDTLS_USE_PSA_CRYPTO because it can be used also - * in mbedtls_pk_sign_ext for RSA keys. */ typedef struct mbedtls_pk_context { const mbedtls_pk_info_t *MBEDTLS_PRIVATE(pk_info); /**< Public key information */ void *MBEDTLS_PRIVATE(pk_ctx); /**< Underlying public key context */ + /* When MBEDTLS_PSA_CRYPTO_C is enabled then the following priv_id field is + * used to store the ID of the opaque key. Differently from the raw public + * key management below, in this case there is no counterpart in the pk_ctx + * field to work in parallel with. + * This priv_id is guarded by MBEDTLS_PSA_CRYPTO_C and not by + * MBEDTLS_USE_PSA_CRYPTO because it can be used also in mbedtls_pk_sign_ext + * for RSA keys. */ #if defined(MBEDTLS_PSA_CRYPTO_C) mbedtls_svc_key_id_t MBEDTLS_PRIVATE(priv_id); /**< Key ID for opaque keys */ #endif /* MBEDTLS_PSA_CRYPTO_C */ + /* The following fields are meant for storing the public key in raw format + * which is handy for: + * - easily importing it into the PSA context + * - reducing the ECP module dependencies in the PK one. + * + * When MBEDTLS_PK_USE_PSA_EC_DATA is enabled: + * - the pk_ctx above is not used anymore for storing the public key + * inside the ecp_keypair structure (only the private part, but also this + * one is going to change in the future) + * - the following fields are used for all public key operations: signature + * verify, key pair check and key write. + * Of course, when MBEDTLS_PK_USE_PSA_EC_DATA is not enabled, the legacy + * ecp_keypair structure is used for storing the public key and perform + * all the operations. + * + * Note: This new public key storing solution only works for EC keys, not + * other ones. The latters is still use pk_ctx to store their own + * context. + */ +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + uint8_t MBEDTLS_PRIVATE(pub_raw)[MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN]; /**< Raw public key */ + size_t MBEDTLS_PRIVATE(pub_raw_len); /**< Valid bytes in "pub_raw" */ + psa_ecc_family_t MBEDTLS_PRIVATE(ec_family); /**< EC family of pk */ + size_t MBEDTLS_PRIVATE(ec_bits); /**< Curve's bits of pk */ +#endif /* MBEDTLS_PK_USE_PSA_EC_PUB_KEY */ } mbedtls_pk_context; #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) diff --git a/library/pk.c b/library/pk.c index 7e772829aa5f..7a047df3fd16 100644 --- a/library/pk.c +++ b/library/pk.c @@ -64,6 +64,12 @@ void mbedtls_pk_init(mbedtls_pk_context *ctx) #if defined(MBEDTLS_PSA_CRYPTO_C) ctx->priv_id = MBEDTLS_SVC_KEY_ID_INIT; #endif /* MBEDTLS_PSA_CRYPTO_C */ +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + memset(ctx->pub_raw, 0, sizeof(ctx->pub_raw)); + ctx->pub_raw_len = 0; + ctx->ec_family = 0; + ctx->ec_bits = 0; +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ } /* From 4064dbbdb2cd5171b0678f7e6e356d573b8a138c Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 17 May 2023 15:33:07 +0200 Subject: [PATCH 03/12] pk: update pkparse and pkwrite to use the new public key storing solution Signed-off-by: Valerio Setti --- library/pk_internal.h | 20 ++++ library/pkparse.c | 212 +++++++++++++++++++++++++++++++++--------- library/pkwrite.c | 35 ++++--- library/pkwrite.h | 28 +----- 4 files changed, 215 insertions(+), 80 deletions(-) diff --git a/library/pk_internal.h b/library/pk_internal.h index 402cb65353be..09f3d85249d2 100644 --- a/library/pk_internal.h +++ b/library/pk_internal.h @@ -63,6 +63,26 @@ static inline mbedtls_ecp_keypair *mbedtls_pk_ec_rw(const mbedtls_pk_context pk) return NULL; } } + +/* Helpers for Montgomery curves */ +#if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) || defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) +#define MBEDTLS_PK_HAVE_RFC8410_CURVES + +static inline int mbedtls_pk_is_rfc8410_curve(mbedtls_ecp_group_id id) +{ +#if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) + if (id == MBEDTLS_ECP_DP_CURVE25519) { + return 1; + } +#endif +#if defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) + if (id == MBEDTLS_ECP_DP_CURVE448) { + return 1; + } +#endif + return 0; +} +#endif /* MBEDTLS_ECP_DP_CURVE25519_ENABLED || MBEDTLS_ECP_DP_CURVE448_ENABLED */ #endif /* MBEDTLS_ECP_LIGHT */ #endif /* MBEDTLS_PK_INTERNAL_H */ diff --git a/library/pkparse.c b/library/pkparse.c index 87b707dc8897..d47b0994936c 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -37,6 +37,9 @@ #if defined(MBEDTLS_RSA_C) || defined(MBEDTLS_ECP_C) #include "pkwrite.h" #endif +#if defined(MBEDTLS_ECP_LIGHT) +#include "pk_internal.h" +#endif #if defined(MBEDTLS_ECDSA_C) #include "mbedtls/ecdsa.h" #endif @@ -455,6 +458,29 @@ static int pk_group_id_from_specified(const mbedtls_asn1_buf *params, } #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) +{ + psa_ecc_family_t ec_family; + size_t bits; + + ec_family = mbedtls_ecc_group_to_psa(grp_id, &bits); + + if ((pk->ec_family != 0) && (pk->ec_family != ec_family)) { + return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; + } + + pk->ec_family = ec_family; + pk->ec_bits = bits; + + return 0; +} +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ + /* * Use EC parameters to initialise an EC group * @@ -463,7 +489,7 @@ static int pk_group_id_from_specified(const mbedtls_asn1_buf *params, * specifiedCurve SpecifiedECDomain -- = SEQUENCE { ... } * -- implicitCurve NULL */ -static int pk_use_ecparams(const mbedtls_asn1_buf *params, mbedtls_ecp_group *grp) +static int pk_use_ecparams(const mbedtls_asn1_buf *params, mbedtls_pk_context *pk) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_ecp_group_id grp_id; @@ -482,39 +508,41 @@ static int pk_use_ecparams(const mbedtls_asn1_buf *params, mbedtls_ecp_group *gr #endif } - /* - * grp may already be initialized; if so, make sure IDs match - */ - if (grp->id != MBEDTLS_ECP_DP_NONE && grp->id != grp_id) { + /* 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(grp, grp_id)) != 0) { + if ((ret = mbedtls_ecp_group_load(&(mbedtls_pk_ec_rw(*pk)->grp), + grp_id)) != 0) { return ret; } +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + ret = pk_update_psa_ecparams(pk, grp_id); +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ - return 0; + return ret; } -#if defined(MBEDTLS_ECP_LIGHT) /* * Helper function for deriving a public key from its private counterpart. */ -static int pk_derive_public_key(mbedtls_ecp_keypair *eck, +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; + mbedtls_ecp_keypair *eck = (mbedtls_ecp_keypair *) pk->pk_ctx; #if defined(MBEDTLS_USE_PSA_CRYPTO) 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(eck->grp.id, &curve_bits); - /* 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. */ +#if !defined(MBEDTLS_PK_USE_PSA_EC_DATA) unsigned char key_buf[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; size_t key_len; +#endif /* !MBEDTLS_PK_USE_PSA_EC_DATA */ mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; (void) f_rng; @@ -529,9 +557,12 @@ static int pk_derive_public_key(mbedtls_ecp_keypair *eck, return ret; } - mbedtls_platform_zeroize(key_buf, sizeof(key_buf)); - +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + status = psa_export_public_key(key_id, pk->pub_raw, sizeof(pk->pub_raw), + &pk->pub_raw_len); +#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ status = psa_export_public_key(key_id, key_buf, sizeof(key_buf), &key_len); +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ ret = psa_pk_status_to_mbedtls(status); destruction_status = psa_destroy_key(key_id); if (ret != 0) { @@ -539,8 +570,9 @@ static int pk_derive_public_key(mbedtls_ecp_keypair *eck, } else if (destruction_status != PSA_SUCCESS) { return psa_pk_status_to_mbedtls(destruction_status); } - +#if !defined(MBEDTLS_PK_USE_PSA_EC_DATA) ret = mbedtls_ecp_point_read_binary(&eck->grp, &eck->Q, key_buf, key_len); +#endif /* !MBEDTLS_PK_USE_PSA_EC_DATA */ #else /* MBEDTLS_USE_PSA_CRYPTO */ (void) d; (void) d_len; @@ -557,13 +589,24 @@ static int pk_derive_public_key(mbedtls_ecp_keypair *eck, */ static int pk_use_ecparams_rfc8410(const mbedtls_asn1_buf *params, mbedtls_ecp_group_id grp_id, - mbedtls_ecp_group *grp) + mbedtls_pk_context *pk) { + mbedtls_ecp_keypair *ecp = mbedtls_pk_ec_rw(*pk); + int ret; + if (params->tag != 0 || params->len != 0) { return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; } - return mbedtls_ecp_group_load(grp, grp_id); + ret = mbedtls_ecp_group_load(&(ecp->grp), grp_id); + if (ret != 0) { + return ret; + } + +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + ret = pk_update_psa_ecparams(pk, grp_id); +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ + return ret; } /* @@ -571,10 +614,11 @@ static int pk_use_ecparams_rfc8410(const mbedtls_asn1_buf *params, * * CurvePrivateKey ::= OCTET STRING */ -static int pk_parse_key_rfc8410_der(mbedtls_ecp_keypair *eck, +static int pk_parse_key_rfc8410_der(mbedtls_pk_context *pk, unsigned char *key, size_t keylen, const unsigned char *end, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) { + mbedtls_ecp_keypair *eck = mbedtls_pk_ec_rw(*pk); int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t len; @@ -591,10 +635,10 @@ static int pk_parse_key_rfc8410_der(mbedtls_ecp_keypair *eck, return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); } - // 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(eck, key, len, f_rng, p_rng)) != 0) { + /* 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) { mbedtls_ecp_keypair_free(eck); return ret; } @@ -607,7 +651,42 @@ static int pk_parse_key_rfc8410_der(mbedtls_ecp_keypair *eck, return 0; } #endif /* MBEDTLS_PK_HAVE_RFC8410_CURVES */ -#endif /* MBEDTLS_ECP_LIGHT */ + +#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 + */ +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) +{ + 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; +} +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ /* * EC public key is an EC point @@ -617,14 +696,60 @@ static int pk_parse_key_rfc8410_der(mbedtls_ecp_keypair *eck, * 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_ecp_keypair *key) + mbedtls_pk_context *pk) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - if ((ret = mbedtls_ecp_point_read_binary(&key->grp, &key->Q, - (const unsigned char *) *p, end - *p)) == 0) { - ret = mbedtls_ecp_check_pubkey(&key->grp, &key->Q); +#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); + + if (len > PSA_EXPORT_PUBLIC_KEY_MAX_SIZE) { + 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)) { + 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 { + /* Uncompressed format */ + if ((end - *p) > MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN) { + return MBEDTLS_ERR_PK_BAD_INPUT_DATA; + } + memcpy(pk->pub_raw, *p, (end - *p)); + pk->pub_raw_len = end - *p; + } + + /* 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 consumed all bytes or failed @@ -796,14 +921,14 @@ int mbedtls_pk_parse_subpubkey(unsigned char **p, const unsigned char *end, if (pk_alg == MBEDTLS_PK_ECKEY_DH || pk_alg == MBEDTLS_PK_ECKEY) { #if defined(MBEDTLS_PK_HAVE_RFC8410_CURVES) if (mbedtls_pk_is_rfc8410_curve(ec_grp_id)) { - ret = pk_use_ecparams_rfc8410(&alg_params, ec_grp_id, &mbedtls_pk_ec_rw(*pk)->grp); + ret = pk_use_ecparams_rfc8410(&alg_params, ec_grp_id, pk); } else #endif { - ret = pk_use_ecparams(&alg_params, &mbedtls_pk_ec_rw(*pk)->grp); + ret = pk_use_ecparams(&alg_params, pk); } if (ret == 0) { - ret = pk_get_ecpubkey(p, end, mbedtls_pk_ec_rw(*pk)); + ret = pk_get_ecpubkey(p, end, pk); } } else #endif /* MBEDTLS_ECP_LIGHT */ @@ -1014,7 +1139,7 @@ static int pk_parse_key_pkcs1_der(mbedtls_rsa_context *rsa, /* * Parse a SEC1 encoded private EC key */ -static int pk_parse_key_sec1_der(mbedtls_ecp_keypair *eck, +static int pk_parse_key_sec1_der(mbedtls_pk_context *pk, const unsigned char *key, size_t keylen, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) { @@ -1026,6 +1151,7 @@ static int pk_parse_key_sec1_der(mbedtls_ecp_keypair *eck, unsigned char *d; unsigned char *end = p + keylen; unsigned char *end2; + mbedtls_ecp_keypair *eck = mbedtls_pk_ec_rw(*pk); /* * RFC 5915, or SEC1 Appendix C.4 @@ -1074,7 +1200,7 @@ static int pk_parse_key_sec1_der(mbedtls_ecp_keypair *eck, MBEDTLS_ASN1_CONTEXT_SPECIFIC | MBEDTLS_ASN1_CONSTRUCTED | 0)) == 0) { if ((ret = pk_get_ecparams(&p, p + len, ¶ms)) != 0 || - (ret = pk_use_ecparams(¶ms, &eck->grp)) != 0) { + (ret = pk_use_ecparams(¶ms, pk)) != 0) { mbedtls_ecp_keypair_free(eck); return ret; } @@ -1103,7 +1229,7 @@ static int pk_parse_key_sec1_der(mbedtls_ecp_keypair *eck, MBEDTLS_ERR_ASN1_LENGTH_MISMATCH); } - if ((ret = pk_get_ecpubkey(&p, end2, eck)) == 0) { + if ((ret = pk_get_ecpubkey(&p, end2, pk)) == 0) { pubkey_done = 1; } else { /* @@ -1121,7 +1247,7 @@ static int pk_parse_key_sec1_der(mbedtls_ecp_keypair *eck, } if (!pubkey_done) { - if ((ret = pk_derive_public_key(eck, d, d_len, f_rng, p_rng)) != 0) { + if ((ret = pk_derive_public_key(pk, d, d_len, f_rng, p_rng)) != 0) { mbedtls_ecp_keypair_free(eck); return ret; } @@ -1232,10 +1358,10 @@ static int pk_parse_key_pkcs8_unencrypted_der( if (pk_alg == MBEDTLS_PK_ECKEY || pk_alg == MBEDTLS_PK_ECKEY_DH) { #if defined(MBEDTLS_PK_HAVE_RFC8410_CURVES) if (mbedtls_pk_is_rfc8410_curve(ec_grp_id)) { - if ((ret = pk_use_ecparams_rfc8410(¶ms, ec_grp_id, - &mbedtls_pk_ec_rw(*pk)->grp)) != 0 || + if ((ret = + pk_use_ecparams_rfc8410(¶ms, ec_grp_id, pk)) != 0 || (ret = - pk_parse_key_rfc8410_der(mbedtls_pk_ec_rw(*pk), p, len, end, f_rng, + pk_parse_key_rfc8410_der(pk, p, len, end, f_rng, p_rng)) != 0) { mbedtls_pk_free(pk); return ret; @@ -1243,8 +1369,8 @@ static int pk_parse_key_pkcs8_unencrypted_der( } else #endif { - if ((ret = pk_use_ecparams(¶ms, &mbedtls_pk_ec_rw(*pk)->grp)) != 0 || - (ret = pk_parse_key_sec1_der(mbedtls_pk_ec_rw(*pk), p, len, f_rng, p_rng)) != 0) { + if ((ret = pk_use_ecparams(¶ms, pk)) != 0 || + (ret = pk_parse_key_sec1_der(pk, p, len, f_rng, p_rng)) != 0) { mbedtls_pk_free(pk); return ret; } @@ -1431,7 +1557,7 @@ int mbedtls_pk_parse_key(mbedtls_pk_context *pk, pk_info = mbedtls_pk_info_from_type(MBEDTLS_PK_ECKEY); if ((ret = mbedtls_pk_setup(pk, pk_info)) != 0 || - (ret = pk_parse_key_sec1_der(mbedtls_pk_ec_rw(*pk), + (ret = pk_parse_key_sec1_der(pk, pem.buf, pem.buflen, f_rng, p_rng)) != 0) { mbedtls_pk_free(pk); @@ -1555,18 +1681,18 @@ int mbedtls_pk_parse_key(mbedtls_pk_context *pk, #if defined(MBEDTLS_ECP_LIGHT) pk_info = mbedtls_pk_info_from_type(MBEDTLS_PK_ECKEY); if (mbedtls_pk_setup(pk, pk_info) == 0 && - pk_parse_key_sec1_der(mbedtls_pk_ec_rw(*pk), + pk_parse_key_sec1_der(pk, key, keylen, f_rng, p_rng) == 0) { return 0; } mbedtls_pk_free(pk); #endif /* MBEDTLS_ECP_LIGHT */ - /* If MBEDTLS_RSA_C is defined but MBEDTLS_ECP_C isn't, + /* If MBEDTLS_RSA_C is defined but MBEDTLS_ECP_LIGHT isn't, * it is ok to leave the PK context initialized but not * freed: It is the caller's responsibility to call pk_init() * before calling this function, and to call pk_free() - * when it fails. If MBEDTLS_ECP_C is defined but MBEDTLS_RSA_C + * when it fails. If MBEDTLS_ECP_LIGHT is defined but MBEDTLS_RSA_C * isn't, this leads to mbedtls_pk_free() being called * twice, once here and once by the caller, but this is * also ok and in line with the mbedtls_pk_free() calls diff --git a/library/pkwrite.c b/library/pkwrite.c index 1f606a4481ae..3577fa1a0fea 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -38,7 +38,10 @@ #include "mbedtls/ecp.h" #include "mbedtls/platform_util.h" #endif -#if defined(MBEDTLS_RSA_C) || defined(MBEDTLS_ECP_C) +#if defined(MBEDTLS_ECP_LIGHT) +#include "pk_internal.h" +#endif +#if defined(MBEDTLS_RSA_C) || defined(MBEDTLS_ECP_LIGHT) #include "pkwrite.h" #endif #if defined(MBEDTLS_ECDSA_C) @@ -100,15 +103,24 @@ static int pk_write_rsa_pubkey(unsigned char **p, unsigned char *start, #endif /* MBEDTLS_RSA_C */ #if defined(MBEDTLS_ECP_LIGHT) -/* - * EC public key is an EC point - */ static int pk_write_ec_pubkey(unsigned char **p, unsigned char *start, - mbedtls_ecp_keypair *ec) + const mbedtls_pk_context *pk) { - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t len = 0; + +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + len = pk->pub_raw_len; + + if (*p < start || (size_t) (*p - start) < len) { + return MBEDTLS_ERR_ASN1_BUF_TOO_SMALL; + } + + memcpy(*p - len, pk->pub_raw, len); + *p -= len; +#else unsigned char buf[MBEDTLS_ECP_MAX_PT_LEN]; + mbedtls_ecp_keypair *ec = mbedtls_pk_ec(*pk); + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; if ((ret = mbedtls_ecp_point_write_binary(&ec->grp, &ec->Q, MBEDTLS_ECP_PF_UNCOMPRESSED, @@ -122,6 +134,7 @@ static int pk_write_ec_pubkey(unsigned char **p, unsigned char *start, *p -= len; memcpy(*p, buf, len); +#endif return (int) len; } @@ -183,7 +196,7 @@ int mbedtls_pk_write_pubkey(unsigned char **p, unsigned char *start, #endif #if defined(MBEDTLS_ECP_LIGHT) if (mbedtls_pk_get_type(key) == MBEDTLS_PK_ECKEY) { - MBEDTLS_ASN1_CHK_ADD(len, pk_write_ec_pubkey(p, start, mbedtls_pk_ec_rw(*key))); + MBEDTLS_ASN1_CHK_ADD(len, pk_write_ec_pubkey(p, start, key)); } else #endif #if defined(MBEDTLS_USE_PSA_CRYPTO) @@ -324,7 +337,7 @@ int mbedtls_pk_write_pubkey_der(const mbedtls_pk_context *key, unsigned char *bu #if defined(MBEDTLS_ECP_LIGHT) #if defined(MBEDTLS_PK_HAVE_RFC8410_CURVES) /* - * RFC8410 + * RFC8410 section 7 * * OneAsymmetricKey ::= SEQUENCE { * version Version, @@ -335,7 +348,7 @@ int mbedtls_pk_write_pubkey_der(const mbedtls_pk_context *key, unsigned char *bu * [[2: publicKey [1] IMPLICIT PublicKey OPTIONAL ]], * ... * } - * + * ... * CurvePrivateKey ::= OCTET STRING */ static int pk_write_ec_rfc8410_der(unsigned char **p, unsigned char *buf, @@ -491,7 +504,7 @@ int mbedtls_pk_write_key_der(const mbedtls_pk_context *key, unsigned char *buf, */ /* publicKey */ - MBEDTLS_ASN1_CHK_ADD(pub_len, pk_write_ec_pubkey(&c, buf, ec)); + MBEDTLS_ASN1_CHK_ADD(pub_len, pk_write_ec_pubkey(&c, buf, key)); if (c - buf < 1) { return MBEDTLS_ERR_ASN1_BUF_TOO_SMALL; @@ -527,7 +540,7 @@ int mbedtls_pk_write_key_der(const mbedtls_pk_context *key, unsigned char *buf, MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_tag(&c, buf, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE)); } else -#endif /* MBEDTLS_ECP_C */ +#endif /* MBEDTLS_ECP_LIGHT */ return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; return (int) len; diff --git a/library/pkwrite.h b/library/pkwrite.h index 537bd0f8c896..8db233373843 100644 --- a/library/pkwrite.h +++ b/library/pkwrite.h @@ -73,7 +73,7 @@ #endif /* MBEDTLS_RSA_C */ -#if defined(MBEDTLS_ECP_C) +#if defined(MBEDTLS_ECP_LIGHT) /* * EC public keys: * SubjectPublicKeyInfo ::= SEQUENCE { 1 + 2 @@ -98,34 +98,10 @@ */ #define MBEDTLS_PK_ECP_PRV_DER_MAX_BYTES (29 + 3 * MBEDTLS_ECP_MAX_BYTES) -#else /* MBEDTLS_ECP_C */ +#else /* MBEDTLS_ECP_LIGHT */ #define MBEDTLS_PK_ECP_PUB_DER_MAX_BYTES 0 #define MBEDTLS_PK_ECP_PRV_DER_MAX_BYTES 0 -#endif /* MBEDTLS_ECP_C */ - -#if defined(MBEDTLS_ECP_LIGHT) -#include "mbedtls/ecp.h" - -#if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) || defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) -#define MBEDTLS_PK_HAVE_RFC8410_CURVES - -static inline int mbedtls_pk_is_rfc8410_curve(mbedtls_ecp_group_id id) -{ -#if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) - if (id == MBEDTLS_ECP_DP_CURVE25519) { - return 1; - } -#endif -#if defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) - if (id == MBEDTLS_ECP_DP_CURVE448) { - return 1; - } -#endif - return 0; -} -#endif /* MBEDTLS_ECP_DP_CURVE25519_ENABLED || MBEDTLS_ECP_DP_CURVE448_ENABLED */ #endif /* MBEDTLS_ECP_LIGHT */ - #endif /* MBEDTLS_PK_WRITE_H */ From a1b8af686916e14cf0b6b4757f808e7e870ce927 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 17 May 2023 15:34:57 +0200 Subject: [PATCH 04/12] pkwrap: update ECDSA verify and EC pair check to use the new public key Signed-off-by: Valerio Setti --- library/pk_wrap.c | 68 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 16 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 0e5e12049a3d..32d697ac03a9 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -23,6 +23,7 @@ #if defined(MBEDTLS_PK_C) #include "pk_wrap.h" +#include "pk_internal.h" #include "mbedtls/error.h" /* Even if RSA not activated, for the sake of RSA-alt */ @@ -653,8 +654,12 @@ static int eckey_can_do(mbedtls_pk_type_t type) static size_t eckey_get_bitlen(mbedtls_pk_context *pk) { +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + return pk->ec_bits; +#else mbedtls_ecp_keypair *ecp = (mbedtls_ecp_keypair *) pk->pk_ctx; return ecp->grp.pbits; +#endif } #if defined(MBEDTLS_PK_CAN_ECDSA_VERIFY) @@ -724,11 +729,20 @@ static int ecdsa_verify_wrap(mbedtls_pk_context *pk, const unsigned char *hash, size_t hash_len, const unsigned char *sig, size_t sig_len) { - mbedtls_ecp_keypair *ctx = pk->pk_ctx; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; psa_status_t status; + unsigned char *p; + psa_algorithm_t psa_sig_md = PSA_ALG_ECDSA_ANY; + size_t signature_len; + ((void) md_alg); +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + unsigned char buf[PSA_VENDOR_ECDSA_SIGNATURE_MAX_SIZE]; + psa_ecc_family_t curve = pk->ec_family; + size_t curve_bits = pk->ec_bits; +#else + mbedtls_ecp_keypair *ctx = pk->pk_ctx; size_t key_len; /* This buffer will initially contain the public key and then the signature * but at different points in time. For all curves except secp224k1, which @@ -736,13 +750,10 @@ static int ecdsa_verify_wrap(mbedtls_pk_context *pk, * (header byte + 2 numbers, while the signature is only 2 numbers), * so use that as the buffer size. */ unsigned char buf[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; - unsigned char *p; - psa_algorithm_t psa_sig_md = PSA_ALG_ECDSA_ANY; size_t curve_bits; psa_ecc_family_t curve = mbedtls_ecc_group_to_psa(ctx->grp.id, &curve_bits); - const size_t signature_part_size = (ctx->grp.nbits + 7) / 8; - ((void) md_alg); +#endif if (curve == 0) { return MBEDTLS_ERR_PK_BAD_INPUT_DATA; @@ -752,6 +763,11 @@ static int ecdsa_verify_wrap(mbedtls_pk_context *pk, psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_VERIFY_HASH); psa_set_key_algorithm(&attributes, psa_sig_md); +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + status = psa_import_key(&attributes, + pk->pub_raw, pk->pub_raw_len, + &key_id); +#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ ret = mbedtls_ecp_point_write_binary(&ctx->grp, &ctx->Q, MBEDTLS_ECP_PF_UNCOMPRESSED, &key_len, buf, sizeof(buf)); @@ -762,27 +778,30 @@ static int ecdsa_verify_wrap(mbedtls_pk_context *pk, status = psa_import_key(&attributes, buf, key_len, &key_id); +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ if (status != PSA_SUCCESS) { ret = PSA_PK_TO_MBEDTLS_ERR(status); goto cleanup; } - /* We don't need the exported key anymore and can - * reuse its buffer for signature extraction. */ - if (2 * signature_part_size > sizeof(buf)) { + signature_len = PSA_ECDSA_SIGNATURE_SIZE(curve_bits); + if (signature_len > sizeof(buf)) { ret = MBEDTLS_ERR_PK_BAD_INPUT_DATA; goto cleanup; } p = (unsigned char *) sig; + /* extract_ecdsa_sig's last parameter is the size + * of each integer to be parse, so it's actually half + * the size of the signature. */ if ((ret = extract_ecdsa_sig(&p, sig + sig_len, buf, - signature_part_size)) != 0) { + signature_len/2)) != 0) { goto cleanup; } status = psa_verify_hash(key_id, psa_sig_md, hash, hash_len, - buf, 2 * signature_part_size); + buf, signature_len); if (status != PSA_SUCCESS) { ret = PSA_PK_ECDSA_TO_MBEDTLS_ERR(status); goto cleanup; @@ -1112,26 +1131,30 @@ static int eckey_check_pair_psa(mbedtls_pk_context *pub, mbedtls_pk_context *prv { psa_status_t status, destruction_status; psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; - mbedtls_ecp_keypair *prv_ctx = prv->pk_ctx; - mbedtls_ecp_keypair *pub_ctx = pub->pk_ctx; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; /* We are using MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH for the size of this * buffer because it will be used to hold the private key at first and * then its public part (but not at the same time). */ uint8_t prv_key_buf[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; size_t prv_key_len; + mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + const psa_ecc_family_t curve = prv->ec_family; + const size_t curve_bits = PSA_BITS_TO_BYTES(prv->ec_bits); +#else /* !MBEDTLS_PK_USE_PSA_EC_DATA */ uint8_t pub_key_buf[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; size_t pub_key_len; - mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; size_t curve_bits; const psa_ecc_family_t curve = - mbedtls_ecc_group_to_psa(prv_ctx->grp.id, &curve_bits); + mbedtls_ecc_group_to_psa(mbedtls_pk_ec_ro(*prv)->grp.id, &curve_bits); const size_t curve_bytes = PSA_BITS_TO_BYTES(curve_bits); +#endif /* !MBEDTLS_PK_USE_PSA_EC_DATA */ psa_set_key_type(&key_attr, PSA_KEY_TYPE_ECC_KEY_PAIR(curve)); psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_EXPORT); - ret = mbedtls_mpi_write_binary(&prv_ctx->d, prv_key_buf, curve_bytes); + ret = mbedtls_mpi_write_binary(&mbedtls_pk_ec_ro(*prv)->d, + prv_key_buf, curve_bytes); if (ret != 0) { return ret; } @@ -1154,7 +1177,13 @@ static int eckey_check_pair_psa(mbedtls_pk_context *pub, mbedtls_pk_context *prv return PSA_PK_TO_MBEDTLS_ERR(destruction_status); } - ret = mbedtls_ecp_point_write_binary(&pub_ctx->grp, &pub_ctx->Q, +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + if (memcmp(prv_key_buf, pub->pub_raw, pub->pub_raw_len) != 0) { + return MBEDTLS_ERR_PK_BAD_INPUT_DATA; + } +#else + ret = mbedtls_ecp_point_write_binary(&mbedtls_pk_ec_rw(*pub)->grp, + &mbedtls_pk_ec_rw(*pub)->Q, MBEDTLS_ECP_PF_UNCOMPRESSED, &pub_key_len, pub_key_buf, sizeof(pub_key_buf)); @@ -1165,6 +1194,7 @@ static int eckey_check_pair_psa(mbedtls_pk_context *pub, mbedtls_pk_context *prv if (memcmp(prv_key_buf, pub_key_buf, curve_bytes) != 0) { return MBEDTLS_ERR_PK_BAD_INPUT_DATA; } +#endif /* !MBEDTLS_PK_USE_PSA_EC_DATA */ return 0; } @@ -1206,10 +1236,16 @@ static void eckey_free_wrap(void *ctx) static void eckey_debug(mbedtls_pk_context *pk, mbedtls_pk_debug_item *items) { +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + items->type = MBEDTLS_PK_DEBUG_PSA_EC; + items->name = "eckey.Q"; + items->value = pk; +#else mbedtls_ecp_keypair *ecp = (mbedtls_ecp_keypair *) pk->pk_ctx; items->type = MBEDTLS_PK_DEBUG_ECP; items->name = "eckey.Q"; items->value = &(ecp->Q); +#endif } const mbedtls_pk_info_t mbedtls_eckey_info = { From 7ca7b90bc7cf917b0d7b662be1d3fa568a1cb2b3 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 17 May 2023 15:35:46 +0200 Subject: [PATCH 05/12] debug: add support for printing the new EC raw format Signed-off-by: Valerio Setti --- library/debug.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/library/debug.c b/library/debug.c index 3969616f4f77..1868cb74240e 100644 --- a/library/debug.c +++ b/library/debug.c @@ -194,6 +194,52 @@ void mbedtls_debug_print_ecp(const mbedtls_ssl_context *ssl, int level, } #endif /* MBEDTLS_ECP_LIGHT */ +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) +void mbedtls_debug_print_psa_ec(const mbedtls_ssl_context *ssl, int level, + const char *file, int line, + const char *text, const mbedtls_pk_context *pk) +{ + char str[DEBUG_BUF_SIZE]; + mbedtls_mpi mpi; + const uint8_t *mpi_start; + size_t mpi_len; + int ret; + + if (NULL == ssl || + NULL == ssl->conf || + NULL == ssl->conf->f_dbg || + level > debug_threshold) { + return; + } + + /* For the description of pk->pk_raw content please refer to the description + * psa_export_public_key() function. */ + mpi_len = (pk->pub_raw_len - 1)/2; + + /* X coordinate */ + mbedtls_mpi_init(&mpi); + mpi_start = pk->pub_raw + 1; + ret = mbedtls_mpi_read_binary(&mpi, mpi_start, mpi_len); + if (ret != 0) { + return; + } + mbedtls_snprintf(str, sizeof(str), "%s(X)", text); + mbedtls_debug_print_mpi(ssl, level, file, line, str, &mpi); + mbedtls_mpi_free(&mpi); + + /* Y coordinate */ + mbedtls_mpi_init(&mpi); + mpi_start = mpi_start + mpi_len; + ret = mbedtls_mpi_read_binary(&mpi, mpi_start, mpi_len); + if (ret != 0) { + return; + } + mbedtls_snprintf(str, sizeof(str), "%s(Y)", text); + mbedtls_debug_print_mpi(ssl, level, file, line, str, &mpi); + mbedtls_mpi_free(&mpi); +} +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ + #if defined(MBEDTLS_BIGNUM_C) void mbedtls_debug_print_mpi(const mbedtls_ssl_context *ssl, int level, const char *file, int line, @@ -277,6 +323,11 @@ static void debug_print_pk(const mbedtls_ssl_context *ssl, int level, if (items[i].type == MBEDTLS_PK_DEBUG_ECP) { mbedtls_debug_print_ecp(ssl, level, file, line, name, items[i].value); } else +#endif +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + if (items[i].type == MBEDTLS_PK_DEBUG_PSA_EC) { + mbedtls_debug_print_psa_ec(ssl, level, file, line, name, items[i].value); + } else #endif { debug_send_line(ssl, level, file, line, "should not happen\n"); } From d7ca39511f8e13af43b8f8ed970758447ad363f5 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 17 May 2023 15:36:18 +0200 Subject: [PATCH 06/12] tls12: use the the raw format for the public key when USE_PSA is enabled Signed-off-by: Valerio Setti --- library/ssl_tls12_client.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index 0940bdb67c7f..070583b138aa 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -2010,7 +2010,6 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) peer_key = mbedtls_pk_ec_ro(*peer_pk); #if defined(MBEDTLS_USE_PSA_CRYPTO) - size_t olen = 0; uint16_t tls_id = 0; psa_ecc_family_t ecc_family; @@ -2034,6 +2033,12 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) ssl->handshake->ecdh_psa_type = PSA_KEY_TYPE_ECC_KEY_PAIR(ecc_family); /* Store peer's public key in psa format. */ +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + memcpy(ssl->handshake->ecdh_psa_peerkey, peer_pk->pub_raw, peer_pk->pub_raw_len); + ssl->handshake->ecdh_psa_peerkey_len = peer_pk->pub_raw_len; + ret = 0; +#else + size_t olen = 0; ret = mbedtls_ecp_point_write_binary(&peer_key->grp, &peer_key->Q, MBEDTLS_ECP_PF_UNCOMPRESSED, &olen, ssl->handshake->ecdh_psa_peerkey, @@ -2043,8 +2048,8 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) MBEDTLS_SSL_DEBUG_RET(1, ("mbedtls_ecp_point_write_binary"), ret); return ret; } - ssl->handshake->ecdh_psa_peerkey_len = olen; +#endif /* MBEDTLS_ECP_C */ #else if ((ret = mbedtls_ecdh_get_params(&ssl->handshake->ecdh_ctx, peer_key, MBEDTLS_ECDH_THEIRS)) != 0) { From 92c3f368666de5e4b5fec84c48ac34c8f0da12a8 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 17 May 2023 15:36:44 +0200 Subject: [PATCH 07/12] test_suite_debug: fix USE_PSA_INIT/DONE guards in a test Signed-off-by: Valerio Setti --- include/mbedtls/pk.h | 1 + tests/suites/test_suite_debug.function | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 9b2cab8fbf1b..4a8e50c9960a 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -224,6 +224,7 @@ typedef enum { MBEDTLS_PK_DEBUG_NONE = 0, MBEDTLS_PK_DEBUG_MPI, MBEDTLS_PK_DEBUG_ECP, + MBEDTLS_PK_DEBUG_PSA_EC, } mbedtls_pk_debug_type; /** diff --git a/tests/suites/test_suite_debug.function b/tests/suites/test_suite_debug.function index da91f445469b..b9610406bb90 100644 --- a/tests/suites/test_suite_debug.function +++ b/tests/suites/test_suite_debug.function @@ -167,11 +167,11 @@ void mbedtls_debug_print_crt(char *crt_file, char *file, int line, mbedtls_ssl_config conf; struct buffer_data buffer; - MD_PSA_INIT(); - mbedtls_ssl_init(&ssl); mbedtls_ssl_config_init(&conf); mbedtls_x509_crt_init(&crt); + MD_OR_USE_PSA_INIT(); + memset(buffer.buf, 0, 2000); buffer.ptr = buffer.buf; @@ -193,7 +193,7 @@ exit: mbedtls_x509_crt_free(&crt); mbedtls_ssl_free(&ssl); mbedtls_ssl_config_free(&conf); - MD_PSA_DONE(); + MD_OR_USE_PSA_DONE(); } /* END_CASE */ From 483738ed6789d2c30b6ffa41dfe14d23229d1204 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 17 May 2023 15:37:29 +0200 Subject: [PATCH 08/12] tests: fixes for using the new public key raw format Signed-off-by: Valerio Setti --- library/pk.c | 33 ++++++++++ library/pk_internal.h | 21 +++++++ tests/suites/test_suite_pk.function | 76 +++++++++++++++++------- tests/suites/test_suite_pkparse.function | 8 ++- 4 files changed, 115 insertions(+), 23 deletions(-) diff --git a/library/pk.c b/library/pk.c index 7a047df3fd16..47c19b208671 100644 --- a/library/pk.c +++ b/library/pk.c @@ -196,6 +196,39 @@ int mbedtls_pk_setup_opaque(mbedtls_pk_context *ctx, } #endif /* MBEDTLS_USE_PSA_CRYPTO */ +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) +int mbedtls_pk_update_public_key_from_keypair(mbedtls_pk_context *pk, + mbedtls_ecp_keypair *ecp_keypair) +{ + int ret = MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; + + if (pk == NULL) { + return MBEDTLS_ERR_PK_BAD_INPUT_DATA; + } + /* The raw public key storing mechanism is only supported for EC keys so + * we fail silently for other ones. */ + if ((pk->pk_info->type != MBEDTLS_PK_ECKEY) && + (pk->pk_info->type != MBEDTLS_PK_ECKEY_DH) && + (pk->pk_info->type != MBEDTLS_PK_ECDSA)) { + return 0; + } + + ret = mbedtls_ecp_point_write_binary(&ecp_keypair->grp, &ecp_keypair->Q, + MBEDTLS_ECP_PF_UNCOMPRESSED, + &pk->pub_raw_len, + pk->pub_raw, + MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN); + if (ret != 0) { + return ret; + } + + pk->ec_family = mbedtls_ecc_group_to_psa(ecp_keypair->grp.id, + &pk->ec_bits); + + return 0; +} +#endif /* MBEDTLS_PK_USE_PSA_EC_PUB_KEY */ + #if defined(MBEDTLS_PK_RSA_ALT_SUPPORT) /* * Initialize an RSA-alt context diff --git a/library/pk_internal.h b/library/pk_internal.h index 09f3d85249d2..dbb7bc1659c9 100644 --- a/library/pk_internal.h +++ b/library/pk_internal.h @@ -23,10 +23,16 @@ #ifndef MBEDTLS_PK_INTERNAL_H #define MBEDTLS_PK_INTERNAL_H +#include "mbedtls/pk.h" + #if defined(MBEDTLS_ECP_LIGHT) #include "mbedtls/ecp.h" #endif +#if defined(MBEDTLS_USE_PSA_CRYPTO) +#include "psa/crypto.h" +#endif + #if defined(MBEDTLS_ECP_LIGHT) /** * Public function mbedtls_pk_ec() can be used to get direct access to the @@ -85,4 +91,19 @@ static inline int mbedtls_pk_is_rfc8410_curve(mbedtls_ecp_group_id id) #endif /* MBEDTLS_ECP_DP_CURVE25519_ENABLED || MBEDTLS_ECP_DP_CURVE448_ENABLED */ #endif /* MBEDTLS_ECP_LIGHT */ +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) +/** + * \brief Copy the public key content in raw format from "ctx->pk_ctx" + * (which is an ecp_keypair) into the internal "ctx->pub_raw" buffer. + * + * \note This is a temporary function that can be removed as soon as the pk + * module is free from ECP_C + * + * \param pk It is the pk_context which is going to be updated. It acts both + * as input and output. + */ +int mbedtls_pk_update_public_key_from_keypair(mbedtls_pk_context *pk, + mbedtls_ecp_keypair *ecp_keypair); +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ + #endif /* MBEDTLS_PK_INTERNAL_H */ diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index f36c6be3c5c7..d39737495194 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -1,5 +1,6 @@ /* BEGIN_HEADER */ #include "mbedtls/pk.h" +#include "pk_internal.h" /* For error codes */ #include "mbedtls/asn1.h" @@ -24,16 +25,15 @@ #define RSA_KEY_SIZE 512 #define RSA_KEY_LEN 64 -#if defined(MBEDTLS_USE_PSA_CRYPTO) && defined(MBEDTLS_ECP_LIGHT) -static int pk_genkey_ec(mbedtls_ecp_group *grp, - mbedtls_mpi *d, mbedtls_ecp_point *Q) +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) +static int pk_genkey_ec(mbedtls_pk_context *pk, mbedtls_ecp_group_id grp_id) { psa_status_t status; + mbedtls_ecp_keypair *eck = mbedtls_pk_ec_rw(*pk); psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; size_t curve_bits; - psa_ecc_family_t curve = mbedtls_ecc_group_to_psa(grp->id, - &curve_bits); + psa_ecc_family_t curve = mbedtls_ecc_group_to_psa(grp_id, &curve_bits); unsigned char key_buf[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; size_t key_len; int ret; @@ -53,26 +53,33 @@ static int pk_genkey_ec(mbedtls_ecp_group *grp, goto exit; } - ret = mbedtls_mpi_read_binary(d, key_buf, key_len); + ret = mbedtls_mpi_read_binary(&eck->d, key_buf, key_len); if (ret != 0) { goto exit; } - status = psa_export_public_key(key_id, key_buf, sizeof(key_buf), - &key_len); + status = psa_export_public_key(key_id, pk->pub_raw, sizeof(pk->pub_raw), + &pk->pub_raw_len); if (status != PSA_SUCCESS) { ret = MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; goto exit; } - ret = mbedtls_ecp_point_read_binary(grp, Q, key_buf, key_len); + pk->ec_family = curve; + pk->ec_bits = curve_bits; -exit: - psa_destroy_key(key_id); + status = psa_destroy_key(key_id); + if (status != PSA_SUCCESS) { + return psa_pk_status_to_mbedtls(status); + } + + return 0; - return ret; +exit: + status = psa_destroy_key(key_id); + return (ret != 0) ? ret : psa_pk_status_to_mbedtls(status); } -#endif /* MBEDTLS_USE_PSA_CRYPTO && MBEDTLS_ECP_LIGHT */ +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ /** Generate a key of the desired type. * @@ -102,22 +109,36 @@ static int pk_genkey(mbedtls_pk_context *pk, int parameter) mbedtls_pk_get_type(pk) == MBEDTLS_PK_ECKEY_DH || mbedtls_pk_get_type(pk) == MBEDTLS_PK_ECDSA) { int ret; - if ((ret = mbedtls_ecp_group_load(&mbedtls_pk_ec_rw(*pk)->grp, - parameter)) != 0) { + + ret = mbedtls_ecp_group_load(&mbedtls_pk_ec_rw(*pk)->grp, parameter); + if (ret != 0) { return ret; } -#if defined(MBEDTLS_USE_PSA_CRYPTO) - return pk_genkey_ec(&mbedtls_pk_ec_rw(*pk)->grp, - &mbedtls_pk_ec_rw(*pk)->d, - &mbedtls_pk_ec_rw(*pk)->Q); -#endif /* MBEDTLS_USE_PSA_CRYPTO */ +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + mbedtls_ecp_group grp; + /* Duplicating the mbedtls_ecp_group_load call to make this part + * more future future proof for when ECP_C will not be defined. */ + mbedtls_ecp_group_init(&grp); + ret = mbedtls_ecp_group_load(&grp, parameter); + if (ret != 0) { + return ret; + } + ret = pk_genkey_ec(pk, grp.id); + if (ret != 0) { + return ret; + } + mbedtls_ecp_group_free(&grp); + + return 0; +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ #if defined(MBEDTLS_ECP_C) return mbedtls_ecp_gen_keypair(&mbedtls_pk_ec_rw(*pk)->grp, &mbedtls_pk_ec_rw(*pk)->d, &mbedtls_pk_ec_rw(*pk)->Q, mbedtls_test_rnd_std_rand, NULL); #endif /* MBEDTLS_ECP_C */ + } #endif /* MBEDTLS_ECP_LIGHT */ return -1; @@ -702,7 +723,6 @@ void pk_ec_test_vec(int type, int id, data_t *key, data_t *hash, data_t *sig, int ret) { mbedtls_pk_context pk; - mbedtls_ecp_keypair *eckey; mbedtls_pk_init(&pk); USE_PSA_INIT(); @@ -710,11 +730,23 @@ void pk_ec_test_vec(int type, int id, data_t *key, data_t *hash, TEST_ASSERT(mbedtls_pk_setup(&pk, mbedtls_pk_info_from_type(type)) == 0); TEST_ASSERT(mbedtls_pk_can_do(&pk, MBEDTLS_PK_ECDSA)); - eckey = mbedtls_pk_ec_rw(pk); +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + mbedtls_ecp_keypair ecp; + mbedtls_ecp_keypair_init(&ecp); + + TEST_ASSERT(mbedtls_ecp_group_load(&ecp.grp, id) == 0); + TEST_ASSERT(mbedtls_ecp_point_read_binary(&ecp.grp, &ecp.Q, + key->x, key->len) == 0); + TEST_ASSERT(mbedtls_pk_update_public_key_from_keypair(&pk, &ecp) == 0); + + mbedtls_ecp_keypair_free(&ecp); +#else + mbedtls_ecp_keypair *eckey = (mbedtls_ecp_keypair *) mbedtls_pk_ec(pk); TEST_ASSERT(mbedtls_ecp_group_load(&eckey->grp, id) == 0); TEST_ASSERT(mbedtls_ecp_point_read_binary(&eckey->grp, &eckey->Q, key->x, key->len) == 0); +#endif // MBEDTLS_MD_NONE is used since it will be ignored. TEST_ASSERT(mbedtls_pk_verify(&pk, MBEDTLS_MD_NONE, diff --git a/tests/suites/test_suite_pkparse.function b/tests/suites/test_suite_pkparse.function index e0e33000da54..a49b6d31959c 100644 --- a/tests/suites/test_suite_pkparse.function +++ b/tests/suites/test_suite_pkparse.function @@ -84,10 +84,16 @@ void pk_parse_public_keyfile_ec(char *key_file, int result) TEST_ASSERT(res == result); if (res == 0) { - const mbedtls_ecp_keypair *eckey; TEST_ASSERT(mbedtls_pk_can_do(&ctx, MBEDTLS_PK_ECKEY)); +#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) + /* No need to check whether the parsed public point is on the curve or + * not because this is already done by the internal "pk_get_ecpubkey()" + * function */ +#else + const mbedtls_ecp_keypair *eckey; eckey = mbedtls_pk_ec_ro(ctx); TEST_ASSERT(mbedtls_ecp_check_pubkey(&eckey->grp, &eckey->Q) == 0); +#endif } exit: From c1541cb3c72b0037566c3aabbf745122ff5374cc Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 17 May 2023 15:49:55 +0200 Subject: [PATCH 09/12] pk: minor fixes (guards and a wrong assignment) Signed-off-by: Valerio Setti --- include/mbedtls/pk.h | 5 +++-- library/debug.c | 2 +- library/pk.c | 2 +- library/pk_wrap.c | 4 ++-- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 4a8e50c9960a..f2cf9fed2596 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -213,7 +213,8 @@ typedef struct mbedtls_pk_rsassa_pss_options { * format and use PSA functions * - if !ECP_C then use new raw data and PSA functions directly. */ -#if defined(MBEDTLS_USE_PSA_CRYPTO) && !defined(MBEDTLS_ECP_C) +#if defined(MBEDTLS_USE_PSA_CRYPTO) && !defined(MBEDTLS_ECP_C) && \ + defined(MBEDTLS_ECP_LIGHT) #define MBEDTLS_PK_USE_PSA_EC_DATA #endif /* MBEDTLS_USE_PSA_CRYPTO && !MBEDTLS_ECP_C */ @@ -290,7 +291,7 @@ typedef struct mbedtls_pk_context { size_t MBEDTLS_PRIVATE(pub_raw_len); /**< Valid bytes in "pub_raw" */ psa_ecc_family_t MBEDTLS_PRIVATE(ec_family); /**< EC family of pk */ size_t MBEDTLS_PRIVATE(ec_bits); /**< Curve's bits of pk */ -#endif /* MBEDTLS_PK_USE_PSA_EC_PUB_KEY */ +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ } mbedtls_pk_context; #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) diff --git a/library/debug.c b/library/debug.c index 1868cb74240e..8236452757b8 100644 --- a/library/debug.c +++ b/library/debug.c @@ -194,6 +194,7 @@ void mbedtls_debug_print_ecp(const mbedtls_ssl_context *ssl, int level, } #endif /* MBEDTLS_ECP_LIGHT */ +#if defined(MBEDTLS_BIGNUM_C) #if defined(MBEDTLS_PK_USE_PSA_EC_DATA) void mbedtls_debug_print_psa_ec(const mbedtls_ssl_context *ssl, int level, const char *file, int line, @@ -240,7 +241,6 @@ void mbedtls_debug_print_psa_ec(const mbedtls_ssl_context *ssl, int level, } #endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ -#if defined(MBEDTLS_BIGNUM_C) void mbedtls_debug_print_mpi(const mbedtls_ssl_context *ssl, int level, const char *file, int line, const char *text, const mbedtls_mpi *X) diff --git a/library/pk.c b/library/pk.c index 47c19b208671..826c29a8cba1 100644 --- a/library/pk.c +++ b/library/pk.c @@ -227,7 +227,7 @@ int mbedtls_pk_update_public_key_from_keypair(mbedtls_pk_context *pk, return 0; } -#endif /* MBEDTLS_PK_USE_PSA_EC_PUB_KEY */ +#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ #if defined(MBEDTLS_PK_RSA_ALT_SUPPORT) /* diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 32d697ac03a9..376af2509a8e 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -1140,15 +1140,15 @@ static int eckey_check_pair_psa(mbedtls_pk_context *pub, mbedtls_pk_context *prv mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; #if defined(MBEDTLS_PK_USE_PSA_EC_DATA) const psa_ecc_family_t curve = prv->ec_family; - const size_t curve_bits = PSA_BITS_TO_BYTES(prv->ec_bits); + const size_t curve_bits = prv->ec_bits; #else /* !MBEDTLS_PK_USE_PSA_EC_DATA */ uint8_t pub_key_buf[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; size_t pub_key_len; size_t curve_bits; const psa_ecc_family_t curve = mbedtls_ecc_group_to_psa(mbedtls_pk_ec_ro(*prv)->grp.id, &curve_bits); - const size_t curve_bytes = PSA_BITS_TO_BYTES(curve_bits); #endif /* !MBEDTLS_PK_USE_PSA_EC_DATA */ + const size_t curve_bytes = PSA_BITS_TO_BYTES(curve_bits); psa_set_key_type(&key_attr, PSA_KEY_TYPE_ECC_KEY_PAIR(curve)); psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_EXPORT); From f57007dd1edb3f323cf396c66204a69349509d6f Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Fri, 19 May 2023 13:54:39 +0200 Subject: [PATCH 10/12] pk: fixing and improving comments Signed-off-by: Valerio Setti --- include/mbedtls/pk.h | 21 ++++++++++++--------- library/pk_wrap.c | 2 +- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index f2cf9fed2596..3a5543a3c12d 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -202,16 +202,21 @@ typedef struct mbedtls_pk_rsassa_pss_options { #define MBEDTLS_PK_CAN_ECDH #endif -/* Helper to define which fields in the pk_context structure below should be - * used for EC keys: legacy ecp_keypair or the raw (PSA friendly) format. - * It should be noticed that this only affect how data is stored, not which - * functions are used for various operations. The overall picture looks like - * this: +/* Internal helper to define which fields in the pk_context structure below + * should be used for EC keys: legacy ecp_keypair or the raw (PSA friendly) + * format. It should be noticed that this only affect how data is stored, not + * which functions are used for various operations. The overall picture looks + * like this: * - if ECP_C is defined then use legacy functions * - if USE_PSA is defined and * - if ECP_C then use ecp_keypair structure, convert data to a PSA friendly * format and use PSA functions * - if !ECP_C then use new raw data and PSA functions directly. + * + * The main reason for the "intermediate" (USE_PSA + ECP_C) above is that as long + * as ECP_C is defined mbedtls_pk_ec() gives the user a read/write access to the + * ecp_keypair structure inside the pk_context so he/she can modify it using + * ECP functions which are not under PK module's control. */ #if defined(MBEDTLS_USE_PSA_CRYPTO) && !defined(MBEDTLS_ECP_C) && \ defined(MBEDTLS_ECP_LIGHT) @@ -258,9 +263,7 @@ typedef struct mbedtls_pk_context { const mbedtls_pk_info_t *MBEDTLS_PRIVATE(pk_info); /**< Public key information */ void *MBEDTLS_PRIVATE(pk_ctx); /**< Underlying public key context */ /* When MBEDTLS_PSA_CRYPTO_C is enabled then the following priv_id field is - * used to store the ID of the opaque key. Differently from the raw public - * key management below, in this case there is no counterpart in the pk_ctx - * field to work in parallel with. + * used to store the ID of the opaque key. * This priv_id is guarded by MBEDTLS_PSA_CRYPTO_C and not by * MBEDTLS_USE_PSA_CRYPTO because it can be used also in mbedtls_pk_sign_ext * for RSA keys. */ @@ -283,7 +286,7 @@ typedef struct mbedtls_pk_context { * all the operations. * * Note: This new public key storing solution only works for EC keys, not - * other ones. The latters is still use pk_ctx to store their own + * other ones. The latters still use pk_ctx to store their own * context. */ #if defined(MBEDTLS_PK_USE_PSA_EC_DATA) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 376af2509a8e..e21ec2b3071e 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -792,7 +792,7 @@ static int ecdsa_verify_wrap(mbedtls_pk_context *pk, p = (unsigned char *) sig; /* extract_ecdsa_sig's last parameter is the size - * of each integer to be parse, so it's actually half + * of each integer to be parsed, so it's actually half * the size of the signature. */ if ((ret = extract_ecdsa_sig(&p, sig + sig_len, buf, signature_len/2)) != 0) { From a7cb845705163ee3c2fa396da5737df5b3daac65 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 22 May 2023 18:39:43 +0200 Subject: [PATCH 11/12] pk: add checks for the returned ECC family Signed-off-by: Valerio Setti --- library/pk.c | 3 +++ library/pk_wrap.c | 4 ++++ tests/suites/test_suite_pk.function | 4 ++++ 3 files changed, 11 insertions(+) diff --git a/library/pk.c b/library/pk.c index 826c29a8cba1..9c4aa16a6baf 100644 --- a/library/pk.c +++ b/library/pk.c @@ -224,6 +224,9 @@ int mbedtls_pk_update_public_key_from_keypair(mbedtls_pk_context *pk, pk->ec_family = mbedtls_ecc_group_to_psa(ecp_keypair->grp.id, &pk->ec_bits); + if (pk->ec_family == 0) { + return MBEDTLS_ERR_PK_BAD_INPUT_DATA; + } return 0; } diff --git a/library/pk_wrap.c b/library/pk_wrap.c index e21ec2b3071e..3a3d3998b01c 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -1150,6 +1150,10 @@ static int eckey_check_pair_psa(mbedtls_pk_context *pub, mbedtls_pk_context *prv #endif /* !MBEDTLS_PK_USE_PSA_EC_DATA */ const size_t curve_bytes = PSA_BITS_TO_BYTES(curve_bits); + if (curve == 0) { + return MBEDTLS_ERR_PK_BAD_INPUT_DATA; + } + psa_set_key_type(&key_attr, PSA_KEY_TYPE_ECC_KEY_PAIR(curve)); psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_EXPORT); diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index d39737495194..7227f9278263 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -38,6 +38,10 @@ static int pk_genkey_ec(mbedtls_pk_context *pk, mbedtls_ecp_group_id grp_id) size_t key_len; int ret; + if (curve == 0) { + return MBEDTLS_ERR_PK_BAD_INPUT_DATA; + } + psa_set_key_type(&key_attr, PSA_KEY_TYPE_ECC_KEY_PAIR(curve)); psa_set_key_bits(&key_attr, curve_bits); psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_EXPORT); From 016264b6cb442d27c436fb1b061cae4b9365a844 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 22 May 2023 18:40:35 +0200 Subject: [PATCH 12/12] pk: fix a return value and a typo in comment Signed-off-by: Valerio Setti --- include/mbedtls/pk.h | 2 +- library/pkparse.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 3a5543a3c12d..ffd1b73b2326 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -282,7 +282,7 @@ typedef struct mbedtls_pk_context { * - the following fields are used for all public key operations: signature * verify, key pair check and key write. * Of course, when MBEDTLS_PK_USE_PSA_EC_DATA is not enabled, the legacy - * ecp_keypair structure is used for storing the public key and perform + * ecp_keypair structure is used for storing the public key and performing * all the operations. * * Note: This new public key storing solution only works for EC keys, not diff --git a/library/pkparse.c b/library/pkparse.c index d47b0994936c..9bc88015afe8 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -722,7 +722,7 @@ static int pk_get_ecpubkey(unsigned char **p, const unsigned char *end, } else { /* Uncompressed format */ if ((end - *p) > MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN) { - return MBEDTLS_ERR_PK_BAD_INPUT_DATA; + return MBEDTLS_ERR_PK_BUFFER_TOO_SMALL; } memcpy(pk->pub_raw, *p, (end - *p)); pk->pub_raw_len = end - *p;