From 39d1f4b29f63a97f4e772c860d6c17e43586d84c Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 31 Oct 2018 05:16:46 -0400 Subject: [PATCH 01/16] pk_wrap.c: add support for ecdsa signature verification using PSA Use PSA internally to verify signatures. Add a conversion to a raw signature format. --- library/pk_wrap.c | 260 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 260 insertions(+) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 87806be33..f48b85039 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -45,6 +45,12 @@ #include "mbedtls/platform_util.h" #endif +#if defined(MBEDTLS_USE_PSA_CRYPTO) +#include "psa/crypto.h" +#include "mbedtls/x509.h" +#include "mbedtls/asn1.h" +#endif + #if defined(MBEDTLS_PLATFORM_C) #include "mbedtls/platform.h" #else @@ -472,6 +478,259 @@ static int ecdsa_can_do( mbedtls_pk_type_t type ) return( type == MBEDTLS_PK_ECDSA ); } +#if defined(MBEDTLS_USE_PSA_CRYPTO) +static psa_status_t mbedtls_psa_get_free_key_slot( psa_key_slot_t *key ) +{ + for( psa_key_slot_t slot = 1; slot <= 32; slot++ ) + { + if( psa_get_key_information( slot, NULL, NULL ) == PSA_ERROR_EMPTY_SLOT ) + { + *key = slot; + return( PSA_SUCCESS ); + } + } + return( PSA_ERROR_INSUFFICIENT_MEMORY ); +} + +static psa_algorithm_t translate_md_to_psa( mbedtls_md_type_t md_alg ) +{ + switch( md_alg ) + { +#if defined(MBEDTLS_MD2_C) + case MBEDTLS_MD_MD2: + return( PSA_ALG_MD2 ); +#endif +#if defined(MBEDTLS_MD4_C) + case MBEDTLS_MD_MD4: + return( PSA_ALG_MD4 ); +#endif +#if defined(MBEDTLS_MD5_C) + case MBEDTLS_MD_MD5: + return( PSA_ALG_MD5 ); +#endif +#if defined(MBEDTLS_SHA1_C) + case MBEDTLS_MD_SHA1: + return( PSA_ALG_SHA_1 ); +#endif +#if defined(MBEDTLS_SHA256_C) + case MBEDTLS_MD_SHA224: + return( PSA_ALG_SHA_224 ); + case MBEDTLS_MD_SHA256: + return( PSA_ALG_SHA_256 ); +#endif +#if defined(MBEDTLS_SHA512_C) + case MBEDTLS_MD_SHA384: + return( PSA_ALG_SHA_384 ); + case MBEDTLS_MD_SHA512: + return( PSA_ALG_SHA_512 ); +#endif +#if defined(MBEDTLS_RIPEMD160_C) + case MBEDTLS_MD_RIPEMD160: + return( PSA_ALG_RIPEMD160 ); +#endif + case MBEDTLS_MD_NONE: // Intentional fallthrough + default: + return( 0 ); + } +} + +/* + * Convert a signature from an ASN.1 sequence of two integers + * to a raw {r,s} buffer. Note: upon a successful call, the caller + * takes ownership of the sig->p buffer. + */ +static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, + mbedtls_asn1_buf *sig ) +{ + int ret; + size_t len_signature; + size_t len_partial; + int tag_type; + if( ( end - *p ) < 1 ) + return( MBEDTLS_ERR_X509_INVALID_SIGNATURE + + MBEDTLS_ERR_ASN1_OUT_OF_DATA ); + tag_type = **p; + + if( ( ret = mbedtls_asn1_get_tag(p, end, &len_partial, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 ) { + return( MBEDTLS_ERR_X509_INVALID_SIGNATURE + ret ); + } + + if( ( ret = mbedtls_asn1_get_tag( p, end, &len_partial, MBEDTLS_ASN1_INTEGER ) ) + != 0 ) + return( ret ); + + if( **p == '\0' ) { + ( *p )++; + len_partial--; + } + + sig->p = mbedtls_calloc( 2, len_partial ); + if( sig->p == NULL ) { + return( MBEDTLS_ERR_ASN1_ALLOC_FAILED ); + } + + memcpy( sig->p, *p, len_partial ); + len_signature = len_partial; + ( *p ) += len_partial; + if( ( ret = mbedtls_asn1_get_tag( p, end, &len_partial, MBEDTLS_ASN1_INTEGER ) ) + != 0 ) + { + mbedtls_free( sig->p ); + return( ret ); + } + + if( **p == '\0' ) { + ( *p )++; + len_partial--; + } + + memcpy( sig->p + len_partial, *p, len_partial ); + len_signature += len_partial; + sig->tag = tag_type; + sig->len = len_signature; + ( *p ) += len_partial; + return( 0 ); +} + +static psa_ecc_curve_t mbedtls_ecc_group_to_psa( mbedtls_ecp_group_id grpid ) +{ + switch( grpid ) + { +#if defined(MBEDTLS_ECP_DP_SECP192R1_ENABLED) + case MBEDTLS_ECP_DP_SECP192R1: + return( PSA_ECC_CURVE_SECP192R1 ); +#endif +#if defined(MBEDTLS_ECP_DP_SECP224R1_ENABLED) + case MBEDTLS_ECP_DP_SECP224R1: + return( PSA_ECC_CURVE_SECP224R1 ); +#endif +#if defined(MBEDTLS_ECP_DP_SECP256R1_ENABLED) + case MBEDTLS_ECP_DP_SECP256R1: + return( PSA_ECC_CURVE_SECP256R1 ); +#endif +#if defined(MBEDTLS_ECP_DP_SECP384R1_ENABLED) + case MBEDTLS_ECP_DP_SECP384R1: + return( PSA_ECC_CURVE_SECP384R1 ); +#endif +#if defined(MBEDTLS_ECP_DP_SECP521R1_ENABLED) + case MBEDTLS_ECP_DP_SECP521R1: + return( PSA_ECC_CURVE_SECP521R1 ); +#endif +#if defined(MBEDTLS_ECP_DP_BP256R1_ENABLED) + case MBEDTLS_ECP_DP_BP256R1: + return( PSA_ECC_CURVE_BRAINPOOL_P256R1 ); +#endif +#if defined(MBEDTLS_ECP_DP_BP384R1_ENABLED) + case MBEDTLS_ECP_DP_BP384R1: + return( PSA_ECC_CURVE_BRAINPOOL_P384R1 ); +#endif +#if defined(MBEDTLS_ECP_DP_BP512R1_ENABLED) + case MBEDTLS_ECP_DP_BP512R1: + return( PSA_ECC_CURVE_BRAINPOOL_P512R1 ); +#endif +#if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) + case MBEDTLS_ECP_DP_CURVE25519: + return( PSA_ECC_CURVE_CURVE25519 ); +#endif +#if defined(MBEDTLS_ECP_DP_SECP192K1_ENABLED) + case MBEDTLS_ECP_DP_SECP192K1: + return( PSA_ECC_CURVE_SECP192K1 ); +#endif +#if defined(MBEDTLS_ECP_DP_SECP224K1_ENABLED) + case MBEDTLS_ECP_DP_SECP224K1: + return( PSA_ECC_CURVE_SECP224K1 ); +#endif +#if defined(MBEDTLS_ECP_DP_SECP256K1_ENABLED) + case MBEDTLS_ECP_DP_SECP256K1: + return( PSA_ECC_CURVE_SECP256K1 ); +#endif +#if defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) + case MBEDTLS_ECP_DP_CURVE448: + return( PSA_ECC_CURVE_CURVE448 ); +#endif + default: + return( 0 ); + } +} + +static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, + const unsigned char *hash, size_t hash_len, + const unsigned char *sig, size_t sig_len ) +{ + int ret; + psa_key_slot_t key_slot; + psa_key_policy_t policy; + psa_key_type_t psa_type; + mbedtls_pk_context key; + mbedtls_asn1_buf signature; + int key_len; + const int buff_len = 30 + 2 * MBEDTLS_ECP_MAX_BYTES; // Equivalent of ECP_PUB_DER_MAX_BYTES + unsigned char buf[buff_len]; + unsigned char *p = ( unsigned char* ) sig; + mbedtls_pk_info_t pk_info = mbedtls_eckey_info; + psa_algorithm_t psa_sig_md = PSA_ALG_ECDSA( translate_md_to_psa( md_alg ) ); + psa_ecc_curve_t curve = mbedtls_ecc_group_to_psa ( ( (mbedtls_ecdsa_context *) ctx )->grp.id ); + ((void) md_alg); + + memset( &signature, 0, sizeof( mbedtls_asn1_buf ) ); + mbedtls_platform_zeroize( buf, buff_len ); + key.pk_info = &pk_info; + key.pk_ctx = ctx; + psa_crypto_init(); + + psa_type = PSA_KEY_TYPE_ECC_PUBLIC_KEY( curve ); + + if( extract_ecdsa_sig( &p, p + sig_len, &signature ) != 0 ) + { + ret = MBEDTLS_ERR_PK_BAD_INPUT_DATA; + goto cleanup; + } + + key_len = mbedtls_pk_write_pubkey_der( &key, buf, buff_len ); + if( key_len <= 0 ) + { + ret = MBEDTLS_ERR_PK_BAD_INPUT_DATA; + goto cleanup; + } + + if( mbedtls_psa_get_free_key_slot( &key_slot ) != PSA_SUCCESS ) + { + ret = MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; + goto cleanup; + } + psa_key_policy_init( &policy ); + psa_key_policy_set_usage( &policy, PSA_KEY_USAGE_VERIFY, psa_sig_md ); + if( psa_set_key_policy( key_slot, &policy ) != PSA_SUCCESS ) + { + ret = MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; + goto cleanup; + } + + if( psa_import_key( key_slot, psa_type, buf+buff_len-key_len, key_len ) + != PSA_SUCCESS ) + { + ret = MBEDTLS_ERR_PK_BAD_INPUT_DATA; + goto cleanup; + } + + if( psa_asymmetric_verify( key_slot, psa_sig_md, + hash, hash_len, + signature.p, signature.len ) + != PSA_SUCCESS ) + { + psa_destroy_key( key_slot ); + ret = MBEDTLS_ERR_ECP_VERIFY_FAILED; + goto cleanup; + } + ret = 0; + psa_destroy_key( key_slot ); + + cleanup: + mbedtls_free( signature.p ); + return( ret ); +} +#else /* MBEDTLS_USE_PSA_CRYPTO */ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, const unsigned char *hash, size_t hash_len, const unsigned char *sig, size_t sig_len ) @@ -487,6 +746,7 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, return( ret ); } +#endif /* MBEDTLS_USE_PSA_CRYPTO */ static int ecdsa_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, const unsigned char *hash, size_t hash_len, From 1e3b6865d7197e023735d364f6316963cb1b2686 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 6 Nov 2018 08:50:04 -0500 Subject: [PATCH 02/16] pk_wrap: cosmetic changes Adjust whitespaces and variable names --- library/pk_wrap.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index f48b85039..4a74621fc 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -547,12 +547,15 @@ static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, size_t len_partial; int tag_type; if( ( end - *p ) < 1 ) + { return( MBEDTLS_ERR_X509_INVALID_SIGNATURE + - MBEDTLS_ERR_ASN1_OUT_OF_DATA ); + MBEDTLS_ERR_ASN1_OUT_OF_DATA ); + } tag_type = **p; - if( ( ret = mbedtls_asn1_get_tag(p, end, &len_partial, - MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 ) { + if( ( ret = mbedtls_asn1_get_tag( p, end, &len_partial, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 ) + { return( MBEDTLS_ERR_X509_INVALID_SIGNATURE + ret ); } @@ -560,15 +563,15 @@ static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, != 0 ) return( ret ); - if( **p == '\0' ) { + if( **p == '\0' ) + { ( *p )++; len_partial--; } sig->p = mbedtls_calloc( 2, len_partial ); - if( sig->p == NULL ) { + if( sig->p == NULL ) return( MBEDTLS_ERR_ASN1_ALLOC_FAILED ); - } memcpy( sig->p, *p, len_partial ); len_signature = len_partial; @@ -580,7 +583,8 @@ static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, return( ret ); } - if( **p == '\0' ) { + if( **p == '\0' ) + { ( *p )++; len_partial--; } @@ -665,16 +669,14 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, mbedtls_pk_context key; mbedtls_asn1_buf signature; int key_len; - const int buff_len = 30 + 2 * MBEDTLS_ECP_MAX_BYTES; // Equivalent of ECP_PUB_DER_MAX_BYTES - unsigned char buf[buff_len]; - unsigned char *p = ( unsigned char* ) sig; + const int buf_len = 30 + 2 * MBEDTLS_ECP_MAX_BYTES; // Equivalent of ECP_PUB_DER_MAX_BYTES + unsigned char buf[buf_len]; + unsigned char *p = (unsigned char*) sig; mbedtls_pk_info_t pk_info = mbedtls_eckey_info; psa_algorithm_t psa_sig_md = PSA_ALG_ECDSA( translate_md_to_psa( md_alg ) ); psa_ecc_curve_t curve = mbedtls_ecc_group_to_psa ( ( (mbedtls_ecdsa_context *) ctx )->grp.id ); - ((void) md_alg); memset( &signature, 0, sizeof( mbedtls_asn1_buf ) ); - mbedtls_platform_zeroize( buf, buff_len ); key.pk_info = &pk_info; key.pk_ctx = ctx; psa_crypto_init(); @@ -687,7 +689,7 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, goto cleanup; } - key_len = mbedtls_pk_write_pubkey_der( &key, buf, buff_len ); + key_len = mbedtls_pk_write_pubkey_der( &key, buf, buf_len ); if( key_len <= 0 ) { ret = MBEDTLS_ERR_PK_BAD_INPUT_DATA; @@ -707,7 +709,7 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, goto cleanup; } - if( psa_import_key( key_slot, psa_type, buf+buff_len-key_len, key_len ) + if( psa_import_key( key_slot, psa_type, buf+buf_len-key_len, key_len ) != PSA_SUCCESS ) { ret = MBEDTLS_ERR_PK_BAD_INPUT_DATA; From 6d49ae92233a6bd1442ecc5aff4ccec4c82719cc Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 7 Nov 2018 03:19:08 -0500 Subject: [PATCH 03/16] pk_wrap: nullify the signature pointer on error in extract_ecdsa_sig Fix a double free error in ecdsa_verify_wrap --- library/pk_wrap.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 4a74621fc..3e150a20d 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -576,10 +576,11 @@ static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, memcpy( sig->p, *p, len_partial ); len_signature = len_partial; ( *p ) += len_partial; - if( ( ret = mbedtls_asn1_get_tag( p, end, &len_partial, MBEDTLS_ASN1_INTEGER ) ) - != 0 ) + if( ( ret = mbedtls_asn1_get_tag( p, end, &len_partial, + MBEDTLS_ASN1_INTEGER ) ) != 0 ) { mbedtls_free( sig->p ); + sig->p = NULL; return( ret ); } @@ -684,10 +685,7 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, psa_type = PSA_KEY_TYPE_ECC_PUBLIC_KEY( curve ); if( extract_ecdsa_sig( &p, p + sig_len, &signature ) != 0 ) - { - ret = MBEDTLS_ERR_PK_BAD_INPUT_DATA; - goto cleanup; - } + return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); key_len = mbedtls_pk_write_pubkey_der( &key, buf, buf_len ); if( key_len <= 0 ) From f8c94a811a45d4e0291dbeba8aa622f1b115c064 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 7 Nov 2018 08:18:52 -0500 Subject: [PATCH 04/16] pk_wrap: check if curve conversion is successful --- library/pk_wrap.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 3e150a20d..6007a23c8 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -677,6 +677,9 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, psa_algorithm_t psa_sig_md = PSA_ALG_ECDSA( translate_md_to_psa( md_alg ) ); psa_ecc_curve_t curve = mbedtls_ecc_group_to_psa ( ( (mbedtls_ecdsa_context *) ctx )->grp.id ); + if( curve == 0 ) + return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + memset( &signature, 0, sizeof( mbedtls_asn1_buf ) ); key.pk_info = &pk_info; key.pk_ctx = ctx; From c097b0fdedd82e5a9369fa418c3c785bd81250b9 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 7 Nov 2018 09:30:50 -0500 Subject: [PATCH 05/16] pk_wrap: add a check for equal signature parts --- library/pk_wrap.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 6007a23c8..2e22ec9d3 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -542,10 +542,9 @@ static psa_algorithm_t translate_md_to_psa( mbedtls_md_type_t md_alg ) static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, mbedtls_asn1_buf *sig ) { - int ret; - size_t len_signature; - size_t len_partial; - int tag_type; + int ret, tag_type; + size_t len_signature, len_partial; + if( ( end - *p ) < 1 ) { return( MBEDTLS_ERR_X509_INVALID_SIGNATURE + @@ -590,6 +589,10 @@ static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, len_partial--; } + // Check if both parts are of the same size + if( len_partial != len_signature ) + return( MBEDTLS_ERR_X509_INVALID_SIGNATURE ); + memcpy( sig->p + len_partial, *p, len_partial ); len_signature += len_partial; sig->tag = tag_type; From 2f69b1a059a32deef1aa2886d290262a91656aa7 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Thu, 8 Nov 2018 04:33:06 -0500 Subject: [PATCH 06/16] pk_wrap: destroy key slot on errors with policy or key importing --- library/pk_wrap.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 2e22ec9d3..469dc253d 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -591,7 +591,7 @@ static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, // Check if both parts are of the same size if( len_partial != len_signature ) - return( MBEDTLS_ERR_X509_INVALID_SIGNATURE ); + return( MBEDTLS_ERR_X509_INVALID_SIGNATURE ); memcpy( sig->p + len_partial, *p, len_partial ); len_signature += len_partial; @@ -696,15 +696,16 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, key_len = mbedtls_pk_write_pubkey_der( &key, buf, buf_len ); if( key_len <= 0 ) { - ret = MBEDTLS_ERR_PK_BAD_INPUT_DATA; - goto cleanup; + mbedtls_free( signature.p ); + return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); } if( mbedtls_psa_get_free_key_slot( &key_slot ) != PSA_SUCCESS ) { - ret = MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; - goto cleanup; + mbedtls_free( signature.p ); + return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE ); } + psa_key_policy_init( &policy ); psa_key_policy_set_usage( &policy, PSA_KEY_USAGE_VERIFY, psa_sig_md ); if( psa_set_key_policy( key_slot, &policy ) != PSA_SUCCESS ) @@ -725,14 +726,13 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, signature.p, signature.len ) != PSA_SUCCESS ) { - psa_destroy_key( key_slot ); ret = MBEDTLS_ERR_ECP_VERIFY_FAILED; goto cleanup; } ret = 0; - psa_destroy_key( key_slot ); cleanup: + psa_destroy_key( key_slot ); mbedtls_free( signature.p ); return( ret ); } From 510ee70501d54c95f5a59aef4bed35a0aad6ae51 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Thu, 8 Nov 2018 05:04:53 -0500 Subject: [PATCH 07/16] pk_wrap: test if a valid md_alg is passed to ecdsa_verify_wrap Adjust tests to pass a valid algorithm --- library/pk_wrap.c | 6 +++++- tests/suites/test_suite_pk.function | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 469dc253d..4fc1a8552 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -677,12 +677,16 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, unsigned char buf[buf_len]; unsigned char *p = (unsigned char*) sig; mbedtls_pk_info_t pk_info = mbedtls_eckey_info; - psa_algorithm_t psa_sig_md = PSA_ALG_ECDSA( translate_md_to_psa( md_alg ) ); + psa_algorithm_t psa_sig_md = translate_md_to_psa( md_alg ); psa_ecc_curve_t curve = mbedtls_ecc_group_to_psa ( ( (mbedtls_ecdsa_context *) ctx )->grp.id ); if( curve == 0 ) return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + if( psa_sig_md == 0 ) + return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + + psa_sig_md = PSA_ALG_ECDSA( psa_sig_md ); memset( &signature, 0, sizeof( mbedtls_asn1_buf ) ); key.pk_info = &pk_info; key.pk_ctx = ctx; diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 4813f71f7..c7c707558 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -246,7 +246,8 @@ void pk_ec_test_vec( int type, int id, data_t * key, data_t * hash, TEST_ASSERT( mbedtls_ecp_point_read_binary( &eckey->grp, &eckey->Q, key->x, key->len ) == 0 ); - TEST_ASSERT( mbedtls_pk_verify( &pk, MBEDTLS_MD_NONE, + // MBEDTLS_MD_SHA1 is a dummy - it is ignored, but has to be other than MBEDTLS_MD_NONE. + TEST_ASSERT( mbedtls_pk_verify( &pk, MBEDTLS_MD_SHA1, hash->x, hash->len, sig->x, sig->len ) == ret ); exit: From ca6330992e2f1f2c51239008612734de6fedd7fa Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 19 Nov 2018 14:33:01 -0500 Subject: [PATCH 08/16] pk_wrap: switch to helper functions defined in psa_util.h Remove duplicated helper functions. Remove an unnecessary call to psa_crypto_init(). --- library/pk_wrap.c | 60 ++--------------------------------------------- 1 file changed, 2 insertions(+), 58 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 4fc1a8552..56ce69c54 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -47,7 +47,7 @@ #if defined(MBEDTLS_USE_PSA_CRYPTO) #include "psa/crypto.h" -#include "mbedtls/x509.h" +#include "mbedtls/psa_util.h" #include "mbedtls/asn1.h" #endif @@ -479,61 +479,6 @@ static int ecdsa_can_do( mbedtls_pk_type_t type ) } #if defined(MBEDTLS_USE_PSA_CRYPTO) -static psa_status_t mbedtls_psa_get_free_key_slot( psa_key_slot_t *key ) -{ - for( psa_key_slot_t slot = 1; slot <= 32; slot++ ) - { - if( psa_get_key_information( slot, NULL, NULL ) == PSA_ERROR_EMPTY_SLOT ) - { - *key = slot; - return( PSA_SUCCESS ); - } - } - return( PSA_ERROR_INSUFFICIENT_MEMORY ); -} - -static psa_algorithm_t translate_md_to_psa( mbedtls_md_type_t md_alg ) -{ - switch( md_alg ) - { -#if defined(MBEDTLS_MD2_C) - case MBEDTLS_MD_MD2: - return( PSA_ALG_MD2 ); -#endif -#if defined(MBEDTLS_MD4_C) - case MBEDTLS_MD_MD4: - return( PSA_ALG_MD4 ); -#endif -#if defined(MBEDTLS_MD5_C) - case MBEDTLS_MD_MD5: - return( PSA_ALG_MD5 ); -#endif -#if defined(MBEDTLS_SHA1_C) - case MBEDTLS_MD_SHA1: - return( PSA_ALG_SHA_1 ); -#endif -#if defined(MBEDTLS_SHA256_C) - case MBEDTLS_MD_SHA224: - return( PSA_ALG_SHA_224 ); - case MBEDTLS_MD_SHA256: - return( PSA_ALG_SHA_256 ); -#endif -#if defined(MBEDTLS_SHA512_C) - case MBEDTLS_MD_SHA384: - return( PSA_ALG_SHA_384 ); - case MBEDTLS_MD_SHA512: - return( PSA_ALG_SHA_512 ); -#endif -#if defined(MBEDTLS_RIPEMD160_C) - case MBEDTLS_MD_RIPEMD160: - return( PSA_ALG_RIPEMD160 ); -#endif - case MBEDTLS_MD_NONE: // Intentional fallthrough - default: - return( 0 ); - } -} - /* * Convert a signature from an ASN.1 sequence of two integers * to a raw {r,s} buffer. Note: upon a successful call, the caller @@ -677,7 +622,7 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, unsigned char buf[buf_len]; unsigned char *p = (unsigned char*) sig; mbedtls_pk_info_t pk_info = mbedtls_eckey_info; - psa_algorithm_t psa_sig_md = translate_md_to_psa( md_alg ); + psa_algorithm_t psa_sig_md = mbedtls_psa_translate_md( md_alg ); psa_ecc_curve_t curve = mbedtls_ecc_group_to_psa ( ( (mbedtls_ecdsa_context *) ctx )->grp.id ); if( curve == 0 ) @@ -690,7 +635,6 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, memset( &signature, 0, sizeof( mbedtls_asn1_buf ) ); key.pk_info = &pk_info; key.pk_ctx = ctx; - psa_crypto_init(); psa_type = PSA_KEY_TYPE_ECC_PUBLIC_KEY( curve ); From 45fc4641562daf97242b68f27286e51990292700 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 19 Nov 2018 14:53:35 -0500 Subject: [PATCH 09/16] pk_wrap: improve error codes returned from ecdsa_verify_wrap Use the shared PSA utilities to translate errors. --- library/pk_wrap.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 56ce69c54..0d1d91b62 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -492,15 +492,14 @@ static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, if( ( end - *p ) < 1 ) { - return( MBEDTLS_ERR_X509_INVALID_SIGNATURE + - MBEDTLS_ERR_ASN1_OUT_OF_DATA ); + return( MBEDTLS_ERR_ASN1_OUT_OF_DATA ); } tag_type = **p; if( ( ret = mbedtls_asn1_get_tag( p, end, &len_partial, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 ) { - return( MBEDTLS_ERR_X509_INVALID_SIGNATURE + ret ); + return( ret ); } if( ( ret = mbedtls_asn1_get_tag( p, end, &len_partial, MBEDTLS_ASN1_INTEGER ) ) @@ -536,7 +535,7 @@ static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, // Check if both parts are of the same size if( len_partial != len_signature ) - return( MBEDTLS_ERR_X509_INVALID_SIGNATURE ); + return( MBEDTLS_ERR_PK_SIG_LEN_MISMATCH ); memcpy( sig->p + len_partial, *p, len_partial ); len_signature += len_partial; @@ -638,8 +637,8 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, psa_type = PSA_KEY_TYPE_ECC_PUBLIC_KEY( curve ); - if( extract_ecdsa_sig( &p, p + sig_len, &signature ) != 0 ) - return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + if( ( ret = extract_ecdsa_sig( &p, p + sig_len, &signature ) ) != 0 ) + return( ret ); key_len = mbedtls_pk_write_pubkey_der( &key, buf, buf_len ); if( key_len <= 0 ) @@ -648,17 +647,17 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); } - if( mbedtls_psa_get_free_key_slot( &key_slot ) != PSA_SUCCESS ) + if( ( ret = mbedtls_psa_get_free_key_slot( &key_slot ) ) != PSA_SUCCESS ) { mbedtls_free( signature.p ); - return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE ); + return( mbedtls_psa_err_translate_pk( ret ) ); } psa_key_policy_init( &policy ); psa_key_policy_set_usage( &policy, PSA_KEY_USAGE_VERIFY, psa_sig_md ); - if( psa_set_key_policy( key_slot, &policy ) != PSA_SUCCESS ) + if( ( ret = psa_set_key_policy( key_slot, &policy ) ) != PSA_SUCCESS ) { - ret = MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; + ret = mbedtls_psa_err_translate_pk( ret ); goto cleanup; } From 7b7808cc7654f524108ecda72eb2b337d60051c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 15 Nov 2018 11:44:11 +0100 Subject: [PATCH 10/16] Add tests for ECDSA verify with short r, s values This is intended to test transcoding the signature to the format expected by PSA (fixed-length encoding of r, s) when r and s have respectively: - full length with initial null byte - full length without initial null byte - non-full length with initial null byte - non-full length without initial null byte The signatures were generated using: programs/pkey/pk_sign tests/data_files/server5.key foo where foo is an empty file, and with a variant of one of the following patches applied: diff --git a/library/ecdsa.c b/library/ecdsa.c index abac015cebc6..e4a27b044516 100644 --- a/library/ecdsa.c +++ b/library/ecdsa.c @@ -305,7 +305,9 @@ static int ecdsa_sign_restartable( mbedtls_ecp_group *grp, ret = MBEDTLS_ERR_ECP_RANDOM_FAILED; goto cleanup; } + printf("\ngenerating r...\n"); +gen: MBEDTLS_MPI_CHK( mbedtls_ecp_gen_privkey( grp, pk, f_rng, p_rng ) ); #if defined(MBEDTLS_ECP_RESTARTABLE) @@ -317,6 +319,11 @@ mul: MBEDTLS_MPI_CHK( mbedtls_ecp_mul_restartable( grp, &R, pk, &grp->G, f_rng, p_rng, ECDSA_RS_ECP ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( pr, &R.X, &grp->N ) ); + + size_t bits = mbedtls_mpi_bitlen( pr ); + printf("%zu ", bits); + if( bits != 255 ) + goto gen; } while( mbedtls_mpi_cmp_int( pr, 0 ) == 0 ); or: diff --git a/library/ecdsa.c b/library/ecdsa.c index abac015cebc6..d704376e0c42 100644 --- a/library/ecdsa.c +++ b/library/ecdsa.c @@ -305,7 +305,9 @@ static int ecdsa_sign_restartable( mbedtls_ecp_group *grp, ret = MBEDTLS_ERR_ECP_RANDOM_FAILED; goto cleanup; } + printf("\ngenerating r...\n"); +gen: MBEDTLS_MPI_CHK( mbedtls_ecp_gen_privkey( grp, pk, f_rng, p_rng ) ); #if defined(MBEDTLS_ECP_RESTARTABLE) @@ -353,6 +355,11 @@ modn: MBEDTLS_MPI_CHK( mbedtls_mpi_inv_mod( s, pk, &grp->N ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( s, s, &e ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( s, s, &grp->N ) ); + + size_t bits = mbedtls_mpi_bitlen( s ); + printf("%zu ", bits); + if( bits != 247 ) + goto gen; } while( mbedtls_mpi_cmp_int( s, 0 ) == 0 ); with the value edited manually between each run to get the desired bit length. --- tests/suites/test_suite_pk.data | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/suites/test_suite_pk.data b/tests/suites/test_suite_pk.data index 478cde7be..11dff2675 100644 --- a/tests/suites/test_suite_pk.data +++ b/tests/suites/test_suite_pk.data @@ -38,6 +38,38 @@ EC(DSA) verify test vector #2 (bad) depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED pk_ec_test_vec:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP192R1:"046FDD3028FA94A863CD4F78DBFF8B3AA561FC6D9CCBBCA88E0AE6FA437F5415F957542D0717FF8B84562DAE99872EF841":"546869732073686F756C64206265207468652068617368206F662061206D6573736167652E00":"30350218185B2A7FB5CD9C9A8488B119B68B47D6EC833509CE9FA1FF021900FB7D259A744A2348BD45D241A39DC915B81CC2084100FA25":MBEDTLS_ERR_ECP_VERIFY_FAILED +EC(DSA) verify test vector: good, bitlen(r) = 256 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +pk_ec_test_vec:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP256R1:"0437cc56d976091e5a723ec7592dff206eee7cf9069174d0ad14b5f768225962924ee500d82311ffea2fd2345d5d16bd8a88c26b770d55cd8a2a0efa01c8b4edff":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855":"3046022100faecc085c6c5362b91ff1fd6dd77da80bc071bee9ff1ac0ef9509c017f13267c022100a7d0b908c938d3dd6c6a9cdc5b0a4a4ee455c519c1ff6cda959806b7e7461ba0":0 + +EC(DSA) verify test vector: good, bitlen(r) = 255 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +pk_ec_test_vec:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP256R1:"0437cc56d976091e5a723ec7592dff206eee7cf9069174d0ad14b5f768225962924ee500d82311ffea2fd2345d5d16bd8a88c26b770d55cd8a2a0efa01c8b4edff":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855":"30450220639f36215b2ff09bb2beb871e122de74c8d5e29ce8a105aa2b95661f42803e72022100becd8f81b2c186f9d5d2c92378d7b9452ce6de231b0c8d17bac2d8537d2331fd":0 + +EC(DSA) verify test vector: good, bitlen(r) = 248 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +pk_ec_test_vec:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP256R1:"0437cc56d976091e5a723ec7592dff206eee7cf9069174d0ad14b5f768225962924ee500d82311ffea2fd2345d5d16bd8a88c26b770d55cd8a2a0efa01c8b4edff":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855":"30450220009109f967f9082abc9c46e5ea07936529b82023a1a49b872c046f430983db2602210085f0b1960d61f8d75109b5b7ff991d3171320d2ab547104f864048455a965090":0 + +EC(DSA) verify test vector: good, bitlen(r) = 247 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +pk_ec_test_vec:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP256R1:"0437cc56d976091e5a723ec7592dff206eee7cf9069174d0ad14b5f768225962924ee500d82311ffea2fd2345d5d16bd8a88c26b770d55cd8a2a0efa01c8b4edff":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855":"3044021f461786833b50247b07194da6cedbd3caefbcd19c73b6283ccff5097cd0d73b022100d85d20b0b8c3b596eb1cdb0381e681fa0a8bccde4e89c139020af3b0f88e099c":0 + +EC(DSA) verify test vector: good, bitlen(s) = 256 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +pk_ec_test_vec:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP256R1:"0437cc56d976091e5a723ec7592dff206eee7cf9069174d0ad14b5f768225962924ee500d82311ffea2fd2345d5d16bd8a88c26b770d55cd8a2a0efa01c8b4edff":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855":"30450220639f36215b2ff09bb2beb871e122de74c8d5e29ce8a105aa2b95661f42803e72022100becd8f81b2c186f9d5d2c92378d7b9452ce6de231b0c8d17bac2d8537d2331fd":0 + +EC(DSA) verify test vector: good, bitlen(s) = 255 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +pk_ec_test_vec:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP256R1:"0437cc56d976091e5a723ec7592dff206eee7cf9069174d0ad14b5f768225962924ee500d82311ffea2fd2345d5d16bd8a88c26b770d55cd8a2a0efa01c8b4edff":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855":"304402206ae26950c606d08fe5e1682efdccfb3a7213ca46bd523ffd20c4213fe1400d3402207612106ada7055926167650b257da7f4c42c190b8aa9e3b680f8751fe90c63a5":0 + +EC(DSA) verify test vector: good, bitlen(s) = 248 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +pk_ec_test_vec:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP256R1:"0437cc56d976091e5a723ec7592dff206eee7cf9069174d0ad14b5f768225962924ee500d82311ffea2fd2345d5d16bd8a88c26b770d55cd8a2a0efa01c8b4edff":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855":"3045022100fd4d718ab483827492e10b89745fad100d2dd257102b99aff179ee596a569f1f022000a1b777e32a8b4909763b615b805e59194e6196eb05719287a36eb5f17aa485":0 + +EC(DSA) verify test vector: good, bitlen(s) = 247 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +pk_ec_test_vec:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP256R1:"0437cc56d976091e5a723ec7592dff206eee7cf9069174d0ad14b5f768225962924ee500d82311ffea2fd2345d5d16bd8a88c26b770d55cd8a2a0efa01c8b4edff":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855":"30430220685a6994daa6a14e4411b5267edc2a00beee907f2dddd956b2a5a1df791c15f8021f675db4538c000c734489ac737fddd5a739c5a23cd6c6eceea70c286ca4fac9":0 + ECDSA sign-verify depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED pk_sign_verify:MBEDTLS_PK_ECDSA:0:0 From 3016de3eebd5147d153f1059ca6b924f6a7a41ee Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 19 Nov 2018 17:01:16 -0500 Subject: [PATCH 11/16] pk_wrap: rework signature extraction to work with small r and s values There is a probability that r will be encoded as 31 or less bytes in DER, so additional padding is added in such case. Added a signature-part extraction function to tidy up the code further. --- library/pk_wrap.c | 108 ++++++++++++++++++++++++++++------------------ 1 file changed, 66 insertions(+), 42 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 0d1d91b62..e33ea3fc5 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -479,70 +479,93 @@ static int ecdsa_can_do( mbedtls_pk_type_t type ) } #if defined(MBEDTLS_USE_PSA_CRYPTO) +/* + * Extract one signature part of an ASN.1 integer type to a given buffer + * and adjust padding according to part_size. + */ +static int extract_ecdsa_sig_part( unsigned char **from, const unsigned char *end, + unsigned char *to, size_t part_size ) +{ + int ret; + size_t len_total, len_partial, zero_padding; + + if( ( ret = mbedtls_asn1_get_tag( from, end, &len_partial, + MBEDTLS_ASN1_INTEGER ) ) != 0 ) + { + return( ret ); + } + + while( **from == '\0' && len_partial > 0 ) + { + ( *from )++; + len_partial--; + } + + if( len_partial > part_size || len_partial == 0 ) + return( MBEDTLS_ERR_PK_SIG_LEN_MISMATCH ); + + zero_padding = part_size - len_partial; + memcpy( to + zero_padding, *from, len_partial ); + len_total = len_partial + zero_padding; + while( zero_padding > 0 ) + { + zero_padding--; + to[zero_padding] = 0; + } + + ( *from ) += len_partial; + return len_total; +} + /* * Convert a signature from an ASN.1 sequence of two integers * to a raw {r,s} buffer. Note: upon a successful call, the caller * takes ownership of the sig->p buffer. */ static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, - mbedtls_asn1_buf *sig ) + mbedtls_asn1_buf *sig, size_t int_size ) { - int ret, tag_type; - size_t len_signature, len_partial; + int ret; if( ( end - *p ) < 1 ) { return( MBEDTLS_ERR_ASN1_OUT_OF_DATA ); } - tag_type = **p; - if( ( ret = mbedtls_asn1_get_tag( p, end, &len_partial, - MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 ) - { - return( ret ); - } - - if( ( ret = mbedtls_asn1_get_tag( p, end, &len_partial, MBEDTLS_ASN1_INTEGER ) ) - != 0 ) - return( ret ); - - if( **p == '\0' ) - { - ( *p )++; - len_partial--; - } - - sig->p = mbedtls_calloc( 2, len_partial ); + sig->p = mbedtls_calloc( 2, int_size ); if( sig->p == NULL ) return( MBEDTLS_ERR_ASN1_ALLOC_FAILED ); - memcpy( sig->p, *p, len_partial ); - len_signature = len_partial; - ( *p ) += len_partial; - if( ( ret = mbedtls_asn1_get_tag( p, end, &len_partial, - MBEDTLS_ASN1_INTEGER ) ) != 0 ) + sig->tag = **p; + + if( ( ret = mbedtls_asn1_get_tag( p, end, &sig->len, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 ) { - mbedtls_free( sig->p ); - sig->p = NULL; - return( ret ); + goto cleanup; } - if( **p == '\0' ) + /* Extract r */ + if( ( ret = extract_ecdsa_sig_part( p, end, sig->p, int_size ) ) < 0) { - ( *p )++; - len_partial--; + goto cleanup; } + sig->len = ret; - // Check if both parts are of the same size - if( len_partial != len_signature ) - return( MBEDTLS_ERR_PK_SIG_LEN_MISMATCH ); + /* Extract s */ + if( ( ret = extract_ecdsa_sig_part( p, end, sig->p + sig->len, int_size ) ) < 0) + { + goto cleanup; + } + sig->len += ret; - memcpy( sig->p + len_partial, *p, len_partial ); - len_signature += len_partial; - sig->tag = tag_type; - sig->len = len_signature; - ( *p ) += len_partial; return( 0 ); + +cleanup: + mbedtls_free( sig->p ); + sig->p = NULL; + sig->len = 0; + sig->tag = 0; + return( ret ); } static psa_ecc_curve_t mbedtls_ecc_group_to_psa( mbedtls_ecp_group_id grpid ) @@ -637,7 +660,8 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, psa_type = PSA_KEY_TYPE_ECC_PUBLIC_KEY( curve ); - if( ( ret = extract_ecdsa_sig( &p, p + sig_len, &signature ) ) != 0 ) + if( ( ret = extract_ecdsa_sig( &p, p + sig_len, &signature, + ( ( (mbedtls_ecdsa_context *) ctx )->grp.nbits + 7) / 8 ) ) != 0 ) return( ret ); key_len = mbedtls_pk_write_pubkey_der( &key, buf, buf_len ); @@ -678,7 +702,7 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, } ret = 0; - cleanup: +cleanup: psa_destroy_key( key_slot ); mbedtls_free( signature.p ); return( ret ); From 688ea8d10de20f46ed3e533992e4886c022c9721 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 19 Nov 2018 17:41:58 -0500 Subject: [PATCH 12/16] pk_wrap: reuse a static buffer for signature extraction Use a buffer left over after importing a key to hold an extracted signature. --- library/pk_wrap.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index e33ea3fc5..46ffe4e27 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -519,8 +519,8 @@ static int extract_ecdsa_sig_part( unsigned char **from, const unsigned char *en /* * Convert a signature from an ASN.1 sequence of two integers - * to a raw {r,s} buffer. Note: upon a successful call, the caller - * takes ownership of the sig->p buffer. + * to a raw {r,s} buffer. Note: the provided sig buffer should be at least + * twice as big as int_size. */ static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, mbedtls_asn1_buf *sig, size_t int_size ) @@ -532,9 +532,8 @@ static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, return( MBEDTLS_ERR_ASN1_OUT_OF_DATA ); } - sig->p = mbedtls_calloc( 2, int_size ); if( sig->p == NULL ) - return( MBEDTLS_ERR_ASN1_ALLOC_FAILED ); + return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); sig->tag = **p; @@ -561,8 +560,6 @@ static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, return( 0 ); cleanup: - mbedtls_free( sig->p ); - sig->p = NULL; sig->len = 0; sig->tag = 0; return( ret ); @@ -640,12 +637,13 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, mbedtls_pk_context key; mbedtls_asn1_buf signature; int key_len; - const int buf_len = 30 + 2 * MBEDTLS_ECP_MAX_BYTES; // Equivalent of ECP_PUB_DER_MAX_BYTES + const unsigned buf_len = 30 + 2 * MBEDTLS_ECP_MAX_BYTES; // Equivalent of ECP_PUB_DER_MAX_BYTES unsigned char buf[buf_len]; unsigned char *p = (unsigned char*) sig; mbedtls_pk_info_t pk_info = mbedtls_eckey_info; psa_algorithm_t psa_sig_md = mbedtls_psa_translate_md( md_alg ); psa_ecc_curve_t curve = mbedtls_ecc_group_to_psa ( ( (mbedtls_ecdsa_context *) ctx )->grp.id ); + size_t signature_part_size = ( ( (mbedtls_ecdsa_context *) ctx ) ->grp.nbits + 7 ) / 8; if( curve == 0 ) return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); @@ -660,22 +658,12 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, psa_type = PSA_KEY_TYPE_ECC_PUBLIC_KEY( curve ); - if( ( ret = extract_ecdsa_sig( &p, p + sig_len, &signature, - ( ( (mbedtls_ecdsa_context *) ctx )->grp.nbits + 7) / 8 ) ) != 0 ) - return( ret ); - key_len = mbedtls_pk_write_pubkey_der( &key, buf, buf_len ); if( key_len <= 0 ) - { - mbedtls_free( signature.p ); return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); - } if( ( ret = mbedtls_psa_get_free_key_slot( &key_slot ) ) != PSA_SUCCESS ) - { - mbedtls_free( signature.p ); return( mbedtls_psa_err_translate_pk( ret ) ); - } psa_key_policy_init( &policy ); psa_key_policy_set_usage( &policy, PSA_KEY_USAGE_VERIFY, psa_sig_md ); @@ -692,6 +680,20 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, goto cleanup; } + /* Reuse the buffer of an already imported key */ + if( 2 * signature_part_size > buf_len ) + { + ret = MBEDTLS_ERR_PK_BAD_INPUT_DATA; + goto cleanup; + } + signature.p = buf; + + if( ( ret = extract_ecdsa_sig( &p, p + sig_len, &signature, + signature_part_size ) ) != 0 ) + { + goto cleanup; + } + if( psa_asymmetric_verify( key_slot, psa_sig_md, hash, hash_len, signature.p, signature.len ) @@ -704,7 +706,6 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, cleanup: psa_destroy_key( key_slot ); - mbedtls_free( signature.p ); return( ret ); } #else /* MBEDTLS_USE_PSA_CRYPTO */ From 73bf6b9e00e35e19c96042f359b495cc54e9d204 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 20 Nov 2018 05:04:35 -0500 Subject: [PATCH 13/16] pk_wrap: rework and tidy up signature extraction Improve comments, use a normal buffer instead of mbedtls_asn1_buf, remove unneeded variables and use shared utilities where possible. --- library/pk_wrap.c | 169 ++++++++++++---------------------------------- 1 file changed, 44 insertions(+), 125 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 46ffe4e27..8d6c0f263 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -480,150 +480,70 @@ static int ecdsa_can_do( mbedtls_pk_type_t type ) #if defined(MBEDTLS_USE_PSA_CRYPTO) /* - * Extract one signature part of an ASN.1 integer type to a given buffer - * and adjust padding according to part_size. + * An ASN.1 encoded signature is a sequence of two ASN.1 integers. Parse one of + * those integers and convert it to the fixed-length encoding expected by PSA. */ -static int extract_ecdsa_sig_part( unsigned char **from, const unsigned char *end, - unsigned char *to, size_t part_size ) +static int extract_ecdsa_sig_int( unsigned char **from, const unsigned char *end, + unsigned char *to, size_t to_len ) { int ret; - size_t len_total, len_partial, zero_padding; + size_t unpadded_len, padding_len; - if( ( ret = mbedtls_asn1_get_tag( from, end, &len_partial, + if( ( ret = mbedtls_asn1_get_tag( from, end, &unpadded_len, MBEDTLS_ASN1_INTEGER ) ) != 0 ) { return( ret ); } - while( **from == '\0' && len_partial > 0 ) + while( unpadded_len > 0 && **from == 0x00 ) { ( *from )++; - len_partial--; + unpadded_len--; } - if( len_partial > part_size || len_partial == 0 ) - return( MBEDTLS_ERR_PK_SIG_LEN_MISMATCH ); + if( unpadded_len > to_len || unpadded_len == 0 ) + return( MBEDTLS_ERR_ASN1_LENGTH_MISMATCH ); - zero_padding = part_size - len_partial; - memcpy( to + zero_padding, *from, len_partial ); - len_total = len_partial + zero_padding; - while( zero_padding > 0 ) - { - zero_padding--; - to[zero_padding] = 0; - } + padding_len = to_len - unpadded_len; + memcpy( to + padding_len, *from, unpadded_len ); + ( *from ) += unpadded_len; - ( *from ) += len_partial; - return len_total; + memset( to, 0x00, padding_len ); + + return( 0 ); } /* * Convert a signature from an ASN.1 sequence of two integers - * to a raw {r,s} buffer. Note: the provided sig buffer should be at least + * to a raw {r,s} buffer. Note: the provided sig buffer must be at least * twice as big as int_size. */ static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, - mbedtls_asn1_buf *sig, size_t int_size ) + unsigned char *sig, size_t int_size ) { int ret; + size_t tmp_size; if( ( end - *p ) < 1 ) { return( MBEDTLS_ERR_ASN1_OUT_OF_DATA ); } - if( sig->p == NULL ) + if( sig == NULL ) return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); - sig->tag = **p; - - if( ( ret = mbedtls_asn1_get_tag( p, end, &sig->len, + if( ( ret = mbedtls_asn1_get_tag( p, end, &tmp_size, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 ) - { - goto cleanup; - } + return( ret ); /* Extract r */ - if( ( ret = extract_ecdsa_sig_part( p, end, sig->p, int_size ) ) < 0) - { - goto cleanup; - } - sig->len = ret; - + if( ( ret = extract_ecdsa_sig_int( p, end, sig, int_size ) ) != 0 ) + return( ret ); /* Extract s */ - if( ( ret = extract_ecdsa_sig_part( p, end, sig->p + sig->len, int_size ) ) < 0) - { - goto cleanup; - } - sig->len += ret; + if( ( ret = extract_ecdsa_sig_int( p, end, sig + int_size, int_size ) ) != 0 ) + return( ret ); return( 0 ); - -cleanup: - sig->len = 0; - sig->tag = 0; - return( ret ); -} - -static psa_ecc_curve_t mbedtls_ecc_group_to_psa( mbedtls_ecp_group_id grpid ) -{ - switch( grpid ) - { -#if defined(MBEDTLS_ECP_DP_SECP192R1_ENABLED) - case MBEDTLS_ECP_DP_SECP192R1: - return( PSA_ECC_CURVE_SECP192R1 ); -#endif -#if defined(MBEDTLS_ECP_DP_SECP224R1_ENABLED) - case MBEDTLS_ECP_DP_SECP224R1: - return( PSA_ECC_CURVE_SECP224R1 ); -#endif -#if defined(MBEDTLS_ECP_DP_SECP256R1_ENABLED) - case MBEDTLS_ECP_DP_SECP256R1: - return( PSA_ECC_CURVE_SECP256R1 ); -#endif -#if defined(MBEDTLS_ECP_DP_SECP384R1_ENABLED) - case MBEDTLS_ECP_DP_SECP384R1: - return( PSA_ECC_CURVE_SECP384R1 ); -#endif -#if defined(MBEDTLS_ECP_DP_SECP521R1_ENABLED) - case MBEDTLS_ECP_DP_SECP521R1: - return( PSA_ECC_CURVE_SECP521R1 ); -#endif -#if defined(MBEDTLS_ECP_DP_BP256R1_ENABLED) - case MBEDTLS_ECP_DP_BP256R1: - return( PSA_ECC_CURVE_BRAINPOOL_P256R1 ); -#endif -#if defined(MBEDTLS_ECP_DP_BP384R1_ENABLED) - case MBEDTLS_ECP_DP_BP384R1: - return( PSA_ECC_CURVE_BRAINPOOL_P384R1 ); -#endif -#if defined(MBEDTLS_ECP_DP_BP512R1_ENABLED) - case MBEDTLS_ECP_DP_BP512R1: - return( PSA_ECC_CURVE_BRAINPOOL_P512R1 ); -#endif -#if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) - case MBEDTLS_ECP_DP_CURVE25519: - return( PSA_ECC_CURVE_CURVE25519 ); -#endif -#if defined(MBEDTLS_ECP_DP_SECP192K1_ENABLED) - case MBEDTLS_ECP_DP_SECP192K1: - return( PSA_ECC_CURVE_SECP192K1 ); -#endif -#if defined(MBEDTLS_ECP_DP_SECP224K1_ENABLED) - case MBEDTLS_ECP_DP_SECP224K1: - return( PSA_ECC_CURVE_SECP224K1 ); -#endif -#if defined(MBEDTLS_ECP_DP_SECP256K1_ENABLED) - case MBEDTLS_ECP_DP_SECP256K1: - return( PSA_ECC_CURVE_SECP256K1 ); -#endif -#if defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) - case MBEDTLS_ECP_DP_CURVE448: - return( PSA_ECC_CURVE_CURVE448 ); -#endif - default: - return( 0 ); - } } static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, @@ -635,36 +555,36 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, psa_key_policy_t policy; psa_key_type_t psa_type; mbedtls_pk_context key; - mbedtls_asn1_buf signature; int key_len; - const unsigned buf_len = 30 + 2 * MBEDTLS_ECP_MAX_BYTES; // Equivalent of ECP_PUB_DER_MAX_BYTES - unsigned char buf[buf_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; mbedtls_pk_info_t pk_info = mbedtls_eckey_info; - psa_algorithm_t psa_sig_md = mbedtls_psa_translate_md( md_alg ); - psa_ecc_curve_t curve = mbedtls_ecc_group_to_psa ( ( (mbedtls_ecdsa_context *) ctx )->grp.id ); + psa_algorithm_t psa_sig_md, psa_md; + psa_ecc_curve_t curve = mbedtls_psa_translate_ecc_group ( + ( (mbedtls_ecdsa_context *) ctx )->grp.id ); size_t signature_part_size = ( ( (mbedtls_ecdsa_context *) ctx ) ->grp.nbits + 7 ) / 8; if( curve == 0 ) return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); - if( psa_sig_md == 0 ) - return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); - - psa_sig_md = PSA_ALG_ECDSA( psa_sig_md ); - memset( &signature, 0, sizeof( mbedtls_asn1_buf ) ); + /* mbedlts_pk_write_pubkey_der() expects a full PK context, + * re-construct one to make it happy */ key.pk_info = &pk_info; key.pk_ctx = ctx; - - psa_type = PSA_KEY_TYPE_ECC_PUBLIC_KEY( curve ); - - key_len = mbedtls_pk_write_pubkey_der( &key, buf, buf_len ); + key_len = mbedtls_pk_write_pubkey_der( &key, buf, sizeof( buf ) ); if( key_len <= 0 ) return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); if( ( ret = mbedtls_psa_get_free_key_slot( &key_slot ) ) != PSA_SUCCESS ) return( mbedtls_psa_err_translate_pk( ret ) ); + psa_md = mbedtls_psa_translate_md( md_alg ); + if( psa_md == 0 ) + return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + psa_sig_md = PSA_ALG_ECDSA( psa_md ); + psa_type = PSA_KEY_TYPE_ECC_PUBLIC_KEY( curve ); + psa_key_policy_init( &policy ); psa_key_policy_set_usage( &policy, PSA_KEY_USAGE_VERIFY, psa_sig_md ); if( ( ret = psa_set_key_policy( key_slot, &policy ) ) != PSA_SUCCESS ) @@ -673,7 +593,7 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, goto cleanup; } - if( psa_import_key( key_slot, psa_type, buf+buf_len-key_len, key_len ) + if( psa_import_key( key_slot, psa_type, buf + sizeof( buf ) - key_len, key_len ) != PSA_SUCCESS ) { ret = MBEDTLS_ERR_PK_BAD_INPUT_DATA; @@ -681,14 +601,13 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, } /* Reuse the buffer of an already imported key */ - if( 2 * signature_part_size > buf_len ) + if( 2 * signature_part_size > sizeof( buf ) ) { ret = MBEDTLS_ERR_PK_BAD_INPUT_DATA; goto cleanup; } - signature.p = buf; - if( ( ret = extract_ecdsa_sig( &p, p + sig_len, &signature, + if( ( ret = extract_ecdsa_sig( &p, p + sig_len, buf, signature_part_size ) ) != 0 ) { goto cleanup; @@ -696,7 +615,7 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, if( psa_asymmetric_verify( key_slot, psa_sig_md, hash, hash_len, - signature.p, signature.len ) + buf, 2 * signature_part_size ) != PSA_SUCCESS ) { ret = MBEDTLS_ERR_ECP_VERIFY_FAILED; From e30ad542a10d2c506e2948f6d445afaf44f82734 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 20 Nov 2018 05:14:46 -0500 Subject: [PATCH 14/16] Cosmetic changes Move memset to a more relevant spot, fix one whitespace error --- library/pk_wrap.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 8d6c0f263..1b626c75a 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -505,11 +505,10 @@ static int extract_ecdsa_sig_int( unsigned char **from, const unsigned char *end return( MBEDTLS_ERR_ASN1_LENGTH_MISMATCH ); padding_len = to_len - unpadded_len; + memset( to, 0x00, padding_len ); memcpy( to + padding_len, *from, unpadded_len ); ( *from ) += unpadded_len; - memset( to, 0x00, padding_len ); - return( 0 ); } @@ -561,7 +560,7 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, unsigned char *p = (unsigned char*) sig; 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 ( + psa_ecc_curve_t curve = mbedtls_psa_translate_ecc_group( ( (mbedtls_ecdsa_context *) ctx )->grp.id ); size_t signature_part_size = ( ( (mbedtls_ecdsa_context *) ctx ) ->grp.nbits + 7 ) / 8; From 96cc1b3def98f62c3e5f87ca2155d7d52a8ad598 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 20 Nov 2018 06:39:06 -0500 Subject: [PATCH 15/16] pk_wrap.c: tidy up signature extraction Add a sanity check for signature length, remove superfluous bounds check. --- library/pk_wrap.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 1b626c75a..9fc7e22b9 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -523,14 +523,6 @@ static int extract_ecdsa_sig( unsigned char **p, const unsigned char *end, int ret; size_t tmp_size; - if( ( end - *p ) < 1 ) - { - return( MBEDTLS_ERR_ASN1_OUT_OF_DATA ); - } - - if( sig == NULL ) - return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); - if( ( ret = mbedtls_asn1_get_tag( p, end, &tmp_size, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 ) return( ret ); @@ -562,7 +554,7 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, psa_algorithm_t psa_sig_md, psa_md; psa_ecc_curve_t curve = mbedtls_psa_translate_ecc_group( ( (mbedtls_ecdsa_context *) ctx )->grp.id ); - size_t signature_part_size = ( ( (mbedtls_ecdsa_context *) ctx ) ->grp.nbits + 7 ) / 8; + const size_t signature_part_size = ( ( (mbedtls_ecdsa_context *) ctx )->grp.nbits + 7 ) / 8; if( curve == 0 ) return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); @@ -599,19 +591,26 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, goto cleanup; } - /* Reuse the buffer of an already imported key */ + /* We don't need the exported key anymore and can + * reuse its buffer for signature extraction. */ if( 2 * signature_part_size > sizeof( buf ) ) { ret = MBEDTLS_ERR_PK_BAD_INPUT_DATA; goto cleanup; } - if( ( ret = extract_ecdsa_sig( &p, p + sig_len, buf, + if( ( ret = extract_ecdsa_sig( &p, sig + sig_len, buf, signature_part_size ) ) != 0 ) { goto cleanup; } + if( p != sig + sig_len ) + { + ret = MBEDTLS_ERR_PK_SIG_LEN_MISMATCH; + goto cleanup; + } + if( psa_asymmetric_verify( key_slot, psa_sig_md, hash, hash_len, buf, 2 * signature_part_size ) From 266d907c87ac43d5f518fad7134ba70d64bfb5df Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 20 Nov 2018 07:59:18 -0500 Subject: [PATCH 16/16] pk_wrap.c: fix length mismatch check placement --- library/pk_wrap.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 9fc7e22b9..3690fef5b 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -605,12 +605,6 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, goto cleanup; } - if( p != sig + sig_len ) - { - ret = MBEDTLS_ERR_PK_SIG_LEN_MISMATCH; - goto cleanup; - } - if( psa_asymmetric_verify( key_slot, psa_sig_md, hash, hash_len, buf, 2 * signature_part_size ) @@ -619,6 +613,12 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, ret = MBEDTLS_ERR_ECP_VERIFY_FAILED; goto cleanup; } + + if( p != sig + sig_len ) + { + ret = MBEDTLS_ERR_PK_SIG_LEN_MISMATCH; + goto cleanup; + } ret = 0; cleanup: