From fbbafa0d2d52d0b78ae52b9e66cb73058b285aa3 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 6 Dec 2023 10:07:34 +0100 Subject: [PATCH 1/3] pkparse: do not set key algorithm for Montgomery keys in pk_ecc_set_key() Signed-off-by: Valerio Setti --- library/pkparse.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index edebf92ff7..18498e5f0b 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -105,16 +105,21 @@ static int pk_ecc_set_key(mbedtls_pk_context *pk, { #if defined(MBEDTLS_PK_USE_PSA_EC_DATA) psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_key_usage_t flags; 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)); + if (pk->ec_family == PSA_ECC_FAMILY_MONTGOMERY) { + /* Do not set algorithm here because Montgomery keys cannot do ECDSA and + * the PK module cannot do ECDH. When the key will be used in TLS for + * ECDH, it will be exported and then re-imported with proper flags + * and algorithm. */ + flags = PSA_KEY_USAGE_EXPORT; + } else { + psa_set_key_algorithm(&attributes, + MBEDTLS_PK_PSA_ALG_ECDSA_MAYBE_DET(PSA_ALG_ANY_HASH)); + flags = PSA_KEY_USAGE_SIGN_HASH | PSA_KEY_USAGE_SIGN_MESSAGE | + PSA_KEY_USAGE_EXPORT; } psa_set_key_usage_flags(&attributes, flags); From bced8bc8d7c77c76dd5cfd962cd97c15ba33cab3 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 6 Dec 2023 10:40:47 +0100 Subject: [PATCH 2/3] ssl_tls12_server: export/import PK parsed key in TLS side Instead of setting both algorithm and enrollement algorithm in the PK module when parsing the key: - for Weierstrass keys we only set ECDSA algorithm, - for Montgomery keys we don't set any algorithm. Reasons: - PK module can only do ECDSA and not ECDH - ECDH is only used in TLS - Montgomery keys cannot be used to do ECDSA, while Weierstrass ones can do both ECDSA and ECDH. So the idea is that once TLS needs the key to do ECDH (either Weierstrass and Montgomery), it exports the one parsed from the PK module and then re-imports it setting proper algorithm and flags. In this way the TLS module will own the new key so it will be its duty to clear it on exit. Signed-off-by: Valerio Setti --- library/ssl_tls12_server.c | 49 ++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index a07d0fb346..f9ce7a6b64 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -2597,12 +2597,12 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) mbedtls_pk_context *pk; mbedtls_pk_type_t pk_type; 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; - size_t key_len; mbedtls_ecp_group_id grp_id; - unsigned char buf[PSA_KEY_EXPORT_ECC_KEY_PAIR_MAX_SIZE(PSA_VENDOR_ECC_MAX_CURVE_BITS)]; mbedtls_ecp_keypair *key; #endif /* !MBEDTLS_PK_USE_PSA_EC_DATA */ @@ -2625,22 +2625,41 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) return MBEDTLS_ERR_SSL_PK_TYPE_MISMATCH; } - ssl->handshake->xxdh_psa_privkey = pk->priv_id; - - /* Key should not be destroyed in the TLS library */ - ssl->handshake->xxdh_psa_privkey_is_external = 1; - - status = psa_get_key_attributes(ssl->handshake->xxdh_psa_privkey, - &key_attributes); + /* Get the attributes of the key previously parsed by PK module in + * order to extract its type and length (in bits). */ + status = psa_get_key_attributes(pk->priv_id, &key_attributes); if (status != PSA_SUCCESS) { - ssl->handshake->xxdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; - return PSA_TO_MBEDTLS_ERR(status); + ret = PSA_TO_MBEDTLS_ERR(status); + goto exit; } - ssl->handshake->xxdh_psa_type = psa_get_key_type(&key_attributes); ssl->handshake->xxdh_psa_bits = psa_get_key_bits(&key_attributes); - psa_reset_key_attributes(&key_attributes); + /* Now export and then re-import the same key with proper flags + * and algorithm. We also set key's type and bits that we just got + * above. */ + 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); + + status = psa_export_key(pk->priv_id, buf, sizeof(buf), &key_len); + if (status != PSA_SUCCESS) { + ret = PSA_TO_MBEDTLS_ERR(status); + goto exit; + } + status = psa_import_key(&key_attributes, buf, key_len, + &ssl->handshake->xxdh_psa_privkey); + if (status != PSA_SUCCESS) { + ret = PSA_TO_MBEDTLS_ERR(status); + goto exit; + } + + /* Set this key as owned by the TLS library: it will be its duty + * to clear it exit. */ + ssl->handshake->xxdh_psa_privkey_is_external = 0; ret = 0; break; @@ -2696,6 +2715,10 @@ static int ssl_get_ecdh_params_from_cert(mbedtls_ssl_context *ssl) ret = MBEDTLS_ERR_SSL_PK_TYPE_MISMATCH; } +exit: + psa_reset_key_attributes(&key_attributes); + mbedtls_platform_zeroize(buf, sizeof(buf)); + return ret; } #else /* MBEDTLS_USE_PSA_CRYPTO */ From 202bb71dcd3c6da9e273fbf21c7145bf5970f851 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 6 Dec 2023 17:05:24 +0100 Subject: [PATCH 3/3] ssl_tls12_server: do not export/import opaque keys Signed-off-by: Valerio Setti --- library/ssl_tls12_server.c | 56 ++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index f9ce7a6b64..923b093af9 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -2635,31 +2635,41 @@ 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); - /* Now export and then re-import the same key with proper flags - * and algorithm. We also set key's type and bits that we just got - * above. */ - 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); + if (pk_type == MBEDTLS_PK_OPAQUE) { + /* Opaque key is created by the user (externally from Mbed TLS) + * so we assume it already has the right algorithm and flags + * set. Just copy its ID as reference. */ + ssl->handshake->xxdh_psa_privkey = pk->priv_id; + ssl->handshake->xxdh_psa_privkey_is_external = 1; + } else { + /* PK_ECKEY[_DH] and PK_ECDSA instead as parsed from the PK + * module and only have ECDSA capabilities. Since we need + * them for ECDH later, we export and then re-import them with + * proper flags and algorithm. Of course We also set key's type + * and bits that we just got above. */ + 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); - status = psa_export_key(pk->priv_id, buf, sizeof(buf), &key_len); - if (status != PSA_SUCCESS) { - ret = PSA_TO_MBEDTLS_ERR(status); - goto exit; - } - status = psa_import_key(&key_attributes, buf, key_len, - &ssl->handshake->xxdh_psa_privkey); - if (status != PSA_SUCCESS) { - ret = PSA_TO_MBEDTLS_ERR(status); - goto exit; - } + status = psa_export_key(pk->priv_id, buf, sizeof(buf), &key_len); + if (status != PSA_SUCCESS) { + ret = PSA_TO_MBEDTLS_ERR(status); + goto exit; + } + status = psa_import_key(&key_attributes, buf, key_len, + &ssl->handshake->xxdh_psa_privkey); + if (status != PSA_SUCCESS) { + ret = PSA_TO_MBEDTLS_ERR(status); + goto exit; + } - /* Set this key as owned by the TLS library: it will be its duty - * to clear it exit. */ - ssl->handshake->xxdh_psa_privkey_is_external = 0; + /* Set this key as owned by the TLS library: it will be its duty + * to clear it exit. */ + ssl->handshake->xxdh_psa_privkey_is_external = 0; + } ret = 0; break;