Change code base on comments

Remove client certificate verify in tests.
Change the layout of structure to fix abi_api check issues.
Add comments of Finished.
Align with the coding styles.

Signed-off-by: XiaokangQian <xiaokang.qian@arm.com>

include/mbedtls/ssl.h

@@ -1124,10 +1124,6 @@ struct mbedtls_ssl_session
      * to be studied whether one of them can be removed. */
     unsigned char MBEDTLS_PRIVATE(minor_ver);    /*!< The TLS version used in the session. */
 
-#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL)
-    mbedtls_ssl_tls1_3_application_secrets MBEDTLS_PRIVATE(app_secrets);
-#endif
-
 #if defined(MBEDTLS_X509_CRT_PARSE_C)
 #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
     mbedtls_x509_crt *MBEDTLS_PRIVATE(peer_cert);       /*!< peer X.509 cert chain */
@@ -1154,6 +1150,10 @@ struct mbedtls_ssl_session
 #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC)
     int MBEDTLS_PRIVATE(encrypt_then_mac);       /*!< flag for EtM activation                */
 #endif
+
+#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL)
+    mbedtls_ssl_tls1_3_application_secrets MBEDTLS_PRIVATE(app_secrets);
+#endif
 };
 
 /*

library/ssl_tls13_generic.c

@@ -876,8 +876,17 @@ static int ssl_tls13_parse_finished_message( mbedtls_ssl_context *ssl,
                                              const unsigned char *buf,
                                              const unsigned char *end )
 {
+    /*
+     * struct {
+     * opaque verify_data[Hash.length];
+     * } Finished;
+     */
+    const unsigned char *expected_verify_data =
+        ssl->handshake->state_local.finished_in.digest;
+    size_t expected_verify_data_len =
+        ssl->handshake->state_local.finished_in.digest_len;
     /* Structural validation */
-    if( (size_t)( end - buf ) != ssl->handshake->state_local.finished_in.digest_len )
+    if( (size_t)( end - buf ) != expected_verify_data_len )
     {
         MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) );
 
@@ -887,19 +896,19 @@ static int ssl_tls13_parse_finished_message( mbedtls_ssl_context *ssl,
     }
 
     MBEDTLS_SSL_DEBUG_BUF( 4, "verify_data (self-computed):",
-                           ssl->handshake->state_local.finished_in.digest,
-                           ssl->handshake->state_local.finished_in.digest_len );
+                           expected_verify_data,
+                           expected_verify_data_len );
     MBEDTLS_SSL_DEBUG_BUF( 4, "verify_data (received message):", buf,
-                           ssl->handshake->state_local.finished_in.digest_len );
+                           expected_verify_data_len );
 
     /* Semantic validation */
     if( mbedtls_ssl_safer_memcmp( buf,
-                   ssl->handshake->state_local.finished_in.digest,
-                   ssl->handshake->state_local.finished_in.digest_len ) != 0 )
+                                  expected_verify_data,
+                                  expected_verify_data_len ) != 0 )
     {
         MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) );
 
-        MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR,
+        MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECRYPT_ERROR,
                               MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE );
         return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE );
     }
