From 5fec0860f91457d82992a940b0c0e1d6952544c4 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 19 Nov 2018 10:07:36 -0500 Subject: [PATCH 1/6] pkwrite: add opaque key handling for public key exporting Return early from mbedtls_pk_write_pubkey_der - public opaque key exporting is expected to contain all of the needed data, therefore it shouldn't be written again. --- library/pkwrite.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/library/pkwrite.c b/library/pkwrite.c index 8eabd889b..d135060e4 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -46,6 +46,9 @@ #include "mbedtls/pem.h" #endif +#if defined(MBEDTLS_USE_PSA_CRYPTO) +#include "psa/crypto.h" +#endif #if defined(MBEDTLS_PLATFORM_C) #include "mbedtls/platform.h" #else @@ -161,6 +164,23 @@ int mbedtls_pk_write_pubkey( unsigned char **p, unsigned char *start, MBEDTLS_ASN1_CHK_ADD( len, pk_write_ec_pubkey( p, start, mbedtls_pk_ec( *key ) ) ); else #endif +#if defined(MBEDTLS_USE_PSA_CRYPTO) + if( mbedtls_pk_get_type( key ) == MBEDTLS_PK_OPAQUE ) + { + size_t buffer_size = *p - start; + psa_key_slot_t* key_slot = ( psa_key_slot_t* ) key->pk_ctx; + if ( psa_export_public_key( *key_slot, start, buffer_size, &len ) + != PSA_SUCCESS ) + { + return MBEDTLS_ERR_PK_BAD_INPUT_DATA; + } + else + { + memmove( *p - len, start, len ); + } + } + else +#endif /* MBEDTLS_USE_PSA_CRYPTO */ return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE ); return( (int) len ); @@ -177,6 +197,10 @@ int mbedtls_pk_write_pubkey_der( mbedtls_pk_context *key, unsigned char *buf, si 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 ); From 5f7bad34bbc61de271922b99cae807f59d7d9c98 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 19 Nov 2018 10:12:37 -0500 Subject: [PATCH 2/6] Add CSR write testing using opaque keys Parse and verify CSR programatically instead of using predetermined data, to not tamper with randomness in tests. --- tests/suites/test_suite_x509write.data | 4 ++ tests/suites/test_suite_x509write.function | 75 ++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/tests/suites/test_suite_x509write.data b/tests/suites/test_suite_x509write.data index 5b54d8588..c932c6816 100644 --- a/tests/suites/test_suite_x509write.data +++ b/tests/suites/test_suite_x509write.data @@ -42,6 +42,10 @@ Certificate Request check Server5 ECDSA, key_usage depends_on:MBEDTLS_SHA1_C:MBEDTLS_ECDSA_C:MBEDTLS_ECDSA_DETERMINISTIC:MBEDTLS_ECP_DP_SECP256R1_ENABLED x509_csr_check:"data_files/server5.key":"data_files/server5.req.ku.sha1":MBEDTLS_MD_SHA1:MBEDTLS_X509_KU_DIGITAL_SIGNATURE | MBEDTLS_X509_KU_NON_REPUDIATION:0 +Certificate Request check opaque Server5 ECDSA, key_usage +depends_on:MBEDTLS_SHA256_C:MBEDTLS_ECDSA_C:MBEDTLS_ECDSA_DETERMINISTIC:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_USE_PSA_CRYPTO +x509_csr_check_opaque:"data_files/server5.key":MBEDTLS_MD_SHA256:MBEDTLS_X509_KU_DIGITAL_SIGNATURE | MBEDTLS_X509_KU_NON_REPUDIATION:0 + Certificate write check Server1 SHA1 depends_on:MBEDTLS_SHA1_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_MD5_C x509_crt_check:"data_files/server1.key":"":"C=NL,O=PolarSSL,CN=PolarSSL Server 1":"data_files/test-ca.key":"PolarSSLTest":"C=NL,O=PolarSSL,CN=PolarSSL Test CA":"1":"20110212144406":"20210212144406":MBEDTLS_MD_SHA1:0:0:1:-1:"data_files/server1.crt":0 diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index c00b1aca8..f1aeaa0c6 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -5,6 +5,11 @@ #include "mbedtls/pem.h" #include "mbedtls/oid.h" #include "mbedtls/rsa.h" +#if defined(MBEDTLS_USE_PSA_CRYPTO) +#include "psa/crypto.h" +#include "mbedtls/psa_util.h" +#endif + #if defined(MBEDTLS_RSA_C) int mbedtls_rsa_decrypt_func( void *ctx, int mode, size_t *olen, @@ -28,6 +33,29 @@ size_t mbedtls_rsa_key_len_func( void *ctx ) } #endif /* MBEDTLS_RSA_C */ +#if defined(MBEDTLS_USE_PSA_CRYPTO) +static int x509_crt_verifycsr( const unsigned char *buf, size_t buflen ) +{ + unsigned char hash[MBEDTLS_MD_MAX_SIZE]; + const mbedtls_md_info_t *md_info; + mbedtls_x509_csr csr; + + if( mbedtls_x509_csr_parse( &csr, buf, buflen ) != 0 ) + return( MBEDTLS_ERR_X509_BAD_INPUT_DATA ); + + md_info = mbedtls_md_info_from_type( csr.sig_md ); + if( mbedtls_md( md_info, csr.cri.p, csr.cri.len, hash ) != 0 ) + return ( MBEDTLS_ERR_X509_BAD_INPUT_DATA );/* Note: this can't happen except after an internal error */ + + if( mbedtls_pk_verify_ext( csr.sig_pk, csr.sig_opts, &csr.pk, + csr.sig_md, hash, mbedtls_md_get_size( md_info ), + csr.sig.p, csr.sig.len ) != 0 ) + return( MBEDTLS_ERR_X509_CERT_VERIFY_FAILED ); + + return( 0 ); +} +#endif /* MBEDTLS_USE_PSA_CRYPTO */ + /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -95,6 +123,53 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_PEM_WRITE_C:MBEDTLS_X509_CSR_WRITE_C:MBEDTLS_USE_PSA_CRYPTO */ +void x509_csr_check_opaque( char *key_file, int md_type, int key_usage, + int cert_type ) +{ + mbedtls_pk_context key; + psa_key_slot_t slot; + psa_algorithm_t md_alg_psa; + mbedtls_x509write_csr req; + unsigned char buf[4096]; + int ret; + size_t pem_len = 0; + const char *subject_name = "C=NL,O=PolarSSL,CN=PolarSSL Server 1"; + rnd_pseudo_info rnd_info; + + psa_crypto_init(); + memset( &rnd_info, 0x2a, sizeof( rnd_pseudo_info ) ); + + md_alg_psa = mbedtls_psa_translate_md( (mbedtls_md_type_t) md_type ); + TEST_ASSERT( md_alg_psa != MBEDTLS_MD_NONE ); + + mbedtls_pk_init( &key ); + TEST_ASSERT( mbedtls_pk_parse_keyfile( &key, key_file, NULL ) == 0 ); + TEST_ASSERT( mbedtls_pk_wrap_as_opaque( &key, &slot, md_alg_psa ) == 0 ); + + mbedtls_x509write_csr_init( &req ); + mbedtls_x509write_csr_set_md_alg( &req, md_type ); + mbedtls_x509write_csr_set_key( &req, &key ); + TEST_ASSERT( mbedtls_x509write_csr_set_subject_name( &req, subject_name ) == 0 ); + if( key_usage != 0 ) + TEST_ASSERT( mbedtls_x509write_csr_set_key_usage( &req, key_usage ) == 0 ); + if( cert_type != 0 ) + TEST_ASSERT( mbedtls_x509write_csr_set_ns_cert_type( &req, cert_type ) == 0 ); + + ret = mbedtls_x509write_csr_pem( &req, buf, sizeof( buf ), + rnd_pseudo_rand, &rnd_info ); + TEST_ASSERT( ret == 0 ); + + pem_len = strlen( (char *) buf ); + buf[pem_len] = '\0'; + TEST_ASSERT( x509_crt_verifycsr( buf, pem_len+1 ) == 0 ); + +exit: + mbedtls_x509write_csr_free( &req ); + mbedtls_pk_free( &key ); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_PEM_WRITE_C:MBEDTLS_X509_CRT_WRITE_C:MBEDTLS_SHA1_C */ void x509_crt_check( char *subject_key_file, char *subject_pwd, char *subject_name, char *issuer_key_file, From 4b1140725871024afe20a1f932bffce2b5cf21f8 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 19 Nov 2018 18:04:01 -0500 Subject: [PATCH 3/6] Cosmetic changes Adjust whitespaces, reduce test dependencies and reduce buffer size passed by 1. --- library/pkwrite.c | 4 ++-- tests/suites/test_suite_x509write.data | 2 +- tests/suites/test_suite_x509write.function | 11 ++++++++--- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/library/pkwrite.c b/library/pkwrite.c index d135060e4..dcd3263b2 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -168,11 +168,11 @@ int mbedtls_pk_write_pubkey( unsigned char **p, unsigned char *start, if( mbedtls_pk_get_type( key ) == MBEDTLS_PK_OPAQUE ) { size_t buffer_size = *p - start; - psa_key_slot_t* key_slot = ( psa_key_slot_t* ) key->pk_ctx; + psa_key_slot_t* key_slot = (psa_key_slot_t*) key->pk_ctx; if ( psa_export_public_key( *key_slot, start, buffer_size, &len ) != PSA_SUCCESS ) { - return MBEDTLS_ERR_PK_BAD_INPUT_DATA; + return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); } else { diff --git a/tests/suites/test_suite_x509write.data b/tests/suites/test_suite_x509write.data index c932c6816..40964258b 100644 --- a/tests/suites/test_suite_x509write.data +++ b/tests/suites/test_suite_x509write.data @@ -43,7 +43,7 @@ depends_on:MBEDTLS_SHA1_C:MBEDTLS_ECDSA_C:MBEDTLS_ECDSA_DETERMINISTIC:MBEDTLS_EC x509_csr_check:"data_files/server5.key":"data_files/server5.req.ku.sha1":MBEDTLS_MD_SHA1:MBEDTLS_X509_KU_DIGITAL_SIGNATURE | MBEDTLS_X509_KU_NON_REPUDIATION:0 Certificate Request check opaque Server5 ECDSA, key_usage -depends_on:MBEDTLS_SHA256_C:MBEDTLS_ECDSA_C:MBEDTLS_ECDSA_DETERMINISTIC:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_USE_PSA_CRYPTO +depends_on:MBEDTLS_SHA256_C:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED x509_csr_check_opaque:"data_files/server5.key":MBEDTLS_MD_SHA256:MBEDTLS_X509_KU_DIGITAL_SIGNATURE | MBEDTLS_X509_KU_NON_REPUDIATION:0 Certificate write check Server1 SHA1 diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index f1aeaa0c6..8fe3b841d 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -45,12 +45,17 @@ static int x509_crt_verifycsr( const unsigned char *buf, size_t buflen ) md_info = mbedtls_md_info_from_type( csr.sig_md ); if( mbedtls_md( md_info, csr.cri.p, csr.cri.len, hash ) != 0 ) - return ( MBEDTLS_ERR_X509_BAD_INPUT_DATA );/* Note: this can't happen except after an internal error */ + { + /* Note: this can't happen except after an internal error */ + return( MBEDTLS_ERR_X509_BAD_INPUT_DATA ); + } if( mbedtls_pk_verify_ext( csr.sig_pk, csr.sig_opts, &csr.pk, csr.sig_md, hash, mbedtls_md_get_size( md_info ), csr.sig.p, csr.sig.len ) != 0 ) + { return( MBEDTLS_ERR_X509_CERT_VERIFY_FAILED ); + } return( 0 ); } @@ -156,13 +161,13 @@ void x509_csr_check_opaque( char *key_file, int md_type, int key_usage, if( cert_type != 0 ) TEST_ASSERT( mbedtls_x509write_csr_set_ns_cert_type( &req, cert_type ) == 0 ); - ret = mbedtls_x509write_csr_pem( &req, buf, sizeof( buf ), + ret = mbedtls_x509write_csr_pem( &req, buf, sizeof( buf ) - 1, rnd_pseudo_rand, &rnd_info ); TEST_ASSERT( ret == 0 ); pem_len = strlen( (char *) buf ); buf[pem_len] = '\0'; - TEST_ASSERT( x509_crt_verifycsr( buf, pem_len+1 ) == 0 ); + TEST_ASSERT( x509_crt_verifycsr( buf, pem_len + 1 ) == 0 ); exit: mbedtls_x509write_csr_free( &req ); From 158c3d10d0fafc6aaf83aaf80041e39ce92da299 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 19 Nov 2018 18:09:59 -0500 Subject: [PATCH 4/6] pkwrite: add a safety check before calculating the buffer size --- library/pkwrite.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/library/pkwrite.c b/library/pkwrite.c index dcd3263b2..3dfc590ad 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -167,8 +167,13 @@ int mbedtls_pk_write_pubkey( unsigned char **p, unsigned char *start, #if defined(MBEDTLS_USE_PSA_CRYPTO) if( mbedtls_pk_get_type( key ) == MBEDTLS_PK_OPAQUE ) { - size_t buffer_size = *p - start; + size_t buffer_size; psa_key_slot_t* key_slot = (psa_key_slot_t*) key->pk_ctx; + + if ( *p < start ) + return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + + buffer_size = *p - start; if ( psa_export_public_key( *key_slot, start, buffer_size, &len ) != PSA_SUCCESS ) { From 967cfd18fdc6495ce62fb8f94eb3a78d46606f4a Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 20 Nov 2018 02:53:17 -0500 Subject: [PATCH 5/6] Remove trailing whitespace --- tests/suites/test_suite_x509write.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index 8fe3b841d..bf43a8001 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -147,7 +147,7 @@ void x509_csr_check_opaque( char *key_file, int md_type, int key_usage, md_alg_psa = mbedtls_psa_translate_md( (mbedtls_md_type_t) md_type ); TEST_ASSERT( md_alg_psa != MBEDTLS_MD_NONE ); - + mbedtls_pk_init( &key ); TEST_ASSERT( mbedtls_pk_parse_keyfile( &key, key_file, NULL ) == 0 ); TEST_ASSERT( mbedtls_pk_wrap_as_opaque( &key, &slot, md_alg_psa ) == 0 ); From b7f3ac6504ed9f28bac3f46a5e472e981df80edf Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 20 Nov 2018 03:03:28 -0500 Subject: [PATCH 6/6] pkwrite: add an explicit cast to size_t --- library/pkwrite.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/pkwrite.c b/library/pkwrite.c index 3dfc590ad..d34714b34 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -173,7 +173,7 @@ int mbedtls_pk_write_pubkey( unsigned char **p, unsigned char *start, if ( *p < start ) return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); - buffer_size = *p - start; + buffer_size = (size_t)( *p - start ); if ( psa_export_public_key( *key_slot, start, buffer_size, &len ) != PSA_SUCCESS ) {