dtls 1.3: allow to skip cookie exchange on resumption

tls 1.3: do cookie exchange when asked too even when found a matching cipher

Author: julek-wolfssl

.github/workflows/os-check.yml

@@ -16,11 +16,14 @@ jobs:
           '--enable-all --enable-asn=original',
           '--enable-harden-tls',
           '--enable-tls13 --enable-session-ticket --enable-dtls --enable-dtls13
-           --enable-opensslextra --enable-sessioncerts 
-           CPPFLAGS=''-DWOLFSSL_DTLS_NO_HVR_ON_RESUME -DHAVE_EXT_CACHE 
-           -DWOLFSSL_TICKET_HAVE_ID -DHAVE_EX_DATA -DSESSION_CACHE_DYNAMIC_MEM'' ',
+             --enable-opensslextra --enable-sessioncerts 
+             CPPFLAGS=''-DWOLFSSL_DTLS_NO_HVR_ON_RESUME -DHAVE_EXT_CACHE 
+             -DWOLFSSL_TICKET_HAVE_ID -DHAVE_EX_DATA -DSESSION_CACHE_DYNAMIC_MEM'' ',
           '--enable-all --enable-secure-renegotiation',
           '--enable-all --enable-haproxy --enable-quic',
+          '--enable-dtls --enable-dtls13 --enable-earlydata 
+             --enable-session-ticket --enable-psk 
+             CPPFLAGS=''-DWOLFSSL_DTLS13_NO_HRR_ON_RESUME'' ',
         ]
     name: make check
     runs-on: ${{ matrix.os }}

examples/client/client.c

@@ -89,7 +89,7 @@ static const char *wolfsentry_config_path = NULL;
 #ifdef WOLFSSL_ALT_TEST_STRINGS
     #define TEST_STR_TERM "\n"
 #else
-    #define TEST_STR_TERM
+    #define TEST_STR_TERM "\n"
 #endif
 
 static const char kHelloMsg[] = "hello wolfssl!" TEST_STR_TERM;
@@ -466,26 +466,6 @@ static void EarlyData(WOLFSSL_CTX* ctx, WOLFSSL* ssl, const char* msg,
         wolfSSL_CTX_free(ctx); ctx = NULL;
         err_sys("SSL_write_early_data failed");
     }
-    do {
-        err = 0; /* reset error */
-        ret = wolfSSL_write_early_data(ssl, msg, msgSz, &msgSz);
-        if (ret <= 0) {
-            err = wolfSSL_get_error(ssl, 0);
-        #ifdef WOLFSSL_ASYNC_CRYPT
-            if (err == WC_PENDING_E) {
-                ret = wolfSSL_AsyncPoll(ssl, WOLF_POLL_FLAG_CHECK_HW);
-                if (ret < 0) break;
-            }
-        #endif
-        }
-    } while (err == WC_PENDING_E);
-    if (ret != msgSz) {
-        LOG_ERROR("SSL_write_early_data msg error %d, %s\n", err,
-                                         wolfSSL_ERR_error_string(err, buffer));
-        wolfSSL_free(ssl);
-        wolfSSL_CTX_free(ctx);
-        err_sys("SSL_write_early_data failed");
-    }
 }
 #endif
 

examples/server/server.c

@@ -3358,7 +3358,7 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args)
                     err = 0; /* reset error */
                     ret = wolfSSL_read_early_data(ssl, input, sizeof(input)-1,
                                                                           &len);
