From ae89dcc4beefeb06a31f030f80726a7e524cc57c Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 5 May 2025 15:34:28 +0200 Subject: [PATCH 1/5] library: tls12: remove usage of MBEDTLS_PK_USE_PSA_EC_DATA PK module will now always use PSA storing pattern when working with EC keys therefore MBEDTLS_PK_USE_PSA_EC_DATA is assumed to be always enabled. Signed-off-by: Valerio Setti --- library/ssl_tls12_client.c | 18 ------------ library/ssl_tls12_server.c | 57 -------------------------------------- 2 files changed, 75 deletions(-) diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index 114c32aea..80b60aeaf 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -1758,10 +1758,6 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) return MBEDTLS_ERR_SSL_PK_TYPE_MISMATCH; } -#if !defined(MBEDTLS_PK_USE_PSA_EC_DATA) - const mbedtls_ecp_keypair *peer_key = mbedtls_pk_ec_ro(*peer_pk); -#endif /* !defined(MBEDTLS_PK_USE_PSA_EC_DATA) */ - uint16_t tls_id = 0; psa_key_type_t key_type = PSA_KEY_TYPE_NONE; mbedtls_ecp_group_id grp_id = mbedtls_pk_get_ec_group_id(peer_pk); @@ -1786,23 +1782,9 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) ssl->handshake->xxdh_psa_type = key_type; /* Store peer's public key in psa format. */ -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) memcpy(ssl->handshake->xxdh_psa_peerkey, peer_pk->pub_raw, peer_pk->pub_raw_len); ssl->handshake->xxdh_psa_peerkey_len = peer_pk->pub_raw_len; ret = 0; -#else /* MBEDTLS_PK_USE_PSA_EC_DATA */ - size_t olen = 0; - ret = mbedtls_ecp_point_write_binary(&peer_key->grp, &peer_key->Q, - MBEDTLS_ECP_PF_UNCOMPRESSED, &olen, - ssl->handshake->xxdh_psa_peerkey, - sizeof(ssl->handshake->xxdh_psa_peerkey)); - - if (ret != 0) { - MBEDTLS_SSL_DEBUG_RET(1, ("mbedtls_ecp_point_write_binary"), ret); - return ret; - } - ssl->handshake->xxdh_psa_peerkey_len = olen; -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ #if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) /* We don't need the peer's public key anymore. Free it, * so that more RAM is available for upcoming expensive diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 2b2b49f2b..b2b5e33c0 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -2525,12 +2525,6 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) psa_key_attributes_t key_attributes = PSA_KEY_ATTRIBUTES_INIT; unsigned char buf[PSA_KEY_EXPORT_ECC_KEY_PAIR_MAX_SIZE(PSA_VENDOR_ECC_MAX_CURVE_BITS)]; size_t key_len; -#if !defined(MBEDTLS_PK_USE_PSA_EC_DATA) - uint16_t tls_id = 0; - psa_key_type_t key_type = PSA_KEY_TYPE_NONE; - mbedtls_ecp_group_id grp_id; - mbedtls_ecp_keypair *key; -#endif /* !MBEDTLS_PK_USE_PSA_EC_DATA */ pk = mbedtls_ssl_own_key(ssl); @@ -2542,11 +2536,9 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) switch (pk_type) { case MBEDTLS_PK_OPAQUE: -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) case MBEDTLS_PK_ECKEY: case MBEDTLS_PK_ECKEY_DH: case MBEDTLS_PK_ECDSA: -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ if (!mbedtls_pk_can_do(pk, MBEDTLS_PK_ECKEY)) { return MBEDTLS_ERR_SSL_PK_TYPE_MISMATCH; } @@ -2561,7 +2553,6 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) ssl->handshake->xxdh_psa_type = psa_get_key_type(&key_attributes); ssl->handshake->xxdh_psa_bits = psa_get_key_bits(&key_attributes); -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) if (pk_type != MBEDTLS_PK_OPAQUE) { /* PK_ECKEY[_DH] and PK_ECDSA instead as parsed from the PK * module and only have ECDSA capabilities. Since we need @@ -2594,7 +2585,6 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) ret = 0; break; } -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ /* Opaque key is created by the user (externally from Mbed TLS) * so we assume it already has the right algorithm and flags @@ -2604,53 +2594,6 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) ret = 0; break; -#if !defined(MBEDTLS_PK_USE_PSA_EC_DATA) - case MBEDTLS_PK_ECKEY: - case MBEDTLS_PK_ECKEY_DH: - case MBEDTLS_PK_ECDSA: - key = mbedtls_pk_ec_rw(*pk); - grp_id = mbedtls_pk_get_ec_group_id(pk); - if (grp_id == MBEDTLS_ECP_DP_NONE) { - return MBEDTLS_ERR_ECP_BAD_INPUT_DATA; - } - tls_id = mbedtls_ssl_get_tls_id_from_ecp_group_id(grp_id); - if (tls_id == 0) { - /* This elliptic curve is not supported */ - return MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE; - } - - /* If the above conversion to TLS ID was fine, then also this one will - be, so there is no need to check the return value here */ - mbedtls_ssl_get_psa_curve_info_from_tls_id(tls_id, &key_type, - &ssl->handshake->xxdh_psa_bits); - - ssl->handshake->xxdh_psa_type = key_type; - - key_attributes = psa_key_attributes_init(); - psa_set_key_usage_flags(&key_attributes, PSA_KEY_USAGE_DERIVE); - psa_set_key_algorithm(&key_attributes, PSA_ALG_ECDH); - psa_set_key_type(&key_attributes, - PSA_KEY_TYPE_ECC_KEY_PAIR(ssl->handshake->xxdh_psa_type)); - psa_set_key_bits(&key_attributes, ssl->handshake->xxdh_psa_bits); - - ret = mbedtls_ecp_write_key_ext(key, &key_len, buf, sizeof(buf)); - if (ret != 0) { - mbedtls_platform_zeroize(buf, sizeof(buf)); - break; - } - - status = psa_import_key(&key_attributes, buf, key_len, - &ssl->handshake->xxdh_psa_privkey); - if (status != PSA_SUCCESS) { - ret = PSA_TO_MBEDTLS_ERR(status); - mbedtls_platform_zeroize(buf, sizeof(buf)); - break; - } - - mbedtls_platform_zeroize(buf, sizeof(buf)); - ret = 0; - break; -#endif /* !MBEDTLS_PK_USE_PSA_EC_DATA */ default: ret = MBEDTLS_ERR_SSL_PK_TYPE_MISMATCH; } From c394fd0ebc0e09654466cf306ccfc16907f09a89 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 5 May 2025 15:42:56 +0200 Subject: [PATCH 2/5] library: debug: replace MBEDTLS_PK_USE_PSA_EC_DATA with PSA_WANT_KEY_TYPE_ECC_PUBLIC_KEY MBEDTLS_PK_USE_PSA_EC_DATA is a legacy symbol that is used in 3.6 LTS branch, but now it is assumed to be always true. It's only kept for legacy reasons so it's better to replace it with PSA_WANT_KEY_TYPE_ECC_PUBLIC_KEY instead. Signed-off-by: Valerio Setti --- library/debug.c | 34 ++++------------------------------ 1 file changed, 4 insertions(+), 30 deletions(-) diff --git a/library/debug.c b/library/debug.c index 8d55b4136..5210f0c68 100644 --- a/library/debug.c +++ b/library/debug.c @@ -219,29 +219,8 @@ void mbedtls_debug_print_mpi(const mbedtls_ssl_context *ssl, int level, #endif /* MBEDTLS_BIGNUM_C */ #if defined(MBEDTLS_X509_CRT_PARSE_C) && !defined(MBEDTLS_X509_REMOVE_INFO) -#if defined(MBEDTLS_ECP_LIGHT) -static void mbedtls_debug_print_ecp(const mbedtls_ssl_context *ssl, int level, - const char *file, int line, - const char *text, const mbedtls_ecp_point *X) -{ - char str[DEBUG_BUF_SIZE]; - if (NULL == ssl || - NULL == ssl->conf || - NULL == ssl->conf->f_dbg || - level > debug_threshold) { - return; - } - - mbedtls_snprintf(str, sizeof(str), "%s(X)", text); - mbedtls_debug_print_mpi(ssl, level, file, line, str, &X->X); - - mbedtls_snprintf(str, sizeof(str), "%s(Y)", text); - mbedtls_debug_print_mpi(ssl, level, file, line, str, &X->Y); -} -#endif /* MBEDTLS_ECP_LIGHT */ - -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) +#if defined(PSA_WANT_KEY_TYPE_ECC_PUBLIC_KEY) static void mbedtls_debug_print_ec_coord(const mbedtls_ssl_context *ssl, int level, const char *file, int line, const char *text, const unsigned char *buf, size_t len) @@ -311,7 +290,7 @@ static void mbedtls_debug_print_psa_ec(const mbedtls_ssl_context *ssl, int level mbedtls_snprintf(str, sizeof(str), "%s(Y)", text); mbedtls_debug_print_ec_coord(ssl, level, file, line, str, coord_start, coord_len); } -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ +#endif /* PSA_WANT_KEY_TYPE_ECC_PUBLIC_KEY */ static void debug_print_pk(const mbedtls_ssl_context *ssl, int level, const char *file, int line, @@ -342,16 +321,11 @@ static void debug_print_pk(const mbedtls_ssl_context *ssl, int level, mbedtls_debug_print_mpi(ssl, level, file, line, name, items[i].value); } else #endif /* MBEDTLS_RSA_C */ -#if defined(MBEDTLS_ECP_LIGHT) - if (items[i].type == MBEDTLS_PK_DEBUG_ECP) { - mbedtls_debug_print_ecp(ssl, level, file, line, name, items[i].value); - } else -#endif /* MBEDTLS_ECP_LIGHT */ -#if defined(MBEDTLS_PK_USE_PSA_EC_DATA) +#if defined(PSA_WANT_KEY_TYPE_ECC_PUBLIC_KEY) if (items[i].type == MBEDTLS_PK_DEBUG_PSA_EC) { mbedtls_debug_print_psa_ec(ssl, level, file, line, name, items[i].value); } else -#endif /* MBEDTLS_PK_USE_PSA_EC_DATA */ +#endif /* PSA_WANT_KEY_TYPE_ECC_PUBLIC_KEY */ { debug_send_line(ssl, level, file, line, "should not happen\n"); } } From eaf578978edd3d91185e5a412d3c8cbf472a7ca0 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Tue, 6 May 2025 17:07:09 +0200 Subject: [PATCH 3/5] library: remove ECDSA_C dependency from ECP_RESTARTABLE Signed-off-by: Valerio Setti --- include/mbedtls/x509_crt.h | 10 +++++----- library/x509_crt.c | 26 +++++++++++++------------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index de9149936..a3f07892f 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -272,7 +272,7 @@ typedef struct { #endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */ } mbedtls_x509_crt_verify_chain; -#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) /** * \brief Context for resuming X.509 verify operations @@ -299,12 +299,12 @@ typedef struct { } mbedtls_x509_crt_restart_ctx; -#else /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ +#else /* MBEDTLS_ECP_RESTARTABLE */ /* Now we can declare functions that take a pointer to that */ typedef void mbedtls_x509_crt_restart_ctx; -#endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ +#endif /* MBEDTLS_ECP_RESTARTABLE */ #if defined(MBEDTLS_X509_CRT_PARSE_C) /** @@ -880,7 +880,7 @@ void mbedtls_x509_crt_init(mbedtls_x509_crt *crt); */ void mbedtls_x509_crt_free(mbedtls_x509_crt *crt); -#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) /** * \brief Initialize a restart context */ @@ -890,7 +890,7 @@ void mbedtls_x509_crt_restart_init(mbedtls_x509_crt_restart_ctx *ctx); * \brief Free the components of a restart context */ void mbedtls_x509_crt_restart_free(mbedtls_x509_crt_restart_ctx *ctx); -#endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ +#endif /* MBEDTLS_ECP_RESTARTABLE */ #endif /* MBEDTLS_X509_CRT_PARSE_C */ /** diff --git a/library/x509_crt.c b/library/x509_crt.c index 0a43d8789..4ac5d9b7e 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -2124,7 +2124,7 @@ static int x509_crt_check_signature(const mbedtls_x509_crt *child, return -1; } -#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) if (rs_ctx != NULL && child->sig_pk == MBEDTLS_PK_ECDSA) { return mbedtls_pk_verify_restartable(&parent->pk, child->sig_md, hash, hash_len, @@ -2234,7 +2234,7 @@ static int x509_crt_find_parent_in( mbedtls_x509_crt *parent, *fallback_parent; int signature_is_good = 0, fallback_signature_is_good; -#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) /* did we have something in progress? */ if (rs_ctx != NULL && rs_ctx->parent != NULL) { /* restore saved state */ @@ -2268,12 +2268,12 @@ static int x509_crt_find_parent_in( } /* Signature */ -#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) check_signature: #endif ret = x509_crt_check_signature(child, parent, rs_ctx); -#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) if (rs_ctx != NULL && ret == MBEDTLS_ERR_ECP_IN_PROGRESS) { /* save state */ rs_ctx->parent = parent; @@ -2358,7 +2358,7 @@ static int x509_crt_find_parent( *parent_is_trusted = 1; -#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) /* restore then clear saved state if we have some stored */ if (rs_ctx != NULL && rs_ctx->parent_is_trusted != -1) { *parent_is_trusted = rs_ctx->parent_is_trusted; @@ -2374,7 +2374,7 @@ static int x509_crt_find_parent( *parent_is_trusted, path_cnt, self_cnt, rs_ctx, now); -#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) if (rs_ctx != NULL && ret == MBEDTLS_ERR_ECP_IN_PROGRESS) { /* save state */ rs_ctx->parent_is_trusted = *parent_is_trusted; @@ -2501,7 +2501,7 @@ static int x509_crt_verify_chain( } #endif -#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) /* resume if we had an operation in progress */ if (rs_ctx != NULL && rs_ctx->in_progress == x509_crt_rs_find_parent) { /* restore saved state */ @@ -2515,7 +2515,7 @@ static int x509_crt_verify_chain( goto find_parent; } -#endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ +#endif /* MBEDTLS_ECP_RESTARTABLE */ child = crt; self_cnt = 0; @@ -2561,7 +2561,7 @@ static int x509_crt_verify_chain( return 0; } -#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) find_parent: #endif @@ -2593,7 +2593,7 @@ find_parent: ver_chain->len - 1, self_cnt, rs_ctx, &now); -#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) if (rs_ctx != NULL && ret == MBEDTLS_ERR_ECP_IN_PROGRESS) { /* save state */ rs_ctx->in_progress = x509_crt_rs_find_parent; @@ -3087,7 +3087,7 @@ exit: ver_chain.trust_ca_cb_result = NULL; #endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */ -#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) if (rs_ctx != NULL && ret != MBEDTLS_ERR_ECP_IN_PROGRESS) { mbedtls_x509_crt_restart_free(rs_ctx); } @@ -3223,7 +3223,7 @@ void mbedtls_x509_crt_free(mbedtls_x509_crt *crt) } } -#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) /* * Initialize a restart context */ @@ -3254,7 +3254,7 @@ void mbedtls_x509_crt_restart_free(mbedtls_x509_crt_restart_ctx *ctx) mbedtls_pk_restart_free(&ctx->pk); mbedtls_x509_crt_restart_init(ctx); } -#endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ +#endif /* MBEDTLS_ECP_RESTARTABLE */ int mbedtls_x509_crt_get_ca_istrue(const mbedtls_x509_crt *crt) { From a81d6dfb05631ac5d8cd0003913665f048287f15 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 21 May 2025 12:37:15 +0200 Subject: [PATCH 4/5] tests|programs: remove usage of mbedtls_ecp_set_max_ops() PK restartable operations are now implemented using PSA interruptible ones, so mbedtls_ecp_set_max_ops() can be removed in favor of psa_interruptible_set_max_ops(). Signed-off-by: Valerio Setti --- programs/ssl/ssl_client2.c | 1 - tests/suites/test_suite_x509parse.function | 1 - 2 files changed, 2 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 4b5ea7c5d..d5e7fdf30 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -2173,7 +2173,6 @@ usage: #if defined(MBEDTLS_ECP_RESTARTABLE) if (opt.ec_max_ops != DFL_EC_MAX_OPS) { psa_interruptible_set_max_ops(opt.ec_max_ops); - mbedtls_ecp_set_max_ops(opt.ec_max_ops); } #endif diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index 127694114..09b248e8f 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -681,7 +681,6 @@ void x509_verify_restart(char *crt_file, char *ca_file, TEST_EQUAL(mbedtls_x509_crt_parse_file(&ca, ca_file), 0); psa_interruptible_set_max_ops(max_ops); - mbedtls_ecp_set_max_ops(max_ops); cnt_restart = 0; do { From d7d0acbeb6b4186a62aa6e7429d5bda56c0cea52 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 12 Jun 2025 06:26:06 +0200 Subject: [PATCH 5/5] tf-psa-crypto: update reference Signed-off-by: Valerio Setti --- tf-psa-crypto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tf-psa-crypto b/tf-psa-crypto index 694fa1b81..1a7ceaf8e 160000 --- a/tf-psa-crypto +++ b/tf-psa-crypto @@ -1 +1 @@ -Subproject commit 694fa1b81cce46e8e160c8bda1a700f8c2a68586 +Subproject commit 1a7ceaf8e28e6b2a48f3743ce706a339dabeb509