commit 53f20b71c58b4af878a3b02cf6801558d47149d7
author XiaokangQian <xiaokang.qian@arm.com> Tue Jan 18 10:47:33 2022 +0000
committer XiaokangQian <xiaokang.qian@arm.com> Wed Jan 26 10:51:13 2022 +0000
tree a9b185e1d787c3d6bb078bfa8bb18134c8b247a9
parent 6db08dd2cb893478e63bbbe389cec941f5ad3276

Improve ssl_tls13_parse_server_hello

Avoid coping random bytes in hrr
Send illegal parameter alert when cipher suite mismatch
Send illegal parameter alert when supported_version not exist

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

library/ssl_tls13_client.c

diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c
index e259b4d..dceef30 100644
--- a/library/ssl_tls13_client.c
+++ b/library/ssl_tls13_client.c
@@ -1030,6 +1030,7 @@
     const unsigned char *extensions_end;
     uint16_t cipher_suite;
     const mbedtls_ssl_ciphersuite_t *ciphersuite_info;
+    int supported_versions_exist = 0;
 #if defined(MBEDTLS_SSL_COOKIE_C)
     size_t cookie_len;
     unsigned char *cookie;
@@ -1071,10 +1072,13 @@
      * with Random defined as:
      * opaque Random[MBEDTLS_SERVER_HELLO_RANDOM_LEN];
      */
-    memcpy( &ssl->handshake->randbytes[MBEDTLS_CLIENT_HELLO_RANDOM_LEN], p,
-            MBEDTLS_SERVER_HELLO_RANDOM_LEN );
-    MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes",
-                           p, MBEDTLS_SERVER_HELLO_RANDOM_LEN );
+    if( !is_hrr )
+    {
+        memcpy( &ssl->handshake->randbytes[MBEDTLS_CLIENT_HELLO_RANDOM_LEN], p,
+                MBEDTLS_SERVER_HELLO_RANDOM_LEN );
+        MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes",
+                               p, MBEDTLS_SERVER_HELLO_RANDOM_LEN );
+    }
     p += MBEDTLS_SERVER_HELLO_RANDOM_LEN;
 
     /* ...
@@ -1116,6 +1120,19 @@
         return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER );
     }
 
+    /*
+     * Check whether this ciphersuite is the same with what we received in HRR.
+     */
+    if( ( !is_hrr ) && ( ssl->handshake->hello_retry_request_count > 0 ) &&
+        ( cipher_suite != ssl->session_negotiate->ciphersuite ) )
+    {
+        MBEDTLS_SSL_DEBUG_MSG( 1, ( "ciphersuite(%04x) not the one from HRR",
+                                    cipher_suite ) );
+
+        MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER,
+                                      MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER );
+        return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER );
+    }
 
     /* Configure ciphersuites */
     mbedtls_ssl_optimize_checksum( ssl, ciphersuite_info );
@@ -1211,6 +1228,7 @@
 #endif /* MBEDTLS_SSL_COOKIE_C */
 
             case MBEDTLS_TLS_EXT_SUPPORTED_VERSIONS:
+                supported_versions_exist = 1;
                 MBEDTLS_SSL_DEBUG_MSG( 3,
                             ( "found supported_versions extension" ) );
 
@@ -1233,14 +1251,12 @@
 #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED)
             case MBEDTLS_TLS_EXT_KEY_SHARE:
                 MBEDTLS_SSL_DEBUG_MSG( 3, ( "found key_shares extension" ) );
-                if( is_hrr == SSL_SERVER_HELLO_COORDINATE_HRR )
+                if( is_hrr )
                     ret = ssl_tls13_parse_hrr_key_share_ext( ssl,
                                             p, p + extension_data_len );
-                else if( is_hrr == SSL_SERVER_HELLO_COORDINATE_HELLO )
+                else
                     ret = ssl_tls13_parse_key_share_ext( ssl,
                                             p, p + extension_data_len );
-                else
-                    ret = is_hrr;
                 if( ret != 0 )
                 {
                     MBEDTLS_SSL_DEBUG_RET( 1,
@@ -1266,6 +1282,14 @@
         p += extension_data_len;
     }
 
+    if( !supported_versions_exist )
+    {
+        MBEDTLS_SSL_DEBUG_MSG( 1, ( "supported_versions not exist" ) );
+        MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER,
+                                      MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER );
+        return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER );
+    }
+
     return( 0 );
 }