Remove useless guards and refine checking

Change-Id: I9cd3073826fc65c203e479d83bed72331ff8963d
Signed-off-by: XiaokangQian <xiaokang.qian@arm.com>
This commit is contained in:
XiaokangQian 2022-05-13 05:55:41 +00:00
parent 189ded2b07
commit c3017f620f
4 changed files with 43 additions and 51 deletions

View File

@ -510,8 +510,9 @@ struct mbedtls_ssl_handshake_params
#endif #endif
#if defined(MBEDTLS_SSL_SRV_C) #if defined(MBEDTLS_SSL_SRV_C)
/** cert_request_send to indicate whether client certitifacte request */ /* Flag indicating if a CertificateRequest message has been sent
uint16_t cert_request_send; * to the client or not. */
uint16_t certificate_request_sent;
#endif /* MBEDTLS_SSL_SRV_C */ #endif /* MBEDTLS_SSL_SRV_C */
#if defined(MBEDTLS_SSL_SESSION_TICKETS) #if defined(MBEDTLS_SSL_SESSION_TICKETS)

View File

@ -315,14 +315,6 @@ int mbedtls_ssl_tls13_process_certificate_verify( mbedtls_ssl_context *ssl )
MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate verify" ) ); 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_PROC_CHK(
mbedtls_ssl_tls13_fetch_handshake_msg( ssl, mbedtls_ssl_tls13_fetch_handshake_msg( ssl,
MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, &buf, &buf_len ) ); 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 ); 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 = if( ( ssl->session_negotiate->peer_cert =
mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) ) ) == NULL ) 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 ); return( MBEDTLS_ERR_SSL_DECODE_ERROR );
} }
exit:
MBEDTLS_SSL_DEBUG_CRT( 3, "peer certificate", ssl->session_negotiate->peer_cert ); MBEDTLS_SSL_DEBUG_CRT( 3, "peer certificate", ssl->session_negotiate->peer_cert );
return( ret ); return( ret );
@ -620,15 +620,15 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl )
} }
#endif /* MBEDTLS_SSL_SRV_C */ #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 MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_NO_CERT,
* server-side case of VERIFY_NONE being handled earlier MBEDTLS_ERR_SSL_FATAL_ALERT_MESSAGE );
* and leading to `ssl->verify_result` being set to return( MBEDTLS_ERR_SSL_FATAL_ALERT_MESSAGE );
* MBEDTLS_X509_BADCERT_SKIP_VERIFY --
* is this difference intentional? */
return( 0 );
} }
#endif /* MBEDTLS_SSL_CLI_C */
#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION)
if( ssl->handshake->sni_ca_chain != NULL ) 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; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate" ) ); 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 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( MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg(
ssl, MBEDTLS_SSL_HS_CERTIFICATE, ssl, MBEDTLS_SSL_HS_CERTIFICATE,
&buf, &buf_len ) ); &buf, &buf_len ) );
/* Parse the certificate chain sent by the peer. */ /* Parse the certificate chain sent by the peer. */
MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate( ssl, buf, MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate( ssl, buf,
buf + buf_len ) ); buf + buf_len ) );
/* Validate the certificate chain and set the verification results. */ /* Validate the certificate chain and set the verification results. */
MBEDTLS_SSL_PROC_CHK( ssl_tls13_validate_certificate( ssl ) ); MBEDTLS_SSL_PROC_CHK( ssl_tls13_validate_certificate( ssl ) );
mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE, mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE,
buf, buf_len ); buf, buf_len );
}
else
#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ #endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate" ) );
ret = 0;
}
cleanup: cleanup:

View File

@ -1342,7 +1342,7 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl )
if( authmode == MBEDTLS_SSL_VERIFY_NONE ) if( authmode == MBEDTLS_SSL_VERIFY_NONE )
return( SSL_CERTIFICATE_REQUEST_SKIP ); return( SSL_CERTIFICATE_REQUEST_SKIP );
ssl->handshake->cert_request_send = 1; ssl->handshake->certificate_request_sent = 1;
return( SSL_CERTIFICATE_REQUEST_SEND_REQUEST ); 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 ); MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE );
return( ret ); return( ret );
} }
if( ssl->handshake->cert_request_send )
if( ssl->handshake->certificate_request_sent )
{ {
mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE ); 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; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
MBEDTLS_SSL_DEBUG_MSG( 1, MBEDTLS_SSL_DEBUG_MSG( 1,
( "Switch to handshake traffic keys for inbound traffic" ) ); ( "Switch to handshake traffic keys for outbound traffic" ) );
mbedtls_ssl_set_inbound_transform( ssl, ssl->handshake->transform_handshake ); if( ! ssl->handshake->certificate_request_sent )
mbedtls_ssl_set_inbound_transform( ssl, ssl->handshake->transform_handshake );
ret = mbedtls_ssl_tls13_process_finished_message( ssl ); ret = mbedtls_ssl_tls13_process_finished_message( ssl );
if( ret != 0 ) if( ret != 0 )
return( ret ); return( ret );
@ -1625,11 +1626,14 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl )
case MBEDTLS_SSL_CLIENT_CERTIFICATE: case MBEDTLS_SSL_CLIENT_CERTIFICATE:
ret = mbedtls_ssl_tls13_process_certificate( ssl ); ret = mbedtls_ssl_tls13_process_certificate( ssl );
if( ret == 0 ) if( ret == 0 && ssl->session_negotiate->peer_cert != NULL)
{ {
mbedtls_ssl_handshake_set_state( mbedtls_ssl_handshake_set_state(
ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY ); ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY );
} }
else
mbedtls_ssl_handshake_set_state(
ssl, MBEDTLS_SSL_CLIENT_FINISHED );
break; break;
case MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY: case MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY:

View File

@ -11291,7 +11291,7 @@ requires_config_enabled MBEDTLS_SSL_SRV_C
run_test "TLS 1.3: Server side check - gnutls with client authentication" \ 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" \ "$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" \ "$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_CLIENT_HELLO" \
-s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \
-s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ -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" \ 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_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" \ "$P_CLI debug_level=4 force_version=tls13" \
1 \ 0 \
-s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \
-s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \
-s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ -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" \ 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_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" \ "$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_CLIENT_HELLO" \
-s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \
-s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \