Check SNI/ALPN in TLS 1.2 stateful session ID resumption

Author: holtrop-wolfssl

src/internal.c

@@ -38153,6 +38153,38 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
             ssl->options.resuming = 0;
             return ret;
         }
+#if defined(HAVE_SESSION_TICKET) && \
+    (defined(HAVE_SNI) || defined(HAVE_ALPN))
+        /* RFC 6066 section 3: a server MUST NOT resume the session if the
+         * server_name in the current ClientHello differs from the one that
+         * was bound to the session.  VerifyTicketBinding() covers the
+         * ticket path early in DoClientHello, but the session-ID cache
+         * path does not load the session until now - so the binding check
+         * must be applied here, after wolfSSL_GetSession().  On mismatch
+         * fall back to a full handshake.  Gated on HAVE_SESSION_TICKET
+         * because session->sniHash/alpnHash and ssl->options.useTicket
+         * only exist in that configuration. */
+        if (!ssl->options.useTicket) {
+            byte curHash[TICKET_BINDING_HASH_SZ];
+#ifdef HAVE_SNI
+            if (TicketSniHash(ssl, curHash) != 0 ||
+                    XMEMCMP(curHash, session->sniHash,
+                            TICKET_BINDING_HASH_SZ) != 0) {
+                WOLFSSL_MSG("Resumed session SNI mismatch, full handshake");
+                ssl->options.resuming = 0;
+            }
+#endif
+#ifdef HAVE_ALPN
+            if (ssl->options.resuming &&
+                    (TicketAlpnHash(ssl, curHash) != 0 ||
+                     XMEMCMP(curHash, session->alpnHash,
+                             TICKET_BINDING_HASH_SZ) != 0)) {
+                WOLFSSL_MSG("Resumed session ALPN mismatch, full handshake");
+                ssl->options.resuming = 0;
+            }
+#endif
+        }
+#endif /* HAVE_SESSION_TICKET && (HAVE_SNI || HAVE_ALPN) */
 #if !defined(WOLFSSL_NO_TICKET_EXPIRE) && !defined(NO_ASN_TIME)
         /* check if the ticket is valid */
         if (LowResTimer() > session->bornOn + ssl->timeout) {
@@ -39520,7 +39552,7 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
 
 #ifdef HAVE_SNI
     /* Hash server-selected SNI; zeros dst when none. */
-    static int TicketSniHash(WOLFSSL* ssl, byte* dst)
+    int TicketSniHash(WOLFSSL* ssl, byte* dst)
     {
         char* name = NULL;
         word16 nameLen;
@@ -39539,17 +39571,28 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
 #endif
 
 #ifdef HAVE_ALPN
-    /* Hash negotiated ALPN; zeros dst when none. */
-    static int TicketAlpnHash(WOLFSSL* ssl, byte* dst)
+    /* Hash negotiated ALPN; zeros dst when none. Peeks at the extension
+     * directly rather than going through TLSX_ALPN_GetRequest(), which
+     * pushes WOLFSSL_ALPN_NOT_FOUND onto the error queue when no ALPN is
+     * present - undesirable for a silent binding probe called on every
+     * session setup. */
+    int TicketAlpnHash(WOLFSSL* ssl, byte* dst)
     {
-        char* proto = NULL;
-        word16 protoLen = 0;
-
-        if (TLSX_ALPN_GetRequest(ssl->extensions, (void**)&proto,
-                                 &protoLen) == WOLFSSL_SUCCESS &&
-                proto != NULL && protoLen > 0) {
-            return wc_Hash(TICKET_BINDING_HASH_TYPE, (const byte*)proto,
-                           protoLen, dst, TICKET_BINDING_HASH_SZ);
+        TLSX* extension;
+        ALPN* alpn;
+
+        extension = TLSX_Find(ssl->extensions, TLSX_APPLICATION_LAYER_PROTOCOL);
+        if (extension != NULL) {
+            alpn = (ALPN*)extension->data;
+            if (alpn != NULL && alpn->negotiated == 1 &&
+                    alpn->protocol_name != NULL) {
+                word32 protoLen = (word32)XSTRLEN(alpn->protocol_name);
+                if (protoLen > 0) {
+                    return wc_Hash(TICKET_BINDING_HASH_TYPE,
+                                   (const byte*)alpn->protocol_name,
+                                   protoLen, dst, TICKET_BINDING_HASH_SZ);
+                }
+            }
         }
 
         XMEMSET(dst, 0, TICKET_BINDING_HASH_SZ);

src/ssl_sess.c

@@ -3647,6 +3647,20 @@ void SetupSession(WOLFSSL* ssl)
         session->sessionCtxSz = ssl->sessionCtxSz;
     }
 #endif
+#ifdef HAVE_SESSION_TICKET
+    /* Bind the current SNI/ALPN to the session so that a later resumption
+     * via the stateful session-ID cache (no ticket) can be rejected when
+     * the resumed ClientHello carries a different SNI/ALPN. RFC 6066 section 3
+     * requires "MUST NOT accept the request to resume the session if the
+     * server_name extension contains a different name". CreateTicket()
+     * overwrites these with the same values for the ticket path. */
+#ifdef HAVE_SNI
+    (void)TicketSniHash(ssl, session->sniHash);
+#endif
+#ifdef HAVE_ALPN
+    (void)TicketAlpnHash(ssl, session->alpnHash);
+#endif
+#endif /* HAVE_SESSION_TICKET */
     session->timeout = ssl->timeout;
 #ifndef NO_ASN_TIME
     session->bornOn  = LowResTimer();

tests/api/test_tls.c

@@ -861,6 +861,93 @@ int test_tls12_etm_failed_resumption(void)
     return EXPECT_RESULT();
 }
 
+/* RFC 6066 Section 3 requires:
+ *   "A server that implements this extension MUST NOT accept the request to
+ *    resume the session if the server_name extension contains a different
+ *    name. Instead, it proceeds with a full handshake to establish a new
+ *    session."
+ *
+ * wolfSSL's SNI/ALPN ticket-binding hardening (see VerifyTicketBinding,
+ * added in PR #10279) covers the session ticket path but short-circuits on
+ * !ssl->options.useTicket, so it does not apply to the TLS 1.2 stateful
+ * session-ID cache resumption path. SetupSession() does not store the
+ * original SNI on the cached WOLFSSL_SESSION, and TlsSessionCacheGetAndLock()
+ * keys only on (sessionID, sessionIDSz, side). The result is that a session
+ * established under one SNI can be resumed under a different SNI via the
+ * session-ID cache, in violation of the MUST NOT above.
+ *
+ * This test forces the session-ID resumption path (no tickets) and offers a
+ * different SNI on the resumption attempt. The server must NOT resume. */
+int test_tls12_session_id_resumption_sni_mismatch(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
+    !defined(WOLFSSL_NO_TLS12) && defined(HAVE_SNI) && \
+    !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";
+
+    /* Step 1: full TLS 1.2 handshake under SNI=public.example, with the
+     * session ticket path disabled so resumption can only happen via the
+     * server's session-ID cache. */
+    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);
+#ifdef HAVE_SESSION_TICKET
+    ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_c), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_s), WOLFSSL_SUCCESS);
+#endif
+    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);
+    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
+     * session cache still holds the entry from step 1). The client offers
+     * the saved session but advertises a *different* SNI. The server's
+     * cache lookup will match by session ID, but per RFC 6066 Section 3 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);
+#ifdef HAVE_SESSION_TICKET
+    ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_c), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_s), WOLFSSL_SUCCESS);
+#endif
+    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);
+
+    /* Post-fix expected behavior: server falls back to a full handshake
+     * because the SNI in the ClientHello does not match the SNI bound to
+     * the cached session. Pre-fix, the server silently resumes - which is
+     * the bug. 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

@@ -32,6 +32,7 @@ int test_tls_certreq_order(void);
 int test_tls12_bad_cv_sig_alg(void);
 int test_tls12_no_null_compression(void);
 int test_tls12_etm_failed_resumption(void);
+int test_tls12_session_id_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);
@@ -47,6 +48,7 @@ int test_tls12_peerauth_failsafe(void);
         TEST_DECL_GROUP("tls", test_tls12_bad_cv_sig_alg),                     \
         TEST_DECL_GROUP("tls", test_tls12_no_null_compression),                \
         TEST_DECL_GROUP("tls", test_tls12_etm_failed_resumption),              \
+        TEST_DECL_GROUP("tls", test_tls12_session_id_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)

wolfssl/internal.h

@@ -6821,6 +6821,12 @@ WOLFSSL_LOCAL int DoClientTicket_ex(const WOLFSSL* ssl, PreSharedKey* psk,
 #endif
 
 WOLFSSL_LOCAL int DoClientTicket(WOLFSSL* ssl, const byte* input, word32 len);
+#ifdef HAVE_SNI
+WOLFSSL_LOCAL int TicketSniHash(WOLFSSL* ssl, byte* dst);
+#endif
+#ifdef HAVE_ALPN
+WOLFSSL_LOCAL int TicketAlpnHash(WOLFSSL* ssl, byte* dst);
+#endif
 #if defined(HAVE_SNI) || defined(HAVE_ALPN)
 WOLFSSL_LOCAL int VerifyTicketBinding(WOLFSSL* ssl);
 #endif