From a98511164f334da4f8ec167eac576933e69ab69c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 25 Jan 2019 16:37:10 +0000 Subject: [PATCH 01/10] Adapt ecdsa_verify_wrap() to new EC public key format Previously, PSA used SubjectPublicKeyInfo structures to serialize EC public keys. This has recently been changed to using ECPoint structures instead, but the wrapper making PSA ECDSA verification available through Mbed TLS' PK API hasn't yet been adapted accordingly - which is what this commit does. Luckily, Mbed TLS' PK API offers two functions mbedtls_pk_write_pubkey() and mbedtls_pk_write_pubkey_der(), the latter exporting a SubjectPublicKeyInfo structure and the former exporting an ECPoint structure in case of EC public keys. For the adaptation of the ECDSA wrapper ecdsa_verify_wrap() it is therefore sufficient to use mbedtls_pk_write_pubkey() instead of mbedtls_pk_write_pubkey_der(). --- library/pk_wrap.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 7f8abd488..332696dfd 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -553,7 +553,7 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, int key_len; /* see ECP_PUB_DER_MAX_BYTES in pkwrite.c */ unsigned char buf[30 + 2 * MBEDTLS_ECP_MAX_BYTES]; - unsigned char *p = (unsigned char*) sig; + unsigned char *p; mbedtls_pk_info_t pk_info = mbedtls_eckey_info; psa_algorithm_t psa_sig_md, psa_md; psa_ecc_curve_t curve = mbedtls_psa_translate_ecc_group( @@ -567,7 +567,8 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, * re-construct one to make it happy */ key.pk_info = &pk_info; key.pk_ctx = ctx; - key_len = mbedtls_pk_write_pubkey_der( &key, buf, sizeof( buf ) ); + p = buf + sizeof( buf ); + key_len = mbedtls_pk_write_pubkey( &p, buf, &key ); if( key_len <= 0 ) return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); @@ -603,6 +604,7 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, goto cleanup; } + p = (unsigned char*) sig; if( ( ret = extract_ecdsa_sig( &p, sig + sig_len, buf, signature_part_size ) ) != 0 ) { From af01b4b387c9817ef1464705cc857a293fbc3c07 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 28 Jan 2019 11:45:50 +0000 Subject: [PATCH 02/10] Update crypto submodule to sibling commit --- crypto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto b/crypto index 2169a5e54..5d5e90a61 160000 --- a/crypto +++ b/crypto @@ -1 +1 @@ -Subproject commit 2169a5e54ded734f7844af1a16c39b993c2a992c +Subproject commit 5d5e90a610ffd78000e056043d2ea9184f060e52 From ccf574e030c214914c7b36ca676242d4a1c08923 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 29 Jan 2019 08:26:15 +0000 Subject: [PATCH 03/10] Fix outdated comment in ecdsa_verify_wrap() --- library/pk_wrap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 332696dfd..c7f879ab5 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -563,7 +563,7 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, if( curve == 0 ) return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); - /* mbedlts_pk_write_pubkey_der() expects a full PK context, + /* 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; From 62efb9c3ba31b36c57c135325124049087aceadb Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 29 Jan 2019 10:14:20 +0000 Subject: [PATCH 04/10] Update crypto submodule to merge commit of sibling PR --- crypto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto b/crypto index 5d5e90a61..472c20b4f 160000 --- a/crypto +++ b/crypto @@ -1 +1 @@ -Subproject commit 5d5e90a610ffd78000e056043d2ea9184f060e52 +Subproject commit 472c20b4f2c8bde9ef8aa8d6d64c254fd14a5f87 From 4fb8db23b942ae1cee42480b036cd4e4c1f11fd1 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 1 Feb 2019 09:57:20 +0000 Subject: [PATCH 05/10] Update pointer in PSA-based mbedtls_pk_write_pubkey() --- library/pkwrite.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/pkwrite.c b/library/pkwrite.c index 11a2a6145..943dbca01 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -181,7 +181,8 @@ int mbedtls_pk_write_pubkey( unsigned char **p, unsigned char *start, } else { - memmove( *p - len, start, len ); + *p -= len; + memmove( *p, start, len ); } } else From 812e12452a4ecb084df2eef5d8d70022ee3b7b5d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 1 Feb 2019 10:06:51 +0000 Subject: [PATCH 06/10] Add function to find OID for PSA ECC curve identifiers --- include/mbedtls/psa_util.h | 76 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/include/mbedtls/psa_util.h b/include/mbedtls/psa_util.h index a78c1a96c..634857c08 100644 --- a/include/mbedtls/psa_util.h +++ b/include/mbedtls/psa_util.h @@ -155,6 +155,82 @@ static inline psa_algorithm_t mbedtls_psa_translate_md( mbedtls_md_type_t md_alg /* Translations for ECC. */ +static inline int mbedtls_psa_get_ecc_oid_from_id( + psa_ecc_curve_t curve, char const **oid, size_t *oid_len ) +{ + switch( curve ) + { +#if defined(MBEDTLS_ECP_DP_SECP192R1_ENABLED) + case PSA_ECC_CURVE_SECP192R1: + *oid = MBEDTLS_OID_EC_GRP_SECP192R1; + *oid_len = MBEDTLS_OID_SIZE( MBEDTLS_OID_EC_GRP_SECP192R1 ); + return( 0 ); +#endif /* MBEDTLS_ECP_DP_SECP192R1_ENABLED */ +#if defined(MBEDTLS_ECP_DP_SECP224R1_ENABLED) + case PSA_ECC_CURVE_SECP224R1: + *oid = MBEDTLS_OID_EC_GRP_SECP224R1; + *oid_len = MBEDTLS_OID_SIZE( MBEDTLS_OID_EC_GRP_SECP224R1 ); + return( 0 ); +#endif /* MBEDTLS_ECP_DP_SECP224R1_ENABLED */ +#if defined(MBEDTLS_ECP_DP_SECP256R1_ENABLED) + case PSA_ECC_CURVE_SECP256R1: + *oid = MBEDTLS_OID_EC_GRP_SECP256R1; + *oid_len = MBEDTLS_OID_SIZE( MBEDTLS_OID_EC_GRP_SECP256R1 ); + return( 0 ); +#endif /* MBEDTLS_ECP_DP_SECP256R1_ENABLED */ +#if defined(MBEDTLS_ECP_DP_SECP384R1_ENABLED) + case PSA_ECC_CURVE_SECP384R1: + *oid = MBEDTLS_OID_EC_GRP_SECP384R1; + *oid_len = MBEDTLS_OID_SIZE( MBEDTLS_OID_EC_GRP_SECP384R1 ); + return( 0 ); +#endif /* MBEDTLS_ECP_DP_SECP384R1_ENABLED */ +#if defined(MBEDTLS_ECP_DP_SECP521R1_ENABLED) + case PSA_ECC_CURVE_SECP521R1: + *oid = MBEDTLS_OID_EC_GRP_SECP521R1; + *oid_len = MBEDTLS_OID_SIZE( MBEDTLS_OID_EC_GRP_SECP521R1 ); + return( 0 ); +#endif /* MBEDTLS_ECP_DP_SECP521R1_ENABLED */ +#if defined(MBEDTLS_ECP_DP_SECP192K1_ENABLED) + case PSA_ECC_CURVE_SECP192K1: + *oid = MBEDTLS_OID_EC_GRP_SECP192K1; + *oid_len = MBEDTLS_OID_SIZE( MBEDTLS_OID_EC_GRP_SECP192K1 ); + return( 0 ); +#endif /* MBEDTLS_ECP_DP_SECP192K1_ENABLED */ +#if defined(MBEDTLS_ECP_DP_SECP224K1_ENABLED) + case PSA_ECC_CURVE_SECP224K1: + *oid = MBEDTLS_OID_EC_GRP_SECP224K1; + *oid_len = MBEDTLS_OID_SIZE( MBEDTLS_OID_EC_GRP_SECP224K1 ); + return( 0 ); +#endif /* MBEDTLS_ECP_DP_SECP224K1_ENABLED */ +#if defined(MBEDTLS_ECP_DP_SECP256K1_ENABLED) + case PSA_ECC_CURVE_SECP256K1: + *oid = MBEDTLS_OID_EC_GRP_SECP256K1; + *oid_len = MBEDTLS_OID_SIZE( MBEDTLS_OID_EC_GRP_SECP256K1 ); + return( 0 ); +#endif /* MBEDTLS_ECP_DP_SECP256K1_ENABLED */ +#if defined(MBEDTLS_ECP_DP_BP256R1_ENABLED) + case PSA_ECC_CURVE_BRAINPOOL_P256R1: + *oid = MBEDTLS_OID_EC_GRP_BP256R1; + *oid_len = MBEDTLS_OID_SIZE( MBEDTLS_OID_EC_GRP_BP256R1 ); + return( 0 ); +#endif /* MBEDTLS_ECP_DP_BP256R1_ENABLED */ +#if defined(MBEDTLS_ECP_DP_BP384R1_ENABLED) + case PSA_ECC_CURVE_BRAINPOOL_P384R1: + *oid = MBEDTLS_OID_EC_GRP_BP384R1; + *oid_len = MBEDTLS_OID_SIZE( MBEDTLS_OID_EC_GRP_BP384R1 ); + return( 0 ); +#endif /* MBEDTLS_ECP_DP_BP384R1_ENABLED */ +#if defined(MBEDTLS_ECP_DP_BP512R1_ENABLED) + case PSA_ECC_CURVE_BRAINPOOL_P512R1: + *oid = MBEDTLS_OID_EC_GRP_BP512R1; + *oid_len = MBEDTLS_OID_SIZE( MBEDTLS_OID_EC_GRP_BP512R1 ); + return( 0 ); +#endif /* MBEDTLS_ECP_DP_BP512R1_ENABLED */ + } + + return( -1 ); +} + static inline psa_ecc_curve_t mbedtls_psa_translate_ecc_group( mbedtls_ecp_group_id grpid ) { switch( grpid ) From 493c171a1c7a86c01a2d0de1fd1f3b301a9b5a29 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 1 Feb 2019 10:07:07 +0000 Subject: [PATCH 07/10] Adapt mbedtls_pk_write_pubkey_der() to the new PSA public key format Context: There are two public key writing functions in Mbed TLS. First, mbedtls_pk_write_pubkey(), which exports a public key in the form of a SubjectPublicKey structure containing the raw keying material (for example, EC point coordinates for an EC public key, without reference to the underlying curve). Secondly, mbedtls_pk_write_pubkey_der(), which exports a public key in the form of a SubjectPublicKeyInfo structure, wrapping the SubjectPublicKey structure by additional information identifying the type of public key (and for ECC, e.g., it'd also contain the ECC group identifier). The implementation of mbedtls_pk_write_pubkey_der() calls mbedtls_pk_write_pubkey() first and then adds the corresponding algorithm identifier wrapper. Both of these functions need to be provided for PSA-based opaque PK contexts, based on PSA's public key export function. Previously, PSA used the SubjectPublicKeyInfo structure as its export format, so mbedtls_pk_write_pubkey_der() could be easily implemented, while mbedtls_pk_write_pubkey() would need to trim the output of the PSA export. The previous implementation of mbedtls_pk_write_pubkey() is not quite right because it calls PSA export doesn't do any trimming, hence exporting the large SubjectPublicKeyInfo structure instead of the small SubjectPublicKey. mbedtls_pk_write_pubkey_der(), in turn, immediately returns after calling mbedtls_pk_write_pubkey(), hence also returning the SubjectPublicKeyInfo structure, which is correct. By now, the PSA public key export format has changed to the smaller SubjectPublicKey structure. This means that, now, mbedtls_pk_write_pubkey() can be implemented by just calling the PSA export, and that mbedtls_pk_write_pubkey_der() needs to add the algorithm information around it, just as in the other types of PK contexts. While not correct for the old format, the existing code for mbedtls_pk_write_pubkey() is therefore correct for the new PSA public key format, and needs no change apart from the missing pointer shift in the last commit. The implementation of mbedtls_pk_write_pubkey_der() needs a special code path for PSA-based opaque PK contexts, as the PK context only contains the PSA key handle, and the PSA API needs to be used to extract the underlying EC curve to be able to write the AlgorithmParameter structure that's part of the SubjectPublicKeyInfo structure. That's what this commit does, (hopefully) making both mbedtls_pk_write_pubkey() and mbedtls_pk_write_pubkey_der() export the correctly formatted public key based on the new PSA public key format. --- include/mbedtls/psa_util.h | 1 + library/pkwrite.c | 52 ++++++++++++++++++++++++++++++-------- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/include/mbedtls/psa_util.h b/include/mbedtls/psa_util.h index 634857c08..3684e9835 100644 --- a/include/mbedtls/psa_util.h +++ b/include/mbedtls/psa_util.h @@ -41,6 +41,7 @@ #include "ecp.h" #include "md.h" #include "pk.h" +#include "oid.h" /* Translations for symmetric crypto. */ diff --git a/library/pkwrite.c b/library/pkwrite.c index 943dbca01..c9b2674c2 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -197,16 +197,13 @@ int mbedtls_pk_write_pubkey_der( mbedtls_pk_context *key, unsigned char *buf, si int ret; unsigned char *c; size_t len = 0, par_len = 0, oid_len; + mbedtls_pk_type_t pk_type; const char *oid; c = buf + size; MBEDTLS_ASN1_CHK_ADD( len, mbedtls_pk_write_pubkey( &c, buf, key ) ); - if( mbedtls_pk_get_type( key ) == MBEDTLS_PK_OPAQUE ) - { - return( (int) len ); - } if( c - buf < 1 ) return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); @@ -221,18 +218,51 @@ int mbedtls_pk_write_pubkey_der( mbedtls_pk_context *key, unsigned char *buf, si MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, buf, len ) ); MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c, buf, MBEDTLS_ASN1_BIT_STRING ) ); - if( ( ret = mbedtls_oid_get_oid_by_pk_alg( mbedtls_pk_get_type( key ), - &oid, &oid_len ) ) != 0 ) - { - return( ret ); - } - + pk_type = mbedtls_pk_get_type( key ); #if defined(MBEDTLS_ECP_C) - if( mbedtls_pk_get_type( key ) == MBEDTLS_PK_ECKEY ) + if( pk_type == MBEDTLS_PK_ECKEY ) { MBEDTLS_ASN1_CHK_ADD( par_len, pk_write_ec_param( &c, buf, mbedtls_pk_ec( *key ) ) ); } #endif +#if defined(MBEDTLS_USE_PSA_CRYPTO) + if( pk_type == MBEDTLS_PK_OPAQUE ) + { + psa_status_t status; + psa_key_type_t key_type; + psa_key_handle_t handle; + psa_ecc_curve_t curve; + + handle = *((psa_key_handle_t*) key->pk_ctx ); + + status = psa_get_key_information( handle, &key_type, + NULL /* bitsize not needed */ ); + if( status != PSA_SUCCESS ) + return( MBEDTLS_ERR_PK_HW_ACCEL_FAILED ); + + curve = PSA_KEY_TYPE_GET_CURVE( key_type ); + if( curve == 0 ) + return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE ); + + ret = mbedtls_psa_get_ecc_oid_from_id( curve, &oid, &oid_len ); + if( ret != 0 ) + return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE ); + + /* Write EC algorithm parameters; that's akin + * to pk_write_ec_param() above. */ + MBEDTLS_ASN1_CHK_ADD( par_len, mbedtls_asn1_write_oid( &c, buf, + oid, oid_len ) ); + + /* The rest of the function works as for legacy EC contexts. */ + pk_type = MBEDTLS_PK_ECKEY; + } +#endif /* MBEDTLS_USE_PSA_CRYPTO */ + + if( ( ret = mbedtls_oid_get_oid_by_pk_alg( pk_type, &oid, + &oid_len ) ) != 0 ) + { + return( ret ); + } MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_algorithm_identifier( &c, buf, oid, oid_len, par_len ) ); From 56104ea9bb216c4438dc48d99834c51a42b1c684 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 1 Feb 2019 11:48:19 +0000 Subject: [PATCH 08/10] Adapt test in test_suite_pk to work with new PSA public key format --- tests/suites/test_suite_pk.function | 57 ++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 120c1716b..e469318bd 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -778,44 +778,65 @@ exit: void pk_psa_sign( ) { mbedtls_pk_context pk; - psa_key_handle_t key; - unsigned char hash[50], sig[100], pkey[100]; - size_t sig_len, klen = 0; + unsigned char hash[50], sig[100], pkey_legacy[100], pkey_psa[100]; + unsigned char *pkey_legacy_start, *pkey_psa_start; + size_t sig_len, klen_legacy, klen_psa; + int ret; + psa_key_handle_t handle; /* * This tests making signatures with a wrapped PSA key: - * - generate a fresh PSA key + * - generate a fresh ECP legacy PK context * - wrap it in a PK context and make a signature this way * - extract the public key * - parse it to a PK context and verify the signature this way */ + /* Create legacy EC public/private key in PK context. */ mbedtls_pk_init( &pk ); + TEST_ASSERT( mbedtls_pk_setup( &pk, + mbedtls_pk_info_from_type( MBEDTLS_PK_ECKEY ) ) == 0 ); + TEST_ASSERT( mbedtls_ecp_gen_key( MBEDTLS_ECP_DP_SECP256R1, + (mbedtls_ecp_keypair*) pk.pk_ctx, + rnd_std_rand, NULL ) == 0 ); + + /* Export underlying public key for re-importing in a legacy context. */ + 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; + + /* Turn PK context into an opaque one. */ + TEST_ASSERT( psa_allocate_key( &handle ) == PSA_SUCCESS ); + TEST_ASSERT( mbedtls_pk_wrap_as_opaque( &pk, &handle, + PSA_ALG_SHA_256 ) == 0 ); memset( hash, 0x2a, sizeof hash ); memset( sig, 0, sizeof sig ); - memset( pkey, 0, sizeof pkey ); - - key = pk_psa_genkey(); - TEST_ASSERT( key != 0 ); - - TEST_ASSERT( mbedtls_pk_setup_opaque( &pk, key ) == 0 ); TEST_ASSERT( mbedtls_pk_sign( &pk, MBEDTLS_MD_SHA256, hash, sizeof hash, sig, &sig_len, NULL, NULL ) == 0 ); - mbedtls_pk_free( &pk ); + /* Export underlying public key for re-importing in a psa context. */ + 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; - TEST_ASSERT( PSA_SUCCESS == psa_export_public_key( - key, pkey, sizeof( pkey ), &klen ) ); - TEST_ASSERT( PSA_SUCCESS == psa_destroy_key( key ) ); + TEST_ASSERT( klen_psa == klen_legacy ); + TEST_ASSERT( memcmp( pkey_psa_start, pkey_legacy_start, klen_psa ) == 0 ); + + mbedtls_pk_free( &pk ); + TEST_ASSERT( PSA_SUCCESS == psa_destroy_key( handle ) ); mbedtls_pk_init( &pk ); - - TEST_ASSERT( mbedtls_pk_parse_public_key( &pk, pkey, klen ) == 0 ); - - + TEST_ASSERT( mbedtls_pk_parse_public_key( &pk, pkey_legacy_start, + klen_legacy ) == 0 ); TEST_ASSERT( mbedtls_pk_verify( &pk, MBEDTLS_MD_SHA256, hash, sizeof hash, sig, sig_len ) == 0 ); From 65935d90c9bbf54e698d26512bdb9574a030d309 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 1 Feb 2019 11:55:03 +0000 Subject: [PATCH 09/10] Add missing include to library/pkwrite.c --- library/pkwrite.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/pkwrite.c b/library/pkwrite.c index c9b2674c2..ab4f7f537 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -48,6 +48,7 @@ #if defined(MBEDTLS_USE_PSA_CRYPTO) #include "psa/crypto.h" +#include "mbedtls/psa_util.h" #endif #if defined(MBEDTLS_PLATFORM_C) #include "mbedtls/platform.h" From 70fc6550cce5b490b7bbc1c56c4b766e21cf5ed7 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 1 Feb 2019 13:08:38 +0000 Subject: [PATCH 10/10] Update Crypto submodule to merge of sibling PR --- crypto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto b/crypto index 472c20b4f..acdf07c03 160000 --- a/crypto +++ b/crypto @@ -1 +1 @@ -Subproject commit 472c20b4f2c8bde9ef8aa8d6d64c254fd14a5f87 +Subproject commit acdf07c033bb7ce11f595ddbe0c66d438b46597c