Ignore failures when sending fatal alerts
In many places in TLS handling, some code detects a fatal error, sends
a fatal alert message, and returns to the caller. If sending the alert
fails, then return the error that triggered the alert, rather than
overriding the return status. This effectively causes alert sending
failures to be ignored. Formerly the code was inconsistently sometimes
doing one, sometimes the other.
In general ignoring the alert is the right thing: what matters to the
caller is the original error. A typical alert failure is that the
connection is already closed.
One case which remains not handled correctly is if the alert remains
in the output buffer (WANT_WRITE). Then it won't be sent, or will be
truncated. We'd need to either delay the application error or record
the write buffering notice; to be done later.
diff --git a/library/ssl_cli.c b/library/ssl_cli.c
index ddb1f9f..d3169d2 100644
--- a/library/ssl_cli.c
+++ b/library/ssl_cli.c
@@ -1057,8 +1057,6 @@
const unsigned char *buf,
size_t len )
{
- int ret;
-
#if defined(MBEDTLS_SSL_RENEGOTIATION)
if( ssl->renego_status != MBEDTLS_SSL_INITIAL_HANDSHAKE )
{
@@ -1071,10 +1069,8 @@
ssl->peer_verify_data, ssl->verify_data_len ) != 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching renegotiation info" ) );
-
- if( ( ret = mbedtls_ssl_send_fatal_handshake_failure( ssl ) ) != 0 )
- return( ret );
-
+ mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+ MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
}
}
@@ -1084,10 +1080,8 @@
if( len != 1 || buf[0] != 0x00 )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-zero length renegotiation info" ) );
-
- if( ( ret = mbedtls_ssl_send_fatal_handshake_failure( ssl ) ) != 0 )
- return( ret );
-
+ mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+ MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
}
@@ -1111,7 +1105,8 @@
buf[0] != ssl->conf->mfl_code )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching max fragment length extension" ) );
- mbedtls_ssl_send_fatal_handshake_failure( ssl );
+ mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+ MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
}
@@ -1128,7 +1123,8 @@
len != 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching truncated HMAC extension" ) );
- mbedtls_ssl_send_fatal_handshake_failure( ssl );
+ mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+ MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
}
@@ -1150,7 +1146,8 @@
len != 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching encrypt-then-MAC extension" ) );
- mbedtls_ssl_send_fatal_handshake_failure( ssl );
+ mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+ MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
}
@@ -1172,7 +1169,8 @@
len != 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching extended master secret extension" ) );
- mbedtls_ssl_send_fatal_handshake_failure( ssl );
+ mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+ MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
}
@@ -1193,7 +1191,8 @@
len != 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching session ticket extension" ) );
- mbedtls_ssl_send_fatal_handshake_failure( ssl );
+ mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+ MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
}
@@ -1244,7 +1243,8 @@
}
MBEDTLS_SSL_DEBUG_MSG( 1, ( "no point format in common" ) );
- mbedtls_ssl_send_fatal_handshake_failure( ssl );
+ mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+ MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
}
#endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C ||
@@ -1273,8 +1273,8 @@
buf, len ) ) != 0 )
{
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_read_round_one", ret );
- if( ( ret = mbedtls_ssl_send_fatal_handshake_failure( ssl ) ) != 0 )
- return( ret );
+ mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+ MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
return( ret );
}
@@ -1293,7 +1293,8 @@
if( ssl->conf->alpn_list == NULL )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching ALPN extension" ) );
- mbedtls_ssl_send_fatal_handshake_failure( ssl );
+ mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+ MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
}
@@ -1343,7 +1344,8 @@
}
MBEDTLS_SSL_DEBUG_MSG( 1, ( "ALPN extension: no matching protocol" ) );
- mbedtls_ssl_send_fatal_handshake_failure( ssl );
+ mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+ MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
}
#endif /* MBEDTLS_SSL_ALPN */
@@ -1910,9 +1912,8 @@
if( handshake_failure == 1 )
{
- if( ( ret = mbedtls_ssl_send_fatal_handshake_failure( ssl ) ) != 0 )
- return( ret );
-
+ mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+ MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
}