Harden TLS 1.3 NewSessionTicket handling per RFC 9846

.wolfssl_known_macro_extras

@@ -1096,6 +1096,7 @@ WOLFSSL_TLS13_DRAFT
 WOLFSSL_TLS13_IGNORE_AEAD_LIMITS
 WOLFSSL_TLS13_IGNORE_PT_ALERT_ON_ENC
 WOLFSSL_TLS13_SHA512
+WOLFSSL_TLS13_TICKET_CHECK_PSK_MODES
 WOLFSSL_TLS13_TICKET_BEFORE_FINISHED
 WOLFSSL_TLSX_PQC_MLKEM_STORE_PRIV_KEY
 WOLFSSL_TRACK_MEMORY_FULL

src/ssl_sess.c

@@ -1052,8 +1052,11 @@ WOLFSSL_SESSION* wolfSSL_GetSessionClient(WOLFSSL* ssl, const byte* id, int len)
 #else
         current = &sessRow->Sessions[clSess[idx].serverIdx];
 #endif
-        if (current && XMEMCMP(current->serverID, id,
-                                                     (unsigned long)len) == 0) {
+        /* Require the same length as well as the same bytes. Comparing only
+         * the requested length lets a short ID alias the prefix of a longer
+         * cached one, mixing sessions the application meant to keep apart. */
+        if (current && current->idLen == (word16)len &&
+                XMEMCMP(current->serverID, id, (unsigned long)len) == 0) {
             WOLFSSL_MSG("Found a serverid match for client");
             if (LowResTimer() < (current->bornOn + current->timeout)) {
                 WOLFSSL_MSG("Session valid");

src/tls.c

@@ -12911,8 +12911,17 @@ static int TLSX_PskKeModes_Parse(WOLFSSL* ssl, const byte* input, word16 length,
     byte modes;
 
     ret = TLSX_PskKeyModes_Parse_Modes(input, length, msgType, &modes);
-    if (ret == 0)
+    if (ret == 0) {
+#if defined(HAVE_SESSION_TICKET) && !defined(NO_WOLFSSL_SERVER) && \
+    defined(WOLFSSL_TLS13_TICKET_CHECK_PSK_MODES)
+        /* Keep the advertised modes for the NewSessionTicket decision. The
+         * extension object is dropped with the rest of the handshake state
+         * once the handshake is done. */
+        ssl->options.pskKeModes = modes;
+        ssl->options.pskKeModesRecvd = 1;
+#endif
         ret = TLSX_PskKeyModes_Use(ssl, modes);
+    }
 
     if (ret != 0) {
         WOLFSSL_ERROR_VERBOSE(ret);

src/tls13.c

@@ -71,6 +71,12 @@
  * WOLFSSL_TICKET_HAVE_ID:   Session tickets include ID            default: off
  *                            Forced on when WOLFSSL_EARLY_DATA is set.
  * WOLFSSL_TICKET_NONCE_MALLOC: Dynamically allocate ticket nonce  default: off
+ * WOLFSSL_TLS13_TICKET_CHECK_PSK_MODES: Withhold NewSessionTicket default: off
+ *                            when the ClientHello advertised no usable
+ *                            psk_key_exchange_modes, as RFC 9846 Sections
+ *                            4.3.9 and 4.7.1 require. Off by default: a peer
+ *                            that omits the extension but expects a ticket
+ *                            stops getting one.
  *
  * TLS 1.3 Key Exchange:
  * HAVE_KEYING_MATERIAL:     Export keying material (RFC 8446 7.5) default: off
@@ -13666,12 +13672,12 @@ static int DoTls13NewSessionTicket(WOLFSSL* ssl, const byte* input,
     *inOutIdx += EXTS_SZ;
     if ((*inOutIdx - begin) + length != size)
         return BUFFER_ERROR;
-    #ifdef WOLFSSL_EARLY_DATA
-    ret = TLSX_Parse(ssl, (byte *)input + (*inOutIdx), length, session_ticket,
-                     NULL);
+    /* RFC 9846 Section 4.7.1: the extensions are Section 4.3 Extension TLVs.
+     * Malformed framing is a syntax error even when no extension in the list
+     * is one we act on. */
+    ret = TLSX_Parse(ssl, input + *inOutIdx, length, session_ticket, NULL);
     if (ret != 0)
         return ret;
-    #endif
     *inOutIdx += length;
 
     SetupSession(ssl);
@@ -13811,6 +13817,48 @@ static int ExpectedResumptionSecret(WOLFSSL* ssl)
 }
 #endif
 
+/* Check the client advertised a PSK key exchange mode a resumption ticket can
+ * be used with.
+ *
+ * RFC 9846 Section 4.3.9: psk_key_exchange_modes restricts both the PSKs
+ * offered in the ClientHello and those the server might supply through
+ * NewSessionTicket, and servers should not send tickets that are incompatible
+ * with the advertised modes. RFC 9846 Section 4.7.1 makes sending a ticket
+ * conditional on the client's hello carrying a suitable extension.
+ *
+ * ssl  The SSL/TLS object.
+ * returns 0 when a ticket may be sent, MISSING_HANDSHAKE_DATA when the
+ *         extension was not received and PSK_KEY_ERROR when none of the
+ *         advertised modes is usable.
+ */
+static int CheckTls13TicketPskModes(WOLFSSL* ssl)
+{
+#ifdef WOLFSSL_TLS13_TICKET_CHECK_PSK_MODES
+    if (!ssl->options.pskKeModesRecvd) {
+        WOLFSSL_MSG("No psk_key_exchange_modes in ClientHello");
+        return MISSING_HANDSHAKE_DATA;
+    }
+
+    if ((ssl->options.pskKeModes & (1 << PSK_KE)) != 0
+    #ifdef HAVE_SUPPORTED_CURVES
+        && !ssl->options.onlyPskDheKe
+    #endif
+        ) {
+        return 0;
+    }
+    if ((ssl->options.pskKeModes & (1 << PSK_DHE_KE)) != 0 &&
+            !ssl->options.noPskDheKe) {
+        return 0;
+    }
+
+    WOLFSSL_MSG("No usable psk_key_exchange_modes advertised by client");
+    return PSK_KEY_ERROR;
+#else
+    (void)ssl;
+    return 0;
+#endif
+}
+
 /* Send New Session Ticket handshake message.
  * Message contains the information required to perform resumption.
  *
@@ -13835,6 +13883,12 @@ static int SendTls13NewSessionTicket(WOLFSSL* ssl)
         return 0;
     }
 
+    if (CheckTls13TicketPskModes(ssl) != 0) {
+        WOLFSSL_MSG("Client advertised no usable PSK key exchange mode; "
+                    "skipping ticket");
+        return 0;
+    }
+
 #ifdef WOLFSSL_DTLS13
     if (ssl->options.dtls)
         idx = Dtls13GetRlHeaderLength(ssl, 1) + DTLS_HANDSHAKE_HEADER_SZ;
@@ -17045,17 +17099,29 @@ int wolfSSL_accept_TLSv13(WOLFSSL* ssl)
  * returns BAD_FUNC_ARG when ssl is NULL, or not using TLS v1.3,
  *         SIDE_ERROR when not a server,
  *         NOT_READY_ERROR when handshake not complete,
+ *         MISSING_HANDSHAKE_DATA when the ClientHello had no
+ *         psk_key_exchange_modes extension and
+ *         WOLFSSL_TLS13_TICKET_CHECK_PSK_MODES is defined,
+ *         PSK_KEY_ERROR when no advertised PSK key exchange mode is usable and
+ *         WOLFSSL_TLS13_TICKET_CHECK_PSK_MODES is defined,
  *         WOLFSSL_FATAL_ERROR when creating or sending message fails, and
  *         WOLFSSL_SUCCESS on success.
  */
 int wolfSSL_send_SessionTicket(WOLFSSL* ssl)
 {
+    int ret;
+
     if (ssl == NULL || !IsAtLeastTLSv1_3(ssl->version))
         return BAD_FUNC_ARG;
     if (ssl->options.side == WOLFSSL_CLIENT_END)
         return SIDE_ERROR;
     if (ssl->options.handShakeState != HANDSHAKE_DONE)
         return NOT_READY_ERROR;
+    ret = CheckTls13TicketPskModes(ssl);
+    if (ret != 0) {
+        WOLFSSL_ERROR_VERBOSE(ret);
+        return ret;
+    }
 
     if ((ssl->error = SendTls13NewSessionTicket(ssl)) != 0) {
         WOLFSSL_ERROR(ssl->error);

tests/api/test_session.c

@@ -1739,3 +1739,82 @@ int test_wolfSSL_GetSessionAtIndex(void)
 
 #endif /* SESSION_INDEX && HAVE_SESSION_TICKET && !NO_SESSION_CACHE &&
         * !NO_WOLFSSL_CLIENT && !NO_TLS */
+
+/* RFC 9846 Appendix C.4: client applications should not offer tickets across
+ * connections meant to be uncorrelated.  wolfSSL_SetServerID() is how an
+ * application keeps such connections apart, so a shorter ID must not match a
+ * cached entry that merely starts with the same bytes. */
+int test_wolfSSL_client_cache_id_prefix(void)
+{
+    EXPECT_DECLS;
+#if !defined(NO_SESSION_CACHE) && !defined(NO_CLIENT_CACHE) && \
+    !defined(NO_TLS) && !defined(WOLFSSL_NO_TLS12) && \
+    defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
+    !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER)
+    WOLFSSL_CTX* ctx_c = NULL;
+    WOLFSSL_CTX* ctx_s = NULL;
+    WOLFSSL* ssl_c = NULL;
+    WOLFSSL* ssl_s = NULL;
+    WOLFSSL* ssl = NULL;
+    struct test_memio_ctx test_ctx;
+    static const byte prefix[] = { 'w', 'o', 'l', 'f', 'S', 'S', 'L', ':' };
+    byte   id[sizeof(prefix) + 4];
+    byte   sessId[ID_LEN];
+    word32 i;
+    /* The cache row is picked from a hash of the ID, so the prefix and any
+     * one long ID rarely share a row.  Every long ID here starts with the
+     * prefix, and there are enough of them to cover all rows, so the prefix
+     * lookup lands on a row holding one of them. */
+    const word32 fill = 4096;
+
+    /* TLS 1.2 so the client has a complete session to cache as soon as the
+     * handshake is done, with or without session tickets. */
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+    ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
+        wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0);
+    ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
+    ExpectIntEQ(ssl_c->session->isSetup, 1);
+
+    XMEMCPY(id, prefix, sizeof(prefix));
+    XMEMSET(sessId, 0, sizeof(sessId));
+    for (i = 0; i < fill && EXPECT_SUCCESS(); i++) {
+        ClientSession* entry = NULL;
+
+        c32toa(i, id + sizeof(prefix));
+        c32toa(i, sessId);
+        ExpectIntEQ(wolfSSL_SetServerID(ssl_c, id, (int)sizeof(id), 1),
+            WOLFSSL_SUCCESS);
+        if (EXPECT_SUCCESS()) {
+            XMEMCPY(ssl_c->session->sessionID, sessId, ID_LEN);
+            XMEMCPY(ssl_c->session->altSessionID, sessId, ID_LEN);
+            ssl_c->session->sessionIDSz = ID_LEN;
+        }
+        ExpectIntEQ(AddSessionToCache(ctx_c, ssl_c->session, sessId, ID_LEN,
+            NULL, WOLFSSL_CLIENT_END, 0, &entry), 0);
+        ExpectNotNull(entry);
+    }
+
+    /* The prefix is a distinct partition key: no cached session for it. */
+    ExpectNotNull(ssl = wolfSSL_new(ctx_c));
+    ExpectIntEQ(ssl->session->isSetup, 0);
+    ExpectIntEQ(wolfSSL_SetServerID(ssl, prefix, (int)sizeof(prefix), 0),
+        WOLFSSL_SUCCESS);
+    ExpectIntEQ(ssl->session->isSetup, 0);
+    ExpectIntEQ(ssl->session->idLen, (int)sizeof(prefix));
+    wolfSSL_free(ssl);
+    ssl = NULL;
+
+    /* Control: the ID it was cached under still finds it. */
+    ExpectNotNull(ssl = wolfSSL_new(ctx_c));
+    ExpectIntEQ(wolfSSL_SetServerID(ssl, id, (int)sizeof(id), 0),
+        WOLFSSL_SUCCESS);
+    ExpectIntEQ(ssl->session->isSetup, 1);
+    wolfSSL_free(ssl);
+
+    wolfSSL_free(ssl_c);
+    wolfSSL_free(ssl_s);
+    wolfSSL_CTX_free(ctx_c);
+    wolfSSL_CTX_free(ctx_s);
+#endif
+    return EXPECT_RESULT();
+}

tests/api/test_session.h

@@ -37,6 +37,7 @@ int test_wolfSSL_CTX_sess_set_remove_cb(void);
 int test_wolfSSL_ticket_keys(void);
 int test_wolfSSL_SESSION_get_ex_new_index(void);
 int test_wolfSSL_GetSessionAtIndex(void);
+int test_wolfSSL_client_cache_id_prefix(void);
 
 #define TEST_SESSION_DECLS                                                     \
     TEST_DECL_GROUP("session", test_wolfSSL_CTX_add_session),                  \
@@ -51,6 +52,7 @@ int test_wolfSSL_GetSessionAtIndex(void);
     TEST_DECL_GROUP("session", test_wolfSSL_CTX_sess_set_remove_cb),           \
     TEST_DECL_GROUP("session", test_wolfSSL_ticket_keys),                      \
     TEST_DECL_GROUP("session", test_wolfSSL_SESSION_get_ex_new_index),         \
-    TEST_DECL_GROUP("session", test_wolfSSL_GetSessionAtIndex)
+    TEST_DECL_GROUP("session", test_wolfSSL_GetSessionAtIndex),                \
+    TEST_DECL_GROUP("session", test_wolfSSL_client_cache_id_prefix)
 
 #endif /* WOLFCRYPT_TEST_SESSION_H */