From e960690b89e22e3c73918257b96417189f7a2579 Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Wed, 9 Feb 2022 14:23:00 +0100 Subject: [PATCH 01/14] PK: ECDSA signing PSA wrap implementation Signed-off-by: Neil Armstrong --- library/pk_wrap.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index f7480c63b..2d4a1e286 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -737,6 +737,106 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, } #endif /* MBEDTLS_USE_PSA_CRYPTO */ +#if defined(MBEDTLS_USE_PSA_CRYPTO) +static int pk_ecdsa_sig_asn1_from_psa( unsigned char *sig, size_t *sig_len, + size_t buf_len ); + +static int find_ecdsa_private_key( unsigned char **buf, unsigned char *end, size_t *key_len) +{ + size_t len; + int ret; + + 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, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) +{ + mbedtls_ecdsa_context *ctx = ctx_arg; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + 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; + /* see ECP_PRV_DER_MAX_BYTES in pkwrite.c */ + unsigned char buf[29 + 3 * MBEDTLS_ECP_MAX_BYTES]; + unsigned char *p; + mbedtls_pk_info_t pk_info = mbedtls_eckey_info; + psa_algorithm_t psa_sig_md = PSA_ALG_ECDSA( mbedtls_psa_translate_md( md_alg ) ); + size_t curve_bits; + psa_ecc_family_t curve = + mbedtls_ecc_group_to_psa( ctx->grp.id, &curve_bits ); + ((void) md_alg); + + /* PSA has its own RNG */ + ((void) f_rng); + ((void) p_rng); + + if( curve == 0 ) + 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 = &pk_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 ); + + p = buf + sizeof( buf ) - key_len; + ret = find_ecdsa_private_key( &p, buf + sizeof( buf ), &key_len ); + if( ret != 0 ) + goto cleanup; + + psa_set_key_type( &attributes, PSA_KEY_TYPE_ECC_KEY_PAIR( curve ) ); + psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_SIGN_HASH ); + psa_set_key_algorithm( &attributes, psa_sig_md ); + + status = psa_import_key( &attributes, + p, key_len, + &key_id ); + if( status != PSA_SUCCESS ) + { + ret = mbedtls_psa_err_translate_pk( status ); + goto cleanup; + } + + if( psa_sign_hash( key_id, psa_sig_md, hash, hash_len, + sig, sig_size, sig_len) + != PSA_SUCCESS ) + { + ret = MBEDTLS_ERR_ECP_VERIFY_FAILED; + goto cleanup; + } + + ret = pk_ecdsa_sig_asn1_from_psa( sig, sig_len, sig_size ); + +cleanup: + psa_destroy_key( key_id ); + return( ret ); +} +#else static int ecdsa_sign_wrap( void *ctx, 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, @@ -747,6 +847,7 @@ static int ecdsa_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, sig, sig_size, sig_len, f_rng, p_rng ) ); } +#endif #if defined(MBEDTLS_ECP_RESTARTABLE) static int ecdsa_verify_rs_wrap( void *ctx, mbedtls_md_type_t md_alg, From 5b32038ff0c1e467cd9a238333ab9ee233c0f795 Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Mon, 21 Feb 2022 17:22:10 +0100 Subject: [PATCH 02/14] Alternative CSR checks in x509_csr_check when USE_PSA_CRYPTO The X509write x509_csr_check reference file depends on mbedtls_test_rnd_pseudo_rand being used to match the pre-generated data. This calls x509_crt_verifycsr() like in x509_csr_check_opaque() when MBEDTLS_USE_PSA_CRYPTO is defined. Notably using PSA_ALG_DETERMINISTIC_ECDSA() in ecdsa_sign_wrap() makes this test run without these changes. Signed-off-by: Neil Armstrong --- tests/suites/test_suite_x509write.function | 23 +++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index 261794c7f..7503b5e81 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -93,6 +93,8 @@ void x509_csr_check( char * key_file, char * cert_req_check_file, int md_type, memset( &rnd_info, 0x2a, sizeof( mbedtls_test_rnd_pseudo_info ) ); + USE_PSA_INIT( ); + mbedtls_pk_init( &key ); TEST_ASSERT( mbedtls_pk_parse_keyfile( &key, key_file, NULL, mbedtls_test_rnd_std_rand, NULL ) == 0 ); @@ -117,6 +119,15 @@ void x509_csr_check( char * key_file, char * cert_req_check_file, int md_type, TEST_ASSERT( buf[buf_index] == 0 ); } +#if defined(MBEDTLS_USE_PSA_CRYPTO) + // When using PSA crypto, RNG isn't controllable, so cert_req_check_file can't be used + (void)f; + (void)olen; + (void)check_buf; + (void)cert_req_check_file; + buf[pem_len] = '\0'; + TEST_ASSERT( x509_crt_verifycsr( buf, pem_len + 1 ) == 0 ); +#else f = fopen( cert_req_check_file, "r" ); TEST_ASSERT( f != NULL ); olen = fread( check_buf, 1, sizeof( check_buf ), f ); @@ -124,6 +135,7 @@ void x509_csr_check( char * key_file, char * cert_req_check_file, int md_type, TEST_ASSERT( olen >= pem_len - 1 ); TEST_ASSERT( memcmp( buf, check_buf, pem_len - 1 ) == 0 ); +#endif der_len = mbedtls_x509write_csr_der( &req, buf, sizeof( buf ), mbedtls_test_rnd_pseudo_rand, @@ -133,13 +145,22 @@ void x509_csr_check( char * key_file, char * cert_req_check_file, int md_type, if( der_len == 0 ) goto exit; - ret = mbedtls_x509write_csr_der( &req, buf, (size_t)( der_len - 1 ), +#if defined(MBEDTLS_USE_PSA_CRYPTO) + // When using PSA crypto, RNG isn't controllable, result length isn't + // deterministic over multiple runs, removing a single byte isn't enough to + // go into the MBEDTLS_ERR_ASN1_BUF_TOO_SMALL error case + der_len /= 2; +#else + der_len -= 1; +#endif + ret = mbedtls_x509write_csr_der( &req, buf, (size_t)( der_len ), mbedtls_test_rnd_pseudo_rand, &rnd_info ); TEST_ASSERT( ret == MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); exit: mbedtls_x509write_csr_free( &req ); mbedtls_pk_free( &key ); + USE_PSA_DONE( ); } /* END_CASE */ From cf5a215a434aab6360d05b2cf7279be005ba65a8 Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Thu, 24 Feb 2022 16:58:54 +0100 Subject: [PATCH 03/14] Check psa_destroy_key() return in rsa_verify_wrap() Signed-off-by: Neil Armstrong --- library/pk_wrap.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 2d4a1e286..7acaf33ad 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -716,7 +716,10 @@ static int ecdsa_verify_wrap( void *ctx_arg, mbedtls_md_type_t md_alg, ret = 0; cleanup: - psa_destroy_key( key_id ); + status = psa_destroy_key( key_id ); + if( ret == 0 && status != PSA_SUCCESS ) + ret = mbedtls_psa_err_translate_pk( status ); + return( ret ); } #else /* MBEDTLS_USE_PSA_CRYPTO */ From 5874aa38f7f4d58bb51bb702d5cda45427ee9ab6 Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Tue, 1 Mar 2022 10:08:02 +0100 Subject: [PATCH 04/14] Fix style issue in find_ecdsa_private_key() Signed-off-by: Neil Armstrong --- 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 7acaf33ad..5e3580ec6 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -744,7 +744,7 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, static int pk_ecdsa_sig_asn1_from_psa( unsigned char *sig, size_t *sig_len, size_t buf_len ); -static int find_ecdsa_private_key( unsigned char **buf, unsigned char *end, size_t *key_len) +static int find_ecdsa_private_key( unsigned char **buf, unsigned char *end, size_t *key_len ) { size_t len; int ret; From 15021659d142a9fc2693e638abfc349aec90daf5 Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Tue, 1 Mar 2022 10:14:17 +0100 Subject: [PATCH 05/14] Move pk_ecdsa_sig_asn1_from_psa() before ecdsa_sign_wrap() Signed-off-by: Neil Armstrong --- library/pk_wrap.c | 167 ++++++++++++++++++++++------------------------ 1 file changed, 80 insertions(+), 87 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 5e3580ec6..095b132c8 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -741,8 +741,87 @@ static int ecdsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, #endif /* MBEDTLS_USE_PSA_CRYPTO */ #if defined(MBEDTLS_USE_PSA_CRYPTO) +/* + * Simultaneously convert and move raw MPI from the beginning of a buffer + * to an ASN.1 MPI at the end of the buffer. + * See also mbedtls_asn1_write_mpi(). + * + * p: pointer to the end of the output buffer + * start: start of the output buffer, and also of the mpi to write at the end + * n_len: length of the mpi to read from start + */ +static int asn1_write_mpibuf( unsigned char **p, unsigned char *start, + size_t n_len ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + size_t len = 0; + + if( (size_t)( *p - start ) < n_len ) + return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); + + len = n_len; + *p -= len; + memmove( *p, start, len ); + + /* ASN.1 DER encoding requires minimal length, so skip leading 0s. + * Neither r nor s should be 0, but as a failsafe measure, still detect + * that rather than overflowing the buffer in case of a PSA error. */ + while( len > 0 && **p == 0x00 ) + { + ++(*p); + --len; + } + + /* this is only reached if the signature was invalid */ + if( len == 0 ) + return( MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED ); + + /* if the msb is 1, ASN.1 requires that we prepend a 0. + * Neither r nor s can be 0, so we can assume len > 0 at all times. */ + if( **p & 0x80 ) + { + if( *p - start < 1 ) + return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); + + *--(*p) = 0x00; + len += 1; + } + + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( p, start, len ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( p, start, + MBEDTLS_ASN1_INTEGER ) ); + + return( (int) len ); +} + +/* Transcode signature from PSA format to ASN.1 sequence. + * See ecdsa_signature_to_asn1 in ecdsa.c, but with byte buffers instead of + * MPIs, and in-place. + * + * [in/out] sig: the signature pre- and post-transcoding + * [in/out] sig_len: signature length pre- and post-transcoding + * [int] buf_len: the available size the in/out buffer + */ static int pk_ecdsa_sig_asn1_from_psa( unsigned char *sig, size_t *sig_len, - size_t buf_len ); + size_t buf_len ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + size_t len = 0; + const size_t rs_len = *sig_len / 2; + unsigned char *p = sig + buf_len; + + MBEDTLS_ASN1_CHK_ADD( len, asn1_write_mpibuf( &p, sig + rs_len, rs_len ) ); + MBEDTLS_ASN1_CHK_ADD( len, asn1_write_mpibuf( &p, sig, rs_len ) ); + + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &p, sig, len ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &p, sig, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ); + + memmove( sig, p, len ); + *sig_len = len; + + return( 0 ); +} static int find_ecdsa_private_key( unsigned char **buf, unsigned char *end, size_t *key_len ) { @@ -1118,92 +1197,6 @@ static int pk_opaque_can_do( mbedtls_pk_type_t type ) type == MBEDTLS_PK_ECDSA ); } -#if defined(MBEDTLS_ECDSA_C) - -/* - * Simultaneously convert and move raw MPI from the beginning of a buffer - * to an ASN.1 MPI at the end of the buffer. - * See also mbedtls_asn1_write_mpi(). - * - * p: pointer to the end of the output buffer - * start: start of the output buffer, and also of the mpi to write at the end - * n_len: length of the mpi to read from start - */ -static int asn1_write_mpibuf( unsigned char **p, unsigned char *start, - size_t n_len ) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t len = 0; - - if( (size_t)( *p - start ) < n_len ) - return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); - - len = n_len; - *p -= len; - memmove( *p, start, len ); - - /* ASN.1 DER encoding requires minimal length, so skip leading 0s. - * Neither r nor s should be 0, but as a failsafe measure, still detect - * that rather than overflowing the buffer in case of a PSA error. */ - while( len > 0 && **p == 0x00 ) - { - ++(*p); - --len; - } - - /* this is only reached if the signature was invalid */ - if( len == 0 ) - return( MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED ); - - /* if the msb is 1, ASN.1 requires that we prepend a 0. - * Neither r nor s can be 0, so we can assume len > 0 at all times. */ - if( **p & 0x80 ) - { - if( *p - start < 1 ) - return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); - - *--(*p) = 0x00; - len += 1; - } - - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( p, start, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( p, start, - MBEDTLS_ASN1_INTEGER ) ); - - return( (int) len ); -} - -/* Transcode signature from PSA format to ASN.1 sequence. - * See ecdsa_signature_to_asn1 in ecdsa.c, but with byte buffers instead of - * MPIs, and in-place. - * - * [in/out] sig: the signature pre- and post-transcoding - * [in/out] sig_len: signature length pre- and post-transcoding - * [int] buf_len: the available size the in/out buffer - */ -static int pk_ecdsa_sig_asn1_from_psa( unsigned char *sig, size_t *sig_len, - size_t buf_len ) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t len = 0; - const size_t rs_len = *sig_len / 2; - unsigned char *p = sig + buf_len; - - MBEDTLS_ASN1_CHK_ADD( len, asn1_write_mpibuf( &p, sig + rs_len, rs_len ) ); - MBEDTLS_ASN1_CHK_ADD( len, asn1_write_mpibuf( &p, sig, rs_len ) ); - - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &p, sig, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &p, sig, - MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ); - - memmove( sig, p, len ); - *sig_len = len; - - return( 0 ); -} - -#endif /* MBEDTLS_ECDSA_C */ - static int pk_opaque_sign_wrap( void *ctx, 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, From dab14de96aa84cec18865f96dde4f110216d36fb Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Tue, 1 Mar 2022 14:00:49 +0100 Subject: [PATCH 06/14] Use now shared ECP_PRV_DER_MAX_BYTES define in pk_wrap.c Signed-off-by: Neil Armstrong --- library/pk_wrap.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 095b132c8..b855cc171 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -861,8 +861,7 @@ static int ecdsa_sign_wrap( void *ctx_arg, mbedtls_md_type_t md_alg, psa_status_t status; mbedtls_pk_context key; size_t key_len; - /* see ECP_PRV_DER_MAX_BYTES in pkwrite.c */ - unsigned char buf[29 + 3 * MBEDTLS_ECP_MAX_BYTES]; + unsigned char buf[MBEDTLS_PK_ECP_PRV_DER_MAX_BYTES]; unsigned char *p; mbedtls_pk_info_t pk_info = mbedtls_eckey_info; psa_algorithm_t psa_sig_md = PSA_ALG_ECDSA( mbedtls_psa_translate_md( md_alg ) ); From edcc73c99286e4b05d1fbdedf2b9d1b5894972cd Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Thu, 3 Mar 2022 12:34:14 +0100 Subject: [PATCH 07/14] Fix 80 characters indentation in ecdsa_sign_wrap() Signed-off-by: Neil Armstrong --- library/pk_wrap.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index b855cc171..90964fe43 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -823,13 +823,15 @@ static int pk_ecdsa_sig_asn1_from_psa( unsigned char *sig, size_t *sig_len, return( 0 ); } -static int find_ecdsa_private_key( unsigned char **buf, unsigned char *end, size_t *key_len ) +static int find_ecdsa_private_key( unsigned char **buf, unsigned char *end, + size_t *key_len ) { size_t len; int ret; if( ( ret = mbedtls_asn1_get_tag( buf, end, &len, - MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 ) + MBEDTLS_ASN1_CONSTRUCTED | + MBEDTLS_ASN1_SEQUENCE ) ) != 0 ) return( ret ); /* version */ @@ -864,7 +866,8 @@ static int ecdsa_sign_wrap( void *ctx_arg, mbedtls_md_type_t md_alg, unsigned char buf[MBEDTLS_PK_ECP_PRV_DER_MAX_BYTES]; unsigned char *p; mbedtls_pk_info_t pk_info = mbedtls_eckey_info; - psa_algorithm_t psa_sig_md = PSA_ALG_ECDSA( mbedtls_psa_translate_md( md_alg ) ); + psa_algorithm_t psa_sig_md = + PSA_ALG_ECDSA( mbedtls_psa_translate_md( md_alg ) ); size_t curve_bits; psa_ecc_family_t curve = mbedtls_ecc_group_to_psa( ctx->grp.id, &curve_bits ); From ff70f0bf77d18058a209dca8cba5213570b0ce00 Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Thu, 3 Mar 2022 14:31:17 +0100 Subject: [PATCH 08/14] Check psa_destroy_key() return in rsa_sign_wrap() Signed-off-by: Neil Armstrong --- library/pk_wrap.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 90964fe43..df67a404e 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -917,7 +917,10 @@ static int ecdsa_sign_wrap( void *ctx_arg, mbedtls_md_type_t md_alg, ret = pk_ecdsa_sig_asn1_from_psa( sig, sig_len, sig_size ); cleanup: - psa_destroy_key( key_id ); + status = psa_destroy_key( key_id ); + if( ret == 0 && status != PSA_SUCCESS ) + ret = mbedtls_psa_err_translate_pk( status ); + return( ret ); } #else From e4edcf761df50cf2d384e6d12ae59861c58fbea8 Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Thu, 3 Mar 2022 16:46:41 +0100 Subject: [PATCH 09/14] Use new PSA to mbedtls PK error mapping functions in ecdsa_sign_wrap() Signed-off-by: Neil Armstrong --- 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 df67a404e..53cf7cbed 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -718,7 +718,7 @@ static int ecdsa_verify_wrap( void *ctx_arg, mbedtls_md_type_t md_alg, cleanup: status = psa_destroy_key( key_id ); if( ret == 0 && status != PSA_SUCCESS ) - ret = mbedtls_psa_err_translate_pk( status ); + ret = mbedtls_pk_error_from_psa( status ); return( ret ); } @@ -902,15 +902,15 @@ static int ecdsa_sign_wrap( void *ctx_arg, mbedtls_md_type_t md_alg, &key_id ); if( status != PSA_SUCCESS ) { - ret = mbedtls_psa_err_translate_pk( status ); + ret = mbedtls_pk_error_from_psa( status ); goto cleanup; } - if( psa_sign_hash( key_id, psa_sig_md, hash, hash_len, - sig, sig_size, sig_len) - != PSA_SUCCESS ) + status = psa_sign_hash( key_id, psa_sig_md, hash, hash_len, + sig, sig_size, sig_len ); + if( status != PSA_SUCCESS ) { - ret = MBEDTLS_ERR_ECP_VERIFY_FAILED; + ret = mbedtls_pk_error_from_psa_ecdca( status ); goto cleanup; } @@ -919,7 +919,7 @@ static int ecdsa_sign_wrap( void *ctx_arg, mbedtls_md_type_t md_alg, cleanup: status = psa_destroy_key( key_id ); if( ret == 0 && status != PSA_SUCCESS ) - ret = mbedtls_psa_err_translate_pk( status ); + ret = mbedtls_pk_error_from_psa( status ); return( ret ); } From 3aca61fdfc5332dcdfba8066e043e44c7823dd9d Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Mon, 14 Mar 2022 14:24:48 +0100 Subject: [PATCH 10/14] Zeroise stack buffer containing private key Signed-off-by: Neil Armstrong --- library/pk_wrap.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 53cf7cbed..59ec307c4 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -917,6 +917,7 @@ static int ecdsa_sign_wrap( void *ctx_arg, mbedtls_md_type_t md_alg, ret = pk_ecdsa_sig_asn1_from_psa( sig, sig_len, sig_size ); cleanup: + mbedtls_platform_zeroize( buf, sizeof( buf ) ); status = psa_destroy_key( key_id ); if( ret == 0 && status != PSA_SUCCESS ) ret = mbedtls_pk_error_from_psa( status ); From cb753a6945e99c14668ab0b9bdde4bad8a5bbfce Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Wed, 16 Mar 2022 15:40:20 +0100 Subject: [PATCH 11/14] Use mbedtls_eckey_info directly in ecdsa_sign_wrap() Signed-off-by: Neil Armstrong --- library/pk_wrap.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 59ec307c4..4df2a2f11 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -865,7 +865,6 @@ static int ecdsa_sign_wrap( void *ctx_arg, mbedtls_md_type_t md_alg, size_t key_len; unsigned char buf[MBEDTLS_PK_ECP_PRV_DER_MAX_BYTES]; unsigned char *p; - mbedtls_pk_info_t pk_info = mbedtls_eckey_info; psa_algorithm_t psa_sig_md = PSA_ALG_ECDSA( mbedtls_psa_translate_md( md_alg ) ); size_t curve_bits; @@ -882,7 +881,7 @@ static int ecdsa_sign_wrap( void *ctx_arg, mbedtls_md_type_t md_alg, /* mbedtls_pk_write_key_der() expects a full PK context; * re-construct one to make it happy */ - key.pk_info = &pk_info; + 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 ) From 05132ed490ba156aa3000c5d352b70f774bb329c Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Wed, 16 Mar 2022 15:40:46 +0100 Subject: [PATCH 12/14] md_alg is used in ecdsa_sign_wrap(), cleanup code Signed-off-by: Neil Armstrong --- library/pk_wrap.c | 1 - 1 file changed, 1 deletion(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 4df2a2f11..663b72d88 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -870,7 +870,6 @@ static int ecdsa_sign_wrap( void *ctx_arg, mbedtls_md_type_t md_alg, size_t curve_bits; psa_ecc_family_t curve = mbedtls_ecc_group_to_psa( ctx->grp.id, &curve_bits ); - ((void) md_alg); /* PSA has its own RNG */ ((void) f_rng); From 17a0655c8d72fc54f432472e5e58beeaa5eae21e Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Fri, 18 Mar 2022 15:27:38 +0100 Subject: [PATCH 13/14] Add documentation to find_ecdsa_private_key() Signed-off-by: Neil Armstrong --- library/pk_wrap.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 663b72d88..5615fb32f 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -823,12 +823,29 @@ 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 ) From c23d2e3ef125a16f21ee5c62d856e0ed185f3ac0 Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Fri, 18 Mar 2022 15:31:59 +0100 Subject: [PATCH 14/14] Wrap unused declaration in #if/#endif when USE_PSA is set in x509_csr_check() Signed-off-by: Neil Armstrong --- tests/suites/test_suite_x509write.function | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index 7503b5e81..4aaacd201 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -83,11 +83,14 @@ void x509_csr_check( char * key_file, char * cert_req_check_file, int md_type, mbedtls_pk_context key; mbedtls_x509write_csr req; unsigned char buf[4096]; - unsigned char check_buf[4000]; int ret; - size_t olen = 0, pem_len = 0, buf_index; - int der_len = -1; +#if !defined(MBEDTLS_USE_PSA_CRYPTO) + unsigned char check_buf[4000]; FILE *f; + size_t olen = 0; +#endif /* !MBEDTLS_USE_PSA_CRYPTO */ + size_t pem_len = 0, buf_index; + int der_len = -1; const char *subject_name = "C=NL,O=PolarSSL,CN=PolarSSL Server 1"; mbedtls_test_rnd_pseudo_info rnd_info; @@ -121,9 +124,6 @@ void x509_csr_check( char * key_file, char * cert_req_check_file, int md_type, #if defined(MBEDTLS_USE_PSA_CRYPTO) // When using PSA crypto, RNG isn't controllable, so cert_req_check_file can't be used - (void)f; - (void)olen; - (void)check_buf; (void)cert_req_check_file; buf[pem_len] = '\0'; TEST_ASSERT( x509_crt_verifycsr( buf, pem_len + 1 ) == 0 ); @@ -135,7 +135,7 @@ void x509_csr_check( char * key_file, char * cert_req_check_file, int md_type, TEST_ASSERT( olen >= pem_len - 1 ); TEST_ASSERT( memcmp( buf, check_buf, pem_len - 1 ) == 0 ); -#endif +#endif /* MBEDTLS_USE_PSA_CRYPTO */ der_len = mbedtls_x509write_csr_der( &req, buf, sizeof( buf ), mbedtls_test_rnd_pseudo_rand,