Don't allow a resumption handshake inside of a SCR

wolfSSL · Jul 6, 2023 · 57e53d1 · 57e53d1
1 parent fb0c769
commit 57e53d1
Show file tree

Hide file tree

Showing 5 changed files with 137 additions and 8 deletions.
diff --git a/src/internal.c b/src/internal.c
@@ -7387,7 +7387,7 @@ int InitSSL(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup)
             ret = wolfSSL_UseSecureRenegotiation(ssl);
             if (ret != WOLFSSL_SUCCESS)
                 return ret;
-            }
+        }
     }
 #endif /* HAVE_SECURE_RENEGOTIATION */
 
@@ -15400,6 +15400,9 @@ int DoFinished(WOLFSSL* ssl, const byte* input, word32* inOutIdx, word32 size,
 #endif
             ssl->options.handShakeState = HANDSHAKE_DONE;
             ssl->options.handShakeDone  = 1;
+#ifdef HAVE_SECURE_RENEGOTIATION
+            ssl->options.resumed = ssl->options.resuming;
+#endif
         }
     }
     else {
@@ -15416,6 +15419,9 @@ int DoFinished(WOLFSSL* ssl, const byte* input, word32* inOutIdx, word32 size,
 #endif
             ssl->options.handShakeState = HANDSHAKE_DONE;
             ssl->options.handShakeDone  = 1;
+#ifdef HAVE_SECURE_RENEGOTIATION
+            ssl->options.resumed = ssl->options.resuming;
+#endif
         }
     }
 #ifdef WOLFSSL_DTLS
@@ -15965,8 +15971,10 @@ static int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx,
     }
 
     if (ssl->options.side == WOLFSSL_CLIENT_END && ssl->options.dtls == 0 &&
-               ssl->options.serverState == NULL_STATE && type != server_hello) {
-        WOLFSSL_MSG("First server message not server hello");
+               ssl->options.serverState == NULL_STATE && type != server_hello &&
+               type != hello_request) {
+        WOLFSSL_MSG("First server message not server hello or "
+                    "hello request");
         SendAlert(ssl, alert_fatal, unexpected_message);
         WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
         return OUT_OF_ORDER_E;
@@ -21917,6 +21925,9 @@ int SendFinished(WOLFSSL* ssl)
         #endif
             ssl->options.handShakeState = HANDSHAKE_DONE;
             ssl->options.handShakeDone  = 1;
+#ifdef HAVE_SECURE_RENEGOTIATION
+            ssl->options.resumed = ssl->options.resuming;
+#endif
         }
     }
     else {
@@ -21929,6 +21940,9 @@ int SendFinished(WOLFSSL* ssl)
         #endif
             ssl->options.handShakeState = HANDSHAKE_DONE;
             ssl->options.handShakeDone  = 1;
+#ifdef HAVE_SECURE_RENEGOTIATION
+            ssl->options.resumed = ssl->options.resuming;
+#endif
         }
     }
 
@@ -27130,13 +27144,20 @@ static int HashSkeData(WOLFSSL* ssl, enum wc_HashType hashType,
             return BAD_FUNC_ARG;
         }
 
-        idSz = ssl->options.resuming ? ssl->session->sessionIDSz : 0;
-
 #ifdef WOLFSSL_TLS13
         if (IsAtLeastTLSv1_3(ssl->version))
             return SendTls13ClientHello(ssl);
 #endif
 
+#ifdef HAVE_SECURE_RENEGOTIATION
+        /* We don't want to resume in SCR */
+        if (IsSCR(ssl))
+            ssl->options.resuming = 0;
+#endif
+
+        idSz = ssl->options.resuming ? ssl->session->sessionIDSz : 0;
+
+
         WOLFSSL_START(WC_FUNC_CLIENT_HELLO_SEND);
         WOLFSSL_ENTER("SendClientHello");
 
@@ -34297,6 +34318,9 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
         ssl->options.dtlsStateful = 1;
 #endif /* WOLFSSL_DTLS */
 
