From: Viktor Dukhovni Date: Sat, 11 Jul 2026 14:35:35 +0000 (+1000) Subject: Fix TLS 1.3 PSKs with SSL_VERIFY_PEER and no sid_ctx. X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=ecd8d329f3476ee54629416738542df0d626a30f;p=thirdparty%2Fopenssl.git Fix TLS 1.3 PSKs with SSL_VERIFY_PEER and no sid_ctx. 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 Reviewed-by: Norbert Pocs MergeDate: Tue Jul 21 14:12:04 2026 (Merged from https://github.com/openssl/openssl/pull/31964) --- diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h index aeb0c170ad1..2cfc5cb8156 100644 --- a/ssl/ssl_local.h +++ b/ssl/ssl_local.h @@ -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. */ diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index ba0dcd229ff..094ab1a74ea 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -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 diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 12c06eef9b5..a597085e1b6 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -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; } diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 2f8bdcea944..fc3769a017b 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -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: