Check SNI/ALPN in TLS 1.3 session resumption

Author: holtrop-wolfssl

src/internal.c

@@ -38761,8 +38761,22 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
     #endif
     #if defined(HAVE_SESSION_TICKET) && \
         (defined(HAVE_SNI) || defined(HAVE_ALPN))
-                if((ret=VerifyTicketBinding(ssl)))
-                    goto out;
+                /* Only verify here for TLS 1.2 ticket-based resumption.
+                 * For stateful (session-ID) resumption ssl->session is
+                 * not loaded until HandleTlsResumption runs below, which
+                 * performs its own binding check against the cached
+                 * session. On mismatch decline the resumption (RFC 6066
+                 * Section 3) but proceed with a full handshake; leave
+                 * useTicket set so the server still issues a fresh
+                 * ticket to the client. */
+                if (ssl->options.useTicket &&
+                        VerifyTicketBinding(ssl) != 0) {
+                    WOLFSSL_MSG("Ticket binding mismatch, "
+                                "declining resumption and falling back "
+                                "to full handshake");
+                    ssl->options.resuming = 0;
+                    ssl->options.peerAuthGood = 0;
+                }
     #endif
 
                 i += totalExtSz;
@@ -39594,13 +39608,23 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
 #if defined(HAVE_SNI) || defined(HAVE_ALPN)
     /* Server-side: verify the SNI/ALPN bindings carried on a resumed
      * session match what was negotiated for the current connection.
-     * Must be called after extension parsing and ALPN_Select.
-     * Returns 0 on match, WOLFSSL_FATAL_ERROR on mismatch. */
+     * Must be called after extension parsing and ALPN_Select, and only
+     * once ssl->session has been loaded with the resumed session's
+     * binding hashes (i.e. after DoClientTicketFinalize for TLS 1.2
+     * tickets / TLS 1.3 PSK resumption). For TLS 1.2 session-ID
+     * resumption the binding check is performed in HandleTlsResumption
+     * against the cached session directly, so this function should not
+     * be called for that path.
+     * Returns 0 on match, WOLFSSL_FATAL_ERROR on mismatch. Callers
+     * decide whether a mismatch is fatal or whether to decline the
+     * resumption and fall back to a full handshake -- RFC 6066 Section 3
+     * mandates the latter for TLS 1.2 SNI, and the same policy is
+     * applied to TLS 1.3 PSK resumption for consistency. */
     int VerifyTicketBinding(WOLFSSL* ssl)
     {
         byte curHash[TICKET_BINDING_HASH_SZ];
 
-        if (!ssl->options.resuming || !ssl->options.useTicket)
+        if (!ssl->options.resuming)
             return 0;
 
 #ifdef HAVE_SNI

src/tls13.c

@@ -7725,8 +7725,72 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
         goto exit_dch;
 #endif
 #if defined(HAVE_SESSION_TICKET) && (defined(HAVE_SNI) || defined(HAVE_ALPN))
-    if ((ret = VerifyTicketBinding(ssl)) != 0)
-        goto exit_dch;
+    /* Only verify when a PSK actually matched, since that's when
+     * DoClientTicketFinalize populated ssl->session with the resumed
+     * session's binding hashes. resuming may be set optimistically by
+     * CheckPreSharedKeys before any PSK has been validated, so gating
+     * on usingPSK avoids comparing against an uninitialized session. */
+    if (args->usingPSK && VerifyTicketBinding(ssl) != 0) {
+        /* RFC 6066 / common practice: decline the PSK resumption and
+         * fall back to a full handshake rather than aborting. Unwind
+         * the PSK state set up by CheckPreSharedKeys/DoPreSharedKeys
+         * so the rest of DoTls13ClientHello proceeds as if no PSK had
+         * been selected. */
+        TLSX* pskExt;
+
+        WOLFSSL_MSG("PSK binding mismatch, declining PSK and falling "
+                    "back to full handshake");
+        args->usingPSK = 0;
+        ssl->options.resuming = 0;
+        ssl->options.peerAuthGood = 0;
+        ssl->options.noPskDheKe = 0;
+        /* DoPreSharedKeys cleared sendVerify because PSK auth obviates
+         * the server's Certificate/CertificateVerify. Restore the
+         * pre-PSK default (set at the top of DoTls13ClientHello) so the
+         * full handshake sends them. */
+        ssl->options.sendVerify = SEND_CERT;
+        /* DoPreSharedKeys derived the resumption PSK into psk_key and
+         * called DeriveEarlySecret with it; drop both so the FINALIZE
+         * block's DeriveEarlySecret call (now reached via !usingPSK)
+         * re-derives the early secret from zeros. */
+        if (ssl->arrays != NULL) {
+            ForceZero(ssl->arrays->psk_key,
+                      sizeof(ssl->arrays->psk_key));
+            ssl->arrays->psk_keySz = 0;
+        }
+        /* Clear the selected-PSK response flag so the server does NOT
+         * echo a pre_shared_key extension in ServerHello, which is what
+         * tells the client the resumption was accepted. */
+        pskExt = TLSX_Find(ssl->extensions, TLSX_PRE_SHARED_KEY);
+        if (pskExt != NULL)
+            pskExt->resp = 0;
+    #ifdef WOLFSSL_EARLY_DATA
+        {
+            TLSX* extEarlyData = TLSX_Find(ssl->extensions,
+                                           TLSX_EARLY_DATA);
+            if (extEarlyData != NULL)
+                extEarlyData->resp = 0;
+            ssl->earlyData = no_early_data;
+        }
+    #endif
+        /* The KeyShare/SigAlgs validation in the !usingPSK block above
+         * was skipped because PSK had been selected. Re-check now to
+         * confirm a full handshake is actually possible. */
+    #ifndef NO_CERTS
+        if (TLSX_Find(ssl->extensions, TLSX_KEY_SHARE) == NULL) {
+            WOLFSSL_MSG("Client did not send a KeyShare extension; "
+                        "cannot fall back to full handshake");
+            ERROR_OUT(INCOMPLETE_DATA, exit_dch);
+        }
+        if (ssl->clSuites->hashSigAlgoSz == 0) {
+            WOLFSSL_MSG("Client did not send a SignatureAlgorithms "
+                        "extension; cannot fall back to full handshake");
+            ERROR_OUT(INCOMPLETE_DATA, exit_dch);
+        }
+    #else
+        ERROR_OUT(INVALID_PARAMETER, exit_dch);
+    #endif
+    }
 #endif
     } /* case TLS_ASYNC_BEGIN */
     FALL_THROUGH;