+        /* Reset to sane value for SCR */
+        ssl->options.resuming = 0;
+
         /* protocol version, random and session id length check */
         if (OPAQUE16_LEN + RAN_LEN + OPAQUE8_LEN > helloSz)
             return BUFFER_ERROR;
@@ -34490,7 +34514,7 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
             ret = BUFFER_ERROR; /* session ID greater than 32 bytes long */
             goto out;
         }
-        else if (b > 0) {
+        else if (b > 0 && !IsSCR(ssl)) {
             if ((i - begin) + b > helloSz) {
                 ret = BUFFER_ERROR;
                 goto out;
@@ -34503,8 +34527,14 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
             if (b == ID_LEN)
                 ssl->options.resuming = 1; /* client wants to resume */
             WOLFSSL_MSG("Client wants to resume session");
-            i += b;
         }
+#ifdef HAVE_SECURE_RENEGOTIATION
+        else {
+            /* We don't want to resume in SCR */
+            ssl->arrays->sessionIDSz = 0;
+        }
+#endif
+        i += b;
 
 #ifdef WOLFSSL_DTLS
             /* cookie */

diff --git a/src/ssl.c b/src/ssl.c
@@ -4103,6 +4103,8 @@ int wolfSSL_Rehandshake(WOLFSSL* ssl)
     if (ssl->options.side == WOLFSSL_SERVER_END) {
         /* Reset option to send certificate verify. */
         ssl->options.sendVerify = 0;
+        /* Reset resuming flag to do full secure handshake. */
+        ssl->options.resuming = 0;
     }
     else {
         /* Reset resuming flag to do full secure handshake. */
@@ -21328,8 +21330,13 @@ int wolfSSL_session_reused(WOLFSSL* ssl)
 {
     int resuming = 0;
     WOLFSSL_ENTER("wolfSSL_session_reused");
-    if (ssl)
+    if (ssl) {
+#ifndef HAVE_SECURE_RENEGOTIATION
         resuming = ssl->options.resuming;
+#else
+        resuming = ssl->options.resuming || ssl->options.resumed;
+#endif
+    }
     WOLFSSL_LEAVE("wolfSSL_session_reused", resuming);
     return resuming;
 }

diff --git a/src/tls.c b/src/tls.c
@@ -5379,6 +5379,13 @@ static int TLSX_SessionTicket_Parse(WOLFSSL* ssl, const byte* input,
             return 0;
         }
 
+#ifdef HAVE_SECURE_RENEGOTIATION
+        if (IsSCR(ssl)) {
+            WOLFSSL_MSG("Client sent session ticket during SCR. Ignoring.");
+            return 0;
+        }
+#endif
+
         if (length > SESSION_TICKET_LEN) {
             ret = BAD_TICKET_MSG_SZ;
             WOLFSSL_ERROR_VERBOSE(ret);

diff --git a/tests/api.c b/tests/api.c
@@ -62040,6 +62040,87 @@ static int test_dtls_ipv6_check(void)
 }
 #endif
 
+#if !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) &&   \
+    defined(HAVE_IO_TESTS_DEPENDENCIES) && defined(HAVE_SECURE_RENEGOTIATION)
+
+static WOLFSSL_SESSION* test_wolfSSL_SCR_after_resumption_session = NULL;
+
+static void test_wolfSSL_SCR_after_resumption_ctx_ready(WOLFSSL_CTX* ctx)
+{
+    AssertIntEQ(wolfSSL_CTX_UseSecureRenegotiation(ctx), WOLFSSL_SUCCESS);
+}
+
+static void test_wolfSSL_SCR_after_resumption_on_result(WOLFSSL* ssl)
+{
+    if (test_wolfSSL_SCR_after_resumption_session == NULL) {
+        test_wolfSSL_SCR_after_resumption_session = wolfSSL_get1_session(ssl);
+        AssertNotNull(test_wolfSSL_SCR_after_resumption_session);
+    }
+    else {
+        char testMsg[] = "Message after SCR";
+        char msgBuf[sizeof(testMsg)];
+        int ret;
+        if (!wolfSSL_is_server(ssl)) {
+            AssertIntEQ(WOLFSSL_SUCCESS,
+                    wolfSSL_set_session(ssl,
+                       test_wolfSSL_SCR_after_resumption_session));
+        }
+        AssertIntEQ(wolfSSL_Rehandshake(ssl), WOLFSSL_SUCCESS);
+        AssertIntEQ(wolfSSL_write(ssl, testMsg, sizeof(testMsg)),
+                    sizeof(testMsg));
+        ret = wolfSSL_read(ssl, msgBuf, sizeof(msgBuf));
+        if (ret != sizeof(msgBuf)) /* Possibly APP_DATA_READY error. Retry. */
+            ret = wolfSSL_read(ssl, msgBuf, sizeof(msgBuf));
+        AssertIntEQ(ret, sizeof(msgBuf));
+    }
+}
+
+static void test_wolfSSL_SCR_after_resumption_ssl_ready(WOLFSSL* ssl)
+{
+    AssertIntEQ(WOLFSSL_SUCCESS,
+           wolfSSL_set_session(ssl, test_wolfSSL_SCR_after_resumption_session));
+}
+
+static int test_wolfSSL_SCR_after_resumption(void)
+{
+    EXPECT_DECLS;
+    callback_functions func_cb_client;
+    callback_functions func_cb_server;
+
+    XMEMSET(&func_cb_client, 0, sizeof(func_cb_client));
+    XMEMSET(&func_cb_server, 0, sizeof(func_cb_server));
+
+    func_cb_client.method = wolfTLSv1_2_client_method;
+    func_cb_client.ctx_ready = test_wolfSSL_SCR_after_resumption_ctx_ready;
+    func_cb_client.on_result = test_wolfSSL_SCR_after_resumption_on_result;
+    func_cb_server.method = wolfTLSv1_2_server_method;
+    func_cb_server.ctx_ready = test_wolfSSL_SCR_after_resumption_ctx_ready;
+
+    test_wolfSSL_client_server_nofail(&func_cb_client, &func_cb_server);
+
+    ExpectIntEQ(func_cb_client.return_code, TEST_SUCCESS);
+    ExpectIntEQ(func_cb_server.return_code, TEST_SUCCESS);
+
+    func_cb_client.ssl_ready = test_wolfSSL_SCR_after_resumption_ssl_ready;
+    func_cb_server.on_result = test_wolfSSL_SCR_after_resumption_on_result;
+
+    test_wolfSSL_client_server_nofail(&func_cb_client, &func_cb_server);
+
+    ExpectIntEQ(func_cb_client.return_code, TEST_SUCCESS);
+    ExpectIntEQ(func_cb_server.return_code, TEST_SUCCESS);
+
+    wolfSSL_SESSION_free(test_wolfSSL_SCR_after_resumption_session);
+
+    return EXPECT_RESULT();
+}
+
+#else
+static int test_wolfSSL_SCR_after_resumption(void)
+{
+    return TEST_SKIPPED;
+}
+#endif
+
 static int test_wolfSSL_configure_args(void)
 {
     EXPECT_DECLS;
@@ -63290,6 +63371,7 @@ TEST_CASE testCases[] = {
     /* Can't memory test as client/server hangs. */
     TEST_DECL(test_dtls_msg_from_other_peer),
     TEST_DECL(test_dtls_ipv6_check),
+    TEST_DECL(test_wolfSSL_SCR_after_resumption),
     /* This test needs to stay at the end to clean up any caches allocated. */
     TEST_DECL(test_wolfSSL_Cleanup)
 };

diff --git a/wolfssl/internal.h b/wolfssl/internal.h
@@ -4517,6 +4517,9 @@ struct Options {
     word16            failNoCertxPSK:1;   /* fail for no cert except with PSK */
     word16            downgrade:1;        /* allow downgrade of versions */
     word16            resuming:1;
+#ifdef HAVE_SECURE_RENEGOTIATION
+    word16            resumed:1;          /* resuming may be reset on SCR */
+#endif
     word16            isPSK:1;
     word16            haveSessionId:1;    /* server may not send */
     word16            tls:1;              /* using TLS ? */