From cbb590369c554c294b885c52b8a57774d810a1d9 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 Mar 2019 14:14:22 +0000 Subject: [PATCH] Minor fixes to CA callback tests --- programs/ssl/ssl_client2.c | 57 ++++++++++++++++---- programs/ssl/ssl_server2.c | 55 +++++++++++++++---- tests/suites/test_suite_x509parse.data | 2 +- tests/suites/test_suite_x509parse.function | 63 +++++++++++++++++----- 4 files changed, 145 insertions(+), 32 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 2e297e362..0540c6589 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -397,7 +397,7 @@ struct options int psk_opaque; #endif #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) - int use_ca_callback /* Use a callback for a trusted certificate list */ + int ca_callback; /* Use callback for trusted certificate list */ #endif const char *psk; /* the pre-shared key */ const char *psk_identity; /* the pre-shared key identity */ @@ -453,21 +453,58 @@ static void my_debug( void *ctx, int level, } #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) -int ca_callback( void *data, mbedtls_x509_crt *child, mbedtls_x509_crt **candidates) +int ca_callback( void *data, mbedtls_x509_crt const *child, + mbedtls_x509_crt **candidates) { + int ret = 0; mbedtls_x509_crt *ca = (mbedtls_x509_crt *) data; - - mbedtls_x509_crt *first = mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) ); - TEST_ASSERT( first != NULL); - TEST_ASSERT( mbedtls_x509_crt_init( first ) == 0 ); - TEST_ASSERT( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) == 0); + mbedtls_x509_crt *first; + + /* This is a test-only implementation of the CA callback + * which always returns the entire list of trusted certificates. + * Production implementations managing a large number of CAs + * should use an efficient presentation and lookup for the + * set of trusted certificates (such as a hashtable) and only + * return those trusted certificates which satisfy basic + * parental checks, such as the matching of child `Issuer` + * and parent `Subject` field. */ + ((void) child); + + first = mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) ); + if( first == NULL ) + { + ret = -1; + goto exit; + } + mbedtls_x509_crt_init( first ); + + if( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) != 0 ) + { + ret = -1; + goto exit; + } + while( ca->next != NULL ) { ca = ca->next; - TEST_ASSERT( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) == 0); + if( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) != 0 ) + { + ret = -1; + goto exit; + } } + +exit: + + if( ret != 0 ) + { + mbedtls_x509_crt_free( first ); + mbedtls_free( first ); + first = NULL; + } + *candidates = first; - return 0; + return( ret ); } #endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */ @@ -1641,7 +1678,7 @@ int main( int argc, char *argv[] ) { #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) if( opt.ca_callback != 0 ) - mbedtls_ssl_conf_ca_cb( &conf, ca_callback, &cacert); + mbedtls_ssl_conf_ca_cb( &conf, ca_callback, &cacert ); else #endif mbedtls_ssl_conf_ca_chain( &conf, &cacert, NULL ); diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 887fe4e96..476a6728a 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -516,7 +516,7 @@ struct options int psk_list_opaque; #endif #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) - int use_ca_callback /* Use a callback for a trusted certificate list */ + int ca_callback; /* Use callback for trusted certificate list */ #endif const char *psk; /* the pre-shared key */ const char *psk_identity; /* the pre-shared key identity */ @@ -576,21 +576,58 @@ static void my_debug( void *ctx, int level, } #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) -int ca_callback( void *data, mbedtls_x509_crt *child, mbedtls_x509_crt **candidates) +int ca_callback( void *data, mbedtls_x509_crt const *child, + mbedtls_x509_crt **candidates) { + int ret = 0; mbedtls_x509_crt *ca = (mbedtls_x509_crt *) data; - - mbedtls_x509_crt *first = mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) ); - TEST_ASSERT( first != NULL); - TEST_ASSERT( mbedtls_x509_crt_init( first ) == 0 ); - TEST_ASSERT( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) == 0); + mbedtls_x509_crt *first; + + /* This is a test-only implementation of the CA callback + * which always returns the entire list of trusted certificates. + * Production implementations managing a large number of CAs + * should use an efficient presentation and lookup for the + * set of trusted certificates (such as a hashtable) and only + * return those trusted certificates which satisfy basic + * parental checks, such as the matching of child `Issuer` + * and parent `Subject` field. */ + ((void) child); + + first = mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) ); + if( first == NULL ) + { + ret = -1; + goto exit; + } + mbedtls_x509_crt_init( first ); + + if( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) != 0 ) + { + ret = -1; + goto exit; + } + while( ca->next != NULL ) { ca = ca->next; - TEST_ASSERT( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) == 0); + if( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) != 0 ) + { + ret = -1; + goto exit; + } } + +exit: + + if( ret != 0 ) + { + mbedtls_x509_crt_free( first ); + mbedtls_free( first ); + first = NULL; + } + *candidates = first; - return 0; + return( ret ); } #endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */ diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 38a75daf1..edd3a6fc5 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -829,7 +829,7 @@ x509_verify:"data_files/cert_sha256.crt":"data_files/test-ca.crt":"data_files/cr X509 Certificate verification with ca callback: failure depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_SHA1_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK -x509_verify_ca_cb_failure:"data_files/server1.crt":"data_files/test-ca.crt":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:"depth 1 - serial 03 - subject C=NL, O=PolarSSL, CN=PolarSSL Test CA - flags 0x00000000\ndepth 0 - serial 01 - subject C=NL, O=PolarSSL, CN=PolarSSL Server 1 - flags 0x00000000\n" +x509_verify_ca_cb_failure:"data_files/server1.crt":"data_files/test-ca.crt":"NULL":MBEDTLS_ERR_X509_FATAL_ERROR X509 Certificate verification callback: bad name depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index d7745a901..e90ddc41d 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -69,7 +69,7 @@ int verify_all( void *data, mbedtls_x509_crt *crt, int certificate_depth, uint32 } #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) -int ca_callback_fail( void *data, mbedtls_x509_crt *child, mbedtls_x509_crt **candidates) +int ca_callback_fail( void *data, mbedtls_x509_crt const *child, mbedtls_x509_crt **candidates) { ((void) data); ((void) child); @@ -78,21 +78,58 @@ int ca_callback_fail( void *data, mbedtls_x509_crt *child, mbedtls_x509_crt **ca return -1; } -int ca_callback( void *data, mbedtls_x509_crt *child, mbedtls_x509_crt **candidates) +int ca_callback( void *data, mbedtls_x509_crt const *child, + mbedtls_x509_crt **candidates) { + int ret = 0; mbedtls_x509_crt *ca = (mbedtls_x509_crt *) data; - - mbedtls_x509_crt *first = mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) ); - TEST_ASSERT( first != NULL); - TEST_ASSERT( mbedtls_x509_crt_init( first ) == 0 ); - TEST_ASSERT( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) == 0); + mbedtls_x509_crt *first; + + /* This is a test-only implementation of the CA callback + * which always returns the entire list of trusted certificates. + * Production implementations managing a large number of CAs + * should use an efficient presentation and lookup for the + * set of trusted certificates (such as a hashtable) and only + * return those trusted certificates which satisfy basic + * parental checks, such as the matching of child `Issuer` + * and parent `Subject` field. */ + ((void) child); + + first = mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) ); + if( first == NULL ) + { + ret = -1; + goto exit; + } + mbedtls_x509_crt_init( first ); + + if( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) != 0 ) + { + ret = -1; + goto exit; + } + while( ca->next != NULL ) { ca = ca->next; - TEST_ASSERT( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) == 0); + if( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) != 0 ) + { + ret = -1; + goto exit; + } } + +exit: + + if( ret != 0 ) + { + mbedtls_x509_crt_free( first ); + mbedtls_free( first ); + first = NULL; + } + *candidates = first; - return 0; + return( ret ); } #endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */ @@ -419,7 +456,7 @@ exit: /* BEGIN_CASE depends_on:MBEDTLS_FS_IO:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_X509_CRL_PARSE_C:MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */ void x509_verify_ca_cb_failure( char *crt_file, char *ca_file, char *name, - int exp_ret, char *exp_vrfy_out ) + int exp_ret ) { int ret; mbedtls_x509_crt crt; @@ -434,8 +471,10 @@ void x509_verify_ca_cb_failure( char *crt_file, char *ca_file, char *name, if( strcmp( name, "NULL" ) == 0 ) name = NULL; - - ret = mbedtls_x509_crt_verify_with_cb( &crt, ca_callback_fail, &ca, &compat_profile, name, &flags, verify_all, NULL ); + + ret = mbedtls_x509_crt_verify_with_cb( &crt, ca_callback_fail, &ca, + &compat_profile, name, &flags, + verify_all, NULL ); TEST_ASSERT( ret == exp_ret ); exit: