Stop a TLSv1.3 server emitting an unsolicited PSK extension

If we attempt to accept a connection on an SSL object, and the
application has set an SSL_SESSION on that SSL object then we
can mistakenly believe that we are resuming and
emit an unsolicited PSK extension back to the client.

This can especially happen when using SSL_clear() which leaves
any SSL_SESSION associated with the SSL object.

See
https://github.com/openssl/openssl/discussions/27563#discussioncomment-13049352
and
https://github.com/openssl/openssl/discussions/24567

Reviewed-by: Todd Short <todd.short@me.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27584)

(cherry picked from commit aa8bca2e81030560d690cb68bbcbe8b7d00d1d29)

diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 8e9aa64de5a7a634dcbe3e316d38b5f9332837a3..a50f7ce96b646e1e66c33d4291b9adc941ef2f3c 100644 (file)
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -595,6 +595,8 @@ int ssl_get_prev_session(SSL_CONNECTION *s, CLIENTHELLO_MSG *hello)
     SSL_TICKET_STATUS r;
 
     if (SSL_CONNECTION_IS_TLS13(s)) {
+        SSL_SESSION_free(s->session);
+        s->session = NULL;
         /*
          * By default we will send a new ticket. This can be overridden in the
          * ticket processing.
@@ -607,6 +609,7 @@ int ssl_get_prev_session(SSL_CONNECTION *s, CLIENTHELLO_MSG *hello)
                                         hello->pre_proc_exts, NULL, 0))
             return -1;
 
+        /* If we resumed, s->session will now be set */
         ret = s->session;
     } else {
         /* sets s->ext.ticket_expected */

diff --git a/test/sslapitest.c b/test/sslapitest.c
index 38d58e938743efa69e918c41d50db8e3052c5362..7143bd01a7bea7ce3c2a0e2515f555008e01027c 100644 (file)
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -7315,18 +7315,6 @@ static int test_ssl_clear(int idx)
         } else {
             SSL_set_accept_state(serverssl);
         }
-        /*
-         * A peculiarity of SSL_clear() is that it does not clear the session.
-         * This is intended behaviour so that a client can create a new
-         * connection and reuse the session. But this doesn't make much sense
-         * on the server side - and causes incorrect behaviour due to the
-         * handshake failing (even though the documentation does say SSL_clear()
-         * is supposed to work on the server side). We clear the session
-         * explicitly - although note that the documentation for
-         * SSL_set_session() says that its only useful for clients!
-         */
-        if (!TEST_true(SSL_set_session(serverssl, NULL)))
-            goto end;
         SSL_free(clientssl);
         clientssl = NULL;
     } else {