Fix implementation of VERIFY_OPTIONAL verification mode
This commit changes the behaviour of mbedtls_ssl_parse_certificate to make the two authentication modes MBEDTLS_SSL_VERIFY_REQUIRED and MBEDTLS_SSL_VERIFY_OPTIONAL be in the following relationship: Mode == MBEDTLS_SSL_VERIFY_REQUIRED <=> Mode == MBEDTLS_SSL_VERIFY_OPTIONAL + check verify result Also, it changes the behaviour to perform the certificate chain verification even if the trusted CA chain is empty. Previously, the function failed in this case, even when using optional verification, which was brought up in #864.
This commit is contained in:
parent
ddc6e52cc1
commit
39ae8cd207
2 changed files with 29 additions and 11 deletions
|
@ -17,6 +17,13 @@ Bugfix
|
||||||
that triggered the alert.
|
that triggered the alert.
|
||||||
* In SSLv3, if refusing a renegotiation attempt, don't process any further
|
* In SSLv3, if refusing a renegotiation attempt, don't process any further
|
||||||
data.
|
data.
|
||||||
|
* Accept empty trusted CA chain in authentication mode
|
||||||
|
MBEDTLS_SSL_VERIFY_OPTIONAL.
|
||||||
|
Fixes #864. Found by jethrogb.
|
||||||
|
* Fix implementation of mbedtls_ssl_parse_certificate
|
||||||
|
to not annihilate fatal errors in authentication mode
|
||||||
|
MBEDTLS_SSL_VERIFY_OPTIONAL and to reflect bad EC curves
|
||||||
|
within verification result.
|
||||||
|
|
||||||
Changes
|
Changes
|
||||||
* Send fatal alerts in many more cases instead of dropping the connection.
|
* Send fatal alerts in many more cases instead of dropping the connection.
|
||||||
|
|
|
@ -4472,14 +4472,6 @@ int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl )
|
||||||
ca_crl = ssl->conf->ca_crl;
|
ca_crl = ssl->conf->ca_crl;
|
||||||
}
|
}
|
||||||
|
|
||||||
if( ca_chain == NULL )
|
|
||||||
{
|
|
||||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "got no CA chain" ) );
|
|
||||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
|
||||||
MBEDTLS_SSL_ALERT_MSG_BAD_CERT );
|
|
||||||
return( MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED );
|
|
||||||
}
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Main check: verify certificate
|
* Main check: verify certificate
|
||||||
*/
|
*/
|
||||||
|
@ -4508,6 +4500,8 @@ int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl )
|
||||||
if( mbedtls_pk_can_do( pk, MBEDTLS_PK_ECKEY ) &&
|
if( mbedtls_pk_can_do( pk, MBEDTLS_PK_ECKEY ) &&
|
||||||
mbedtls_ssl_check_curve( ssl, mbedtls_pk_ec( *pk )->grp.id ) != 0 )
|
mbedtls_ssl_check_curve( ssl, mbedtls_pk_ec( *pk )->grp.id ) != 0 )
|
||||||
{
|
{
|
||||||
|
ssl->session_negotiate->verify_result |= MBEDTLS_X509_BADCERT_BAD_KEY;
|
||||||
|
|
||||||
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate (EC key curve)" ) );
|
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate (EC key curve)" ) );
|
||||||
if( ret == 0 )
|
if( ret == 0 )
|
||||||
ret = MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE;
|
ret = MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE;
|
||||||
|
@ -4525,8 +4519,24 @@ int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl )
|
||||||
ret = MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE;
|
ret = MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE;
|
||||||
}
|
}
|
||||||
|
|
||||||
if( authmode == MBEDTLS_SSL_VERIFY_OPTIONAL )
|
/* 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_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_HS_CERTIFICATE ) )
|
||||||
|
{
|
||||||
ret = 0;
|
ret = 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
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;
|
||||||
|
}
|
||||||
|
|
||||||
if( ret != 0 )
|
if( ret != 0 )
|
||||||
{
|
{
|
||||||
|
@ -4558,6 +4568,7 @@ int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl )
|
||||||
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
|
||||||
alert );
|
alert );
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate" ) );
|
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate" ) );
|
||||||
|
|
Loading…
Reference in a new issue