diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 01420dde23..2d513339f6 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -510,8 +510,9 @@ struct mbedtls_ssl_handshake_params #endif #if defined(MBEDTLS_SSL_SRV_C) - /** cert_request_send to indicate whether client certitifacte request */ - uint16_t cert_request_send; + /* Flag indicating if a CertificateRequest message has been sent + * to the client or not. */ + uint16_t certificate_request_sent; #endif /* MBEDTLS_SSL_SRV_C */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index ab4d077a55..786a72c025 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -315,14 +315,6 @@ int mbedtls_ssl_tls13_process_certificate_verify( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate verify" ) ); - if( ssl->handshake->cert_request_send && - ssl->session_negotiate->peer_cert == NULL ) - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate verify" ) ); - ret = 0; - goto cleanup; - } - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( ssl, MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, &buf, &buf_len ) ); @@ -472,6 +464,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 ) { @@ -557,6 +556,7 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } +exit: MBEDTLS_SSL_DEBUG_CRT( 3, "peer certificate", ssl->session_negotiate->peer_cert ); return( ret ); @@ -620,15 +620,15 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_SRV_C */ - if( authmode == MBEDTLS_SSL_VERIFY_NONE ) +#if defined(MBEDTLS_SSL_CLI_C) + if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT && + ssl->session_negotiate->peer_cert == NULL ) { - /* NOTE: This happens on client-side only, with the - * server-side case of VERIFY_NONE being handled earlier - * and leading to `ssl->verify_result` being set to - * MBEDTLS_X509_BADCERT_SKIP_VERIFY -- - * is this difference intentional? */ - return( 0 ); + 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 ) @@ -746,36 +746,23 @@ int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl ) int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate" ) ); - /* Coordination: - * Check if we expect a certificate, and if yes, - * check if a non-empty certificate has been sent. - */ #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) - if( ssl->handshake->cert_request_send ) - { - unsigned char *buf; - size_t buf_len; + unsigned char *buf; + size_t buf_len; - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( - ssl, MBEDTLS_SSL_HS_CERTIFICATE, - &buf, &buf_len ) ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( + ssl, MBEDTLS_SSL_HS_CERTIFICATE, + &buf, &buf_len ) ); - /* Parse the certificate chain sent by the peer. */ - 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 ) ); + /* Parse the certificate chain sent by the peer. */ + 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 ) ); - mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE, - buf, buf_len ); - } - else + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE, + buf, buf_len ); #endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate" ) ); - ret = 0; - } - cleanup: diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index fa297cb78f..43018d0dbf 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1342,7 +1342,7 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) if( authmode == MBEDTLS_SSL_VERIFY_NONE ) return( SSL_CERTIFICATE_REQUEST_SKIP ); - ssl->handshake->cert_request_send = 1; + ssl->handshake->certificate_request_sent = 1; return( SSL_CERTIFICATE_REQUEST_SEND_REQUEST ); } @@ -1497,7 +1497,8 @@ static int ssl_tls13_write_server_finished( mbedtls_ssl_context *ssl ) MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); return( ret ); } - if( ssl->handshake->cert_request_send ) + + if( ssl->handshake->certificate_request_sent ) { mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE ); @@ -1517,9 +1518,9 @@ 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 ); - + ( "Switch to handshake traffic keys for outbound traffic" ) ); + if( ! ssl->handshake->certificate_request_sent ) + mbedtls_ssl_set_inbound_transform( ssl, ssl->handshake->transform_handshake ); ret = mbedtls_ssl_tls13_process_finished_message( ssl ); if( ret != 0 ) return( ret ); @@ -1625,11 +1626,14 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) case MBEDTLS_SSL_CLIENT_CERTIFICATE: ret = mbedtls_ssl_tls13_process_certificate( ssl ); - if( ret == 0 ) + 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: diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 2aeee4ffbc..eb9c1f48f9 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11291,7 +11291,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" \ @@ -11310,7 +11310,7 @@ requires_config_enabled MBEDTLS_SSL_CLI_C run_test "TLS 1.3: Server side check - mbedtls" \ "$P_SRV debug_level=4 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" \ - 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 +11329,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" \