Merge pull request #9391 from holtrop/check-dup-extensions-fix

Check for duplicate extensions in client hello when HAVE_TLS_EXTENSIONS is not set - fix #9377

Author: dgarske

.github/workflows/os-check.yml

@@ -63,6 +63,7 @@ jobs:
           '--enable-coding=no',
           '--enable-dtls --enable-dtls13 --enable-ocspstapling --enable-ocspstapling2
            --enable-cert-setup-cb --enable-sessioncerts',
+          '--disable-sni --disable-ecc --disable-tls13 --disable-secure-renegotiation-info',
         ]
     name: make check
     if: github.repository_owner == 'wolfssl'

src/internal.c

@@ -37869,7 +37869,6 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
         return ret;
     }
 
-
     /* handle processing of client_hello (1) */
     int DoClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
                              word32 helloSz)
@@ -38310,6 +38309,8 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
                 /* auto populate extensions supported unless user defined */
                 if ((ret = TLSX_PopulateExtensions(ssl, 1)) != 0)
                     goto out;
+#else
+                word32 extensions_seen = 0U;
 #endif
 
                 if ((i - begin) + OPAQUE16_LEN > helloSz) {
@@ -38363,6 +38364,17 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
                     ato16(&input[i], &extSz);
                     i += OPAQUE16_LEN;
 
+                    if (extId < (word16)(sizeof(extensions_seen) * 8U)) {
+                        word32 mask = 1U << extId;
+                        if ((extensions_seen & mask) != 0U) {
+                            WOLFSSL_MSG(
+                                    "DoClientHello: duplicate extension found");
+                            ret = DUPLICATE_TLS_EXT_E;
+                            goto out;
+                        }
+                        extensions_seen |= mask;
+                    }
+
                     if (OPAQUE16_LEN + OPAQUE16_LEN + extSz > totalExtSz) {
                         ret = BUFFER_ERROR;
                         goto out;

tests/api.c

@@ -12977,10 +12977,10 @@ static int test_tls_bad_legacy_version(void)
 #if defined(WOLFSSL_TLS13) && !defined(WOLFSSL_ALLOW_BAD_TLS_LEGACY_VERSION)
 #if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS) && \
     !defined(NO_FILESYSTEM) && (!defined(NO_RSA) || defined(HAVE_ECC))
-    /* This is exactly the same as the buffer in test_tls_ext_duplicate() except
-     * the 11th byte is set to 0x04. That change means the legacy protocol
-     * version field is invalid. That will be caught before the dulplicate
-     * signature algorithms extension. */
+    /* This buffer (prior to Extensions) is exactly the same as the buffer in
+     * test_tls_ext_duplicate() except the 11th byte is set to 0x04. That
+     * change means the legacy protocol version field is invalid. That will be
+     * caught before the dulplicate signature algorithms extension. */
     const unsigned char clientHelloBadLegacyVersion[] = {
         0x16, 0x03, 0x03, 0x00, 0x6a, 0x01, 0x00, 0x00,
         0x66, 0x03, 0x04, 0xf4, 0x65, 0xbd, 0x22, 0xfe,
@@ -12993,9 +12993,9 @@ static int test_tls_bad_legacy_version(void)
         0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
         0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x13, 0x01,
         0x00, 0x9e, 0x01, 0x00,
-        /* Extensions - duplicate signature algorithms. */
+        /* Extensions */
                                 0x00, 0x19, 0x00, 0x0d,
-        0x00, 0x04, 0x00, 0x02, 0x04, 0x01, 0x00, 0x0d,
+        0x00, 0x04, 0x00, 0x02, 0x04, 0x01, 0x00, 0x15,
         0x00, 0x04, 0x00, 0x02, 0x04, 0x01,
         /* Supported Versions extension for TLS 1.3. */
                                             0x00, 0x2b,