From b4a87b07f8e2791eb368bb79fb9ae8c32c7cb82e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 4 Oct 2022 22:54:26 +0200 Subject: [PATCH 01/13] Don't use pk_write in ecdsa_verify_wrap with USE_PSA_CRYPTO Under MBEDTLS_USE_PSA_CRYPTO, ecdsa_verify_wrap() was calling mbedtls_pk_write_pubkey() to write a public key in the form of a subjectPublicKey, only to then extract the part that represents the EC point which psa_import_key() actually wants. Instead, call an ecp function to directly get the public key in the desired format (just the point). This slightly reduces the code size and stack usage, and removes a dependency on pk_write. Signed-off-by: Gilles Peskine --- library/pk_wrap.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index fa296e824..0f0ae5b2b 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -691,11 +691,13 @@ static int ecdsa_verify_wrap(void *ctx_arg, mbedtls_md_type_t md_alg, 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; - mbedtls_pk_context key; - int key_len; - unsigned char buf[MBEDTLS_PK_ECP_PUB_DER_MAX_BYTES]; + size_t key_len; + /* This buffer contains first the public key (consisting of two public + * points plus a header byte), then the signature (consisting of two + * public points). Size it for the public key which is one byte larger. */ + unsigned char buf[PSA_KEY_EXPORT_ECC_PUBLIC_KEY_MAX_SIZE( + PSA_VENDOR_ECC_MAX_CURVE_BITS )]; unsigned char *p; - mbedtls_pk_info_t pk_info = mbedtls_eckey_info; psa_algorithm_t psa_sig_md = PSA_ALG_ECDSA_ANY; size_t curve_bits; psa_ecc_family_t curve = @@ -707,25 +709,22 @@ static int ecdsa_verify_wrap(void *ctx_arg, mbedtls_md_type_t md_alg, return MBEDTLS_ERR_PK_BAD_INPUT_DATA; } - /* mbedtls_pk_write_pubkey() expects a full PK context; - * re-construct one to make it happy */ - key.pk_info = &pk_info; - key.pk_ctx = ctx; - p = buf + sizeof(buf); - key_len = mbedtls_pk_write_pubkey(&p, buf, &key); - if (key_len <= 0) { - return MBEDTLS_ERR_PK_BAD_INPUT_DATA; + psa_set_key_type( &attributes, PSA_KEY_TYPE_ECC_PUBLIC_KEY( curve ) ); + psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_VERIFY_HASH ); + psa_set_key_algorithm( &attributes, psa_sig_md ); + + ret = mbedtls_ecp_point_write_binary(&ctx->grp, &ctx->Q, + MBEDTLS_ECP_PF_UNCOMPRESSED, + &key_len, buf, sizeof(buf)); + if (ret != 0) { + goto cleanup; } - psa_set_key_type(&attributes, PSA_KEY_TYPE_ECC_PUBLIC_KEY(curve)); - psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_VERIFY_HASH); - psa_set_key_algorithm(&attributes, psa_sig_md); - status = psa_import_key(&attributes, - buf + sizeof(buf) - key_len, key_len, + buf, key_len, &key_id); if (status != PSA_SUCCESS) { - ret = mbedtls_pk_error_from_psa(status); + ret = mbedtls_pk_error_from_psa( status ); goto cleanup; } From 13caa94746354ecd0b8fe8ba1529916aed0d599e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 4 Oct 2022 22:59:26 +0200 Subject: [PATCH 02/13] Don't use pk_write in ecdsa_sign_wrap with USE_PSA_CRYPTO Under MBEDTLS_USE_PSA_CRYPTO, ecdsa_sign_wrap() was calling mbedtls_pk_write_key_der() to write a private key in SEC1 format, only to then extract the part that represents the private value which is what psa_import_key() actually wants. Instead, call an mpi function to directly get the private key in the desired format. This slightly reduces the code size and stack usage, and removes a dependency on pk_write. Signed-off-by: Gilles Peskine --- library/pk_wrap.c | 82 +++++++---------------------------------------- 1 file changed, 11 insertions(+), 71 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 0f0ae5b2b..464634db2 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -869,54 +869,6 @@ static int pk_ecdsa_sig_asn1_from_psa(unsigned char *sig, size_t *sig_len, return 0; } -/* Locate an ECDSA privateKey in a RFC 5915, or SEC1 Appendix C.4 ASN.1 buffer - * - * [in/out] buf: ASN.1 buffer start as input - ECDSA privateKey start as output - * [in] end: ASN.1 buffer end - * [out] key_len: the ECDSA privateKey length in bytes - */ -static int find_ecdsa_private_key(unsigned char **buf, unsigned char *end, - size_t *key_len) -{ - size_t len; - int ret; - - /* - * RFC 5915, or SEC1 Appendix C.4 - * - * ECPrivateKey ::= SEQUENCE { - * version INTEGER { ecPrivkeyVer1(1) } (ecPrivkeyVer1), - * privateKey OCTET STRING, - * parameters [0] ECParameters {{ NamedCurve }} OPTIONAL, - * publicKey [1] BIT STRING OPTIONAL - * } - */ - - if ((ret = mbedtls_asn1_get_tag(buf, end, &len, - MBEDTLS_ASN1_CONSTRUCTED | - MBEDTLS_ASN1_SEQUENCE)) != 0) { - return ret; - } - - /* version */ - if ((ret = mbedtls_asn1_get_tag(buf, end, &len, - MBEDTLS_ASN1_INTEGER)) != 0) { - return ret; - } - - *buf += len; - - /* privateKey */ - if ((ret = mbedtls_asn1_get_tag(buf, end, &len, - MBEDTLS_ASN1_OCTET_STRING)) != 0) { - return ret; - } - - *key_len = len; - - return 0; -} - static int ecdsa_sign_wrap(void *ctx_arg, mbedtls_md_type_t md_alg, const unsigned char *hash, size_t hash_len, unsigned char *sig, size_t sig_size, size_t *sig_len, @@ -927,19 +879,14 @@ static int ecdsa_sign_wrap(void *ctx_arg, mbedtls_md_type_t md_alg, 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; - mbedtls_pk_context key; - size_t key_len; - unsigned char buf[MBEDTLS_PK_ECP_PRV_DER_MAX_BYTES]; - unsigned char *p; - psa_algorithm_t psa_hash = mbedtls_hash_info_psa_from_md(md_alg); -#if defined(MBEDTLS_ECDSA_DETERMINISTIC) - psa_algorithm_t psa_sig_md = PSA_ALG_DETERMINISTIC_ECDSA(psa_hash); -#else - psa_algorithm_t psa_sig_md = PSA_ALG_ECDSA(psa_hash); -#endif + unsigned char buf[PSA_KEY_EXPORT_ECC_KEY_PAIR_MAX_SIZE( + PSA_VENDOR_ECC_MAX_CURVE_BITS )]; + psa_algorithm_t psa_sig_md = + PSA_ALG_ECDSA( mbedtls_hash_info_psa_from_md( md_alg ) ); size_t curve_bits; psa_ecc_family_t curve = - mbedtls_ecc_group_to_psa(ctx->grp.id, &curve_bits); + mbedtls_ecc_group_to_psa( ctx->grp.id, &curve_bits ); + size_t key_len = PSA_BITS_TO_BYTES(curve_bits); /* PSA has its own RNG */ ((void) f_rng); @@ -949,18 +896,11 @@ static int ecdsa_sign_wrap(void *ctx_arg, mbedtls_md_type_t md_alg, return MBEDTLS_ERR_PK_BAD_INPUT_DATA; } - /* mbedtls_pk_write_key_der() expects a full PK context; - * re-construct one to make it happy */ - key.pk_info = &mbedtls_eckey_info; - key.pk_ctx = ctx; - key_len = mbedtls_pk_write_key_der(&key, buf, sizeof(buf)); - if (key_len <= 0) { - return MBEDTLS_ERR_PK_BAD_INPUT_DATA; + if (key_len > sizeof(buf)) { + return( MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED ); } - - p = buf + sizeof(buf) - key_len; - ret = find_ecdsa_private_key(&p, buf + sizeof(buf), &key_len); - if (ret != 0) { + ret = mbedtls_mpi_write_binary(&ctx->d, buf, key_len); + if( ret != 0 ) { goto cleanup; } @@ -969,7 +909,7 @@ static int ecdsa_sign_wrap(void *ctx_arg, mbedtls_md_type_t md_alg, psa_set_key_algorithm(&attributes, psa_sig_md); status = psa_import_key(&attributes, - p, key_len, + buf, key_len, &key_id); if (status != PSA_SUCCESS) { ret = mbedtls_pk_error_from_psa(status); From bbccdd485c808d77b7cc11356832d547bbd0db2b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 4 Oct 2022 23:00:42 +0200 Subject: [PATCH 03/13] pk no longer needs pk_write for ECDSA with MBEDTLS_USE_PSA_CRYPTO The dependency is still useful for RSA, for which PSA encodes keys with an ASN.1 structure. Signed-off-by: Gilles Peskine --- include/mbedtls/build_info.h | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/include/mbedtls/build_info.h b/include/mbedtls/build_info.h index bbfd5d48d..6cf1176de 100644 --- a/include/mbedtls/build_info.h +++ b/include/mbedtls/build_info.h @@ -82,21 +82,9 @@ /* The PK wrappers need pk_write functions to format RSA key objects * when they are dispatching to the PSA API. This happens under USE_PSA_CRYPTO, - * and also even without USE_PSA_CRYPTO for mbedtls_pk_sign_ext(). - * PSA crypto also needs pk_write to export RSA keys (otherwise the build - * goes through but psa_export_key() and psa_export_public_key() fail on - * RSA keys), and pk_parse to work with RSA keys in almost any way. - */ -#if defined(MBEDTLS_PSA_CRYPTO_C) && defined(MBEDTLS_RSA_C) -#define MBEDTLS_PK_C -#define MBEDTLS_PK_WRITE_C -#define MBEDTLS_PK_PARSE_C -#endif - -/* Under MBEDTLS_USE_PSA_CRYPTO, the pk module needs pk_write functions - * to pass ECC keys to PSA. */ -#if defined(MBEDTLS_PK_C) && \ - defined(MBEDTLS_USE_PSA_CRYPTO) && defined(MBEDTLS_ECP_C) + * and also even without USE_PSA_CRYPTO for mbedtls_pk_sign_ext(). */ +#if defined(MBEDTLS_PK_C) && defined(MBEDTLS_PSA_CRYPTO_C) && \ + defined(MBEDTLS_RSA_C) #define MBEDTLS_PK_WRITE_C #endif From 8a6022e94885ba73e34019b45849718afdbad446 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 4 Oct 2022 23:01:59 +0200 Subject: [PATCH 04/13] Clean up header inclusions in pk_wrap.c To better reflect what the code relies on, limit the headers that are included when MBEDTLS_USE_PSA_CRYPTO is disabled. Also stop including "pkwrite.h" when it is no longer needed. Include "mbedlts/platform_util.h" unconditionally. It was only included for RSA ALT but was also used for MBEDTLS_USE_PSA_CRYPTO (the code worked because other headers include "mbedtls/platform_util.h"). Signed-off-by: Gilles Peskine --- library/pk_wrap.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 464634db2..ab19a47af 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -19,6 +19,8 @@ #include "common.h" +#include "mbedtls/platform_util.h" + #if defined(MBEDTLS_PK_C) #include "pk_wrap.h" #include "mbedtls/error.h" @@ -26,39 +28,34 @@ /* Even if RSA not activated, for the sake of RSA-alt */ #include "mbedtls/rsa.h" -#include - #if defined(MBEDTLS_ECP_C) #include "mbedtls/ecp.h" #endif -#if defined(MBEDTLS_RSA_C) || defined(MBEDTLS_ECP_C) -#include "pkwrite.h" -#endif - #if defined(MBEDTLS_ECDSA_C) #include "mbedtls/ecdsa.h" #endif -#if defined(MBEDTLS_USE_PSA_CRYPTO) -#include "mbedtls/asn1write.h" -#endif - -#if defined(MBEDTLS_PK_RSA_ALT_SUPPORT) -#include "mbedtls/platform_util.h" +#if defined(MBEDTLS_RSA_C) && defined(MBEDTLS_PSA_CRYPTO_C) +#include "pkwrite.h" #endif #if defined(MBEDTLS_USE_PSA_CRYPTO) #include "psa/crypto.h" #include "mbedtls/psa_util.h" -#include "mbedtls/asn1.h" #include "hash_info.h" + +#if defined(MBEDTLS_ECDSA_C) +#include "mbedtls/asn1write.h" +#include "mbedtls/asn1.h" #endif +#endif /* MBEDTLS_USE_PSA_CRYPTO */ #include "mbedtls/platform.h" #include #include +#include #if defined(MBEDTLS_PSA_CRYPTO_C) int mbedtls_pk_error_from_psa(psa_status_t status) From be9e2a1634ce4798d6ba36316ed2cd4ebc5acd7b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 6 Oct 2022 17:37:07 +0200 Subject: [PATCH 05/13] The pk_psa_sign test function needs pk_write Signed-off-by: Gilles Peskine --- tests/suites/test_suite_pk.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index d8a8f863e..9ec354eed 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -1182,7 +1182,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_USE_PSA_CRYPTO */ +/* BEGIN_CASE depends_on:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_USE_PSA_CRYPTO:MBEDTLS_PK_WRITE_C */ void pk_psa_sign(int parameter_arg, int psa_type_arg, int expected_bits_arg) { From 5bc52248eff9a7833c0b256b1b82409eee2fbff2 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 30 Jan 2023 15:48:28 +0100 Subject: [PATCH 06/13] pk_wrap: fix for DETERMINISTIC_ECDSA case in ecdsa_sign_wrap() Signed-off-by: Valerio Setti --- library/pk_wrap.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index ab19a47af..525f6bc90 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -878,8 +878,13 @@ static int ecdsa_sign_wrap(void *ctx_arg, mbedtls_md_type_t md_alg, psa_status_t status; unsigned char buf[PSA_KEY_EXPORT_ECC_KEY_PAIR_MAX_SIZE( PSA_VENDOR_ECC_MAX_CURVE_BITS )]; +#if defined(MBEDTLS_ECDSA_DETERMINISTIC) + psa_algorithm_t psa_sig_md = + PSA_ALG_DETERMINISTIC_ECDSA( mbedtls_hash_info_psa_from_md( md_alg ) ); +#else psa_algorithm_t psa_sig_md = PSA_ALG_ECDSA( mbedtls_hash_info_psa_from_md( md_alg ) ); +#endif size_t curve_bits; psa_ecc_family_t curve = mbedtls_ecc_group_to_psa( ctx->grp.id, &curve_bits ); From 1337a4f334854f9482b80244b4a90f7df7694773 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 30 Jan 2023 15:54:55 +0100 Subject: [PATCH 07/13] pk_wrap: use specific lengths for EC's private key and key-pair Signed-off-by: Valerio Setti --- include/mbedtls/psa_util.h | 3 +++ library/pk_wrap.c | 6 ++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/psa_util.h b/include/mbedtls/psa_util.h index dc74ac60c..f6070dcba 100644 --- a/include/mbedtls/psa_util.h +++ b/include/mbedtls/psa_util.h @@ -257,6 +257,9 @@ static inline int mbedtls_psa_get_ecc_oid_from_id( #define MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH \ PSA_KEY_EXPORT_ECC_PUBLIC_KEY_MAX_SIZE(PSA_VENDOR_ECC_MAX_CURVE_BITS) +#define MBEDTLS_PSA_MAX_EC_KEY_PAIR_LENGTH \ + PSA_KEY_EXPORT_ECC_KEY_PAIR_MAX_SIZE(PSA_VENDOR_ECC_MAX_CURVE_BITS) + /* Expose whatever RNG the PSA subsystem uses to applications using the * mbedtls_xxx API. The declarations and definitions here need to be * consistent with the implementation in library/psa_crypto_random_impl.h. diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 525f6bc90..6fba6e9b2 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -692,8 +692,7 @@ static int ecdsa_verify_wrap(void *ctx_arg, mbedtls_md_type_t md_alg, /* This buffer contains first the public key (consisting of two public * points plus a header byte), then the signature (consisting of two * public points). Size it for the public key which is one byte larger. */ - unsigned char buf[PSA_KEY_EXPORT_ECC_PUBLIC_KEY_MAX_SIZE( - PSA_VENDOR_ECC_MAX_CURVE_BITS )]; + 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; @@ -876,8 +875,7 @@ static int ecdsa_sign_wrap(void *ctx_arg, mbedtls_md_type_t md_alg, 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 buf[PSA_KEY_EXPORT_ECC_KEY_PAIR_MAX_SIZE( - PSA_VENDOR_ECC_MAX_CURVE_BITS )]; + unsigned char buf[MBEDTLS_PSA_MAX_EC_KEY_PAIR_LENGTH]; #if defined(MBEDTLS_ECDSA_DETERMINISTIC) psa_algorithm_t psa_sig_md = PSA_ALG_DETERMINISTIC_ECDSA( mbedtls_hash_info_psa_from_md( md_alg ) ); From d0b83e1fc7e08fe7633f52b1b59771b8e53806fe Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Tue, 31 Jan 2023 10:16:23 +0100 Subject: [PATCH 08/13] build_info: fix PK's requirements for RSA_C Signed-off-by: Valerio Setti --- include/mbedtls/build_info.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/build_info.h b/include/mbedtls/build_info.h index 6cf1176de..b10b1ad4e 100644 --- a/include/mbedtls/build_info.h +++ b/include/mbedtls/build_info.h @@ -83,9 +83,10 @@ /* The PK wrappers need pk_write functions to format RSA key objects * when they are dispatching to the PSA API. This happens under USE_PSA_CRYPTO, * and also even without USE_PSA_CRYPTO for mbedtls_pk_sign_ext(). */ -#if defined(MBEDTLS_PK_C) && defined(MBEDTLS_PSA_CRYPTO_C) && \ - defined(MBEDTLS_RSA_C) +#if defined(MBEDTLS_PSA_CRYPTO_C) && defined(MBEDTLS_RSA_C) +#define MBEDTLS_PK_C #define MBEDTLS_PK_WRITE_C +#define MBEDTLS_PK_PARSE_C #endif #if !defined(MBEDTLS_SSL_PROTO_TLS1_2) From b761b15f060dc6ec0c43b20c9d67ae2d58a03c3c Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Tue, 31 Jan 2023 14:56:04 +0100 Subject: [PATCH 09/13] fix code style Signed-off-by: Valerio Setti --- library/pk_wrap.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 6fba6e9b2..2a71bd852 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -705,9 +705,9 @@ static int ecdsa_verify_wrap(void *ctx_arg, mbedtls_md_type_t md_alg, return MBEDTLS_ERR_PK_BAD_INPUT_DATA; } - psa_set_key_type( &attributes, PSA_KEY_TYPE_ECC_PUBLIC_KEY( curve ) ); - psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_VERIFY_HASH ); - psa_set_key_algorithm( &attributes, psa_sig_md ); + psa_set_key_type(&attributes, PSA_KEY_TYPE_ECC_PUBLIC_KEY(curve)); + psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_VERIFY_HASH); + psa_set_key_algorithm(&attributes, psa_sig_md); ret = mbedtls_ecp_point_write_binary(&ctx->grp, &ctx->Q, MBEDTLS_ECP_PF_UNCOMPRESSED, @@ -720,7 +720,7 @@ static int ecdsa_verify_wrap(void *ctx_arg, mbedtls_md_type_t md_alg, buf, key_len, &key_id); if (status != PSA_SUCCESS) { - ret = mbedtls_pk_error_from_psa( status ); + ret = mbedtls_pk_error_from_psa(status); goto cleanup; } @@ -878,14 +878,14 @@ static int ecdsa_sign_wrap(void *ctx_arg, mbedtls_md_type_t md_alg, unsigned char buf[MBEDTLS_PSA_MAX_EC_KEY_PAIR_LENGTH]; #if defined(MBEDTLS_ECDSA_DETERMINISTIC) psa_algorithm_t psa_sig_md = - PSA_ALG_DETERMINISTIC_ECDSA( mbedtls_hash_info_psa_from_md( md_alg ) ); + PSA_ALG_DETERMINISTIC_ECDSA(mbedtls_hash_info_psa_from_md(md_alg)); #else psa_algorithm_t psa_sig_md = - PSA_ALG_ECDSA( mbedtls_hash_info_psa_from_md( md_alg ) ); + PSA_ALG_ECDSA(mbedtls_hash_info_psa_from_md(md_alg)); #endif size_t curve_bits; psa_ecc_family_t curve = - mbedtls_ecc_group_to_psa( ctx->grp.id, &curve_bits ); + mbedtls_ecc_group_to_psa(ctx->grp.id, &curve_bits); size_t key_len = PSA_BITS_TO_BYTES(curve_bits); /* PSA has its own RNG */ @@ -897,10 +897,10 @@ static int ecdsa_sign_wrap(void *ctx_arg, mbedtls_md_type_t md_alg, } if (key_len > sizeof(buf)) { - return( MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED ); + return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; } ret = mbedtls_mpi_write_binary(&ctx->d, buf, key_len); - if( ret != 0 ) { + if (ret != 0) { goto cleanup; } From 5c032b5e1bd3b072bc4d54be2f3db1f95eb5a78b Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 2 Feb 2023 15:10:32 +0100 Subject: [PATCH 10/13] pk_wrap: fix comment in ecdsa_verify_wrap Signed-off-by: Valerio Setti --- library/pk_wrap.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 2a71bd852..7f266d23d 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -689,9 +689,11 @@ static int ecdsa_verify_wrap(void *ctx_arg, mbedtls_md_type_t md_alg, mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; psa_status_t status; size_t key_len; - /* This buffer contains first the public key (consisting of two public - * points plus a header byte), then the signature (consisting of two - * public points). Size it for the public key which is one byte larger. */ + /* This buffer will initially contain the public key and then the signature + * but at different points in time. For all curves except secp224k1, which + * is not currently supported in PSA, the public key is one byte longer + * (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; From a1e3e3a28fe404c2e166f6095844d75718b1800f Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 2 Feb 2023 15:21:48 +0100 Subject: [PATCH 11/13] test: pk: keep PK_WRITE_C only in RSA tests Signed-off-by: Valerio Setti --- tests/suites/test_suite_pk.data | 2 +- tests/suites/test_suite_pk.function | 41 ++++++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_pk.data b/tests/suites/test_suite_pk.data index 531a2f1e0..01d8d2dc0 100644 --- a/tests/suites/test_suite_pk.data +++ b/tests/suites/test_suite_pk.data @@ -618,7 +618,7 @@ depends_on:MBEDTLS_PK_CAN_ECDSA_SIGN:MBEDTLS_ECP_DP_BP512R1_ENABLED pk_psa_sign:MBEDTLS_ECP_DP_BP512R1:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_BRAINPOOL_P_R1):512 PSA wrapped sign: RSA PKCS1 v1.5 -depends_on:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_GENPRIME +depends_on:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_GENPRIME:MBEDTLS_PK_WRITE_C pk_psa_sign:1024:PSA_KEY_TYPE_RSA_KEY_PAIR:1024 PK Sign ext:RSA2048,PK_RSA,MD_SHA256 diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 9ec354eed..f124c9a5f 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -1182,7 +1182,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_USE_PSA_CRYPTO:MBEDTLS_PK_WRITE_C */ +/* BEGIN_CASE depends_on:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_USE_PSA_CRYPTO */ void pk_psa_sign(int parameter_arg, int psa_type_arg, int expected_bits_arg) { @@ -1242,12 +1242,22 @@ void pk_psa_sign(int parameter_arg, } /* Export underlying public key for re-importing in a legacy context. */ +#if defined(MBEDTLS_PK_WRITE_C) ret = mbedtls_pk_write_pubkey_der(&pk, pkey_legacy, sizeof(pkey_legacy)); TEST_ASSERT(ret >= 0); klen_legacy = (size_t) ret; /* mbedtls_pk_write_pubkey_der() writes backwards in the data buffer. */ pkey_legacy_start = pkey_legacy + sizeof(pkey_legacy) - klen_legacy; +#else + ret = mbedtls_ecp_point_write_binary(&(mbedtls_pk_ec(pk)->grp), + &(mbedtls_pk_ec(pk)->Q), + MBEDTLS_ECP_PF_UNCOMPRESSED, + &klen_legacy, pkey_legacy, + sizeof(pkey_legacy)); + TEST_EQUAL(ret, 0); + pkey_legacy_start = pkey_legacy; +#endif /* MBEDTLS_PK_WRITE_C */ /* Turn PK context into an opaque one. */ TEST_ASSERT(mbedtls_pk_wrap_as_opaque(&pk, &key_id, alg_psa, @@ -1268,12 +1278,21 @@ void pk_psa_sign(int parameter_arg, NULL, NULL) == 0); /* Export underlying public key for re-importing in a psa context. */ +#if defined(MBEDTLS_PK_WRITE_C) ret = mbedtls_pk_write_pubkey_der(&pk, pkey_psa, sizeof(pkey_psa)); TEST_ASSERT(ret >= 0); klen_psa = (size_t) ret; /* mbedtls_pk_write_pubkey_der() writes backwards in the data buffer. */ pkey_psa_start = pkey_psa + sizeof(pkey_psa) - klen_psa; +#else + psa_status_t status; + + status = psa_export_public_key(key_id, pkey_psa, sizeof(pkey_psa), + &klen_psa); + TEST_EQUAL(status, PSA_SUCCESS); + pkey_psa_start = pkey_psa; +#endif /* MBEDTLS_PK_WRITE_C */ TEST_ASSERT(klen_psa == klen_legacy); TEST_ASSERT(memcmp(pkey_psa_start, pkey_legacy_start, klen_psa) == 0); @@ -1282,8 +1301,24 @@ void pk_psa_sign(int parameter_arg, TEST_ASSERT(PSA_SUCCESS == psa_destroy_key(key_id)); mbedtls_pk_init(&pk); - TEST_ASSERT(mbedtls_pk_parse_public_key(&pk, pkey_legacy_start, - klen_legacy) == 0); + + /* If we used "pk_write" previously, then we go for a "pk_parse" here; + * otherwise if we went for "ecp_point_write_binary" then we'll go + * for a "ecp_point_read_binary" here. This allows to drop dependencies + * on "PK_WRITE" and "PK_PARSE" if required */ +#if defined(MBEDTLS_PK_WRITE_C) && defined(MBEDTLS_PK_PARSE_C) + TEST_EQUAL(mbedtls_pk_parse_public_key(&pk, pkey_legacy_start, + klen_legacy), 0); +#else + TEST_EQUAL(mbedtls_pk_setup(&pk, + mbedtls_pk_info_from_type(MBEDTLS_PK_ECKEY)), 0); + TEST_EQUAL(mbedtls_ecp_group_load( + &(mbedtls_pk_ec(pk)->grp), + (mbedtls_ecp_group_id) parameter_arg), 0); + TEST_EQUAL(mbedtls_ecp_point_read_binary(&(mbedtls_pk_ec(pk)->grp) , + &(mbedtls_pk_ec(pk)->Q), + pkey_legacy_start, klen_legacy), 0); +#endif TEST_ASSERT(mbedtls_pk_verify(&pk, MBEDTLS_MD_SHA256, hash, sizeof(hash), sig, sig_len) == 0); From 683a432a7f99f88a93e52efd47a661ca3f92a0b9 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 8 Feb 2023 09:52:40 +0100 Subject: [PATCH 12/13] fix code style Signed-off-by: Valerio Setti --- tests/suites/test_suite_pk.function | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index f124c9a5f..c252cc664 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -1251,10 +1251,10 @@ void pk_psa_sign(int parameter_arg, pkey_legacy_start = pkey_legacy + sizeof(pkey_legacy) - klen_legacy; #else ret = mbedtls_ecp_point_write_binary(&(mbedtls_pk_ec(pk)->grp), - &(mbedtls_pk_ec(pk)->Q), - MBEDTLS_ECP_PF_UNCOMPRESSED, - &klen_legacy, pkey_legacy, - sizeof(pkey_legacy)); + &(mbedtls_pk_ec(pk)->Q), + MBEDTLS_ECP_PF_UNCOMPRESSED, + &klen_legacy, pkey_legacy, + sizeof(pkey_legacy)); TEST_EQUAL(ret, 0); pkey_legacy_start = pkey_legacy; #endif /* MBEDTLS_PK_WRITE_C */ @@ -1289,7 +1289,7 @@ void pk_psa_sign(int parameter_arg, psa_status_t status; status = psa_export_public_key(key_id, pkey_psa, sizeof(pkey_psa), - &klen_psa); + &klen_psa); TEST_EQUAL(status, PSA_SUCCESS); pkey_psa_start = pkey_psa; #endif /* MBEDTLS_PK_WRITE_C */ @@ -1308,16 +1308,16 @@ void pk_psa_sign(int parameter_arg, * on "PK_WRITE" and "PK_PARSE" if required */ #if defined(MBEDTLS_PK_WRITE_C) && defined(MBEDTLS_PK_PARSE_C) TEST_EQUAL(mbedtls_pk_parse_public_key(&pk, pkey_legacy_start, - klen_legacy), 0); + klen_legacy), 0); #else TEST_EQUAL(mbedtls_pk_setup(&pk, - mbedtls_pk_info_from_type(MBEDTLS_PK_ECKEY)), 0); + mbedtls_pk_info_from_type(MBEDTLS_PK_ECKEY)), 0); TEST_EQUAL(mbedtls_ecp_group_load( - &(mbedtls_pk_ec(pk)->grp), - (mbedtls_ecp_group_id) parameter_arg), 0); - TEST_EQUAL(mbedtls_ecp_point_read_binary(&(mbedtls_pk_ec(pk)->grp) , - &(mbedtls_pk_ec(pk)->Q), - pkey_legacy_start, klen_legacy), 0); + &(mbedtls_pk_ec(pk)->grp), + (mbedtls_ecp_group_id) parameter_arg), 0); + TEST_EQUAL(mbedtls_ecp_point_read_binary(&(mbedtls_pk_ec(pk)->grp), + &(mbedtls_pk_ec(pk)->Q), + pkey_legacy_start, klen_legacy), 0); #endif TEST_ASSERT(mbedtls_pk_verify(&pk, MBEDTLS_MD_SHA256, hash, sizeof(hash), sig, sig_len) == 0); From 80d0798ae89316bb78e8fa41934c455d5a781c9d Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 8 Feb 2023 13:49:17 +0100 Subject: [PATCH 13/13] pk_wrap: use new macros for ECDSA capabilities Signed-off-by: Valerio Setti --- library/pk_wrap.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 7f266d23d..378c36833 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -45,7 +45,7 @@ #include "mbedtls/psa_util.h" #include "hash_info.h" -#if defined(MBEDTLS_ECDSA_C) +#if defined(MBEDTLS_PK_CAN_ECDSA_SOME) #include "mbedtls/asn1write.h" #include "mbedtls/asn1.h" #endif @@ -950,8 +950,7 @@ static int ecdsa_sign_wrap(void *ctx, mbedtls_md_type_t md_alg, #endif /* MBEDTLS_USE_PSA_CRYPTO */ #endif /* MBEDTLS_PK_CAN_ECDSA_SIGN */ -#if defined(MBEDTLS_ECDSA_C) -#if defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) /* Forward declarations */ static int ecdsa_verify_rs_wrap(void *ctx, mbedtls_md_type_t md_alg, const unsigned char *hash, size_t hash_len, @@ -1057,8 +1056,7 @@ static int eckey_sign_rs_wrap(void *ctx, mbedtls_md_type_t md_alg, cleanup: return ret; } -#endif /* MBEDTLS_ECP_RESTARTABLE */ -#endif /* MBEDTLS_ECDSA_C */ +#endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ static int eckey_check_pair(const void *pub, const void *prv, int (*f_rng)(void *, unsigned char *, size_t),