Update code base on comments

Move reset transcript for hrr to generic
Reset SHA256 or SHA384 other than both
Rename message layer reset
Add check log for hrr parse successfully

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

library/ssl_misc.h

@@ -1447,8 +1447,8 @@ void mbedtls_ssl_update_out_pointers( mbedtls_ssl_context *ssl,
 void mbedtls_ssl_update_in_pointers( mbedtls_ssl_context *ssl );
 
 int mbedtls_ssl_session_reset_int( mbedtls_ssl_context *ssl, int partial );
-void mbedtls_ssl_tls13_session_reset_msg_layer( mbedtls_ssl_context *ssl,
-                                                int partial );
+void mbedtls_ssl_session_reset_msg_layer( mbedtls_ssl_context *ssl,
+                                          int partial );
 
 /*
  * Send pending alert

library/ssl_tls.c

@@ -3400,8 +3400,8 @@ error:
  * If partial is non-zero, keep data in the input buffer and client ID.
  * (Use when a DTLS client reconnects from the same port.)
  */
-void mbedtls_ssl_tls13_session_reset_msg_layer( mbedtls_ssl_context *ssl,
-                                                int partial )
+void mbedtls_ssl_session_reset_msg_layer( mbedtls_ssl_context *ssl,
+                                          int partial )
 {
 #if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH)
     size_t in_buf_len = ssl->in_buf_len;
@@ -3467,7 +3467,7 @@ int mbedtls_ssl_session_reset_int( mbedtls_ssl_context *ssl, int partial )
 
     ssl->state = MBEDTLS_SSL_HELLO_REQUEST;
 
-    mbedtls_ssl_tls13_session_reset_msg_layer( ssl, partial );
+    mbedtls_ssl_session_reset_msg_layer( ssl, partial );
 
     /* Reset renegotiation state */
 #if defined(MBEDTLS_SSL_RENEGOTIATION)
@@ -7615,105 +7615,5 @@ int mbedtls_ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf,
     return( 0 );
 }
 #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */
-#if defined(MBEDTLS_SSL_PROTO_TLS1_3)
-
-static int ssl_hash_transcript_core( mbedtls_ssl_context *ssl,
-                                     mbedtls_md_type_t md,
-                                     unsigned char *transcript,
-                                     size_t len,
-                                     size_t *olen )
-{
-    int ret;
-    size_t hash_size;
-
-    if( len < 4 )
-        return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
-
-    ret = mbedtls_ssl_get_handshake_transcript( ssl, md,
-                                                transcript + 4,
-                                                len - 4,
-                                                &hash_size );
-    if( ret != 0 )
-    {
-        MBEDTLS_SSL_DEBUG_RET( 4, "mbedtls_ssl_get_handshake_transcript", ret );
-        return( ret );
-    }
-
-    transcript[0] = MBEDTLS_SSL_HS_MESSAGE_HASH;
-    transcript[1] = 0;
-    transcript[2] = 0;
-    transcript[3] = (unsigned char) hash_size;
-
-    *olen = 4 + hash_size;
-    return( 0 );
-}
-
-/* Reset SSL context and update hash for handling HRR.
- *
- * Replace Transcript-Hash(X) by
- * Transcript-Hash( message_hash     ||
- *                 00 00 Hash.length ||
- *                 X )
- * A few states of the handshake are preserved, including:
- *   - session ID
- *   - session ticket
- *   - negotiated ciphersuite
- */
-int mbedtls_ssl_reset_transcript_for_hrr( mbedtls_ssl_context *ssl )
-{
-    int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
-    unsigned char hash_transcript[ MBEDTLS_MD_MAX_SIZE + 4 ];
-    size_t hash_olen;
-
-    MBEDTLS_SSL_DEBUG_MSG( 3, ( "Reset SSL session for HRR" ) );
-
-#if defined(MBEDTLS_SHA256_C)
-    ret = ssl_hash_transcript_core( ssl, MBEDTLS_MD_SHA256,
-                                    hash_transcript,
-                                    sizeof( hash_transcript ),
-                                    &hash_olen );
-    if( ret != 0 )
-    {
-        MBEDTLS_SSL_DEBUG_RET( 4, "ssl_hash_transcript_core", ret );
-        return( ret );
-    }
-    MBEDTLS_SSL_DEBUG_BUF( 4, "Truncated SHA-256 handshake transcript",
-                           hash_transcript, hash_olen );
-
-#if defined(MBEDTLS_USE_PSA_CRYPTO)
-    psa_hash_abort( &ssl->handshake->fin_sha256_psa );
-    psa_hash_setup( &ssl->handshake->fin_sha256_psa, PSA_ALG_SHA_256 );
-#else
-    mbedtls_sha256_starts( &ssl->handshake->fin_sha256, 0 );
-#endif
-    ssl_update_checksum_sha256( ssl, hash_transcript, hash_olen );
-#endif /* MBEDTLS_SHA256_C */
-
-#if defined(MBEDTLS_SHA384_C)
-    ret = ssl_hash_transcript_core( ssl, MBEDTLS_MD_SHA384,
-                                    hash_transcript,
-                                    sizeof( hash_transcript ),
-                                    &hash_olen );
-    if( ret != 0 )
-    {
-        MBEDTLS_SSL_DEBUG_RET( 4, "ssl_hash_transcript_core", ret );
-        return( ret );
-    }
-    MBEDTLS_SSL_DEBUG_BUF( 4, "Truncated SHA-384 handshake transcript",
-                           hash_transcript, hash_olen );
-
-#if defined(MBEDTLS_USE_PSA_CRYPTO)
-    psa_hash_abort( &ssl->handshake->fin_sha384_psa );
-    psa_hash_setup( &ssl->handshake->fin_sha384_psa, PSA_ALG_SHA_384 );
-#else
-    mbedtls_sha512_starts( &ssl->handshake->fin_sha512, 1 );
-#endif
-    ssl_update_checksum_sha384( ssl, hash_transcript, hash_olen );
-#endif /* MBEDTLS_SHA384_C */
-
-    return( ret );
-}
-
-#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */
 
 #endif /* MBEDTLS_SSL_TLS_C */

library/ssl_tls13_client.c

@@ -136,12 +136,6 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl,
  */
 
 #if defined(MBEDTLS_ECDH_C)
-static int ssl_tls13_reset_ecdhe_share( mbedtls_ssl_context *ssl )
-{
-    mbedtls_ecdh_free( &ssl->handshake->ecdh_ctx );
-    return( 0 );
-}
-
 static int ssl_tls13_reset_key_share( mbedtls_ssl_context *ssl )
 {
     uint16_t group_id = ssl->handshake->offered_group_id;
@@ -149,7 +143,10 @@ static int ssl_tls13_reset_key_share( mbedtls_ssl_context *ssl )
         return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
 
     if( mbedtls_ssl_tls13_named_group_is_ecdhe( group_id ) )
-        return( ssl_tls13_reset_ecdhe_share( ssl ) );
+    {
+        mbedtls_ecdh_free( &ssl->handshake->ecdh_ctx );
+        return( 0 );
+    }
     else if( 0 /* other KEMs? */ )
     {
         /* Do something */
@@ -425,7 +422,6 @@ static int ssl_tls13_parse_hrr_key_share_ext( mbedtls_ssl_context *ssl,
                                               const unsigned char *buf,
                                               const unsigned char *end )
 {
-    /* Variables for parsing the key_share */
     const mbedtls_ecp_curve_info *curve_info = NULL;
     const unsigned char *p = buf;
     int tls_id;
@@ -1030,7 +1026,7 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl,
     const unsigned char *extensions_end;
     uint16_t cipher_suite;
     const mbedtls_ssl_ciphersuite_t *ciphersuite_info;
-    int supported_versions_exist = 0;
+    int supported_versions_ext_found = 0;
 #if defined(MBEDTLS_SSL_COOKIE_C)
     size_t cookie_len;
     unsigned char *cookie;
@@ -1228,7 +1224,7 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl,
 #endif /* MBEDTLS_SSL_COOKIE_C */
 
             case MBEDTLS_TLS_EXT_SUPPORTED_VERSIONS:
-                supported_versions_exist = 1;
+                supported_versions_ext_found = 1;
                 MBEDTLS_SSL_DEBUG_MSG( 3,
                             ( "found supported_versions extension" ) );
 
@@ -1282,9 +1278,9 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl,
         p += extension_data_len;
     }
 
-    if( !supported_versions_exist )
+    if( !supported_versions_ext_found )
     {
-        MBEDTLS_SSL_DEBUG_MSG( 1, ( "supported_versions not exist" ) );
+        MBEDTLS_SSL_DEBUG_MSG( 1, ( "supported_versions not found" ) );
         MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER,
                                       MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER );
         return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER );
@@ -1424,14 +1420,12 @@ static int ssl_tls13_finalize_hrr( mbedtls_ssl_context *ssl )
     mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO );
 #endif /* MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE */
 
-    mbedtls_ssl_tls13_session_reset_msg_layer( ssl, 0 );
+    mbedtls_ssl_session_reset_msg_layer( ssl, 0 );
 
-    /* Reset everything that's going to be re-generated in the new ClientHello.
-     *
-     * Currently, we're always resetting the key share, even if the server
-     * was fine with it. Once we have separated key share generation from
-     * key share writing, we can confine this to the case where the server
-     * requested a different share.
+    /*
+     * We are going to re-generate a shared secret corresponding to the group selected by the server,
+     * which is different from the group for which we generated a shared secret in the first client
+     * hello.  Thus, reset the shared secret.
      */
 #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED)
     ret = ssl_tls13_reset_key_share( ssl );
@@ -1451,7 +1445,7 @@ static int ssl_tls13_process_server_hello( mbedtls_ssl_context *ssl )
     int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
     unsigned char *buf = NULL;
     size_t buf_len = 0;
-    int is_hrr = -1;
+    int is_hrr = 0;
 
     MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> %s", __func__ ) );
 
@@ -1488,7 +1482,7 @@ static int ssl_tls13_process_server_hello( mbedtls_ssl_context *ssl )
         MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_server_hello( ssl ) );
 
 cleanup:
