From dd5f624f32395e64e29dc537bdf208058544b226 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 28 Jun 2021 21:58:56 +0100 Subject: [PATCH 1/4] Fix TLS alert codes Signed-off-by: Dave Rodgman --- ChangeLog.d/fix_tls_alert_codes.txt | 6 ++++++ library/ssl_cli.c | 2 +- library/ssl_tls.c | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 ChangeLog.d/fix_tls_alert_codes.txt diff --git a/ChangeLog.d/fix_tls_alert_codes.txt b/ChangeLog.d/fix_tls_alert_codes.txt new file mode 100644 index 0000000000..abe3a5e6df --- /dev/null +++ b/ChangeLog.d/fix_tls_alert_codes.txt @@ -0,0 +1,6 @@ +Bugfix + * Fix the alert raised when a client requests an invalid + * fragment length, as per RFC6066 section 4. We now alert with + * MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER. Similarly, raise + * MBEDTLS_SSL_ALERT_MSG_DECRYPT_ERROR for an invalid finished + * message, as per RFC5247 section 7.2.2. diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 01e3f111e8..6710f133e8 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1525,7 +1525,7 @@ static int ssl_parse_max_fragment_length_ext( mbedtls_ssl_context *ssl, mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } diff --git a/library/ssl_tls.c b/library/ssl_tls.c index a4cf44c788..976a87c514 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3610,7 +3610,7 @@ int mbedtls_ssl_parse_finished( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + MBEDTLS_SSL_ALERT_MSG_DECRYPT_ERROR ); return( MBEDTLS_ERR_SSL_BAD_HS_FINISHED ); } From de8c91bbae8e96fa5c5641fa2e0384eea7e75c7f Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 29 Jun 2021 14:43:09 +0100 Subject: [PATCH 2/4] Improve Changelog for TLS alert codes Signed-off-by: Dave Rodgman --- ChangeLog.d/fix_tls_alert_codes.txt | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/ChangeLog.d/fix_tls_alert_codes.txt b/ChangeLog.d/fix_tls_alert_codes.txt index abe3a5e6df..10235d7bc3 100644 --- a/ChangeLog.d/fix_tls_alert_codes.txt +++ b/ChangeLog.d/fix_tls_alert_codes.txt @@ -1,6 +1,5 @@ Bugfix - * Fix the alert raised when a client requests an invalid - * fragment length, as per RFC6066 section 4. We now alert with - * MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER. Similarly, raise - * MBEDTLS_SSL_ALERT_MSG_DECRYPT_ERROR for an invalid finished - * message, as per RFC5247 section 7.2.2. + * Fix which alert is sent in some cases to conform to the + applicable RFC: on an invalid Finished message value, an + invalid max_fragment_length extension, or an + unsupported extension used by the server. From 0dfb7dbe15734b2a3f430d4e02b937705402c64e Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 29 Jun 2021 15:09:58 +0100 Subject: [PATCH 3/4] TLS UNSUPPORTED_EXTENSION error code changes Signed-off-by: Dave Rodgman --- library/ssl_cli.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 6710f133e8..f49178cf41 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1572,7 +1572,7 @@ static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl, { MBEDTLS_SSL_DEBUG_MSG( 1, ( "CID extension unexpected" ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_EXT ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -1628,7 +1628,7 @@ static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_EXT ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -1654,7 +1654,7 @@ static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_EXT ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -1679,7 +1679,7 @@ static int ssl_parse_session_ticket_ext( mbedtls_ssl_context *ssl, mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_EXT ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -1785,7 +1785,7 @@ static int ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_EXT ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } From 7ed75e21c9fda8e2ca3ee1b0f257b68241dc9cde Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 29 Jun 2021 19:05:34 +0100 Subject: [PATCH 4/4] Correct required config flag in ECJPAKE tests Signed-off-by: Dave Rodgman --- tests/ssl-opt.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 2200f9036d..5954004d47 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5992,7 +5992,7 @@ run_test "ECJPAKE: working, TLS" \ -S "SSL - Verification of the message MAC failed" server_needs_more_time 1 -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECJPAKE +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED run_test "ECJPAKE: password mismatch, TLS" \ "$P_SRV debug_level=3 ecjpake_pw=bla" \ "$P_CLI debug_level=3 ecjpake_pw=bad \ @@ -6001,7 +6001,7 @@ run_test "ECJPAKE: password mismatch, TLS" \ -C "re-using cached ecjpake parameters" \ -s "SSL - Verification of the message MAC failed" -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECJPAKE +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED run_test "ECJPAKE: working, DTLS" \ "$P_SRV debug_level=3 dtls=1 ecjpake_pw=bla" \ "$P_CLI debug_level=3 dtls=1 ecjpake_pw=bla \ @@ -6010,7 +6010,7 @@ run_test "ECJPAKE: working, DTLS" \ -c "re-using cached ecjpake parameters" \ -S "SSL - Verification of the message MAC failed" -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECJPAKE +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED run_test "ECJPAKE: working, DTLS, no cookie" \ "$P_SRV debug_level=3 dtls=1 ecjpake_pw=bla cookies=0" \ "$P_CLI debug_level=3 dtls=1 ecjpake_pw=bla \ @@ -6020,7 +6020,7 @@ run_test "ECJPAKE: working, DTLS, no cookie" \ -S "SSL - Verification of the message MAC failed" server_needs_more_time 1 -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECJPAKE +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED run_test "ECJPAKE: password mismatch, DTLS" \ "$P_SRV debug_level=3 dtls=1 ecjpake_pw=bla" \ "$P_CLI debug_level=3 dtls=1 ecjpake_pw=bad \ @@ -6030,7 +6030,7 @@ run_test "ECJPAKE: password mismatch, DTLS" \ -s "SSL - Verification of the message MAC failed" # for tests with configs/config-thread.h -requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECJPAKE +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED run_test "ECJPAKE: working, DTLS, nolog" \ "$P_SRV dtls=1 ecjpake_pw=bla" \ "$P_CLI dtls=1 ecjpake_pw=bla \