-                    if (ret != WOLFSSL_SUCCESS) {
+                    if (ret <= 0) {
                         err = SSL_get_error(ssl, 0);
                     #ifdef WOLFSSL_ASYNC_CRYPT
                         if (err == WC_PENDING_E) {

src/dtls.c

@@ -21,11 +21,13 @@
 
 /*
  * WOLFSSL_DTLS_NO_HVR_ON_RESUME
+ * WOLFSSL_DTLS13_NO_HRR_ON_RESUME
  *     If defined, a DTLS server will not do a cookie exchange on successful
  *     client resumption: the resumption will be faster (one RTT less) and
- *     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.
+ *     will consume less bandwidth (one ClientHello and one
+ *     HelloVerifyRequest/HelloRetryRequest less). On the other hand, if a valid
+ *     SessionID/ticket/psk 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
@@ -769,6 +771,15 @@ static int SendStatelessReplyDtls13(const WOLFSSL* ssl, WolfSSL_CH* ch)
         }
     }
 
+#ifdef WOLFSSL_DTLS13_NO_HRR_ON_RESUME
+    if (ssl->options.dtls13NoHrrOnResume && usePSK && pskInfo.isValid &&
+            !cs.doHelloRetry) {
+        /* Skip HRR on resumption */
+        ((WOLFSSL*)ssl)->options.dtlsStateful = 1;
+        goto dtls13_cleanup;
+    }
+#endif
+
 #ifdef HAVE_SUPPORTED_CURVES
     if (cs.doHelloRetry) {
         ret = TLSX_KeyShare_SetSupported(ssl, &parsedExts);
@@ -949,7 +960,7 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, word32 helloSz,
             ret = COOKIE_ERROR;
         else
 #endif
-            ret = SendStatelessReply((WOLFSSL*)ssl, &ch, isTls13);
+            ret = SendStatelessReply(ssl, &ch, isTls13);
     }
     else {
         byte cookieGood;
@@ -970,7 +981,7 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, word32 helloSz,
                 ret = COOKIE_ERROR;
             else
 #endif
-                ret = SendStatelessReply((WOLFSSL*)ssl, &ch, isTls13);
+                ret = SendStatelessReply(ssl, &ch, isTls13);
         }
         else {
             ssl->options.dtlsStateful = 1;

src/dtls13.c

@@ -2842,5 +2842,15 @@ int wolfSSL_dtls13_allow_ch_frag(WOLFSSL *ssl, int enabled)
 }
 #endif
 
+#ifdef WOLFSSL_DTLS13_NO_HRR_ON_RESUME
+int wolfSSL_dtls13_hrr_on_resume(WOLFSSL *ssl, int enabled)
+{
+    if (ssl->options.side == WOLFSSL_CLIENT_END) {
+        return WOLFSSL_FAILURE;
+    }
+    ssl->options.dtls13NoHrrOnResume = !!enabled;
+    return WOLFSSL_SUCCESS;
+}
+#endif
 
 #endif /* WOLFSSL_DTLS13 */

src/internal.c

@@ -20179,20 +20179,8 @@ static int DtlsShouldDrop(WOLFSSL* ssl, int retcode)
 #ifndef NO_WOLFSSL_SERVER
     if (ssl->options.side == WOLFSSL_SERVER_END
             && ssl->curRL.type != handshake && !IsSCR(ssl)) {
-        int beforeCookieVerified = 0;
-        if (!IsAtLeastTLSv1_3(ssl->version)) {
-            beforeCookieVerified =
-                ssl->options.acceptState < ACCEPT_FIRST_REPLY_DONE;
-        }
-#ifdef WOLFSSL_DTLS13
-        else {
-            beforeCookieVerified =
-                ssl->options.acceptState < TLS13_ACCEPT_SECOND_REPLY_DONE;
-        }
-#endif /* WOLFSSL_DTLS13 */
-
-        if (beforeCookieVerified) {
-            WOLFSSL_MSG("Drop non-handshake record before handshake");
+        if (!ssl->options.dtlsStateful) {
+            WOLFSSL_MSG("Drop non-handshake record when not stateful");
             return 1;
         }
     }
@@ -34435,6 +34423,9 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
 
 #if defined(WOLFSSL_TLS13) && defined(HAVE_SUPPORTED_CURVES)
         if (cs.doHelloRetry) {
+            /* Make sure we don't send HRR twice */
+            if (ssl->options.serverState == SERVER_HELLO_RETRY_REQUEST_COMPLETE)
+                return INVALID_PARAMETER;
             ssl->options.serverState = SERVER_HELLO_RETRY_REQUEST_COMPLETE;
             return TLSX_KeyShare_SetSupported(ssl, &ssl->extensions);
         }

src/ssl.c

@@ -16790,11 +16790,13 @@ int wolfSSL_set_compression(WOLFSSL* ssl)
 #endif /* OPENSSL_EXTRA || WOLFSSL_EXTRA || WOLFSSL_WPAS_SMALL */
 
     /* return true if connection established */
-    int wolfSSL_is_init_finished(WOLFSSL* ssl)
+    int wolfSSL_is_init_finished(const WOLFSSL* ssl)
     {
         if (ssl == NULL)
             return 0;
 
+        /* Can't use ssl->options.connectState and ssl->options.acceptState because
+         * they differ in meaning for TLS <=1.2 and 1.3 */
         if (ssl->options.handShakeState == HANDSHAKE_DONE)
             return 1;
 
@@ -31935,12 +31937,7 @@ int wolfSSL_SSL_in_init(WOLFSSL *ssl)
 {
     WOLFSSL_ENTER("wolfSSL_SSL_in_init");
 
-    if (ssl == NULL)
-        return WOLFSSL_FAILURE;
-
-    /* Can't use ssl->options.connectState and ssl->options.acceptState because
-     * they differ in meaning for TLS <=1.2 and 1.3 */
-    return ssl->options.handShakeState != HANDSHAKE_DONE;
+    return !wolfSSL_is_init_finished(ssl);
 }
 
 int wolfSSL_SSL_in_connect_init(WOLFSSL* ssl)

src/tls13.c

@@ -6197,6 +6197,8 @@ static int CheckPreSharedKeys(WOLFSSL* ssl, const byte* input, word32 helloSz,
                 if ((ret = SetKeysSide(ssl, DECRYPT_SIDE_ONLY)) != 0)
                     return ret;
 
+                ssl->keys.encryptionOn = 1;
+
 #ifdef WOLFSSL_DTLS13
                 if (ssl->options.dtls) {
                     ret = Dtls13NewEpoch(ssl,
@@ -6909,7 +6911,11 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
             }
         }
         else {
-            ERROR_OUT(HRR_COOKIE_ERROR, exit_dch);
+#if defined(WOLFSSL_DTLS13) && defined(WOLFSSL_DTLS13_NO_HRR_ON_RESUME)
+            /* Don't error out as we may be resuming. We confirm this later. */
+            if (!ssl->options.dtls)
+#endif
+                ERROR_OUT(HRR_COOKIE_ERROR, exit_dch);
         }
     }
 #endif
@@ -6975,14 +6981,16 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
             goto exit_dch;
         }
     }
-    else
 #endif
 #ifdef HAVE_SUPPORTED_CURVES
     if (args->usingPSK == 2) {
         /* Pick key share and Generate a new key if not present. */
         int doHelloRetry = 0;
         ret = TLSX_KeyShare_Establish(ssl, &doHelloRetry);
         if (doHelloRetry) {
+            /* Make sure we don't send HRR twice */
+            if (ssl->options.serverState == SERVER_HELLO_RETRY_REQUEST_COMPLETE)
+                ERROR_OUT(INVALID_PARAMETER, exit_dch);
             ssl->options.serverState = SERVER_HELLO_RETRY_REQUEST_COMPLETE;
             if (ret != WC_PENDING_E)
                 ret = 0; /* for hello_retry return 0 */
@@ -7075,32 +7083,58 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
         ret = INPUT_CASE_ERROR;
     } /* switch (ssl->options.asyncState) */
 
-#if defined(WOLFSSL_SEND_HRR_COOKIE)
-    if (ret == 0 && ssl->options.sendCookie && ssl->options.cookieGood &&
-            (ssl->options.serverState == SERVER_HELLO_RETRY_REQUEST_COMPLETE
+#ifdef WOLFSSL_SEND_HRR_COOKIE
+    if (ret == 0 && ssl->options.sendCookie) {
+        if (ssl->options.cookieGood &&
+                ssl->options.acceptState == TLS13_ACCEPT_FIRST_REPLY_DONE) {
+            /* Processing second ClientHello. Clear HRR state. */
+            ssl->options.serverState = NULL_STATE;
+        }
+
+        if (ssl->options.cookieGood &&
+            ssl->options.serverState == SERVER_HELLO_RETRY_REQUEST_COMPLETE) {
+            /* If we already verified the peer with a cookie then we can't
+             * do another HRR for cipher negotiation. Send alert and restart
+             * the entire handshake. */
+            ERROR_OUT(INVALID_PARAMETER, exit_dch);
+        }
 #ifdef WOLFSSL_DTLS13
-                    /* DTLS cookie exchange should be done in stateless code in
-                     * DoClientHelloStateless. If we verified the cookie then
-                     * always advance the state. */
-                    || ssl->options.dtls
+        if (ssl->options.dtls &&
+            ssl->options.serverState == SERVER_HELLO_RETRY_REQUEST_COMPLETE) {
+            /* Cookie and key share negotiation should be handled in
+             * DoClientHelloStateless. If we enter here then something went
+             * wrong in our logic. */
+            ERROR_OUT(BAD_HELLO, exit_dch);
+        }
 #endif
-                    ))
-        ssl->options.serverState = SERVER_HELLO_COMPLETE;
+        /* Send a cookie */
+        if (!ssl->options.cookieGood &&
+            ssl->options.serverState != SERVER_HELLO_RETRY_REQUEST_COMPLETE) {
+#ifdef WOLFSSL_DTLS13
+            if (ssl->options.dtls) {
+#ifdef WOLFSSL_DTLS13_NO_HRR_ON_RESUME
+                /* We can skip cookie on resumption */
+                if (!ssl->options.dtls || !ssl->options.dtls13NoHrrOnResume ||
+                        !args->usingPSK)
+#endif
+                    ERROR_OUT(BAD_HELLO, exit_dch);
+            }
+            else
 #endif
+            {
+                /* Need to remove the keyshare ext if we found a common group
+                 * and are not doing curve negotiation. */
+                TLSX_Remove(&ssl->extensions, TLSX_KEY_SHARE, ssl->heap);
+                ssl->options.serverState = SERVER_HELLO_RETRY_REQUEST_COMPLETE;
+            }
 
-#if defined(WOLFSSL_DTLS13) && defined(WOLFSSL_SEND_HRR_COOKIE)
-    if (ret == 0 && ssl->options.dtls && ssl->options.sendCookie &&
-        ssl->options.serverState <= SERVER_HELLO_RETRY_REQUEST_COMPLETE) {
-        /* Cookie and key share negotiation should be handled in
-         * DoClientHelloStateless. If we enter here then something went wrong
-         * in our logic. */
-        ERROR_OUT(BAD_HELLO, exit_dch);
+        }
     }
 #endif /* WOLFSSL_DTLS13 */
 
 #ifdef WOLFSSL_DTLS_CID
     /* do not modify CID state if we are sending an HRR  */
-    if (ssl->options.useDtlsCID &&
+    if (ret == 0 && ssl->options.dtls && ssl->options.useDtlsCID &&
             ssl->options.serverState != SERVER_HELLO_RETRY_REQUEST_COMPLETE)
         DtlsCIDOnExtensionsParsed(ssl);
 #endif /* WOLFSSL_DTLS_CID */