diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 06860e22c6..6295b1816c 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -492,6 +492,7 @@ #define MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_EXT 110 /* 0x6E */ #define MBEDTLS_SSL_ALERT_MSG_UNRECOGNIZED_NAME 112 /* 0x70 */ #define MBEDTLS_SSL_ALERT_MSG_UNKNOWN_PSK_IDENTITY 115 /* 0x73 */ +#define MBEDTLS_SSL_ALERT_MSG_CERT_REQUIRED 116 /* 0x74 */ #define MBEDTLS_SSL_ALERT_MSG_NO_APPLICATION_PROTOCOL 120 /* 0x78 */ #define MBEDTLS_SSL_HS_HELLO_REQUEST 0 diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 9fcb2b2963..9912d6c29a 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -509,6 +509,12 @@ struct mbedtls_ssl_handshake_params uint8_t sni_authmode; /*!< authmode from SNI callback */ #endif +#if defined(MBEDTLS_SSL_SRV_C) + /* Flag indicating if a CertificateRequest message has been sent + * to the client or not. */ + uint8_t certificate_request_sent; +#endif /* MBEDTLS_SSL_SRV_C */ + #if defined(MBEDTLS_SSL_SESSION_TICKETS) uint8_t new_session_ticket; /*!< use NewSessionTicket? */ #endif /* MBEDTLS_SSL_SESSION_TICKETS */ diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index e69fd7b622..239be53a18 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -367,14 +367,10 @@ cleanup: /* * - * STATE HANDLING: Incoming Certificate, client-side only currently. + * STATE HANDLING: Incoming Certificate. * */ -/* - * Implementation - */ - #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) /* @@ -439,6 +435,13 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, mbedtls_free( ssl->session_negotiate->peer_cert ); } + if( certificate_list_len == 0 ) + { + ssl->session_negotiate->peer_cert = NULL; + ret = 0; + goto exit; + } + if( ( ssl->session_negotiate->peer_cert = mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) ) ) == NULL ) { @@ -515,6 +518,7 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, p += extensions_len; } +exit: /* Check that all the message is consumed. */ if( p != end ) { @@ -547,10 +551,58 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) { int ret = 0; + int authmode = MBEDTLS_SSL_VERIFY_REQUIRED; mbedtls_x509_crt *ca_chain; mbedtls_x509_crl *ca_crl; uint32_t verify_result = 0; + /* If SNI was used, overwrite authentication mode + * from the configuration. */ +#if defined(MBEDTLS_SSL_SRV_C) + if( ssl->conf->endpoint == MBEDTLS_SSL_IS_SERVER ) + authmode = ssl->conf->authmode; +#endif + + /* + * If the peer hasn't sent a certificate ( i.e. it sent + * an empty certificate chain ), this is reflected in the peer CRT + * structure being unset. + * Check for that and handle it depending on the + * authentication mode. + */ + if( ssl->session_negotiate->peer_cert == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "peer has not sent a certificate" ) ); + +#if defined(MBEDTLS_SSL_SRV_C) + if( ssl->conf->endpoint == MBEDTLS_SSL_IS_SERVER ) + { + /* The client was asked for a certificate but didn't send + * one. The client should know what's going on, so we + * don't send an alert. + */ + ssl->session_negotiate->verify_result = MBEDTLS_X509_BADCERT_MISSING; + if( authmode == MBEDTLS_SSL_VERIFY_OPTIONAL ) + return( 0 ); + else + { + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_NO_CERT, + MBEDTLS_ERR_SSL_NO_CLIENT_CERTIFICATE ); + return( MBEDTLS_ERR_SSL_NO_CLIENT_CERTIFICATE ); + } + } +#endif /* MBEDTLS_SSL_SRV_C */ + +#if defined(MBEDTLS_SSL_CLI_C) + if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) + { + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_NO_CERT, + MBEDTLS_ERR_SSL_FATAL_ALERT_MESSAGE ); + return( MBEDTLS_ERR_SSL_FATAL_ALERT_MESSAGE ); + } +#endif /* MBEDTLS_SSL_CLI_C */ + } + #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) if( ssl->handshake->sni_ca_chain != NULL ) { @@ -593,8 +645,20 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; } + /* mbedtls_x509_crt_verify_with_profile is supposed to report a + * verification failure through MBEDTLS_ERR_X509_CERT_VERIFY_FAILED, + * with details encoded in the verification flags. All other kinds + * of error codes, including those from the user provided f_vrfy + * functions, are treated as fatal and lead to a failure of + * ssl_tls13_parse_certificate even if verification was optional. */ + if( authmode == MBEDTLS_SSL_VERIFY_OPTIONAL && + ( ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || + ret == MBEDTLS_ERR_SSL_BAD_CERTIFICATE ) ) + { + ret = 0; + } - if( ca_chain == NULL ) + if( ca_chain == NULL && authmode == MBEDTLS_SSL_VERIFY_REQUIRED ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "got no CA chain" ) ); ret = MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED; @@ -663,7 +727,8 @@ int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl ) &buf, &buf_len ) ); /* Parse the certificate chain sent by the peer. */ - MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate( ssl, buf, buf + buf_len ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate( ssl, buf, + buf + buf_len ) ); /* Validate the certificate chain and set the verification results. */ MBEDTLS_SSL_PROC_CHK( ssl_tls13_validate_certificate( ssl ) ); @@ -671,12 +736,9 @@ int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl ) buf, buf_len ); cleanup: +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate" ) ); -#else - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ return( ret ); } #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 8c8eb2ab6a..f3843b1e85 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1342,6 +1342,8 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) if( authmode == MBEDTLS_SSL_VERIFY_NONE ) return( SSL_CERTIFICATE_REQUEST_SKIP ); + ssl->handshake->certificate_request_sent = 1; + return( SSL_CERTIFICATE_REQUEST_SEND_REQUEST ); } @@ -1495,7 +1497,16 @@ static int ssl_tls13_write_server_finished( mbedtls_ssl_context *ssl ) MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); return( ret ); } - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_FINISHED ); + + if( ssl->handshake->certificate_request_sent ) + { + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE ); + + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Switch to handshake keys for inbound traffic" ) ); + mbedtls_ssl_set_inbound_transform( ssl, ssl->handshake->transform_handshake ); + } + else + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_FINISHED ); return( 0 ); } @@ -1506,10 +1517,12 @@ static int ssl_tls13_process_client_finished( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - MBEDTLS_SSL_DEBUG_MSG( 1, - ( "Switch to handshake traffic keys for inbound traffic" ) ); - mbedtls_ssl_set_inbound_transform( ssl, ssl->handshake->transform_handshake ); - + if( ! ssl->handshake->certificate_request_sent ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "Switch to handshake traffic keys for inbound traffic" ) ); + mbedtls_ssl_set_inbound_transform( ssl, ssl->handshake->transform_handshake ); + } ret = mbedtls_ssl_tls13_process_finished_message( ssl ); if( ret != 0 ) return( ret ); @@ -1613,6 +1626,27 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) ret = ssl_tls13_handshake_wrapup( ssl ); break; + case MBEDTLS_SSL_CLIENT_CERTIFICATE: + ret = mbedtls_ssl_tls13_process_certificate( ssl ); + if( ret == 0 && ssl->session_negotiate->peer_cert != NULL ) + { + mbedtls_ssl_handshake_set_state( + ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY ); + } + else + mbedtls_ssl_handshake_set_state( + ssl, MBEDTLS_SSL_CLIENT_FINISHED ); + break; + + case MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY: + ret = mbedtls_ssl_tls13_process_certificate_verify( ssl ); + if( ret == 0 ) + { + mbedtls_ssl_handshake_set_state( + ssl, MBEDTLS_SSL_CLIENT_FINISHED ); + } + break; + default: MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid state %d", ssl->state ) ); return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 6eaa53477d..21d17f9728 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11241,10 +11241,6 @@ run_test "TLS 1.3: Server side check - openssl" \ -s "tls13 server state: MBEDTLS_SSL_CLIENT_FINISHED" \ -s "tls13 server state: MBEDTLS_SSL_HANDSHAKE_WRAPUP" -# Skip this test before openssl exit code issue fixed -# On fail, openssl return different exit code on OpenCI and internal CI for -# this test. -skip_next_test requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_SRV_C @@ -11252,7 +11248,7 @@ requires_openssl_tls1_3 run_test "TLS 1.3: Server side check - openssl with client authentication" \ "$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" \ "$O_NEXT_CLI -msg -debug -cert data_files/server5.crt -key data_files/server5.key -tls1_3 -no_middlebox" \ - 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" \ @@ -11291,7 +11287,7 @@ requires_config_enabled MBEDTLS_SSL_SRV_C run_test "TLS 1.3: Server side check - gnutls with client authentication" \ "$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" \ "$G_NEXT_CLI localhost -d 4 --x509certfile data_files/server5.crt --x509keyfile data_files/server5.key --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%NO_TICKETS:%DISABLE_TLS13_COMPAT_MODE -V" \ - 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" \ @@ -11329,7 +11325,7 @@ requires_config_enabled MBEDTLS_SSL_CLI_C run_test "TLS 1.3: Server side check - mbedtls with client authentication" \ "$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" \ "$P_CLI debug_level=4 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" \ @@ -11339,6 +11335,40 @@ run_test "TLS 1.3: Server side check - mbedtls with client authentication" \ -s "=> parse client hello" \ -s "<= parse client hello" +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 client empty certificate" \ + "$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" \ + "$P_CLI debug_level=4 crt_file=none key_file=none 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" \ + -s "=> write certificate request" \ + -s "SSL - No client certification received from the client, but required by the authentication mode" \ + -c "client state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ + -s "=> parse client hello" \ + -s "<= parse client hello" + +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 optional client authentication" \ + "$P_SRV debug_level=4 auth_mode=optional crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ + "$P_CLI debug_level=4 force_version=tls13 crt_file=none key_file=none" \ + 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 "=> write certificate request" \ + -c "client state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ + -s "=> parse client hello" \ + -s "<= parse client hello" requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_CLI_C