Merge pull request #9246 from julek-wolfssl/gh/9240

Abort connection if we are about to send the same CH

Author: douzzer

src/ssl.c

@@ -14316,6 +14316,12 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out,
         ssl->options.haveSessionId = 0;
         ssl->options.tls = 0;
         ssl->options.tls1_1 = 0;
+    #ifdef WOLFSSL_TLS13
+    #ifdef WOLFSSL_SEND_HRR_COOKIE
+        ssl->options.hrrSentCookie = 0;
+    #endif
+        ssl->options.hrrSentKeyShare = 0;
+    #endif
     #ifdef WOLFSSL_DTLS
         ssl->options.dtlsStateful = 0;
     #endif

src/tls.c

@@ -7318,9 +7318,11 @@ static int TLSX_Cookie_Parse(WOLFSSL* ssl, const byte* input, word16 length,
     if (length - idx != len)
         return BUFFER_E;
 
-    if (msgType == hello_retry_request)
+    if (msgType == hello_retry_request) {
+        ssl->options.hrrSentCookie = 1;
         return TLSX_Cookie_Use(ssl, input + idx, len, NULL, 0, 1,
                                &ssl->extensions);
+    }
 
     /* client_hello */
     extension = TLSX_Find(ssl->extensions, TLSX_COOKIE);
@@ -10145,6 +10147,8 @@ int TLSX_KeyShare_Parse(WOLFSSL* ssl, const byte* input, word16 length,
         if (length != OPAQUE16_LEN)
             return BUFFER_ERROR;
 
+        ssl->options.hrrSentKeyShare = 1;
+
         /* The data is the named group the server wants to use. */
         ato16(input, &group);
 

src/tls13.c

@@ -5635,6 +5635,21 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
 
     }
     else {
+        /* https://datatracker.ietf.org/doc/html/rfc8446#section-4.1.4
+         * Clients MUST abort the handshake with an
+         * "illegal_parameter" alert if the HelloRetryRequest would not result
+         * in any change in the ClientHello.
+         */
+        /* Check if the HRR contained a cookie or a keyshare */
+        if (!ssl->options.hrrSentKeyShare
+#ifdef WOLFSSL_SEND_HRR_COOKIE
+                && !ssl->options.hrrSentCookie
+#endif
+                ) {
+            SendAlert(ssl, alert_fatal, illegal_parameter);
+            return DUPLICATE_MSG_E;
+        }
+
         ssl->options.tls1_3 = 1;
         ssl->options.serverState = SERVER_HELLO_RETRY_REQUEST_COMPLETE;
 

tests/api/test_tls13.c

@@ -2141,3 +2141,53 @@ int test_tls13_early_data(void)
     return EXPECT_RESULT();
 }
 
+
+/* Check that the client won't send the same CH after a HRR. An HRR without
+ * a KeyShare or a Cookie extension will trigger the error. */
+int test_tls13_same_ch(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
+    defined(WOLFSSL_TLS13) && defined(WOLFSSL_AES_128) && \
+    defined(HAVE_AESGCM) && !defined(NO_SHA256) && \
+    /* middlebox compat requires that the session ID is echoed */ \
+    !defined(WOLFSSL_TLS13_MIDDLEBOX_COMPAT)
+    WOLFSSL_CTX *ctx_c = NULL;
+    WOLFSSL *ssl_c = NULL;
+    struct test_memio_ctx test_ctx;
+    /* Transport Layer Security
+     *     TLSv1.3 Record Layer: Handshake Protocol: Hello Retry Request
+     *         Content Type: Handshake (22)
+     *         Version: TLS 1.2 (0x0303)
+     *         Length: 50
+     *         Handshake Protocol: Hello Retry Request
+     *             Handshake Type: Server Hello (2)
+     *             Length: 46
+     *             Version: TLS 1.2 (0x0303)
+     *             Random: cf21ad74e59a6111be1d8c021e65b891c2a211167abb8c5e079e09e2c8a8339c (HelloRetryRequest magic)
+     *             Session ID Length: 0
+     *             Cipher Suite: TLS_AES_128_GCM_SHA256 (0x1301)
+     *             Compression Method: null (0)
+     *             Extensions Length: 6
+     *             Extension: supported_versions (len=2) TLS 1.3 */
+    unsigned char hrr[] = {
+      0x16, 0x03, 0x03, 0x00, 0x32, 0x02, 0x00, 0x00, 0x2e, 0x03, 0x03, 0xcf,
+      0x21, 0xad, 0x74, 0xe5, 0x9a, 0x61, 0x11, 0xbe, 0x1d, 0x8c, 0x02, 0x1e,
+      0x65, 0xb8, 0x91, 0xc2, 0xa2, 0x11, 0x16, 0x7a, 0xbb, 0x8c, 0x5e, 0x07,
+      0x9e, 0x09, 0xe2, 0xc8, 0xa8, 0x33, 0x9c, 0x00, 0x13, 0x01, 0x00, 0x00,
+      0x06, 0x00, 0x2b, 0x00, 0x02, 0x03, 0x04
+    };
+
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+    ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, NULL, &ssl_c, NULL,
+            wolfTLSv1_3_client_method, NULL), 0);
+    ExpectIntEQ(test_memio_inject_message(&test_ctx, 1, (char*)hrr,
+            sizeof(hrr)), 0);
+    ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), DUPLICATE_MSG_E);
+
+    wolfSSL_free(ssl_c);
+    wolfSSL_CTX_free(ctx_c);
+#endif
+    return EXPECT_RESULT();
+}

tests/api/test_tls13.h

@@ -30,13 +30,15 @@ int test_tls13_bad_psk_binder(void);
 int test_tls13_rpk_handshake(void);
 int test_tls13_pq_groups(void);
 int test_tls13_early_data(void);
+int test_tls13_same_ch(void);
 
 #define TEST_TLS13_DECLS                                   \
     TEST_DECL_GROUP("tls13", test_tls13_apis),             \
     TEST_DECL_GROUP("tls13", test_tls13_cipher_suites),    \
     TEST_DECL_GROUP("tls13", test_tls13_bad_psk_binder),   \
     TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake),    \
     TEST_DECL_GROUP("tls13", test_tls13_pq_groups),        \
-    TEST_DECL_GROUP("tls13", test_tls13_early_data)
+    TEST_DECL_GROUP("tls13", test_tls13_early_data),       \
+    TEST_DECL_GROUP("tls13", test_tls13_same_ch)
 
 #endif /* WOLFCRYPT_TEST_TLS13_H */

wolfssl/internal.h

@@ -5061,6 +5061,12 @@ struct Options {
 #if defined(HAVE_DANE)
     word16            useDANE:1;
 #endif /* HAVE_DANE */
+#ifdef WOLFSSL_TLS13
+#ifdef WOLFSSL_SEND_HRR_COOKIE
+    word16            hrrSentCookie:1;    /* HRR sent with cookie */
+#endif
+    word16            hrrSentKeyShare:1;  /* HRR sent with key share */
+#endif
     word16            disableRead:1;
 #ifdef WOLFSSL_DTLS
     byte              haveMcast;          /* using multicast ? */