tests/api/test_tls.c

@@ -906,8 +906,9 @@ int test_tls_set_session_min_downgrade(void)
 }
 
 #if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
-    !defined(WOLFSSL_NO_TLS12) && defined(HAVE_SNI) && \
-    defined(HAVE_SESSION_TICKET) && !defined(NO_SESSION_CACHE)
+    (!defined(WOLFSSL_NO_TLS12) || defined(WOLFSSL_TLS13)) && \
+    defined(HAVE_SNI) && defined(HAVE_SESSION_TICKET) && \
+    !defined(NO_SESSION_CACHE)
 /* Accept-all SNI callback. */
 static int accept_any_sni_cb(WOLFSSL* ssl, int* ret, void* arg)
 {
@@ -987,6 +988,76 @@ int test_tls12_session_id_resumption_sni_mismatch(void)
     return EXPECT_RESULT();
 }
 
+/* TLS 1.3 PSK resumption must fall back to a full handshake if the SNI in
+ * the resumed ClientHello does not match the SNI bound to the original
+ * session (RFC 6066 Section 3 / RFC 8446 Section 4.6.1). */
+int test_tls13_session_resumption_sni_mismatch(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \
+    defined(HAVE_SNI) && defined(HAVE_SESSION_TICKET) && \
+    !defined(NO_SESSION_CACHE)
+    WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
+    WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
+    WOLFSSL_SESSION *sess = NULL;
+    struct test_memio_ctx test_ctx;
+    const char* sniA = "public.example";
+    const char* sniB = "admin.example";
+    byte readBuf[16];
+
+    /* Step 1: full TLS 1.3 handshake under SNI=public.example to obtain a
+     * session ticket. The server-side SNI callback ensures ssl->extensions
+     * retains the client's SNI in builds that don't compile in
+     * WOLFSSL_ALWAYS_KEEP_SNI. */
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+    ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
+                    wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0);
+    wolfSSL_CTX_set_servername_callback(ctx_s, accept_any_sni_cb);
+    ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME,
+                    sniA, (word16)XSTRLEN(sniA)), WOLFSSL_SUCCESS);
+    ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
+    /* Sanity: the first handshake was not a resumption. */
+    ExpectIntEQ(wolfSSL_session_reused(ssl_s), 0);
+    /* Drive the post-handshake NewSessionTicket through to the client so
+     * the saved session is a real resumption ticket. */
+    ExpectIntEQ(wolfSSL_read(ssl_c, readBuf, sizeof(readBuf)), -1);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
+    ExpectNotNull(sess = wolfSSL_get1_session(ssl_c));
+
+    wolfSSL_free(ssl_c); ssl_c = NULL;
+    wolfSSL_free(ssl_s); ssl_s = NULL;
+
+    /* Step 2: new SSL objects on the SAME WOLFSSL_CTX (so the server's
+     * ticket key still matches). The client offers the saved session but
+     * advertises a *different* SNI. The server MUST NOT resume because the
+     * SNI differs from the original. */
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+    ExpectNotNull(ssl_c = wolfSSL_new(ctx_c));
+    wolfSSL_SetIOReadCtx(ssl_c, &test_ctx);
+    wolfSSL_SetIOWriteCtx(ssl_c, &test_ctx);
+    ExpectNotNull(ssl_s = wolfSSL_new(ctx_s));
+    wolfSSL_SetIOReadCtx(ssl_s, &test_ctx);
+    wolfSSL_SetIOWriteCtx(ssl_s, &test_ctx);
+    ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME,
+                    sniB, (word16)XSTRLEN(sniB)), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_set_session(ssl_c, sess), WOLFSSL_SUCCESS);
+    ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
+
+    /* Desired behavior: server falls back to a full handshake because the
+     * SNI in the ClientHello does not match the SNI bound to the cached
+     * ticket. Both sides should report no resumption. */
+    ExpectIntEQ(wolfSSL_session_reused(ssl_s), 0);
+    ExpectIntEQ(wolfSSL_session_reused(ssl_c), 0);
+
+    wolfSSL_SESSION_free(sess);
+    wolfSSL_free(ssl_c);
+    wolfSSL_free(ssl_s);
+    wolfSSL_CTX_free(ctx_c);
+    wolfSSL_CTX_free(ctx_s);
+#endif
+    return EXPECT_RESULT();
+}
+
 int test_tls_set_curves_list_ecc_fallback(void)
 {
     EXPECT_DECLS;

tests/api/test_tls.h

@@ -34,6 +34,7 @@ int test_tls12_no_null_compression(void);
 int test_tls12_etm_failed_resumption(void);
 int test_tls_set_session_min_downgrade(void);
 int test_tls12_session_id_resumption_sni_mismatch(void);
+int test_tls13_session_resumption_sni_mismatch(void);
 int test_tls_set_curves_list_ecc_fallback(void);
 int test_tls12_corrupted_finished(void);
 int test_tls12_peerauth_failsafe(void);
@@ -51,6 +52,7 @@ int test_tls12_peerauth_failsafe(void);
         TEST_DECL_GROUP("tls", test_tls12_etm_failed_resumption),              \
         TEST_DECL_GROUP("tls", test_tls_set_session_min_downgrade),            \
         TEST_DECL_GROUP("tls", test_tls12_session_id_resumption_sni_mismatch), \
+        TEST_DECL_GROUP("tls", test_tls13_session_resumption_sni_mismatch),    \
         TEST_DECL_GROUP("tls", test_tls_set_curves_list_ecc_fallback),         \
         TEST_DECL_GROUP("tls", test_tls12_corrupted_finished),                 \
         TEST_DECL_GROUP("tls", test_tls12_peerauth_failsafe)