From 40a3523eb7735b34c4b4aa79eeeca5e331809c2c Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Sat, 7 May 2022 09:02:40 +0000 Subject: [PATCH 01/16] Add support of server name extension to server side Change-Id: Iccf5017e306ba6ead2e1026a29f397ead084cc4d Signed-off-by: XiaokangQian --- library/ssl_misc.h | 6 +++ library/ssl_tls.c | 51 +++++++++++++++++++++++++ library/ssl_tls12_server.c | 77 +------------------------------------- library/ssl_tls13_server.c | 20 ++++++++++ tests/ssl-opt.sh | 20 ++++++++++ 5 files changed, 99 insertions(+), 75 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 9912d6c29a..f0f4465c86 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2270,4 +2270,10 @@ int mbedtls_ssl_validate_ciphersuite( int mbedtls_ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, size_t *out_len ); +#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) +int mbedtls_ssl_parse_servername_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ); +#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ + #endif /* ssl_misc.h */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 53318650cc..29a33f49e0 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8210,4 +8210,55 @@ int mbedtls_ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, } #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ +#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) +int mbedtls_ssl_parse_servername_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + const unsigned char *p = buf; + size_t servername_list_size, hostname_len; + const unsigned char *servername_end; + + if( ssl->conf->p_sni == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "No SNI callback configured. Skip SNI parsing." ) ); + return( 0 ); + } + + MBEDTLS_SSL_DEBUG_MSG( 3, ( "Parse ServerName extension" ) ); + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); + servername_list_size = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, servername_list_size ); + servername_end = p + servername_list_size; + while ( p < servername_end ) + { + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, servername_end, 3 ); + hostname_len = MBEDTLS_GET_UINT16_BE( p, 1 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, servername_end, hostname_len + 3 ); + + if( p[0] == MBEDTLS_TLS_EXT_SERVERNAME_HOSTNAME ) + { + ret = ssl->conf->f_sni( ssl->conf->p_sni, + ssl, p + 3, hostname_len ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "sni_wrapper", ret ); + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_UNRECOGNIZED_NAME ); + return( MBEDTLS_ERR_SSL_UNRECOGNIZED_NAME ); + } + return( 0 ); + } + + p += hostname_len + 3; + } + + return( 0 ); +} +#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ + #endif /* MBEDTLS_SSL_TLS_C */ diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index d09263344f..e48a8ca004 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -77,80 +77,6 @@ void mbedtls_ssl_conf_dtls_cookies( mbedtls_ssl_config *conf, } #endif /* MBEDTLS_SSL_DTLS_HELLO_VERIFY */ -#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) -static int ssl_parse_servername_ext( mbedtls_ssl_context *ssl, - const unsigned char *buf, - size_t len ) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t servername_list_size, hostname_len; - const unsigned char *p; - - MBEDTLS_SSL_DEBUG_MSG( 3, ( "parse ServerName extension" ) ); - - if( len < 2 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } - servername_list_size = ( ( buf[0] << 8 ) | ( buf[1] ) ); - if( servername_list_size + 2 != len ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } - - p = buf + 2; - while( servername_list_size > 2 ) - { - hostname_len = ( ( p[1] << 8 ) | p[2] ); - if( hostname_len + 3 > servername_list_size ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } - - if( p[0] == MBEDTLS_TLS_EXT_SERVERNAME_HOSTNAME ) - { - ssl->handshake->sni_name = p + 3; - ssl->handshake->sni_name_len = hostname_len; - if( ssl->conf->f_sni == NULL ) - return( 0 ); - - ret = ssl->conf->f_sni( ssl->conf->p_sni, - ssl, p + 3, hostname_len ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_sni_wrapper", ret ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_UNRECOGNIZED_NAME ); - return( MBEDTLS_ERR_SSL_UNRECOGNIZED_NAME ); - } - return( 0 ); - } - - servername_list_size -= hostname_len + 3; - p += hostname_len + 3; - } - - if( servername_list_size != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } - - return( 0 ); -} -#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ - #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) static int ssl_conf_has_psk_or_cb( mbedtls_ssl_config const *conf ) { @@ -1483,7 +1409,8 @@ read_record_header: #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) case MBEDTLS_TLS_EXT_SERVERNAME: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found ServerName extension" ) ); - ret = ssl_parse_servername_ext( ssl, ext + 4, ext_size ); + ret = mbedtls_ssl_parse_servername_ext( ssl, ext + 4, + ext + 4 + ext_size ); if( ret != 0 ) return( ret ); break; diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index f3843b1e85..9d2c8eccac 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -580,6 +580,21 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, switch( extension_type ) { +#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + case MBEDTLS_TLS_EXT_SERVERNAME: + MBEDTLS_SSL_DEBUG_MSG( 3, ( "found ServerName extension" ) ); + ret = mbedtls_ssl_parse_servername_ext( ssl, p, + extension_data_end ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( + 1, "mbedtls_ssl_parse_servername_ext", ret ); + return( ret ); + } + ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_SERVERNAME; + break; +#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ + #if defined(MBEDTLS_ECDH_C) case MBEDTLS_TLS_EXT_SUPPORTED_GROUPS: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported group extension" ) ); @@ -1337,6 +1352,11 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) { int authmode; +#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + if( ssl->handshake->sni_authmode != MBEDTLS_SSL_VERIFY_UNSET ) + authmode = ssl->handshake->sni_authmode; + else +#endif authmode = ssl->conf->authmode; if( authmode == MBEDTLS_SSL_VERIFY_NONE ) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 21d17f9728..4ef37f2bbb 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11398,6 +11398,26 @@ run_test "TLS 1.3: Server side check, no server certificate available" \ -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ -s "No certificate available." +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_SRV_C +requires_config_enabled MBEDTLS_SSL_CLI_C +run_test "TLS 1.3: Server side check - mbedtls with server name indication" \ + "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0 \ + sni=localhost,data_files/server2.crt,data_files/server2.key,-,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ + "$P_CLI debug_level=4 server_name=localhost crt_file=data_files/server5.crt key_file=data_files/server5.key \ + force_version=tls13" \ + 1 \ + -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ + -c "client state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ + -s "Parse ServerName extension" \ + -s "SSL - The requested feature is not available" \ + -s "=> parse client hello" \ + -s "<= parse client hello" + for i in opt-testcases/*.sh do TEST_SUITE_NAME=${i##*/} From 9b2b7716b083afe59bc1af7dfa567095f1b689b6 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 17 May 2022 02:57:00 +0000 Subject: [PATCH 02/16] Change mbedtls_ssl_parse_server_name_ext base on comments Change-Id: I4ae831925cb1899afafb7dc626bfad9be24a5c8c Signed-off-by: XiaokangQian --- library/ssl_misc.h | 6 ++--- library/ssl_tls.c | 53 +++++++++++++++++++++++++++----------- library/ssl_tls12_server.c | 4 +-- library/ssl_tls13_server.c | 4 +-- 4 files changed, 45 insertions(+), 22 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index f0f4465c86..3b5aadb0bd 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2271,9 +2271,9 @@ int mbedtls_ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, size_t *out_len ); #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) -int mbedtls_ssl_parse_servername_ext( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ); +int mbedtls_ssl_parse_server_name_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ); #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ #endif /* ssl_misc.h */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 29a33f49e0..015c38a67f 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8211,44 +8211,67 @@ int mbedtls_ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) -int mbedtls_ssl_parse_servername_ext( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ) +/* + * mbedtls_ssl_parse_server_name_ext + * + * Structure of server_name extension: + * + * enum { + * host_name(0), (255) + * } NameType; + * opaque HostName<1..2^16-1>; + * + * struct { + * NameType name_type; + * select (name_type) { + * case host_name: HostName; + * } name; + * } ServerName; + * struct { + * ServerName server_name_list<1..2^16-1> + * } ServerNameList; + */ +int mbedtls_ssl_parse_server_name_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const unsigned char *p = buf; - size_t servername_list_size, hostname_len; - const unsigned char *servername_end; + size_t server_name_list_len, hostname_len; + const unsigned char *server_name_list_end; if( ssl->conf->p_sni == NULL ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "No SNI callback configured. Skip SNI parsing." ) ); + MBEDTLS_SSL_DEBUG_MSG( + 3, ( "No SNI callback configured. Skip SNI parsing." ) ); return( 0 ); } MBEDTLS_SSL_DEBUG_MSG( 3, ( "Parse ServerName extension" ) ); MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); - servername_list_size = MBEDTLS_GET_UINT16_BE( p, 0 ); + server_name_list_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, servername_list_size ); - servername_end = p + servername_list_size; - while ( p < servername_end ) + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, server_name_list_len ); + server_name_list_end = p + server_name_list_len; + while ( p < server_name_list_end ) { - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, servername_end, 3 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, server_name_list_end, 3 ); hostname_len = MBEDTLS_GET_UINT16_BE( p, 1 ); - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, servername_end, hostname_len + 3 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, server_name_list_end, + hostname_len + 3 ); if( p[0] == MBEDTLS_TLS_EXT_SERVERNAME_HOSTNAME ) { ret = ssl->conf->f_sni( ssl->conf->p_sni, - ssl, p + 3, hostname_len ); + ssl, p + 3, hostname_len ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "sni_wrapper", ret ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_UNRECOGNIZED_NAME ); + mbedtls_ssl_send_alert_message( + ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_UNRECOGNIZED_NAME ); return( MBEDTLS_ERR_SSL_UNRECOGNIZED_NAME ); } return( 0 ); diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index e48a8ca004..854ec637b8 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -1409,8 +1409,8 @@ read_record_header: #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) case MBEDTLS_TLS_EXT_SERVERNAME: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found ServerName extension" ) ); - ret = mbedtls_ssl_parse_servername_ext( ssl, ext + 4, - ext + 4 + ext_size ); + ret = mbedtls_ssl_parse_server_name_ext( ssl, ext + 4, + ext + 4 + ext_size ); if( ret != 0 ) return( ret ); break; diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 9d2c8eccac..7508685fa4 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -583,8 +583,8 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) case MBEDTLS_TLS_EXT_SERVERNAME: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found ServerName extension" ) ); - ret = mbedtls_ssl_parse_servername_ext( ssl, p, - extension_data_end ); + ret = mbedtls_ssl_parse_server_name_ext( ssl, p, + extension_data_end ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( From f2a942073e9c3d0a8e2cff78527d3fc33890a355 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Fri, 20 May 2022 06:44:24 +0000 Subject: [PATCH 03/16] Fix SNI test failure Change-Id: Id3fce36af9bc52cac858b473168451945aa974f4 Signed-off-by: XiaokangQian --- library/ssl_tls.c | 8 +++++-- tests/ssl-opt.sh | 58 +++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 015c38a67f..f1f6e84eb8 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8247,7 +8247,7 @@ int mbedtls_ssl_parse_server_name_ext( mbedtls_ssl_context *ssl, return( 0 ); } - MBEDTLS_SSL_DEBUG_MSG( 3, ( "Parse ServerName extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "parse ServerName extension" ) ); MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); server_name_list_len = MBEDTLS_GET_UINT16_BE( p, 0 ); @@ -8264,11 +8264,15 @@ int mbedtls_ssl_parse_server_name_ext( mbedtls_ssl_context *ssl, if( p[0] == MBEDTLS_TLS_EXT_SERVERNAME_HOSTNAME ) { + ssl->handshake->sni_name = p + 3; + ssl->handshake->sni_name_len = hostname_len; + if( ssl->conf->f_sni == NULL ) + return( 0 ); ret = ssl->conf->f_sni( ssl->conf->p_sni, ssl, p + 3, hostname_len ); if( ret != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "sni_wrapper", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_sni_wrapper", ret ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, MBEDTLS_SSL_ALERT_MSG_UNRECOGNIZED_NAME ); diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 4ef37f2bbb..e2f8c2e074 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11399,6 +11399,53 @@ run_test "TLS 1.3: Server side check, no server certificate available" \ -s "No certificate available." requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_SRV_C +requires_config_enabled MBEDTLS_SSL_CLI_C +run_test "TLS 1.3: Server side check - openssl with server name indication" \ + "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0 \ + sni=localhost,data_files/server2.crt,data_files/server2.key,data_files/test-ca2.crt,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ + "$O_NEXT_CLI -msg -debug -servername localhost -CAfile data_files/test-ca_cat12.crt -cert data_files/server5.crt -key data_files/server5.key -tls1_3" \ + 0 \ + -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ + -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ + -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_VERIFY" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_FINISHED" \ + -s "tls13 server state: MBEDTLS_SSL_CLIENT_FINISHED" \ + -s "tls13 server state: MBEDTLS_SSL_HANDSHAKE_WRAPUP" \ + -s "parse ServerName extension" \ + -s "=> parse client hello" \ + -s "<= parse client hello" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_SRV_C +requires_config_enabled MBEDTLS_SSL_CLI_C +run_test "TLS 1.3: Server side check - gnutls with server name indication" \ + "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0 \ + sni=localhost,data_files/server2.crt,data_files/server2.key,data_files/test-ca2.crt,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ + "$G_NEXT_CLI localhost -d 4 --sni-hostname=localhost --x509certfile data_files/server5.crt --x509keyfile data_files/server5.key --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%NO_TICKETS -V" \ + 0 \ + -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ + -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ + -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_VERIFY" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_FINISHED" \ + -s "tls13 server state: MBEDTLS_SSL_CLIENT_FINISHED" \ + -s "tls13 server state: MBEDTLS_SSL_HANDSHAKE_WRAPUP" \ + -s "parse ServerName extension" \ + -s "=> parse client hello" \ + -s "<= parse client hello" + +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_SRV_C requires_config_enabled MBEDTLS_SSL_CLI_C @@ -11407,14 +11454,17 @@ run_test "TLS 1.3: Server side check - mbedtls with server name indication" \ sni=localhost,data_files/server2.crt,data_files/server2.key,-,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ "$P_CLI debug_level=4 server_name=localhost crt_file=data_files/server5.crt key_file=data_files/server5.key \ force_version=tls13" \ - 1 \ + 0 \ -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ - -c "client state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ - -s "Parse ServerName extension" \ - -s "SSL - The requested feature is not available" \ + -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ + -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_VERIFY" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_FINISHED" \ + -s "tls13 server state: MBEDTLS_SSL_CLIENT_FINISHED" \ + -s "tls13 server state: MBEDTLS_SSL_HANDSHAKE_WRAPUP" \ + -s "parse ServerName extension" \ -s "=> parse client hello" \ -s "<= parse client hello" From 0557c94fef2cf725afaf50c17adcad6eeb8e5fcb Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Mon, 30 May 2022 08:10:53 +0000 Subject: [PATCH 04/16] Add back SNI related code to validate_certificate Change-Id: I75883858016d4163cd7c64c3418eb3ca24fa46ea Signed-off-by: XiaokangQian --- library/ssl_tls13_generic.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 239be53a18..17efa8c22b 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -560,7 +560,14 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) * from the configuration. */ #if defined(MBEDTLS_SSL_SRV_C) if( ssl->conf->endpoint == MBEDTLS_SSL_IS_SERVER ) - authmode = ssl->conf->authmode; + { +#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + if( ssl->handshake->sni_authmode != MBEDTLS_SSL_VERIFY_UNSET ) + authmode = ssl->handshake->sni_authmode; + else +#endif + authmode = ssl->conf->authmode; + } #endif /* From 2ccd97b8ef65842b032e4ea1cc9e4aaae39b0517 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 31 May 2022 08:30:17 +0000 Subject: [PATCH 05/16] Change test case name to sni Change-Id: I8f6e68deab71cc49741cbdf233cf876e29683db9 Signed-off-by: XiaokangQian --- tests/ssl-opt.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index e2f8c2e074..8a6723e5ec 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11403,7 +11403,7 @@ requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_SRV_C requires_config_enabled MBEDTLS_SSL_CLI_C -run_test "TLS 1.3: Server side check - openssl with server name indication" \ +run_test "TLS 1.3: Server side check - openssl with sni" \ "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0 \ sni=localhost,data_files/server2.crt,data_files/server2.key,data_files/test-ca2.crt,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ "$O_NEXT_CLI -msg -debug -servername localhost -CAfile data_files/test-ca_cat12.crt -cert data_files/server5.crt -key data_files/server5.key -tls1_3" \ @@ -11426,7 +11426,7 @@ requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_SRV_C requires_config_enabled MBEDTLS_SSL_CLI_C -run_test "TLS 1.3: Server side check - gnutls with server name indication" \ +run_test "TLS 1.3: Server side check - gnutls with sni" \ "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0 \ sni=localhost,data_files/server2.crt,data_files/server2.key,data_files/test-ca2.crt,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ "$G_NEXT_CLI localhost -d 4 --sni-hostname=localhost --x509certfile data_files/server5.crt --x509keyfile data_files/server5.key --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%NO_TICKETS -V" \ @@ -11449,7 +11449,7 @@ requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_SRV_C requires_config_enabled MBEDTLS_SSL_CLI_C -run_test "TLS 1.3: Server side check - mbedtls with server name indication" \ +run_test "TLS 1.3: Server side check - mbedtls with sni" \ "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0 \ sni=localhost,data_files/server2.crt,data_files/server2.key,-,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ "$P_CLI debug_level=4 server_name=localhost crt_file=data_files/server5.crt key_file=data_files/server5.key \ From ac41edfc5e2093bce4737ccaff095cf10880fd94 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 31 May 2022 13:22:13 +0000 Subject: [PATCH 06/16] Enable requires_gnutls_tls1_3 in sni test cases Change-Id: Iea18f4e6a6b4c6b90612b43a5bcd396cdd506335 Signed-off-by: XiaokangQian --- tests/ssl-opt.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 8a6723e5ec..07424276d0 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11398,6 +11398,7 @@ run_test "TLS 1.3: Server side check, no server certificate available" \ -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ -s "No certificate available." +requires_gnutls_tls1_3 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE requires_config_enabled MBEDTLS_DEBUG_C @@ -11421,6 +11422,7 @@ run_test "TLS 1.3: Server side check - openssl with sni" \ -s "=> parse client hello" \ -s "<= parse client hello" +requires_gnutls_tls1_3 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE requires_config_enabled MBEDTLS_DEBUG_C From f4f0f6961aa7650f73f6a63adeb0d6c9d2a3094b Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 1 Jun 2022 00:42:27 +0000 Subject: [PATCH 07/16] Enable requires_openssl_tls1_3 in sni test cases Change-Id: I71fbabe0b2ff80d5f1f15ae7df2b048503ccf965 Signed-off-by: XiaokangQian --- tests/ssl-opt.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 07424276d0..6eb2f250bf 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11398,7 +11398,7 @@ run_test "TLS 1.3: Server side check, no server certificate available" \ -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ -s "No certificate available." -requires_gnutls_tls1_3 +requires_openssl_tls1_3 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE requires_config_enabled MBEDTLS_DEBUG_C From 129aeb9b0e985e77f628714fb1291cafbd1e7ccb Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 2 Jun 2022 09:29:18 +0000 Subject: [PATCH 08/16] Update test cases and support sni ca override Change-Id: I6052acde0b0ec1c25537f8dd81a35562da05a393 Signed-off-by: XiaokangQian --- library/ssl_tls.c | 12 ++---------- library/ssl_tls13_server.c | 13 +++++++++++++ tests/ssl-opt.sh | 36 +++--------------------------------- 3 files changed, 18 insertions(+), 43 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index f1f6e84eb8..6c2ba7263d 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8240,13 +8240,6 @@ int mbedtls_ssl_parse_server_name_ext( mbedtls_ssl_context *ssl, size_t server_name_list_len, hostname_len; const unsigned char *server_name_list_end; - if( ssl->conf->p_sni == NULL ) - { - MBEDTLS_SSL_DEBUG_MSG( - 3, ( "No SNI callback configured. Skip SNI parsing." ) ); - return( 0 ); - } - MBEDTLS_SSL_DEBUG_MSG( 3, ( "parse ServerName extension" ) ); MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); @@ -8273,9 +8266,8 @@ int mbedtls_ssl_parse_server_name_ext( mbedtls_ssl_context *ssl, if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "ssl_sni_wrapper", ret ); - mbedtls_ssl_send_alert_message( - ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_UNRECOGNIZED_NAME ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNRECOGNIZED_NAME, + MBEDTLS_ERR_SSL_UNRECOGNIZED_NAME ); return( MBEDTLS_ERR_SSL_UNRECOGNIZED_NAME ); } return( 0 ); diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 7508685fa4..e130fa53a1 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -688,6 +688,19 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, p += extension_data_len; } + /* + * Server certification selection (after processing TLS extensions) + */ + if( ssl->conf->f_cert_cb && ( ret = ssl->conf->f_cert_cb( ssl ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "f_cert_cb", ret ); + return( ret ); + } +#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + ssl->handshake->sni_name = NULL; + ssl->handshake->sni_name_len = 0; +#endif + /* Update checksum with either * - The entire content of the CH message, if no PSK extension is present * - The content up to but excluding the PSK extension, if present. diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 6eb2f250bf..d3996c4227 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11409,18 +11409,8 @@ run_test "TLS 1.3: Server side check - openssl with sni" \ sni=localhost,data_files/server2.crt,data_files/server2.key,data_files/test-ca2.crt,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ "$O_NEXT_CLI -msg -debug -servername localhost -CAfile data_files/test-ca_cat12.crt -cert data_files/server5.crt -key data_files/server5.key -tls1_3" \ 0 \ - -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ - -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ - -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ - -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ - -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ - -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_VERIFY" \ - -s "tls13 server state: MBEDTLS_SSL_SERVER_FINISHED" \ - -s "tls13 server state: MBEDTLS_SSL_CLIENT_FINISHED" \ - -s "tls13 server state: MBEDTLS_SSL_HANDSHAKE_WRAPUP" \ -s "parse ServerName extension" \ - -s "=> parse client hello" \ - -s "<= parse client hello" + -s "HTTP/1.0 200 OK" requires_gnutls_tls1_3 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 @@ -11433,18 +11423,8 @@ run_test "TLS 1.3: Server side check - gnutls with sni" \ sni=localhost,data_files/server2.crt,data_files/server2.key,data_files/test-ca2.crt,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ "$G_NEXT_CLI localhost -d 4 --sni-hostname=localhost --x509certfile data_files/server5.crt --x509keyfile data_files/server5.key --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%NO_TICKETS -V" \ 0 \ - -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ - -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ - -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ - -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ - -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ - -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_VERIFY" \ - -s "tls13 server state: MBEDTLS_SSL_SERVER_FINISHED" \ - -s "tls13 server state: MBEDTLS_SSL_CLIENT_FINISHED" \ - -s "tls13 server state: MBEDTLS_SSL_HANDSHAKE_WRAPUP" \ -s "parse ServerName extension" \ - -s "=> parse client hello" \ - -s "<= parse client hello" + -s "HTTP/1.0 200 OK" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE @@ -11457,18 +11437,8 @@ run_test "TLS 1.3: Server side check - mbedtls with sni" \ "$P_CLI debug_level=4 server_name=localhost crt_file=data_files/server5.crt key_file=data_files/server5.key \ force_version=tls13" \ 0 \ - -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ - -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ - -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ - -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ - -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ - -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_VERIFY" \ - -s "tls13 server state: MBEDTLS_SSL_SERVER_FINISHED" \ - -s "tls13 server state: MBEDTLS_SSL_CLIENT_FINISHED" \ - -s "tls13 server state: MBEDTLS_SSL_HANDSHAKE_WRAPUP" \ -s "parse ServerName extension" \ - -s "=> parse client hello" \ - -s "<= parse client hello" + -s "HTTP/1.0 200 OK" for i in opt-testcases/*.sh do From 23c5be6b9476bf2d524a970133db8ab6b7574eeb Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 7 Jun 2022 02:04:34 +0000 Subject: [PATCH 09/16] Enable SNI test for both tls12 and tls13 Change-Id: Iae5c39668db7caa1a59d7e67f226a5286d91db22 CustomizedGitHooks: yes Signed-off-by: XiaokangQian --- library/ssl_tls13_client.c | 4 +- library/ssl_tls13_server.c | 103 ++++++++++++++++++++++++++++++++++++- tests/ssl-opt.sh | 18 ++----- 3 files changed, 107 insertions(+), 18 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index e9250fcd3c..b498fd4909 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1687,7 +1687,7 @@ static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) } else if( ret == SSL_CERTIFICATE_REQUEST_SKIP ) { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate request" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip tls13 parse certificate request" ) ); ret = 0; } else @@ -1793,7 +1793,7 @@ static int ssl_tls13_write_client_certificate( mbedtls_ssl_context *ssl ) } else { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "No certificate message to send." ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "skip write certificate" ) ); } #endif diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index e130fa53a1..25faa067d0 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -335,6 +335,98 @@ static int ssl_tls13_check_ephemeral_key_exchange( mbedtls_ssl_context *ssl ) return( 1 ); } +#if defined(MBEDTLS_X509_CRT_PARSE_C) +/* + * Return 0 if the given key uses one of the acceptable curves, -1 otherwise + */ +#if defined(MBEDTLS_ECDSA_C) +static int ssl_check_key_curve( mbedtls_pk_context *pk, + const mbedtls_ecp_curve_info **curves ) +{ + const mbedtls_ecp_curve_info **crv = curves; + mbedtls_ecp_group_id grp_id = mbedtls_pk_ec( *pk )->grp.id; + + while( *crv != NULL ) + { + if( (*crv)->grp_id == grp_id ) + return( 0 ); + crv++; + } + + return( -1 ); +} +#endif /* MBEDTLS_ECDSA_C */ + +/* + * Try picking a certificate for this ciphersuite, + * return 0 on success and -1 on failure. + */ +static int ssl_pick_cert( mbedtls_ssl_context *ssl, + const mbedtls_ssl_ciphersuite_t * ciphersuite_info ) +{ + mbedtls_ssl_key_cert *cur, *list; +#if defined(MBEDTLS_ECDSA_C) + mbedtls_pk_type_t pk_alg = + mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); +#endif /* MBEDTLS_ECDSA_C */ + uint32_t flags; + +#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + if( ssl->handshake->sni_key_cert != NULL ) + list = ssl->handshake->sni_key_cert; + else +#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ + list = ssl->conf->key_cert; + + if( list == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "server has no certificate" ) ); + return( -1 ); + } + + for( cur = list; cur != NULL; cur = cur->next ) + { + flags = 0; + MBEDTLS_SSL_DEBUG_CRT( 3, "candidate certificate chain, certificate", + cur->cert ); + + /* + * This avoids sending the client a cert it'll reject based on + * keyUsage or other extensions. + */ + if( mbedtls_ssl_check_cert_usage( cur->cert, ciphersuite_info, + MBEDTLS_SSL_IS_SERVER, &flags ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "certificate mismatch: " + "(extended) key usage extension" ) ); + continue; + } + +#if defined(MBEDTLS_ECDSA_C) + if( pk_alg == MBEDTLS_PK_ECDSA && + ssl_check_key_curve( &cur->cert->pk, ssl->handshake->curves ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "certificate mismatch: elliptic curve" ) ); + continue; + } +#endif /* MBEDTLS_ECDSA_C */ + + break; + } + + /* Do not update ssl->handshake->key_cert unless there is a match */ + if( cur != NULL ) + { + ssl->handshake->key_cert = cur; + MBEDTLS_SSL_DEBUG_CRT( 3, "selected certificate chain, certificate", + ssl->handshake->key_cert->cert ); + return( 0 ); + } + + return( -1 ); +} +#endif /* MBEDTLS_X509_CRT_PARSE_C */ + /* * * STATE HANDLING: ClientHello @@ -699,7 +791,16 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) ssl->handshake->sni_name = NULL; ssl->handshake->sni_name_len = 0; -#endif +#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ + +#if defined(MBEDTLS_X509_CRT_PARSE_C) + if( (ssl_pick_cert( ssl, ssl->handshake->ciphersuite_info ) != 0) ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ciphersuite mismatch: " + "no suitable certificate" ) ); + return( 0 ); + } +#endif /* MBEDTLS_X509_CRT_PARSE_C */ /* Update checksum with either * - The entire content of the CH message, if no PSK extension is present diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index d3996c4227..7124c8f3ec 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5363,7 +5363,6 @@ run_test "Certificate hash: client TLS 1.2 -> SHA-2" \ # tests for SNI requires_config_disabled MBEDTLS_X509_REMOVE_INFO -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "SNI: no SNI callback" \ "$P_SRV debug_level=3 \ crt_file=data_files/server5.crt key_file=data_files/server5.key" \ @@ -5373,7 +5372,6 @@ run_test "SNI: no SNI callback" \ -c "subject name *: C=NL, O=PolarSSL, CN=localhost" requires_config_disabled MBEDTLS_X509_REMOVE_INFO -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "SNI: matching cert 1" \ "$P_SRV debug_level=3 \ crt_file=data_files/server5.crt key_file=data_files/server5.key \ @@ -5385,7 +5383,6 @@ run_test "SNI: matching cert 1" \ -c "subject name *: C=NL, O=PolarSSL, CN=localhost" requires_config_disabled MBEDTLS_X509_REMOVE_INFO -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "SNI: matching cert 2" \ "$P_SRV debug_level=3 \ crt_file=data_files/server5.crt key_file=data_files/server5.key \ @@ -5397,7 +5394,6 @@ run_test "SNI: matching cert 2" \ -c "subject name *: C=NL, O=PolarSSL, CN=polarssl.example" requires_config_disabled MBEDTLS_X509_REMOVE_INFO -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "SNI: no matching cert" \ "$P_SRV debug_level=3 \ crt_file=data_files/server5.crt key_file=data_files/server5.key \ @@ -5410,7 +5406,6 @@ run_test "SNI: no matching cert" \ -c "mbedtls_ssl_handshake returned" \ -c "SSL - A fatal alert message was received from our peer" -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "SNI: client auth no override: optional" \ "$P_SRV debug_level=3 auth_mode=optional \ crt_file=data_files/server5.crt key_file=data_files/server5.key \ @@ -5424,7 +5419,6 @@ run_test "SNI: client auth no override: optional" \ -C "skip write certificate verify" \ -S "skip parse certificate verify" -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "SNI: client auth override: none -> optional" \ "$P_SRV debug_level=3 auth_mode=none \ crt_file=data_files/server5.crt key_file=data_files/server5.key \ @@ -5438,7 +5432,6 @@ run_test "SNI: client auth override: none -> optional" \ -C "skip write certificate verify" \ -S "skip parse certificate verify" -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "SNI: client auth override: optional -> none" \ "$P_SRV debug_level=3 auth_mode=optional \ crt_file=data_files/server5.crt key_file=data_files/server5.key \ @@ -5448,11 +5441,8 @@ run_test "SNI: client auth override: optional -> none" \ -s "skip write certificate request" \ -C "skip parse certificate request" \ -c "got no certificate request" \ - -c "skip write certificate" \ - -c "skip write certificate verify" \ - -s "skip parse certificate verify" + -c "skip write certificate" -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "SNI: CA no override" \ "$P_SRV debug_level=3 auth_mode=optional \ crt_file=data_files/server5.crt key_file=data_files/server5.key \ @@ -5471,7 +5461,6 @@ run_test "SNI: CA no override" \ -s "! The certificate is not correctly signed by the trusted CA" \ -S "The certificate has been revoked (is on a CRL)" -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "SNI: CA override" \ "$P_SRV debug_level=3 auth_mode=optional \ crt_file=data_files/server5.crt key_file=data_files/server5.key \ @@ -5490,7 +5479,6 @@ run_test "SNI: CA override" \ -S "! The certificate is not correctly signed by the trusted CA" \ -S "The certificate has been revoked (is on a CRL)" -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "SNI: CA override with CRL" \ "$P_SRV debug_level=3 auth_mode=optional \ crt_file=data_files/server5.crt key_file=data_files/server5.key \ @@ -11406,7 +11394,7 @@ requires_config_enabled MBEDTLS_SSL_SRV_C requires_config_enabled MBEDTLS_SSL_CLI_C run_test "TLS 1.3: Server side check - openssl with sni" \ "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0 \ - sni=localhost,data_files/server2.crt,data_files/server2.key,data_files/test-ca2.crt,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ + sni=localhost,data_files/server5.crt,data_files/server5.key,data_files/test-ca_cat12.crt,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ "$O_NEXT_CLI -msg -debug -servername localhost -CAfile data_files/test-ca_cat12.crt -cert data_files/server5.crt -key data_files/server5.key -tls1_3" \ 0 \ -s "parse ServerName extension" \ @@ -11420,7 +11408,7 @@ requires_config_enabled MBEDTLS_SSL_SRV_C requires_config_enabled MBEDTLS_SSL_CLI_C run_test "TLS 1.3: Server side check - gnutls with sni" \ "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0 \ - sni=localhost,data_files/server2.crt,data_files/server2.key,data_files/test-ca2.crt,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ + sni=localhost,data_files/server5.crt,data_files/server5.key,data_files/test-ca_cat12.crt,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ "$G_NEXT_CLI localhost -d 4 --sni-hostname=localhost --x509certfile data_files/server5.crt --x509keyfile data_files/server5.key --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%NO_TICKETS -V" \ 0 \ -s "parse ServerName extension" \ From 9850fa8e8d0b0c250114d36d064a1a9821160827 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 8 Jun 2022 06:58:05 +0000 Subject: [PATCH 10/16] Refine ssl_tls13_pick_cert() Change-Id: I5448095e280d8968b20ade8b304d139e399e54f1 CustomizedGitHooks: yes Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 41 +++++++++++++++----------------------- tests/ssl-opt.sh | 1 + 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 25faa067d0..6ddacba4f8 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -339,37 +339,31 @@ static int ssl_tls13_check_ephemeral_key_exchange( mbedtls_ssl_context *ssl ) /* * Return 0 if the given key uses one of the acceptable curves, -1 otherwise */ -#if defined(MBEDTLS_ECDSA_C) -static int ssl_check_key_curve( mbedtls_pk_context *pk, - const mbedtls_ecp_curve_info **curves ) +static int ssl_tls13_check_key_sigs( mbedtls_pk_context *pk, + uint16_t *sig_alg ) { - const mbedtls_ecp_curve_info **crv = curves; - mbedtls_ecp_group_id grp_id = mbedtls_pk_ec( *pk )->grp.id; + mbedtls_pk_type_t pk_type; + mbedtls_md_type_t md_alg; - while( *crv != NULL ) + while( *sig_alg != MBEDTLS_TLS1_3_SIG_NONE ) { - if( (*crv)->grp_id == grp_id ) + mbedtls_ssl_tls13_get_pk_type_and_md_alg_from_sig_alg( + *sig_alg, &pk_type, &md_alg ); + if( pk_type == mbedtls_pk_get_type(pk) ) return( 0 ); - crv++; + sig_alg++; } return( -1 ); } -#endif /* MBEDTLS_ECDSA_C */ /* * Try picking a certificate for this ciphersuite, * return 0 on success and -1 on failure. */ -static int ssl_pick_cert( mbedtls_ssl_context *ssl, - const mbedtls_ssl_ciphersuite_t * ciphersuite_info ) +static int ssl_tls13_pick_cert( mbedtls_ssl_context *ssl ) { mbedtls_ssl_key_cert *cur, *list; -#if defined(MBEDTLS_ECDSA_C) - mbedtls_pk_type_t pk_alg = - mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); -#endif /* MBEDTLS_ECDSA_C */ - uint32_t flags; #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) if( ssl->handshake->sni_key_cert != NULL ) @@ -386,7 +380,6 @@ static int ssl_pick_cert( mbedtls_ssl_context *ssl, for( cur = list; cur != NULL; cur = cur->next ) { - flags = 0; MBEDTLS_SSL_DEBUG_CRT( 3, "candidate certificate chain, certificate", cur->cert ); @@ -394,22 +387,20 @@ static int ssl_pick_cert( mbedtls_ssl_context *ssl, * This avoids sending the client a cert it'll reject based on * keyUsage or other extensions. */ - if( mbedtls_ssl_check_cert_usage( cur->cert, ciphersuite_info, - MBEDTLS_SSL_IS_SERVER, &flags ) != 0 ) + if( mbedtls_x509_crt_check_key_usage( cur->cert, MBEDTLS_X509_KU_DIGITAL_SIGNATURE ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "certificate mismatch: " "(extended) key usage extension" ) ); continue; } -#if defined(MBEDTLS_ECDSA_C) - if( pk_alg == MBEDTLS_PK_ECDSA && - ssl_check_key_curve( &cur->cert->pk, ssl->handshake->curves ) != 0 ) + if( ssl_tls13_check_key_sigs( &cur->cert->pk, + ssl->handshake->received_sig_algs ) != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "certificate mismatch: elliptic curve" ) ); + MBEDTLS_SSL_DEBUG_MSG( + 3, ( "certificate key mismatch: received_sig_algs" ) ); continue; } -#endif /* MBEDTLS_ECDSA_C */ break; } @@ -794,7 +785,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ #if defined(MBEDTLS_X509_CRT_PARSE_C) - if( (ssl_pick_cert( ssl, ssl->handshake->ciphersuite_info ) != 0) ) + if( (ssl_tls13_pick_cert( ssl ) != 0) ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "ciphersuite mismatch: " "no suitable certificate" ) ); diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 7124c8f3ec..c451708ca9 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5363,6 +5363,7 @@ run_test "Certificate hash: client TLS 1.2 -> SHA-2" \ # tests for SNI requires_config_disabled MBEDTLS_X509_REMOVE_INFO +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "SNI: no SNI callback" \ "$P_SRV debug_level=3 \ crt_file=data_files/server5.crt key_file=data_files/server5.key" \ From 96287d98d899e7cf22577da9f0068ebe849b404e Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 8 Jun 2022 08:37:53 +0000 Subject: [PATCH 11/16] Remove the certificate key check against the received signature Change-Id: I07d8d46c58dec499f96cb7307fc0af15149d9df7 CustomizedGitHooks: yes Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 29 ----------------------------- tests/ssl-opt.sh | 1 - 2 files changed, 30 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 6ddacba4f8..22562a5ae8 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -336,27 +336,6 @@ static int ssl_tls13_check_ephemeral_key_exchange( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_X509_CRT_PARSE_C) -/* - * Return 0 if the given key uses one of the acceptable curves, -1 otherwise - */ -static int ssl_tls13_check_key_sigs( mbedtls_pk_context *pk, - uint16_t *sig_alg ) -{ - mbedtls_pk_type_t pk_type; - mbedtls_md_type_t md_alg; - - while( *sig_alg != MBEDTLS_TLS1_3_SIG_NONE ) - { - mbedtls_ssl_tls13_get_pk_type_and_md_alg_from_sig_alg( - *sig_alg, &pk_type, &md_alg ); - if( pk_type == mbedtls_pk_get_type(pk) ) - return( 0 ); - sig_alg++; - } - - return( -1 ); -} - /* * Try picking a certificate for this ciphersuite, * return 0 on success and -1 on failure. @@ -394,14 +373,6 @@ static int ssl_tls13_pick_cert( mbedtls_ssl_context *ssl ) continue; } - if( ssl_tls13_check_key_sigs( &cur->cert->pk, - ssl->handshake->received_sig_algs ) != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( - 3, ( "certificate key mismatch: received_sig_algs" ) ); - continue; - } - break; } diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index c451708ca9..7124c8f3ec 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5363,7 +5363,6 @@ run_test "Certificate hash: client TLS 1.2 -> SHA-2" \ # tests for SNI requires_config_disabled MBEDTLS_X509_REMOVE_INFO -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "SNI: no SNI callback" \ "$P_SRV debug_level=3 \ crt_file=data_files/server5.crt key_file=data_files/server5.key" \ From 81802f43a283d0215ff14c0dee6a6b4e86972c36 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Fri, 10 Jun 2022 13:25:22 +0000 Subject: [PATCH 12/16] Select certificate base on the received signature list Change-Id: Ife707db7fcfdb1e761ba86804cbf5dd766a5ee33 Signed-off-by: XiaokangQian --- library/ssl_misc.h | 4 +++ library/ssl_tls13_generic.c | 8 ++--- library/ssl_tls13_server.c | 64 ++++++++++++++++++++++--------------- 3 files changed, 47 insertions(+), 29 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 3b5aadb0bd..b1f0c90b5f 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2276,4 +2276,8 @@ int mbedtls_ssl_parse_server_name_ext( mbedtls_ssl_context *ssl, const unsigned char *end ); #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ +int mbedtls_ssl_tls13_get_sig_alg_from_pk( mbedtls_ssl_context *ssl, + mbedtls_pk_context *own_key, + uint16_t *algorithm ); + #endif /* ssl_misc.h */ diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 17efa8c22b..842a708a9d 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -866,9 +866,9 @@ cleanup: /* * STATE HANDLING: Output Certificate Verify */ -static int ssl_tls13_get_sig_alg_from_pk( mbedtls_ssl_context *ssl, - mbedtls_pk_context *own_key, - uint16_t *algorithm ) +int mbedtls_ssl_tls13_get_sig_alg_from_pk( mbedtls_ssl_context *ssl, + mbedtls_pk_context *own_key, + uint16_t *algorithm ) { mbedtls_pk_type_t sig = mbedtls_ssl_sig_from_pk( own_key ); /* Determine the size of the key */ @@ -1035,7 +1035,7 @@ static int ssl_tls13_write_certificate_verify_body( mbedtls_ssl_context *ssl, * opaque signature<0..2^16-1>; * } CertificateVerify; */ - ret = ssl_tls13_get_sig_alg_from_pk( ssl, own_key, &algorithm ); + ret = mbedtls_ssl_tls13_get_sig_alg_from_pk( ssl, own_key, &algorithm ); if( ret != 0 || ! mbedtls_ssl_sig_alg_is_received( ssl, algorithm ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 22562a5ae8..494208f816 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -335,7 +335,8 @@ static int ssl_tls13_check_ephemeral_key_exchange( mbedtls_ssl_context *ssl ) return( 1 ); } -#if defined(MBEDTLS_X509_CRT_PARSE_C) +#if defined(MBEDTLS_X509_CRT_PARSE_C) && \ + defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* * Try picking a certificate for this ciphersuite, * return 0 on success and -1 on failure. @@ -343,6 +344,8 @@ static int ssl_tls13_check_ephemeral_key_exchange( mbedtls_ssl_context *ssl ) static int ssl_tls13_pick_cert( mbedtls_ssl_context *ssl ) { mbedtls_ssl_key_cert *cur, *list; + const uint16_t *sig_alg = ssl->handshake->received_sig_algs; + uint16_t own_sig_alg; #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) if( ssl->handshake->sni_key_cert != NULL ) @@ -357,37 +360,46 @@ static int ssl_tls13_pick_cert( mbedtls_ssl_context *ssl ) return( -1 ); } - for( cur = list; cur != NULL; cur = cur->next ) + for( ; *sig_alg != MBEDTLS_TLS1_3_SIG_NONE; sig_alg++ ) { - MBEDTLS_SSL_DEBUG_CRT( 3, "candidate certificate chain, certificate", - cur->cert ); - - /* - * This avoids sending the client a cert it'll reject based on - * keyUsage or other extensions. - */ - if( mbedtls_x509_crt_check_key_usage( cur->cert, MBEDTLS_X509_KU_DIGITAL_SIGNATURE ) != 0 ) + for( cur = list; cur != NULL; cur = cur->next ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "certificate mismatch: " + MBEDTLS_SSL_DEBUG_CRT( 3, "candidate certificate chain, certificate", + cur->cert ); + + /* + * This avoids sending the client a cert it'll reject based on + * keyUsage or other extensions. + */ + if( mbedtls_x509_crt_check_key_usage( + cur->cert, MBEDTLS_X509_KU_DIGITAL_SIGNATURE ) != 0 || + mbedtls_x509_crt_check_extended_key_usage( + cur->cert, MBEDTLS_OID_SERVER_AUTH, + MBEDTLS_OID_SIZE( MBEDTLS_OID_SERVER_AUTH ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "certificate mismatch: " "(extended) key usage extension" ) ); - continue; - } + continue; + } - break; - } - - /* Do not update ssl->handshake->key_cert unless there is a match */ - if( cur != NULL ) - { - ssl->handshake->key_cert = cur; - MBEDTLS_SSL_DEBUG_CRT( 3, "selected certificate chain, certificate", + if( ! mbedtls_ssl_tls13_get_sig_alg_from_pk( + ssl, &cur->cert->pk, &own_sig_alg ) && + ( *sig_alg == own_sig_alg ) ) + { + ssl->handshake->key_cert = cur; + MBEDTLS_SSL_DEBUG_CRT( 3, "selected certificate chain, certificate", ssl->handshake->key_cert->cert ); - return( 0 ); + return( 0 ); + } + + + } } return( -1 ); } -#endif /* MBEDTLS_X509_CRT_PARSE_C */ +#endif /* MBEDTLS_X509_CRT_PARSE_C && + MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ /* * @@ -755,14 +767,16 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, ssl->handshake->sni_name_len = 0; #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ -#if defined(MBEDTLS_X509_CRT_PARSE_C) +#if defined(MBEDTLS_X509_CRT_PARSE_C) && \ + defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) if( (ssl_tls13_pick_cert( ssl ) != 0) ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "ciphersuite mismatch: " "no suitable certificate" ) ); return( 0 ); } -#endif /* MBEDTLS_X509_CRT_PARSE_C */ +#endif /* MBEDTLS_X509_CRT_PARSE_C && + MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ /* Update checksum with either * - The entire content of the CH message, if no PSK extension is present From 07aad0710cf2e6584336e741c305c0b2f1935689 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 14 Jun 2022 05:35:09 +0000 Subject: [PATCH 13/16] Refine function name ssl_tls13_pick_key_cert Change-Id: I821e1485d9cfcca88fa3e18d345766ea48c64250 Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 494208f816..74df4b3fab 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -341,7 +341,7 @@ static int ssl_tls13_check_ephemeral_key_exchange( mbedtls_ssl_context *ssl ) * Try picking a certificate for this ciphersuite, * return 0 on success and -1 on failure. */ -static int ssl_tls13_pick_cert( mbedtls_ssl_context *ssl ) +static int ssl_tls13_pick_key_cert( mbedtls_ssl_context *ssl ) { mbedtls_ssl_key_cert *cur, *list; const uint16_t *sig_alg = ssl->handshake->received_sig_algs; @@ -391,8 +391,6 @@ static int ssl_tls13_pick_cert( mbedtls_ssl_context *ssl ) ssl->handshake->key_cert->cert ); return( 0 ); } - - } } @@ -769,7 +767,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_X509_CRT_PARSE_C) && \ defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) - if( (ssl_tls13_pick_cert( ssl ) != 0) ) + if( (ssl_tls13_pick_key_cert( ssl ) != 0) ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "ciphersuite mismatch: " "no suitable certificate" ) ); From 3ed16231ab8d6af2b5009cf8ce9c43e9264d8bae Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 14 Jun 2022 08:24:04 +0000 Subject: [PATCH 14/16] Refine server side SNI test cases Change-Id: Icdc91ed382e81702e3b46645d3ce3534e62d4a13 Signed-off-by: XiaokangQian --- tests/ssl-opt.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 7124c8f3ec..02f1a32750 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11391,7 +11391,6 @@ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_SRV_C -requires_config_enabled MBEDTLS_SSL_CLI_C run_test "TLS 1.3: Server side check - openssl with sni" \ "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0 \ sni=localhost,data_files/server5.crt,data_files/server5.key,data_files/test-ca_cat12.crt,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ @@ -11405,7 +11404,6 @@ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_SRV_C -requires_config_enabled MBEDTLS_SSL_CLI_C run_test "TLS 1.3: Server side check - gnutls with sni" \ "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0 \ sni=localhost,data_files/server5.crt,data_files/server5.key,data_files/test-ca_cat12.crt,-,-,polarssl.example,data_files/server1-nospace.crt,data_files/server1.key,-,-,-" \ From fb665a8452982e92dfd8011fe0d80b78a614defe Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 15 Jun 2022 03:57:21 +0000 Subject: [PATCH 15/16] Adress the comments about styles and pick_cert Change-Id: Iee89a27aaea6ebc8eb01c6c9985487f081ef7343 Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 88 +++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 45 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 74df4b3fab..e4ca6aa2df 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -338,23 +338,23 @@ static int ssl_tls13_check_ephemeral_key_exchange( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_X509_CRT_PARSE_C) && \ defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* - * Try picking a certificate for this ciphersuite, - * return 0 on success and -1 on failure. + * Pick best ( private key, certificate chain ) pair based on the signature + * algorithms supported by the client. */ static int ssl_tls13_pick_key_cert( mbedtls_ssl_context *ssl ) { - mbedtls_ssl_key_cert *cur, *list; + mbedtls_ssl_key_cert *key_cert, *key_cert_list; const uint16_t *sig_alg = ssl->handshake->received_sig_algs; - uint16_t own_sig_alg; + uint16_t key_sig_alg; #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) if( ssl->handshake->sni_key_cert != NULL ) - list = ssl->handshake->sni_key_cert; + key_cert_list = ssl->handshake->sni_key_cert; else #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ - list = ssl->conf->key_cert; + key_cert_list = ssl->conf->key_cert; - if( list == NULL ) + if( key_cert_list == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "server has no certificate" ) ); return( -1 ); @@ -362,33 +362,38 @@ static int ssl_tls13_pick_key_cert( mbedtls_ssl_context *ssl ) for( ; *sig_alg != MBEDTLS_TLS1_3_SIG_NONE; sig_alg++ ) { - for( cur = list; cur != NULL; cur = cur->next ) + for( key_cert = key_cert_list; key_cert != NULL; + key_cert = key_cert->next ) { - MBEDTLS_SSL_DEBUG_CRT( 3, "candidate certificate chain, certificate", - cur->cert ); + int ret; + MBEDTLS_SSL_DEBUG_CRT( 3, "certificate (chain) candidate", + key_cert->cert ); /* * This avoids sending the client a cert it'll reject based on * keyUsage or other extensions. */ if( mbedtls_x509_crt_check_key_usage( - cur->cert, MBEDTLS_X509_KU_DIGITAL_SIGNATURE ) != 0 || + key_cert->cert, MBEDTLS_X509_KU_DIGITAL_SIGNATURE ) != 0 || mbedtls_x509_crt_check_extended_key_usage( - cur->cert, MBEDTLS_OID_SERVER_AUTH, - MBEDTLS_OID_SIZE( MBEDTLS_OID_SERVER_AUTH ) ) != 0 ) + key_cert->cert, MBEDTLS_OID_SERVER_AUTH, + MBEDTLS_OID_SIZE( MBEDTLS_OID_SERVER_AUTH ) ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "certificate mismatch: " - "(extended) key usage extension" ) ); + "(extended) key usage extension" ) ); continue; } - if( ! mbedtls_ssl_tls13_get_sig_alg_from_pk( - ssl, &cur->cert->pk, &own_sig_alg ) && - ( *sig_alg == own_sig_alg ) ) + ret = mbedtls_ssl_tls13_get_sig_alg_from_pk( + ssl, &key_cert->cert->pk, &key_sig_alg ); + if( ret != 0 ) + continue; + if( *sig_alg == key_sig_alg ) { - ssl->handshake->key_cert = cur; - MBEDTLS_SSL_DEBUG_CRT( 3, "selected certificate chain, certificate", - ssl->handshake->key_cert->cert ); + ssl->handshake->key_cert = key_cert; + MBEDTLS_SSL_DEBUG_CRT( + 3, "selected certificate chain, certificate", + ssl->handshake->key_cert->cert ); return( 0 ); } } @@ -752,30 +757,6 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, p += extension_data_len; } - /* - * Server certification selection (after processing TLS extensions) - */ - if( ssl->conf->f_cert_cb && ( ret = ssl->conf->f_cert_cb( ssl ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "f_cert_cb", ret ); - return( ret ); - } -#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) - ssl->handshake->sni_name = NULL; - ssl->handshake->sni_name_len = 0; -#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ - -#if defined(MBEDTLS_X509_CRT_PARSE_C) && \ - defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) - if( (ssl_tls13_pick_key_cert( ssl ) != 0) ) - { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "ciphersuite mismatch: " - "no suitable certificate" ) ); - return( 0 ); - } -#endif /* MBEDTLS_X509_CRT_PARSE_C && - MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ - /* Update checksum with either * - The entire content of the CH message, if no PSK extension is present * - The content up to but excluding the PSK extension, if present. @@ -811,6 +792,19 @@ static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + /* + * Server certificate selection + */ + if( ssl->conf->f_cert_cb && ( ret = ssl->conf->f_cert_cb( ssl ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "f_cert_cb", ret ); + return( ret ); + } +#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + ssl->handshake->sni_name = NULL; + ssl->handshake->sni_name_len = 0; +#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ + ret = mbedtls_ssl_tls13_key_schedule_stage_early( ssl ); if( ret != 0 ) { @@ -1558,13 +1552,17 @@ cleanup: static int ssl_tls13_write_server_certificate( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - if( mbedtls_ssl_own_cert( ssl ) == NULL ) + +#if defined(MBEDTLS_X509_CRT_PARSE_C) + if( ( ssl_tls13_pick_key_cert( ssl ) != 0 ) || + mbedtls_ssl_own_cert( ssl ) == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "No certificate available." ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE); return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); } +#endif /* MBEDTLS_X509_CRT_PARSE_C */ ret = mbedtls_ssl_tls13_write_certificate( ssl ); if( ret != 0 ) From 75fe8c7e548f4159d3cd5ec2199a1be3e27aa205 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 15 Jun 2022 09:42:45 +0000 Subject: [PATCH 16/16] Change place of ssl_tls13_check_ephemeral_key_exchange Change-Id: Id49172f7375e2a0771ad1216fb7eead808f0db3e Signed-off-by: XiaokangQian --- library/ssl_tls.c | 7 ++++++- library/ssl_tls13_server.c | 21 ++++++++++----------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 6c2ba7263d..8332461412 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8248,7 +8248,7 @@ int mbedtls_ssl_parse_server_name_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, server_name_list_len ); server_name_list_end = p + server_name_list_len; - while ( p < server_name_list_end ) + while( p < server_name_list_end ) { MBEDTLS_SSL_CHK_BUF_READ_PTR( p, server_name_list_end, 3 ); hostname_len = MBEDTLS_GET_UINT16_BE( p, 1 ); @@ -8257,6 +8257,11 @@ int mbedtls_ssl_parse_server_name_ext( mbedtls_ssl_context *ssl, if( p[0] == MBEDTLS_TLS_EXT_SERVERNAME_HOSTNAME ) { + /* sni_name is intended to be used only during the parsing of the + * ClientHello message (it is reset to NULL before the end of + * the message parsing). Thus it is ok to just point to the + * reception buffer and not make a copy of it. + */ ssl->handshake->sni_name = p + 3; ssl->handshake->sni_name_len = hostname_len; if( ssl->conf->f_sni == NULL ) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index e4ca6aa2df..6527e21775 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -392,7 +392,7 @@ static int ssl_tls13_pick_key_cert( mbedtls_ssl_context *ssl ) { ssl->handshake->key_cert = key_cert; MBEDTLS_SSL_DEBUG_CRT( - 3, "selected certificate chain, certificate", + 3, "selected certificate (chain)", ssl->handshake->key_cert->cert ); return( 0 ); } @@ -769,10 +769,18 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, ssl_tls13_debug_print_client_hello_exts( ssl ); #endif /* MBEDTLS_DEBUG_C */ + return( hrr_required ? SSL_CLIENT_HELLO_HRR_REQUIRED : SSL_CLIENT_HELLO_OK ); +} + +/* Update the handshake state machine */ + +static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + /* * Here we only support the ephemeral or (EC)DHE key echange mode */ - if( !ssl_tls13_check_ephemeral_key_exchange( ssl ) ) { MBEDTLS_SSL_DEBUG_MSG( @@ -783,15 +791,6 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } - return( hrr_required ? SSL_CLIENT_HELLO_HRR_REQUIRED : SSL_CLIENT_HELLO_OK ); -} - -/* Update the handshake state machine */ - -static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl ) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - /* * Server certificate selection */