diff --git a/ChangeLog.d/tls13-fix-finished-fetch.txt b/ChangeLog.d/tls13-fix-finished-fetch.txt new file mode 100644 index 000000000..28c30f909 --- /dev/null +++ b/ChangeLog.d/tls13-fix-finished-fetch.txt @@ -0,0 +1,5 @@ +Bugfix + * Fix handshake failure when the peer Finished message has not been received + yet when we first try to fetch it. The fetching is moved before the + preprocessing computations to avoid doing them multiple times, which was + causing the handshake to fail. diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 0a9969344..79d7dddea 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -642,15 +642,11 @@ typedef enum MBEDTLS_SSL_HANDSHAKE_OVER, MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET, MBEDTLS_SSL_SERVER_HELLO_VERIFY_REQUEST_SENT, -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) MBEDTLS_SSL_HELLO_RETRY_REQUEST, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS, MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY, -#if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE) MBEDTLS_SSL_CLIENT_CCS_AFTER_SERVER_FINISHED, MBEDTLS_SSL_CLIENT_CCS_BEFORE_2ND_CLIENT_HELLO, -#endif /* MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE */ -#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ } mbedtls_ssl_states; diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 044693af1..3b49ec5f7 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1179,13 +1179,15 @@ int mbedtls_ssl_tls13_process_finished_message( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse finished message" ) ); - /* Preprocessing step: Compute handshake digest */ - MBEDTLS_SSL_PROC_CHK( ssl_tls13_preprocess_finished_message( ssl ) ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( ssl, MBEDTLS_SSL_HS_FINISHED, &buf, &buf_len ) ); + + /* Preprocessing step: Compute handshake digest */ + MBEDTLS_SSL_PROC_CHK( ssl_tls13_preprocess_finished_message( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_finished_message( ssl, buf, buf + buf_len ) ); + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_FINISHED, buf, buf_len ); diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index f3843b1e8..65023075c 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1628,14 +1628,17 @@ 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 && ssl->session_negotiate->peer_cert != NULL ) + if( ret == 0 ) { - mbedtls_ssl_handshake_set_state( - ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY ); + if( 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 ); } - else - mbedtls_ssl_handshake_set_state( - ssl, MBEDTLS_SSL_CLIENT_FINISHED ); break; case MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY: diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 274f0de01..5192342c8 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -105,19 +105,15 @@ depends_on:MBEDTLS_SSL_PROTO_TLS1_2 mbedtls_endpoint_sanity:MBEDTLS_SSL_IS_SERVER Test moving clients handshake to state: HELLO_REQUEST -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_HELLO_REQUEST:1 Test moving clients handshake to state: CLIENT_HELLO -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_CLIENT_HELLO:1 Test moving clients handshake to state: SERVER_HELLO -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_SERVER_HELLO:1 Test moving clients handshake to state: SERVER_CERTIFICATE -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_SERVER_CERTIFICATE:1 Test moving clients handshake to state: SERVER_KEY_EXCHANGE @@ -125,7 +121,6 @@ depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_SERVER_KEY_EXCHANGE:1 Test moving clients handshake to state: CERTIFICATE_REQUEST -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_CERTIFICATE_REQUEST:1 Test moving clients handshake to state: SERVER_HELLO_DONE @@ -133,7 +128,6 @@ depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_SERVER_HELLO_DONE:1 Test moving clients handshake to state: CLIENT_CERTIFICATE -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_CLIENT_CERTIFICATE:1 Test moving clients handshake to state: CLIENT_KEY_EXCHANGE @@ -141,7 +135,6 @@ depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_CLIENT_KEY_EXCHANGE:1 Test moving clients handshake to state: CERTIFICATE_VERIFY -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_CERTIFICATE_VERIFY:1 Test moving clients handshake to state: CLIENT_CHANGE_CIPHER_SPEC @@ -149,7 +142,6 @@ depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC:1 Test moving clients handshake to state: CLIENT_FINISHED -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_CLIENT_FINISHED:1 Test moving clients handshake to state: SERVER_CHANGE_CIPHER_SPEC @@ -157,35 +149,27 @@ depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC:1 Test moving clients handshake to state: SERVER_FINISHED -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_SERVER_FINISHED:1 Test moving clients handshake to state: FLUSH_BUFFERS -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_FLUSH_BUFFERS:1 Test moving clients handshake to state: HANDSHAKE_WRAPUP -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_HANDSHAKE_WRAPUP:1 Test moving clients handshake to state: HANDSHAKE_OVER -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_HANDSHAKE_OVER:1 Test moving servers handshake to state: HELLO_REQUEST -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_HELLO_REQUEST:1 Test moving servers handshake to state: CLIENT_HELLO -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_CLIENT_HELLO:1 Test moving servers handshake to state: SERVER_HELLO -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_SERVER_HELLO:1 Test moving servers handshake to state: SERVER_CERTIFICATE -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_SERVER_CERTIFICATE:1 Test moving servers handshake to state: SERVER_KEY_EXCHANGE @@ -193,7 +177,6 @@ depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_SERVER_KEY_EXCHANGE:1 Test moving servers handshake to state: CERTIFICATE_REQUEST -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_CERTIFICATE_REQUEST:1 Test moving servers handshake to state: SERVER_HELLO_DONE @@ -201,7 +184,6 @@ depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_SERVER_HELLO_DONE:1 Test moving servers handshake to state: CLIENT_CERTIFICATE -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_CLIENT_CERTIFICATE:1 Test moving servers handshake to state: CLIENT_KEY_EXCHANGE @@ -209,7 +191,6 @@ depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_CLIENT_KEY_EXCHANGE:1 Test moving servers handshake to state: CERTIFICATE_VERIFY -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_CERTIFICATE_VERIFY:1 Test moving servers handshake to state: CLIENT_CHANGE_CIPHER_SPEC @@ -217,7 +198,6 @@ depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC:1 Test moving servers handshake to state: CLIENT_FINISHED -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_CLIENT_FINISHED:1 Test moving servers handshake to state: SERVER_CHANGE_CIPHER_SPEC @@ -225,7 +205,6 @@ depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC:1 Test moving servers handshake to state: SERVER_FINISHED -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_SERVER_FINISHED:1 Test moving servers handshake to state: FLUSH_BUFFERS @@ -233,11 +212,9 @@ depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_FLUSH_BUFFERS:1 Test moving servers handshake to state: HANDSHAKE_WRAPUP -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_HANDSHAKE_WRAPUP:1 Test moving servers handshake to state: HANDSHAKE_OVER -depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_HANDSHAKE_OVER:1 Negative test moving clients ssl to state: VERIFY_REQUEST_SENT @@ -248,10 +225,30 @@ Negative test moving servers ssl to state: NEW_SESSION_TICKET depends_on:MBEDTLS_SSL_PROTO_TLS1_2 move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET:0 +TLS 1.3:Test moving clients handshake to state: ENCRYPTED_EXTENSIONS +depends_on:MBEDTLS_SSL_PROTO_TLS1_3:!MBEDTLS_SSL_PROTO_TLS1_2 +move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_ENCRYPTED_EXTENSIONS:1 + +TLS 1.3:Test moving servers handshake to state: ENCRYPTED_EXTENSIONS +depends_on:MBEDTLS_SSL_PROTO_TLS1_3:!MBEDTLS_SSL_PROTO_TLS1_2 +move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_ENCRYPTED_EXTENSIONS:1 + +TLS 1.3:Test moving clients handshake to state: CLIENT_CERTIFICATE_VERIFY +depends_on:MBEDTLS_SSL_PROTO_TLS1_3:!MBEDTLS_SSL_PROTO_TLS1_2 +move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY:1 + +TLS 1.3:Test moving servers handshake to state: CLIENT_CERTIFICATE_VERIFY +depends_on:MBEDTLS_SSL_PROTO_TLS1_3:!MBEDTLS_SSL_PROTO_TLS1_2 +move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY:1 + Handshake, tls1_2 depends_on:MBEDTLS_SSL_PROTO_TLS1_2 handshake_version:0:MBEDTLS_SSL_VERSION_TLS1_2:MBEDTLS_SSL_VERSION_TLS1_2:MBEDTLS_SSL_VERSION_TLS1_2:MBEDTLS_SSL_VERSION_TLS1_2:MBEDTLS_SSL_VERSION_TLS1_2 +Handshake, tls1_3 +depends_on:MBEDTLS_SSL_PROTO_TLS1_3 +handshake_version:0:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_SSL_VERSION_TLS1_3 + Handshake, ECDHE-RSA-WITH-AES-256-GCM-SHA384 depends_on:MBEDTLS_SHA384_C:MBEDTLS_AES_C:MBEDTLS_GCM_C:MBEDTLS_RSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED handshake_cipher:"TLS-ECDHE-RSA-WITH-AES-256-GCM-SHA384":MBEDTLS_PK_RSA:0 diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 2685e6ada..8d683adb4 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -886,6 +886,7 @@ exit: * * \retval 0 on success, otherwise error code. */ + int mbedtls_endpoint_init( mbedtls_endpoint *ep, int endpoint_type, int pk_alg, mbedtls_test_message_socket_context *dtls_context, mbedtls_test_message_queue *input_queue, @@ -966,6 +967,8 @@ int mbedtls_endpoint_init( mbedtls_endpoint *ep, int endpoint_type, int pk_alg, if( group_list != NULL ) mbedtls_ssl_conf_groups( &(ep->conf), group_list ); + mbedtls_ssl_conf_authmode( &( ep->conf ), MBEDTLS_SSL_VERIFY_REQUIRED ); + ret = mbedtls_ssl_setup( &( ep->ssl ), &( ep->conf ) ); TEST_ASSERT( ret == 0 ); @@ -2096,6 +2099,17 @@ void perform_handshake( handshake_test_options* options ) } TEST_ASSERT( mbedtls_ssl_is_handshake_over( &client.ssl ) == 1 ); + + /* Make sure server state is moved to HANDSHAKE_OVER also. */ + TEST_ASSERT( mbedtls_move_handshake_to_state( &(server.ssl), + &(client.ssl), + MBEDTLS_SSL_HANDSHAKE_OVER ) + == expected_handshake_result ); + if( expected_handshake_result != 0 ) + { + goto exit; + } + TEST_ASSERT( mbedtls_ssl_is_handshake_over( &server.ssl ) == 1 ); /* Check that both sides have negotiated the expected version. */ @@ -4867,7 +4881,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED:!MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_PKCS1_V15:MBEDTLS_ENTROPY_C:MBEDTLS_ENTROPY_C:MBEDTLS_CTR_DRBG_C */ +/* BEGIN_CASE depends_on:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_PKCS1_V15:MBEDTLS_ENTROPY_C:MBEDTLS_CTR_DRBG_C */ void move_handshake_to_state(int endpoint_type, int state, int need_pass) { enum { BUFFSIZE = 1024 }; @@ -4896,12 +4910,16 @@ void move_handshake_to_state(int endpoint_type, int state, int need_pass) state ); if( need_pass ) { - TEST_ASSERT( ret == 0 ); + TEST_ASSERT( ret == 0 || + ret == MBEDTLS_ERR_SSL_WANT_READ || + ret == MBEDTLS_ERR_SSL_WANT_WRITE ); TEST_ASSERT( base_ep.ssl.state == state ); } else { - TEST_ASSERT( ret != 0 ); + TEST_ASSERT( ret != 0 && + ret != MBEDTLS_ERR_SSL_WANT_READ && + ret != MBEDTLS_ERR_SSL_WANT_WRITE ); TEST_ASSERT( base_ep.ssl.state != state ); }