Merge pull request #5792 from yuhaoth/pr/add-tls13-moving-state-tests

Pr/add-tls13-moving-state-tests

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.

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;
 

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 );
 

library/ssl_tls13_server.c

@@ -1628,7 +1628,9 @@ 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 )
+            {
+                if( ssl->session_negotiate->peer_cert != NULL )
                 {
                     mbedtls_ssl_handshake_set_state(
                         ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY );
@@ -1636,6 +1638,7 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl )
                 else
                     mbedtls_ssl_handshake_set_state(
                         ssl, MBEDTLS_SSL_CLIENT_FINISHED );
+            }
             break;
 
         case MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY:

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

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 );
     }