-    MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= %s", __func__ ) );
+    MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= %s:is_hrr = %d", __func__, is_hrr ) );
     return( ret );
 }
 

library/ssl_tls13_generic.c

@@ -1128,6 +1128,111 @@ cleanup:
 
 #endif /* MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE */
 
+static int ssl_hash_transcript_core( mbedtls_ssl_context *ssl,
+                                     mbedtls_md_type_t md,
+                                     unsigned char *transcript,
+                                     size_t len,
+                                     size_t *olen )
+{
+    int ret;
+    size_t hash_size;
+
+    if( len < 4 )
+        return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
+
+    ret = mbedtls_ssl_get_handshake_transcript( ssl, md,
+                                                transcript + 4,
+                                                len - 4,
+                                                &hash_size );
+    if( ret != 0 )
+    {
+        MBEDTLS_SSL_DEBUG_RET( 4, "mbedtls_ssl_get_handshake_transcript", ret );
+        return( ret );
+    }
+
+    transcript[0] = MBEDTLS_SSL_HS_MESSAGE_HASH;
+    transcript[1] = 0;
+    transcript[2] = 0;
+    transcript[3] = (unsigned char) hash_size;
+
+    *olen = 4 + hash_size;
+    return( 0 );
+}
+
+/* Reset SSL context and update hash for handling HRR.
+ *
+ * Replace Transcript-Hash(X) by
+ * Transcript-Hash( message_hash     ||
+ *                 00 00 Hash.length ||
+ *                 X )
+ * A few states of the handshake are preserved, including:
+ *   - session ID
+ *   - session ticket
+ *   - negotiated ciphersuite
+ */
+int mbedtls_ssl_reset_transcript_for_hrr( mbedtls_ssl_context *ssl )
+{
+    int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
+    unsigned char hash_transcript[ MBEDTLS_MD_MAX_SIZE + 4 ];
+    size_t hash_olen;
+    const mbedtls_ssl_ciphersuite_t *ciphersuite_info;
+    uint16_t cipher_suite = ssl->session_negotiate->ciphersuite;
+    ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( cipher_suite );
+
+    MBEDTLS_SSL_DEBUG_MSG( 3, ( "Reset SSL session for HRR" ) );
+
+    if( ciphersuite_info->mac == MBEDTLS_MD_SHA256 )
+    {
+#if defined(MBEDTLS_SHA256_C)
+        ret = ssl_hash_transcript_core( ssl, MBEDTLS_MD_SHA256,
+                                        hash_transcript,
+                                        sizeof( hash_transcript ),
+                                        &hash_olen );
+        if( ret != 0 )
+        {
+            MBEDTLS_SSL_DEBUG_RET( 4, "ssl_hash_transcript_core", ret );
+            return( ret );
+        }
+        MBEDTLS_SSL_DEBUG_BUF( 4, "Truncated SHA-256 handshake transcript",
+                               hash_transcript, hash_olen );
+
+#if defined(MBEDTLS_USE_PSA_CRYPTO)
+        psa_hash_abort( &ssl->handshake->fin_sha256_psa );
+        psa_hash_setup( &ssl->handshake->fin_sha256_psa, PSA_ALG_SHA_256 );
+#else
+        mbedtls_sha256_starts( &ssl->handshake->fin_sha256, 0 );
+#endif
+    ssl->handshake->update_checksum( ssl, hash_transcript, hash_olen );
+#endif /* MBEDTLS_SHA256_C */
+    }
+    else if( ciphersuite_info->mac == MBEDTLS_MD_SHA384 )
+    {
+#if defined(MBEDTLS_SHA384_C)
+        ret = ssl_hash_transcript_core( ssl, MBEDTLS_MD_SHA384,
+                                        hash_transcript,
+                                        sizeof( hash_transcript ),
+                                        &hash_olen );
+        if( ret != 0 )
+        {
+            MBEDTLS_SSL_DEBUG_RET( 4, "ssl_hash_transcript_core", ret );
+            return( ret );
+        }
+        MBEDTLS_SSL_DEBUG_BUF( 4, "Truncated SHA-384 handshake transcript",
+                               hash_transcript, hash_olen );
+
+#if defined(MBEDTLS_USE_PSA_CRYPTO)
+        psa_hash_abort( &ssl->handshake->fin_sha384_psa );
+        psa_hash_setup( &ssl->handshake->fin_sha384_psa, PSA_ALG_SHA_384 );
+#else
+        mbedtls_sha512_starts( &ssl->handshake->fin_sha512, 1 );
+#endif
+    ssl->handshake->update_checksum( ssl, hash_transcript, hash_olen );
+#endif /* MBEDTLS_SHA384_C */
+    }
+
+    return( ret );
+}
+
 #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */
 
 #endif /* MBEDTLS_SSL_TLS_C */

tests/ssl-opt.sh

@@ -9219,6 +9219,7 @@ run_test    "TLS 1.3: HelloRetryRequest check - openssl" \
             "$P_CLI debug_level=4 force_version=tls13" \
             0 \
             -c "received HelloRetryRequest message" \
+            -c "<= ssl_tls13_process_server_hello:is_hrr = 1" \
             -c "tls13 client state: MBEDTLS_SSL_CLIENT_HELLO(1)" \
             -c "HTTP/1.0 200 ok"
 
@@ -9234,6 +9235,7 @@ run_test    "TLS 1.3: HelloRetryRequest check - gnutls" \
             "$P_CLI debug_level=4 force_version=tls13" \
             1 \
             -c "received HelloRetryRequest message" \
+            -c "<= ssl_tls13_process_server_hello:is_hrr = 1" \
             -c "tls13 client state: MBEDTLS_SSL_CLIENT_HELLO(1)" \
             -c "Last error was: -0x6E00 - SSL - The handshake negotiation failed" \
             -s "HELLO RETRY REQUEST was queued"