]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
Fix TLS 1.3 PSKs with SSL_VERIFY_PEER and no sid_ctx.
authorViktor Dukhovni <viktor@openssl.org>
Sat, 11 Jul 2026 14:35:35 +0000 (00:35 +1000)
committerNorbert Pocs <norbertp@openssl.org>
Tue, 21 Jul 2026 14:11:36 +0000 (16:11 +0200)
A server with client certificate verification requested, but no
session ID context configured, wrongly rejected every TLS 1.3
PSK-based connection, whether a resumption ticket, or an external
PSK.  After a full non-PSK handshake the same server issued a
poison resumption PSK (session ticket) that led to handshake
failure if/when used.

The session ID context check exists to stop SSL acceptors with
distinct authentication policies that share a session cache from
resuming each other's sessions and trusting their authentication
results; it doesn't apply to a just-validated external PSK, so
the corresponding sessions are now exempted.  Ticket issuance is
also suppressed when it's already known that the ticket would lead
to a handshake failure with the same server's configuration.

Reviewed-by: Bob Beck <beck@openssl.org>
Reviewed-by: Norbert Pocs <norbertp@openssl.org>
MergeDate: Tue Jul 21 14:12:04 2026
(Merged from https://github.com/openssl/openssl/pull/31964)

ssl/ssl_local.h
ssl/ssl_sess.c
ssl/statem/extensions_srvr.c
ssl/statem/statem_srvr.c

index aeb0c170ad1e97d58825ef6da3dbddca1ee4a56f..2cfc5cb8156cd32f831414b4f7bd78f58e13e442 100644 (file)
@@ -512,6 +512,21 @@ struct ssl_session_st {
      * to disable session caching and tickets.
      */
     int not_resumable;
+    /*
+     * Set when this session's master key was resolved from an external PSK
+     * identity (psk_find_session_cb(), or the legacy psk_server_callback())
+     * rather than from a resumption ticket or session-cache lookup.
+     * ssl_get_prev_session() uses this to exempt such sessions from sid_ctx
+     * checks that only make sense for a real cache lookup.
+     *
+     * Deliberately not part of the SSL_SESSION ASN.1 encoding: it must not
+     * survive a real ticket round-trip (a session reconstructed by
+     * d2i_SSL_SESSION() from a genuine, previously-issued ticket is by
+     * definition not an external-PSK match, and should get the ordinary
+     * sid_ctx treatment). ssl_session_dup() resets it to 0 on every copy,
+     * mirroring not_resumable just above, for the same reason.
+     */
+    int psk_external;
     /* Peer raw public key, if available */
     EVP_PKEY *peer_rpk;
     /* This is the cert and type for the other end. */
index ba0dcd229ff420b847f0f5fdfcb504fb3587bf03..094ab1a74eadb7f7c7cbc71ef27fc723d6ed0408 100644 (file)
@@ -279,8 +279,16 @@ SSL_SESSION *ssl_session_dup(const SSL_SESSION *src, int ticket)
 {
     SSL_SESSION *sess = ssl_session_dup_intern(src, ticket);
 
-    if (sess != NULL)
+    if (sess != NULL) {
         sess->not_resumable = 0;
+        /*
+         * A duplicated session can land in the stateful session cache, and is
+         * not necessarily a live session just built for an external PSK.  The
+         * caller must explicitly set this field non-zero after duplication as
+         * needed.
+         */
+        sess->psk_external = 0;
+    }
 
     return sess;
 }
@@ -648,7 +656,13 @@ int ssl_get_prev_session(SSL_CONNECTION *s, CLIENTHELLO_MSG *hello)
         goto err; /* treat like cache miss */
     }
 
-    if ((s->verify_mode & SSL_VERIFY_PEER) && s->sid_ctx_length == 0) {
+    /*
+     * sid_ctx exists to keep multiple services that happen to share one
+     * session cache from resuming each other's sessions.  This check is not
+     * relevant to external PSK sessions that are not restored from a cache.
+     */
+    if (!ret->psk_external
+        && (s->verify_mode & SSL_VERIFY_PEER) && s->sid_ctx_length == 0) {
         /*
          * We can't be sure if this session is being used out of context,
          * which is especially important for SSL_VERIFY_PEER. The application
index 12c06eef9b54d37579ec86b52dd8f9d10bcc0b2d..a597085e1b6cfd4d32c1ffaf1c418625aeccb8df 100644 (file)
@@ -1397,7 +1397,10 @@ int tls_parse_ctos_psk(SSL_CONNECTION *s, PACKET *pkt, unsigned int context,
 #endif /* OPENSSL_NO_PSK */
 
         if (sess != NULL) {
-            /* We found a PSK */
+            /*
+             * We found an external (not a resumption) PSK - duplicate the
+             * session, set the session id to our own, and mark it as external.
+             */
             SSL_SESSION *sesstmp = ssl_session_dup(sess, 0);
 
             if (sesstmp == NULL) {
@@ -1413,7 +1416,7 @@ int tls_parse_ctos_psk(SSL_CONNECTION *s, PACKET *pkt, unsigned int context,
              */
             memcpy(sess->sid_ctx, s->sid_ctx, s->sid_ctx_length);
             sess->sid_ctx_length = s->sid_ctx_length;
-            ext = 1;
+            sess->psk_external = ext = 1;
             if (id == 0)
                 s->ext.early_data_ok = 1;
             s->ext.ticket_expected = 1;
@@ -1484,6 +1487,8 @@ int tls_parse_ctos_psk(SSL_CONNECTION *s, PACKET *pkt, unsigned int context,
                  */
                 s->ext.early_data_ok = 1;
             }
+            /* This PSK is not external, use the correct binder label, ... */
+            ext = 0;
         }
 
         md = ssl_md(sctx, sess->cipher->algorithm2);
@@ -1760,7 +1765,15 @@ EXT_RETURN tls_construct_stoc_session_ticket(SSL_CONNECTION *s, WPACKET *pkt,
     unsigned int context, X509 *x,
     size_t chainidx)
 {
-    if (!s->ext.ticket_expected || !tls_use_ticket(s)) {
+    /*
+     * Don't tell the client to expect a NewSessionTicket when any
+     * ticket we'd mint would be rejected by ssl_get_prev_session()
+     * whenever SSL_VERIFY_PEER is set with no sid_ctx configured (see
+     * the checks there).  In TLS 1.2, once promised the ticket MUST
+     * be sent.
+     */
+    if (!s->ext.ticket_expected || !tls_use_ticket(s)
+        || ((s->verify_mode & SSL_VERIFY_PEER) != 0 && s->sid_ctx_length == 0)) {
         s->ext.ticket_expected = 0;
         return EXT_RETURN_NOT_SENT;
     }
index 2f8bdcea944093e57fa7026c7aa94cd1e0d019e1..fc3769a017b738bd6c3944b4469dea385c591ced 100644 (file)
@@ -712,17 +712,20 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL_CONNECTION *s)
          * session tickets or resumption (e.g. new_session_count = 0 or
          * resumption_count = 0), this implementation does not currently
          * interpret or enforce those parameters.
+         *
+         * Also skip issuance when SSL_VERIFY_PEER is set with no sid_ctx
+         * configured: any ticket minted here would be rejected by
+         * ssl_get_prev_session() in that configuration.
          */
-        if (((s->options & SSL_OP_NO_TICKET) != 0
+        if (s->num_tickets <= s->sent_tickets
+            || ((s->options & SSL_OP_NO_TICKET) != 0
                 && (SSL_CONNECTION_GET_CTX(s)->session_cache_mode & SSL_SESS_CACHE_SERVER)
                     == 0)
-            || s->ext.psk_kex_mode == TLSEXT_KEX_MODE_FLAG_NONE) {
+            || s->ext.psk_kex_mode == TLSEXT_KEX_MODE_FLAG_NONE
+            || ((s->verify_mode & SSL_VERIFY_PEER) != 0 && s->sid_ctx_length == 0))
             st->hand_state = TLS_ST_OK;
-        } else if (s->num_tickets > s->sent_tickets) {
+        else
             st->hand_state = TLS_ST_SW_SESSION_TICKET;
-        } else {
-            st->hand_state = TLS_ST_OK;
-        }
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SR_KEY_UPDATE: