Merge pull request #6748 from julek-wolfssl/dtls13-frag-ch2

DTLS 1.3: allow fragmenting the second ClientHello message

Author: JacobBarthelmeh

.github/workflows/curl.yml

@@ -4,15 +4,16 @@ on:
   workflow_call:
 
 jobs:
-  build:
+  build-and-test:
     runs-on: ubuntu-latest
     # This should be a safe limit for the tests to run.
-    timeout-minutes: 14
+    timeout-minutes: 25
     steps:
     - name: Install test dependencies
       run: |
         sudo apt-get update
         sudo apt-get install nghttp2
+        sudo pip install impacket
 
     - name: Build wolfSSL
       uses: wolfSSL/actions-build-autotools-project@v1
@@ -21,10 +22,14 @@ jobs:
         configure: --enable-curl
         install: true
 
-    - name: Build and test curl
+    - name: Build curl
       uses: wolfSSL/actions-build-autotools-project@v1
       with:
         repository: curl/curl
         path: curl
         configure: --with-wolfssl=$GITHUB_WORKSPACE/build-dir
-        check: true
+        check: false
+
+    - name: Test curl
+      working-directory: curl
+      run: make -j test-ci

Docker/OpenWrt/runTests.sh

@@ -1,11 +1,12 @@
 #!/bin/sh
 
 runCMD() { # usage: runCMD "<command>" "<retVal>"
-    eval $1 >/dev/null 2>&1
+    TMP_FILE=$(mktemp)
+    eval $1 > $TMP_FILE 2>&1
     RETVAL=$?
     if [ "$RETVAL" != "$2" ]; then
-        echo "Command ($1) returned ${RETVAL}, but expected $2. Rerunning with output to terminal:"
-        eval $1
+        echo "Command ($1) returned ${RETVAL}, but expected $2. Error output:"
+        cat $TMP_FILE
         exit 1
     fi
 }
@@ -16,11 +17,11 @@ runCMD "ldd /lib/libustream-ssl.so" 0
 # Remove after fixed upstream.
 runCMD "sed '\/src\/gz openwrt_kmods https:\/\/downloads.openwrt.org\/releases\/21.02-SNAPSHOT\/targets\/x86\/64\/kmods\/5.4.238-1-5a722da41bc36de95a7195be6fce1b45/s//#&/' -i /etc/opkg/distfeeds.conf" 0
 runCMD "opkg update" 0
-runCMD "uclient-fetch -O /dev/null 'https://letsencrypt.org'" 0
+runCMD "uclient-fetch 'https://letsencrypt.org'" 0
 # Negative tests
-runCMD "uclient-fetch --ca-certificate=/dev/null -O /dev/null 'https://letsencrypt.org'" 5
-runCMD "uclient-fetch -O /dev/null 'https://self-signed.badssl.com/'" 5
-runCMD "uclient-fetch -O /dev/null 'https://untrusted-root.badssl.com/'" 5
-runCMD "uclient-fetch -O /dev/null 'https://expired.badssl.com/'" 5
+runCMD "uclient-fetch --ca-certificate=/dev/null 'https://letsencrypt.org'" 5
+runCMD "uclient-fetch 'https://self-signed.badssl.com/'" 5
+runCMD "uclient-fetch 'https://untrusted-root.badssl.com/'" 5
+runCMD "uclient-fetch 'https://expired.badssl.com/'" 5
 
 echo "All tests passed."

configure.ac

@@ -4516,6 +4516,21 @@ then
   AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_DTLS_CID"
 fi
 
+# DTLS 1.3 Fragment Second ClientHello
+AC_ARG_ENABLE([dtls-frag-ch],
+    [AS_HELP_STRING([--enable-dtls-frag-ch],[Enable wolfSSL DTLS 1.3 ClientHello fragmenting (default: disabled)])],
+    [ ENABLED_DTLS_CH_FRAG=$enableval ],
+    [ ENABLED_DTLS_CH_FRAG=no ]
+    )
+if test "x$ENABLED_DTLS_CH_FRAG" = "xyes"
+then
+  if test "x$ENABLED_DTLS13" != "xyes"
+  then
+    AC_MSG_ERROR([You need to enable DTLSv1.3 to use DTLS ClientHello fragmenting])
+  fi
+  AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_DTLS_CH_FRAG"
+fi
+
 # CODING
 AC_ARG_ENABLE([coding],
     [AS_HELP_STRING([--enable-coding],[Enable Coding base 16/64 (default: enabled)])],

examples/server/server.c

@@ -3322,6 +3322,11 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args)
     }
 #endif /* WOLFSSL_DTLS_CID */
 
+#ifdef WOLFSSL_DTLS_CH_FRAG
+    if (doDTLS)
+        wolfSSL_dtls13_allow_ch_frag(ssl, 1);
+#endif
+
 #ifndef WOLFSSL_CALLBACKS
         if (nonBlocking) {
             #ifdef WOLFSSL_DTLS

src/dtls.c

@@ -26,6 +26,13 @@
  *     will consume less bandwidth (one ClientHello and one HelloVerifyRequest
  *     less). On the other hand, if a valid SessionID is collected, forged
  *     clientHello messages will consume resources on the server.
+ * WOLFSSL_DTLS_CH_FRAG
+ *     Allow a server to process a fragmented second/verified (one containing a
+ *     valid cookie response) ClientHello message. The first/unverified (one
+ *     without a cookie extension) ClientHello MUST be unfragmented so that the
+ *     DTLS server can process it statelessly. This is only implemented for
+ *     DTLS 1.3. The user MUST call wolfSSL_dtls13_allow_ch_frag() on the server
+ *     to explicitly enable this during runtime.
  */
 
 #ifdef HAVE_CONFIG_H
@@ -263,10 +270,13 @@ static int CheckDtlsCookie(const WOLFSSL* ssl, WolfSSL_CH* ch,
     return ret;
 }
 
-static int ParseClientHello(const byte* input, word32 helloSz, WolfSSL_CH* ch)
+static int ParseClientHello(const byte* input, word32 helloSz, WolfSSL_CH* ch,
+        byte isFirstCHFrag)
 {
     word32 idx = 0;
 
+    (void)isFirstCHFrag;
+
     /* protocol version, random and session id length check */
     if (OPAQUE16_LEN + RAN_LEN + OPAQUE8_LEN > helloSz)
         return BUFFER_ERROR;
@@ -288,9 +298,20 @@ static int ParseClientHello(const byte* input, word32 helloSz, WolfSSL_CH* ch)
     idx += ReadVector8(input + idx, &ch->compression);
     if (idx < helloSz - OPAQUE16_LEN) {
         /* Extensions are optional */
+#ifdef WOLFSSL_DTLS_CH_FRAG
+        word32 extStart = idx + OPAQUE16_LEN;
+#endif
         idx += ReadVector16(input + idx, &ch->extension);
-        if (idx > helloSz)
-            return BUFFER_ERROR;
+        if (idx > helloSz) {
+#ifdef WOLFSSL_DTLS_CH_FRAG
+            idx = helloSz;
+            /* Allow incomplete extensions if we are parsing a fragment */
+            if (isFirstCHFrag && extStart < helloSz)
+                ch->extension.size = helloSz - extStart;
+            else
+#endif
+                return BUFFER_ERROR;
+        }
     }
     if (idx != helloSz)
         return BUFFER_ERROR;
@@ -860,17 +881,30 @@ static int ClientHelloSanityCheck(WolfSSL_CH* ch, byte isTls13)
     return 0;
 }
 
-int DoClientHelloStateless(WOLFSSL* ssl, const byte* input,
-                           word32* inOutIdx, word32 helloSz)
+int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, word32 helloSz,
+        byte isFirstCHFrag, byte* tls13)
 {
     int ret;
     WolfSSL_CH ch;
     byte isTls13 = 0;
 
+    WOLFSSL_ENTER("DoClientHelloStateless");
+    if (isFirstCHFrag) {
+#ifdef WOLFSSL_DTLS_CH_FRAG
+        WOLFSSL_MSG("\tProcessing fragmented ClientHello");
+#else
+        WOLFSSL_MSG("\tProcessing fragmented ClientHello but "
+                "WOLFSSL_DTLS_CH_FRAG is not defined. This should not happen.");
+        return BAD_STATE_E;
+#endif
+    }
+    if (tls13 != NULL)
+        *tls13 = 0;
+
     XMEMSET(&ch, 0, sizeof(ch));
 
     ssl->options.dtlsStateful = 0;
-    ret = ParseClientHello(input + *inOutIdx, helloSz, &ch);
+    ret = ParseClientHello(input, helloSz, &ch, isFirstCHFrag);
     if (ret != 0)
         return ret;
 
@@ -879,6 +913,8 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input,
         ret = TlsCheckSupportedVersion(ssl, &ch, &isTls13);
         if (ret != 0)
             return ret;
+        if (tls13 != NULL)
+            *tls13 = isTls13;
         if (isTls13) {
             int tlsxFound;
             ret = FindExtByType(&ch.cookieExt, TLSX_COOKIE, ch.extension,
@@ -894,7 +930,7 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input,
         return ret;
 
 #ifdef WOLFSSL_DTLS_NO_HVR_ON_RESUME
-    if (!isTls13) {
+    if (!isTls13 && !isFirstCHFrag) {
         int resume = FALSE;
         ret = TlsResumptionIsValid(ssl, &ch, &resume);
         if (ret != 0)
@@ -907,7 +943,13 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input,
 #endif
 
     if (ch.cookie.size == 0 && ch.cookieExt.size == 0) {
-        ret = SendStatelessReply((WOLFSSL*)ssl, &ch, isTls13);
+#ifdef WOLFSSL_DTLS_CH_FRAG
+        /* Don't send anything here when processing fragment */
+        if (isFirstCHFrag)
+            ret = COOKIE_ERROR;
+        else
+#endif
+            ret = SendStatelessReply((WOLFSSL*)ssl, &ch, isTls13);
     }
     else {
         byte cookieGood;
@@ -921,11 +963,25 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input,
             if (isTls13)
                 ret = INVALID_PARAMETER;
             else
+#endif
+#ifdef WOLFSSL_DTLS_CH_FRAG
+            /* Don't send anything here when processing fragment */
+            if (isFirstCHFrag)
+                ret = COOKIE_ERROR;
+            else
 #endif
                 ret = SendStatelessReply((WOLFSSL*)ssl, &ch, isTls13);
         }
-        else
+        else {
             ssl->options.dtlsStateful = 1;
+            /* Update the window now that we enter the stateful parsing */
+#ifdef WOLFSSL_DTLS13
+            if (isTls13)
+                ret = Dtls13UpdateWindowRecordRecvd(ssl);
+            else
+#endif
+                DtlsUpdateWindow(ssl);
+        }
     }
 
     return ret;

src/dtls13.c

@@ -1573,6 +1573,19 @@ static int Dtls13RtxSendBuffered(WOLFSSL* ssl)
     return 0;
 }
 
+static int Dtls13AcceptFragmented(WOLFSSL *ssl, enum HandShakeType type)
+{
+    if (IsEncryptionOn(ssl, 0))
+        return 1;
+    if (ssl->options.side == WOLFSSL_CLIENT_END && type == server_hello)
+        return 1;
+#ifdef WOLFSSL_DTLS_CH_FRAG
+    if (ssl->options.side == WOLFSSL_SERVER_END && type == client_hello &&
+            ssl->options.dtls13ChFrag && ssl->options.dtlsStateful)
+        return 1;
+#endif
+    return 0;
+}
 /**
  * Dtls13HandshakeRecv() - process an handshake message. Deal with
  fragmentation if needed
@@ -1646,13 +1659,35 @@ static int _Dtls13HandshakeRecv(WOLFSSL* ssl, byte* input, word32 size,
     isFirst = fragOff == 0;
     isComplete = isFirst && fragLength == messageLength;
 
-    if (!isComplete && !IsEncryptionOn(ssl, 0)) {
+    if (!isComplete && !Dtls13AcceptFragmented(ssl, handshakeType)) {
+#ifdef WOLFSSL_DTLS_CH_FRAG
+        byte tls13 = 0;
+        /* check if the first CH fragment contains a valid cookie */
+        if (ssl->options.dtls13ChFrag && !ssl->options.dtlsStateful &&
+                isFirst && handshakeType == client_hello &&
+                DoClientHelloStateless(ssl, input + idx, fragLength, 1, &tls13)
+                    == 0 && tls13) {
+            /* We can save this message and continue as stateful. */
+            if (ssl->chGoodCb != NULL) {
+                int cbret = ssl->chGoodCb(ssl, ssl->chGoodCtx);
+                if (cbret < 0) {
+                    ssl->error = cbret;
+                    WOLFSSL_MSG("ClientHello Good Cb don't continue error");
+                    return WOLFSSL_FATAL_ERROR;
+                }
+            }
+            WOLFSSL_MSG("ClientHello fragment verified");
+        }
+        else
+#endif
+        {
 #ifdef WOLFSSL_DEBUG_TLS
-        WOLFSSL_MSG("DTLS1.3 not accepting fragmented plaintext message");
+            WOLFSSL_MSG("DTLS1.3 not accepting fragmented plaintext message");
 #endif /* WOLFSSL_DEBUG_TLS */
-        /* ignore the message */
-        *processedSize = idx + fragLength + ssl->keys.padSz;
-        return 0;
+            /* ignore the message */
+            *processedSize = idx + fragLength + ssl->keys.padSz;
+            return 0;
+        }
     }
 
     usingAsyncCrypto = ssl->devId != INVALID_DEVID;
@@ -2369,7 +2404,11 @@ static int Dtls13WriteAckMessage(WOLFSSL* ssl,
     c16toa(msgSz, ackMessage);
     ackMessage += OPAQUE16_LEN;
 
+    WOLFSSL_MSG("write ack records");
+
     while (recordNumberList != NULL) {
+        WOLFSSL_MSG_EX("epoch %d seq %d", recordNumberList->epoch,
+                recordNumberList->seq);
         c64toa(&recordNumberList->epoch, ackMessage);
         ackMessage += OPAQUE64_LEN;
         c64toa(&recordNumberList->seq, ackMessage);
@@ -2561,10 +2600,13 @@ int DoDtls13Ack(WOLFSSL* ssl, const byte* input, word32 inputSize,
     if (length % (DTLS13_RN_SIZE) != 0)
         return PARSE_ERROR;
 
+    WOLFSSL_MSG("read ack records");
+
     ackMessage = input + OPAQUE16_LEN;
     for (i = 0; i < length; i += DTLS13_RN_SIZE) {
         ato64(ackMessage + i, &epoch);
         ato64(ackMessage + i + OPAQUE64_LEN, &seq);
+        WOLFSSL_MSG_EX("epoch %d seq %d", epoch, seq);
         Dtls13RtxRemoveRecord(ssl, epoch, seq);
     }
 
@@ -2635,28 +2677,20 @@ int SendDtls13Ack(WOLFSSL* ssl)
     if (ret != 0)
         return ret;
 
-    if (w64IsZero(ssl->dtls13EncryptEpoch->epochNumber)) {
-
-        ret = Dtls13WriteAckMessage(ssl, ssl->dtls13Rtx.seenRecords, &length);
-        if (ret != 0)
-            return ret;
+    ret = Dtls13WriteAckMessage(ssl, ssl->dtls13Rtx.seenRecords, &length);
+    if (ret != 0)
+        return ret;
 
-        output = GetOutputBuffer(ssl);
+    output = GetOutputBuffer(ssl);
 
+    if (w64IsZero(ssl->dtls13EncryptEpoch->epochNumber)) {
         ret = Dtls13RlAddPlaintextHeader(ssl, output, ack, (word16)length);
         if (ret != 0)
             return ret;
 
         ssl->buffers.outputBuffer.length += length + DTLS_RECORD_HEADER_SZ;
     }
     else {
-
-        ret = Dtls13WriteAckMessage(ssl, ssl->dtls13Rtx.seenRecords, &length);
-        if (ret != 0)
-            return ret;
-
-        output = GetOutputBuffer(ssl);
-
         outputSize = ssl->buffers.outputBuffer.bufferSize -
                      ssl->buffers.outputBuffer.idx -
                      ssl->buffers.outputBuffer.length;
@@ -2797,4 +2831,16 @@ int Dtls13CheckAEADFailLimit(WOLFSSL* ssl)
 }
 #endif
 
+#ifdef WOLFSSL_DTLS_CH_FRAG
+int wolfSSL_dtls13_allow_ch_frag(WOLFSSL *ssl, int enabled)
+{
+    if (ssl->options.side == WOLFSSL_CLIENT_END) {
+        return WOLFSSL_FAILURE;
+    }
+    ssl->options.dtls13ChFrag = !!enabled;
+    return WOLFSSL_SUCCESS;
+}
+#endif
+
+
 #endif /* WOLFSSL_DTLS13 */