Merge pull request #6067 from AndrzejKurek/cid-session-resumption-2-28

[Backport 2.28] Fix DTLS 1.2 session resumption
Mbed-TLS · Jul 6, 2022 · ada62f2 · ada62f2
2 parents 06986de + 135afdc
commit ada62f2
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 10 deletions.
diff --git a/ChangeLog.d/resumption_cid.txt b/ChangeLog.d/resumption_cid.txt
@@ -0,0 +1,5 @@
+Bugfix
+   * Fix server connection identifier setting for outgoing encrypted records
+     on DTLS 1.2 session resumption. After DTLS 1.2 session resumption with
+     connection identifier, the Mbed TLS client now properly sends the server
+     connection identifier in encrypted record headers. Fix #5872.
diff --git a/library/ssl_cli.c b/library/ssl_cli.c
@@ -2276,16 +2276,6 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl )
     else
     {
         ssl->state = MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC;
-
-        if( ( ret = mbedtls_ssl_derive_keys( ssl ) ) != 0 )
-        {
-            MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_derive_keys", ret );
-            mbedtls_ssl_send_alert_message(
-                ssl,
-                MBEDTLS_SSL_ALERT_LEVEL_FATAL,
-                MBEDTLS_SSL_ALERT_MSG_INTERNAL_ERROR );
-            return( ret );
-        }
     }
 
     MBEDTLS_SSL_DEBUG_MSG( 3, ( "%s session has been resumed",
@@ -2537,6 +2527,24 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl )
         }
     }
 
+    /*
+     * mbedtls_ssl_derive_keys() has to be called after the parsing of the
+     * extensions. It sets the transform data for the resumed session which in
+     * case of DTLS includes the server CID extracted from the CID extension.
+     */
+    if( ssl->handshake->resume )
+    {
+        if( ( ret = mbedtls_ssl_derive_keys( ssl ) ) != 0 )
+        {
+            MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_derive_keys", ret );
+            mbedtls_ssl_send_alert_message(
+                ssl,
+                MBEDTLS_SSL_ALERT_LEVEL_FATAL,
+                MBEDTLS_SSL_ALERT_MSG_INTERNAL_ERROR );
+            return( ret );
+        }
+    }
+
     /*
      * Renegotiation security checks
      */

diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
@@ -3365,6 +3365,29 @@ run_test    "Session resume using cache: openssl server" \
             -C "parse new session ticket" \
             -c "a session has been resumed"
 
+# Tests for Session resume and extensions
+
+requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2
+requires_config_enabled MBEDTLS_SSL_DTLS_CONNECTION_ID
+run_test    "Session resume and connection ID" \
+            "$P_SRV debug_level=3 cid=1 cid_val=dead dtls=1 tickets=0" \
+            "$P_CLI debug_level=3 cid=1 cid_val=beef dtls=1 tickets=0 reconnect=1" \
+            0 \
+            -c "Enable use of CID extension." \
+            -s "Enable use of CID extension." \
+            -c "client hello, adding CID extension" \
+            -s "found CID extension"           \
+            -s "Use of CID extension negotiated" \
+            -s "server hello, adding CID extension" \
+            -c "found CID extension" \
+            -c "Use of CID extension negotiated" \
+            -s "Copy CIDs into SSL transform" \
+            -c "Copy CIDs into SSL transform" \
+            -c "Peer CID (length 2 Bytes): de ad" \
+            -s "Peer CID (length 2 Bytes): be ef" \
+            -s "Use of Connection ID has been negotiated" \
+            -c "Use of Connection ID has been negotiated"
+
 # Tests for Session Resume based on session-ID and cache, DTLS
 
 requires_config_enabled MBEDTLS_SSL_CACHE_C