@@ -908,7 +917,7 @@ static int ssl_tls13_parse_finished_message( mbedtls_ssl_context *ssl,
 
 static int ssl_tls13_postprocess_server_finished_message( mbedtls_ssl_context *ssl )
 {
-    int ret = 0;
+    int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
     mbedtls_ssl_key_set traffic_keys;
     mbedtls_ssl_transform *transform_application = NULL;
 
@@ -920,8 +929,7 @@ static int ssl_tls13_postprocess_server_finished_message( mbedtls_ssl_context *s
         goto cleanup;
     }
 
-    ret = mbedtls_ssl_tls13_generate_application_keys(
-        ssl, &traffic_keys );
+    ret = mbedtls_ssl_tls13_generate_application_keys( ssl, &traffic_keys );
     if( ret != 0 )
     {
         MBEDTLS_SSL_DEBUG_RET( 1,
@@ -953,7 +961,7 @@ static int ssl_tls13_postprocess_server_finished_message( mbedtls_ssl_context *s
 
 cleanup:
 
-    mbedtls_platform_zeroize( &traffic_keys, sizeof( mbedtls_ssl_key_set ) );
+    mbedtls_platform_zeroize( &traffic_keys, sizeof( traffic_keys ) );
     if( ret != 0 )
     {
         mbedtls_free( transform_application );
@@ -977,7 +985,7 @@ static int ssl_tls13_postprocess_finished_message( mbedtls_ssl_context* ssl )
 
 int mbedtls_ssl_tls13_process_finished_message( mbedtls_ssl_context *ssl )
 {
-    int ret = 0;
+    int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
     unsigned char *buf;
     size_t buflen;
 

library/ssl_tls13_keys.c

@@ -568,7 +568,8 @@ int mbedtls_ssl_tls13_key_schedule_stage_application(
     mbedtls_ssl_context *ssl )
 {
     int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
-    mbedtls_md_type_t const md_type = ssl->handshake->ciphersuite_info->mac;
+    mbedtls_ssl_handshake_params *handshake = ssl->handshake;
+    mbedtls_md_type_t const md_type = handshake->ciphersuite_info->mac;
 #if defined(MBEDTLS_DEBUG_C)
     mbedtls_md_info_t const * const md_info = mbedtls_md_info_from_type( md_type );
     size_t const md_size = mbedtls_md_get_size( md_info );
@@ -578,9 +579,9 @@ int mbedtls_ssl_tls13_key_schedule_stage_application(
      * Compute MasterSecret
      */
     ret = mbedtls_ssl_tls1_3_evolve_secret( md_type,
-                    ssl->handshake->tls1_3_master_secrets.handshake,
+                    handshake->tls1_3_master_secrets.handshake,
                     NULL, 0,
-                    ssl->handshake->tls1_3_master_secrets.app );
+                    handshake->tls1_3_master_secrets.app );
     if( ret != 0 )
     {
         MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_evolve_secret", ret );
@@ -588,7 +589,7 @@ int mbedtls_ssl_tls13_key_schedule_stage_application(
     }
 
     MBEDTLS_SSL_DEBUG_BUF( 4, "Master secret",
-             ssl->handshake->tls1_3_master_secrets.app, md_size );
+             handshake->tls1_3_master_secrets.app, md_size );
 
     return( 0 );
 }
@@ -690,7 +691,7 @@ int mbedtls_ssl_tls13_calculate_verify_data( mbedtls_ssl_context* ssl,
 
 exit:
 
-    mbedtls_platform_zeroize( transcript, sizeof( transcript) );
+    mbedtls_platform_zeroize( transcript, sizeof( transcript ) );
     return( ret );
 }
 
@@ -1116,13 +1117,14 @@ int mbedtls_ssl_tls13_generate_application_keys(
                                         mbedtls_ssl_key_set *traffic_keys )
 {
     int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
+    mbedtls_ssl_handshake_params *handshake = ssl->handshake;
 
     /* Address at which to store the application secrets */
     mbedtls_ssl_tls1_3_application_secrets * const app_secrets =
         &ssl->session_negotiate->app_secrets;
 
     /* Holding the transcript up to and including the ServerFinished */
-    unsigned char transcript[MBEDTLS_MD_MAX_SIZE];
+    unsigned char transcript[MBEDTLS_TLS1_3_MD_MAX_SIZE];
     size_t transcript_len;
 
     /* Variables relating to the hash for the chosen ciphersuite. */
@@ -1139,11 +1141,11 @@ int mbedtls_ssl_tls13_generate_application_keys(
     /* Extract basic information about hash and ciphersuite */
 
     cipher_info = mbedtls_cipher_info_from_type(
-                                  ssl->handshake->ciphersuite_info->cipher );
+                                  handshake->ciphersuite_info->cipher );
     keylen = cipher_info->key_bitlen / 8;
     ivlen = cipher_info->iv_size;
 
-    md_type = ssl->handshake->ciphersuite_info->mac;
+    md_type = handshake->ciphersuite_info->mac;
     md_info = mbedtls_md_info_from_type( md_type );
     md_size = mbedtls_md_get_size( md_info );
 
@@ -1159,7 +1161,7 @@ int mbedtls_ssl_tls13_generate_application_keys(
     /* Compute application secrets from master secret and transcript hash. */
 
     ret = mbedtls_ssl_tls1_3_derive_application_secrets( md_type,
-                                   ssl->handshake->tls1_3_master_secrets.app,
+                                   handshake->tls1_3_master_secrets.app,
                                    transcript, transcript_len,
                                    app_secrets );
     if( ret != 0 )
@@ -1197,16 +1199,16 @@ int mbedtls_ssl_tls13_generate_application_keys(
         ssl->f_export_keys( ssl->p_export_keys,
                 MBEDTLS_SSL_KEY_EXPORT_TLS13_CLIENT_APPLICATION_TRAFFIC_SECRET,
                 app_secrets->client_application_traffic_secret_N, md_size,
-                ssl->handshake->randbytes + 32,
-                ssl->handshake->randbytes,
+                handshake->randbytes + 32,
+                handshake->randbytes,
                 MBEDTLS_SSL_TLS_PRF_NONE /* TODO: this should be replaced by
                                             a new constant for TLS 1.3! */ );
 
         ssl->f_export_keys( ssl->p_export_keys,
                 MBEDTLS_SSL_KEY_EXPORT_TLS13_SERVER_APPLICATION_TRAFFIC_SECRET,
                 app_secrets->server_application_traffic_secret_N, md_size,
-                ssl->handshake->randbytes + 32,
-                ssl->handshake->randbytes,
+                handshake->randbytes + 32,
+                handshake->randbytes,
                 MBEDTLS_SSL_TLS_PRF_NONE /* TODO: this should be replaced by
                                             a new constant for TLS 1.3! */ );
     }
@@ -1224,7 +1226,7 @@ int mbedtls_ssl_tls13_generate_application_keys(
 
  cleanup:
 
-    mbedtls_platform_zeroize( transcript, sizeof(transcript) );
+    mbedtls_platform_zeroize( transcript, sizeof( transcript ) );
     return( ret );
 }
 

tests/ssl-opt.sh

@@ -8820,8 +8820,8 @@ run_test    "TLS1.3: Test client hello msg work - openssl" \
             -c "tls1_3 client state: 19"    \
             -c "tls1_3 client state: 5"     \
             -c "tls1_3 client state: 3"     \
+            -c "tls1_3 client state: 9"     \
             -c "tls1_3 client state: 13"    \
-            -c "tls1_3 client state: 20"    \
             -c "tls1_3 client state: 11"    \
             -c "tls1_3 client state: 14"    \
             -c "tls1_3 client state: 15"    \
@@ -8852,8 +8852,8 @@ run_test    "TLS1.3: Test client hello msg work - gnutls" \
             -c "tls1_3 client state: 19"    \
             -c "tls1_3 client state: 5"     \
             -c "tls1_3 client state: 3"     \
+            -c "tls1_3 client state: 9"     \
             -c "tls1_3 client state: 13"    \
-            -c "tls1_3 client state: 20"    \
             -c "tls1_3 client state: 11"    \
             -c "tls1_3 client state: 14"    \
             -c "tls1_3 client state: 15"    \