From e6706e62d8296d5d9630c0c6edec278f6cc02698 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 15 May 2017 16:05:15 +0100 Subject: [PATCH] Add tests for missing CA chains and bad curves. This commit adds four tests to tests/ssl-opt.sh: (1) & (2): Check behaviour of optional/required verification when the trusted CA chain is empty. (3) & (4): Check behaviour of optional/required verification when the client receives a server certificate with an unsupported curve. --- library/ssl_tls.c | 11 +++++ programs/ssl/ssl_client2.c | 88 ++++++++++++++++++++++++++++++++++++++ programs/ssl/ssl_server2.c | 87 +++++++++++++++++++++++++++++++++++++ tests/ssl-opt.sh | 48 +++++++++++++++++++++ 4 files changed, 234 insertions(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index ff951a536..208500a66 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4569,6 +4569,17 @@ int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl ) alert ); } +#if defined(MBEDTLS_DEBUG_C) + if( ssl->session_negotiate->verify_result != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "! Certificate verification flags %x", + ssl->session_negotiate->verify_result ) ); + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "Certificate verification flags clear" ) ); + } +#endif /* MBEDTLS_DEBUG_C */ } MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate" ) ); diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 212f47a6c..9a75c7f0e 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -98,6 +98,7 @@ int main( void ) #define DFL_RECONNECT_HARD 0 #define DFL_TICKETS MBEDTLS_SSL_SESSION_TICKETS_ENABLED #define DFL_ALPN_STRING NULL +#define DFL_CURVES NULL #define DFL_TRANSPORT MBEDTLS_SSL_TRANSPORT_STREAM #define DFL_HS_TO_MIN 0 #define DFL_HS_TO_MAX 0 @@ -178,6 +179,17 @@ int main( void ) #define USAGE_ALPN "" #endif /* MBEDTLS_SSL_ALPN */ +#if defined(MBEDTLS_ECP_C) +#define USAGE_CURVES \ + " curves=a,b,c,d default: \"default\" (library default)\n" \ + " example: \"secp521r1,brainpoolP512r1\"\n" \ + " - use \"none\" for empty list\n" \ + " - see mbedtls_ecp_curve_list()\n" \ + " for acceptable curve names\n" +#else +#define USAGE_CURVES "" +#endif + #if defined(MBEDTLS_SSL_PROTO_DTLS) #define USAGE_DTLS \ " dtls=%%d default: 0 (TLS)\n" \ @@ -260,6 +272,7 @@ int main( void ) USAGE_FALLBACK \ USAGE_EMS \ USAGE_ETM \ + USAGE_CURVES \ USAGE_RECSPLIT \ USAGE_DHMLEN \ "\n" \ @@ -313,6 +326,7 @@ struct options int reco_delay; /* delay in seconds before resuming session */ int reconnect_hard; /* unexpectedly reconnect from the same port */ int tickets; /* enable / disable session tickets */ + const char *curves; /* list of supported elliptic curves */ const char *alpn_string; /* ALPN supported protocols */ int transport; /* TLS or DTLS? */ uint32_t hs_to_min; /* Initial value of DTLS handshake timer */ @@ -428,6 +442,11 @@ int main( int argc, char *argv[] ) #if defined(MBEDTLS_SSL_ALPN) const char *alpn_list[10]; #endif +#if defined(MBEDTLS_ECP_C) + mbedtls_ecp_group_id curve_list[20]; + const mbedtls_ecp_curve_info *curve_cur; +#endif + const char *pers = "ssl_client2"; #if defined(MBEDTLS_X509_CRT_PARSE_C) @@ -524,6 +543,7 @@ int main( int argc, char *argv[] ) opt.reconnect_hard = DFL_RECONNECT_HARD; opt.tickets = DFL_TICKETS; opt.alpn_string = DFL_ALPN_STRING; + opt.curves = DFL_CURVES; opt.transport = DFL_TRANSPORT; opt.hs_to_min = DFL_HS_TO_MIN; opt.hs_to_max = DFL_HS_TO_MAX; @@ -680,6 +700,8 @@ int main( int argc, char *argv[] ) default: goto usage; } } + else if( strcmp( p, "curves" ) == 0 ) + opt.curves = q; else if( strcmp( p, "etm" ) == 0 ) { switch( atoi( q ) ) @@ -937,6 +959,64 @@ int main( int argc, char *argv[] ) } #endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ +#if defined(MBEDTLS_ECP_C) + if( opt.curves != NULL ) + { + p = (char *) opt.curves; + i = 0; + + if( strcmp( p, "none" ) == 0 ) + { + curve_list[0] = MBEDTLS_ECP_DP_NONE; + } + else if( strcmp( p, "default" ) != 0 ) + { + /* Leave room for a final NULL in curve list */ + while( i < (int) ( sizeof( curve_list ) / sizeof( *curve_list ) ) - 1 + && *p != '\0' ) + { + q = p; + + /* Terminate the current string */ + while( *p != ',' && *p != '\0' ) + p++; + if( *p == ',' ) + *p++ = '\0'; + + if( ( curve_cur = mbedtls_ecp_curve_info_from_name( q ) ) != NULL ) + { + curve_list[i++] = curve_cur->grp_id; + } + else + { + mbedtls_printf( "unknown curve %s\n", q ); + mbedtls_printf( "supported curves: " ); + for( curve_cur = mbedtls_ecp_curve_list(); + curve_cur->grp_id != MBEDTLS_ECP_DP_NONE; + curve_cur++ ) + { + mbedtls_printf( "%s ", curve_cur->name ); + } + mbedtls_printf( "\n" ); + goto exit; + } + } + + mbedtls_printf("Number of curves: %d\n", i ); + + if( i == (int) ( sizeof( curve_list ) / sizeof( *curve_list ) ) - 1 + && *p != '\0' ) + { + mbedtls_printf( "curves list too long, maximum %zu", + (size_t) ( sizeof( curve_list ) / sizeof( *curve_list ) - 1 ) ); + goto exit; + } + + curve_list[i] = MBEDTLS_ECP_DP_NONE; + } + } +#endif /* MBEDTLS_ECP_C */ + #if defined(MBEDTLS_SSL_ALPN) if( opt.alpn_string != NULL ) { @@ -1226,6 +1306,14 @@ int main( int argc, char *argv[] ) } #endif +#if defined(MBEDTLS_ECP_C) + if( opt.curves != NULL && + strcmp( opt.curves, "default" ) != 0 ) + { + mbedtls_ssl_conf_curves( &conf, curve_list ); + } +#endif + #if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) if( ( ret = mbedtls_ssl_conf_psk( &conf, psk, psk_len, (const unsigned char *) opt.psk_identity, diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 05827106c..cbc654e90 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -134,6 +134,7 @@ int main( void ) #define DFL_CACHE_TIMEOUT -1 #define DFL_SNI NULL #define DFL_ALPN_STRING NULL +#define DFL_CURVES NULL #define DFL_DHM_FILE NULL #define DFL_TRANSPORT MBEDTLS_SSL_TRANSPORT_STREAM #define DFL_COOKIES 1 @@ -311,6 +312,17 @@ int main( void ) #define USAGE_ECJPAKE "" #endif +#if defined(MBEDTLS_ECP_C) +#define USAGE_CURVES \ + " curves=a,b,c,d default: \"default\" (library default)\n" \ + " example: \"secp521r1,brainpoolP512r1\"\n" \ + " - use \"none\" for empty list\n" \ + " - see mbedtls_ecp_curve_list()\n" \ + " for acceptable curve names\n" +#else +#define USAGE_CURVES "" +#endif + #define USAGE \ "\n usage: ssl_server2 param=<>...\n" \ "\n acceptable parameters:\n" \ @@ -347,6 +359,7 @@ int main( void ) USAGE_ALPN \ USAGE_EMS \ USAGE_ETM \ + USAGE_CURVES \ "\n" \ " arc4=%%d default: (library default: 0)\n" \ " allow_sha1=%%d default: 0\n" \ @@ -415,6 +428,7 @@ struct options int cache_max; /* max number of session cache entries */ int cache_timeout; /* expiration delay of session cache entries */ char *sni; /* string describing sni information */ + const char *curves; /* list of supported elliptic curves */ const char *alpn_string; /* ALPN supported protocols */ const char *dhm_file; /* the file with the DH parameters */ int extended_ms; /* allow negotiation of extended MS? */ @@ -871,6 +885,10 @@ int main( int argc, char *argv[] ) #if defined(SNI_OPTION) sni_entry *sni_info = NULL; #endif +#if defined(MBEDTLS_ECP_C) + mbedtls_ecp_group_id curve_list[20]; + const mbedtls_ecp_curve_info * curve_cur; +#endif #if defined(MBEDTLS_SSL_ALPN) const char *alpn_list[10]; #endif @@ -982,6 +1000,7 @@ int main( int argc, char *argv[] ) opt.cache_timeout = DFL_CACHE_TIMEOUT; opt.sni = DFL_SNI; opt.alpn_string = DFL_ALPN_STRING; + opt.curves = DFL_CURVES; opt.dhm_file = DFL_DHM_FILE; opt.transport = DFL_TRANSPORT; opt.cookies = DFL_COOKIES; @@ -1060,6 +1079,8 @@ int main( int argc, char *argv[] ) } opt.force_ciphersuite[1] = 0; } + else if( strcmp( p, "curves" ) == 0 ) + opt.curves = q; else if( strcmp( p, "version_suites" ) == 0 ) opt.version_suites = q; else if( strcmp( p, "renegotiation" ) == 0 ) @@ -1419,6 +1440,64 @@ int main( int argc, char *argv[] ) } #endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ +#if defined(MBEDTLS_ECP_C) + if( opt.curves != NULL ) + { + p = (char *) opt.curves; + i = 0; + + if( strcmp( p, "none" ) == 0 ) + { + curve_list[0] = MBEDTLS_ECP_DP_NONE; + } + else if( strcmp( p, "default" ) != 0 ) + { + /* Leave room for a final NULL in curve list */ + while( i < (int) ( sizeof( curve_list ) / sizeof( *curve_list ) ) - 1 + && *p != '\0' ) + { + q = p; + + /* Terminate the current string */ + while( *p != ',' && *p != '\0' ) + p++; + if( *p == ',' ) + *p++ = '\0'; + + if( ( curve_cur = mbedtls_ecp_curve_info_from_name( q ) ) != NULL ) + { + curve_list[i++] = curve_cur->grp_id; + } + else + { + mbedtls_printf( "unknown curve %s\n", q ); + mbedtls_printf( "supported curves: " ); + for( curve_cur = mbedtls_ecp_curve_list(); + curve_cur->grp_id != MBEDTLS_ECP_DP_NONE; + curve_cur++ ) + { + mbedtls_printf( "%s ", curve_cur->name ); + } + mbedtls_printf( "\n" ); + goto exit; + } + } + + mbedtls_printf("Number of curves: %d\n", i ); + + if( i == (int) ( sizeof( curve_list ) / sizeof( *curve_list ) ) - 1 + && *p != '\0' ) + { + mbedtls_printf( "curves list too long, maximum %zu", + (size_t) ( sizeof( curve_list ) / sizeof( *curve_list ) - 1 ) ); + goto exit; + } + + curve_list[i] = MBEDTLS_ECP_DP_NONE; + } + } +#endif /* MBEDTLS_ECP_C */ + #if defined(MBEDTLS_SSL_ALPN) if( opt.alpn_string != NULL ) { @@ -1870,6 +1949,14 @@ int main( int argc, char *argv[] ) mbedtls_ssl_conf_sni( &conf, sni_callback, sni_info ); #endif +#if defined(MBEDTLS_ECP_C) + if( opt.curves != NULL && + strcmp( opt.curves, "default" ) != 0 ) + { + mbedtls_ssl_conf_curves( &conf, curve_list ); + } +#endif + #if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) if( strlen( opt.psk ) != 0 && strlen( opt.psk_identity ) != 0 ) { diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index b57edd7fe..688baa7ef 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1837,6 +1837,54 @@ run_test "Authentication: server badcert, client optional" \ -C "! mbedtls_ssl_handshake returned" \ -C "X509 - Certificate verification failed" +run_test "Authentication: server goodcert, client optional, no trusted CA" \ + "$P_SRV" \ + "$P_CLI debug_level=3 auth_mode=optional ca_file=none ca_path=none" \ + 0 \ + -c "x509_verify_cert() returned" \ + -c "! The certificate is not correctly signed by the trusted CA" \ + -c "! Certificate verification flags"\ + -C "! mbedtls_ssl_handshake returned" \ + -C "X509 - Certificate verification failed" \ + -C "SSL - No CA Chain is set, but required to operate" + +run_test "Authentication: server goodcert, client required, no trusted CA" \ + "$P_SRV" \ + "$P_CLI debug_level=3 auth_mode=required ca_file=none ca_path=none" \ + 1 \ + -c "x509_verify_cert() returned" \ + -c "! The certificate is not correctly signed by the trusted CA" \ + -c "! Certificate verification flags"\ + -c "! mbedtls_ssl_handshake returned" \ + -c "SSL - No CA Chain is set, but required to operate" + +# The purpose of the next two tests is to test the client's behaviour when receiving a server +# certificate with an unsupported elliptic curve. This should usually not happen because +# the client informs the server about the supported curves - it does, though, in the +# corner case of a static ECDH suite, because the server doesn't check the curve on that +# occasion (to be fixed). If that bug's fixed, the test needs to be altered to use a +# different means to have the server ignoring the client's supported curve list. + +requires_config_enabled MBEDTLS_ECP_C +run_test "Authentication: server ECDH p256v1, client required, p256v1 unsupported" \ + "$P_SRV debug_level=1 key_file=data_files/server5.key \ + crt_file=data_files/server5.ku-ka.crt" \ + "$P_CLI debug_level=3 auth_mode=required curves=secp521r1" \ + 1 \ + -c "bad certificate (EC key curve)"\ + -c "! Certificate verification flags"\ + -C "bad server certificate (ECDH curve)" # Expect failure at earlier verification stage + +requires_config_enabled MBEDTLS_ECP_C +run_test "Authentication: server ECDH p256v1, client optional, p256v1 unsupported" \ + "$P_SRV debug_level=1 key_file=data_files/server5.key \ + crt_file=data_files/server5.ku-ka.crt" \ + "$P_CLI debug_level=3 auth_mode=optional curves=secp521r1" \ + 1 \ + -c "bad certificate (EC key curve)"\ + -c "! Certificate verification flags"\ + -c "bad server certificate (ECDH curve)" # Expect failure only at ECDH params check + run_test "Authentication: server badcert, client none" \ "$P_SRV crt_file=data_files/server5-badsign.crt \ key_file=data_files/server5.key" \