SSL_get_error(): Do not depend on the state of the error stack

We check in relevant functions (SSL_handshake(), SSL_read(), etc.) whether
a new error has been pushed onto the error stack, and if so, memorise this
fact in the SSL structure. After that SSL_get_error() uses this memorised
information instead of checking the error stack itself.

Fixes #11889
Fixes openssl/project#1715

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
MergeDate: Wed Feb 18 15:27:38 2026
(Merged from https://github.com/openssl/openssl/pull/29991)

diff --git a/CHANGES.md b/CHANGES.md
index a2621cb75cac65a53e790362f6553b0617ac1d88..d30b96ace321b93ae6c2ba521e366eaf44c8d95e 100644 (file)
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -73,6 +73,12 @@ OpenSSL 4.0
 
    *Neil Horman*
 
+ * SSL_get_error() no longer depends on the state of the error stack,
+   so it is no longer necessary to empty the error queue before the
+   TLS/SSL I/O operations.
+
+   *Igor Ustinov*
+
  * Added configure options to disable KDF algorithms for
    hmac-drbg-kdf, kbkdf, krb5kdf, pvkkdf, snmpkdf, sskdf, sshkdf, x942kdf and x963kdf.
 

diff --git a/doc/man3/SSL_get_error.pod b/doc/man3/SSL_get_error.pod
index db750cc37fad55ae3d131cddcbae073cb26c2794..07a79d667aa263df7a6a8b798a0d5a9166d1c090 100644 (file)
--- a/doc/man3/SSL_get_error.pod
+++ b/doc/man3/SSL_get_error.pod
@@ -18,14 +18,6 @@ SSL_read_ex(), SSL_read(), SSL_peek_ex(), SSL_peek(), SSL_shutdown(),
 SSL_write_ex() or SSL_write() on B<ssl>.  The value returned by that TLS/SSL I/O
 function must be passed to SSL_get_error() in parameter B<ret>.
 
-In addition to B<ssl> and B<ret>, SSL_get_error() inspects the
-current thread's OpenSSL error queue.  Thus, SSL_get_error() must be
-used in the same thread that performed the TLS/SSL I/O operation, and no
-other OpenSSL function calls should appear in between.  The current
-thread's error queue must be empty before the TLS/SSL I/O operation is
-attempted, or SSL_get_error() will not work reliably.  Emptying the
-current thread's error queue is done with L<ERR_clear_error(3)>.
-
 =head1 NOTES
 
 Some TLS implementations do not send a close_notify alert on shutdown.
@@ -195,6 +187,10 @@ L<ERR_clear_error(3)>, ERR_print_errors(3), ERR_peek_last_error_all(3)
 The SSL_ERROR_WANT_ASYNC error code was added in OpenSSL 1.1.0.
 The SSL_ERROR_WANT_CLIENT_HELLO_CB error code was added in OpenSSL 1.1.1.
 
+Since OpenSSL 4.0 SSL_get_error() no longer depends on the state of the
+error stack, so it is no longer necessary to empty the error queue
+before the TLS/SSL I/O operations.
+
 =head1 COPYRIGHT
 
 Copyright 2000-2024 The OpenSSL Project Authors. All Rights Reserved.

diff --git a/include/internal/statem.h b/include/internal/statem.h
index a41ed3227031e89124f84a2f67a94c5a8ba8f654..aab3dcbab5d5e82eb0b8c1b04a02cac962fa58ce 100644 (file)
--- a/include/internal/statem.h
+++ b/include/internal/statem.h
@@ -81,6 +81,12 @@ typedef enum {
     CON_FUNC_DONT_SEND
 } CON_FUNC_RETURN;
 
+typedef enum {
+    ERROR_STATE_NOERROR = 0,
+    ERROR_STATE_SSL,
+    ERROR_STATE_SYSCALL
+} ERROR_STATE;
+
 typedef int (*ossl_statem_mutate_handshake_cb)(const unsigned char *msgin,
     size_t inlen,
     unsigned char **msgout,
@@ -106,6 +112,7 @@ struct ossl_statem_st {
     OSSL_HANDSHAKE_STATE hand_state;
     /* The handshake state requested by an API call (e.g. HelloRequest) */
     OSSL_HANDSHAKE_STATE request_state;
+    ERROR_STATE error_state;
     int in_init;
     int read_state_first_init;
     /* true when we are actually in SSL_accept() or SSL_connect() */

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index f105408ecd01821b3706073627a57a5d196ab054..4949fc9229caeca2ef95c60d22d9ee8c652d3572 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2185,23 +2185,58 @@ int SSL_get_async_status(SSL *s, int *status)
     return 1;
 }
 
+static void ssl_clear_error_state(SSL *s)
+{
+    SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(s);
+
+    if (sc != NULL)
+        sc->statem.error_state = ERROR_STATE_NOERROR;
+    ERR_set_mark();
+}
+
+static void ssl_check_error_stack(SSL *s)
+{
+    SSL_CONNECTION *sc;
+    unsigned long l;
+    if (ERR_count_to_mark() > 0) {
+        sc = SSL_CONNECTION_FROM_SSL(s);
+        l = ERR_peek_error();
+        if (sc != NULL && l != 0) {
+            if (ERR_GET_LIB(l) == ERR_LIB_SYS)
+                sc->statem.error_state = ERROR_STATE_SYSCALL;
+            else
+                sc->statem.error_state = ERROR_STATE_SSL;
+        }
+    }
+    ERR_clear_last_mark();
+}
+
 int SSL_accept(SSL *s)
 {
     SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(s);
 
+    ssl_clear_error_state(s);
+
 #ifndef OPENSSL_NO_QUIC
-    if (IS_QUIC(s))
-        return s->method->ssl_accept(s);
+    if (IS_QUIC(s)) {
+        int ret = s->method->ssl_accept(s);
+        ssl_check_error_stack(s);
+        return ret;
+    }
 #endif
 
-    if (sc == NULL)
+    if (sc == NULL) {
+        ERR_clear_last_mark();
         return 0;
+    }
 
     if (sc->handshake_func == NULL) {
         /* Not properly initialized yet */
         SSL_set_accept_state(s);
     }
 
+    ssl_check_error_stack(s);
+
     return SSL_do_handshake(s);
 }
 
@@ -2209,19 +2244,28 @@ int SSL_connect(SSL *s)
 {
     SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(s);
 
+    ssl_clear_error_state(s);
+
 #ifndef OPENSSL_NO_QUIC
-    if (IS_QUIC(s))
-        return s->method->ssl_connect(s);
+    if (IS_QUIC(s)) {
+        int ret = s->method->ssl_connect(s);
+        ssl_check_error_stack(s);
+        return ret;
+    }
 #endif
 
-    if (sc == NULL)
+    if (sc == NULL) {
+        ERR_clear_last_mark();
         return 0;
+    }
 
     if (sc->handshake_func == NULL) {
         /* Not properly initialized yet */
         SSL_set_connect_state(s);
     }
 
+    ssl_check_error_stack(s);
+
     return SSL_do_handshake(s);
 }
 
@@ -2366,8 +2410,11 @@ int SSL_read(SSL *s, void *buf, int num)
     int ret;
     size_t readbytes;
 
+    ssl_clear_error_state(s);
+
     if (num < 0) {
         ERR_raise(ERR_LIB_SSL, SSL_R_BAD_LENGTH);
+        ssl_check_error_stack(s);
         return -1;
     }
 
@@ -2380,15 +2427,23 @@ int SSL_read(SSL *s, void *buf, int num)
     if (ret > 0)
         ret = (int)readbytes;
 
+    ssl_check_error_stack(s);
+
     return ret;
 }
 
 int SSL_read_ex(SSL *s, void *buf, size_t num, size_t *readbytes)
 {
-    int ret = ssl_read_internal(s, buf, num, readbytes);
+    int ret;
+
+    ssl_clear_error_state(s);
 
+    ret = ssl_read_internal(s, buf, num, readbytes);
     if (ret < 0)
         ret = 0;
+
+    ssl_check_error_stack(s);
+
     return ret;
 }
 
@@ -2501,8 +2556,11 @@ int SSL_peek(SSL *s, void *buf, int num)
     int ret;
     size_t readbytes;
 
+    ssl_clear_error_state(s);
+
     if (num < 0) {
         ERR_raise(ERR_LIB_SSL, SSL_R_BAD_LENGTH);
+        ssl_check_error_stack(s);
         return -1;
     }
 
@@ -2515,15 +2573,23 @@ int SSL_peek(SSL *s, void *buf, int num)
     if (ret > 0)
         ret = (int)readbytes;
 
+    ssl_check_error_stack(s);
+
     return ret;
 }
 
 int SSL_peek_ex(SSL *s, void *buf, size_t num, size_t *readbytes)
 {
-    int ret = ssl_peek_internal(s, buf, num, readbytes);
+    int ret;
+
+    ssl_clear_error_state(s);
 
+    ret = ssl_peek_internal(s, buf, num, readbytes);
     if (ret < 0)
         ret = 0;
+
+    ssl_check_error_stack(s);
+
     return ret;
 }
 
@@ -2656,8 +2722,11 @@ int SSL_write(SSL *s, const void *buf, int num)
     int ret;
     size_t written;
 
+    ssl_clear_error_state(s);
+
     if (num < 0) {
         ERR_raise(ERR_LIB_SSL, SSL_R_BAD_LENGTH);
+        ssl_check_error_stack(s);
         return -1;
     }
 
@@ -2670,6 +2739,8 @@ int SSL_write(SSL *s, const void *buf, int num)
     if (ret > 0)
         ret = (int)written;
 
+    ssl_check_error_stack(s);
+
     return ret;
 }
 
@@ -2681,10 +2752,16 @@ int SSL_write_ex(SSL *s, const void *buf, size_t num, size_t *written)
 int SSL_write_ex2(SSL *s, const void *buf, size_t num, uint64_t flags,
     size_t *written)
 {
-    int ret = ssl_write_internal(s, buf, num, flags, written);
+    int ret;
+
+    ssl_clear_error_state(s);
 
+    ret = ssl_write_internal(s, buf, num, flags, written);
     if (ret < 0)
         ret = 0;
+
+    ssl_check_error_stack(s);
+
     return ret;
 }
 
@@ -2773,18 +2850,26 @@ int SSL_shutdown(SSL *s)
      * (see ssl3_shutdown).
      */
     SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(s);
+    int ret;
+
+    ssl_clear_error_state(s);
 
 #ifndef OPENSSL_NO_QUIC
-    if (IS_QUIC(s))
-        return ossl_quic_conn_shutdown(s, 0, NULL, 0);
+    if (IS_QUIC(s)) {
+        ret = ossl_quic_conn_shutdown(s, 0, NULL, 0);
+        goto end;
+    }
 #endif
 
-    if (sc == NULL)
-        return -1;
+    if (sc == NULL) {
+        ret = -1;
+        goto end;
+    }
 
     if (sc->handshake_func == NULL) {
         ERR_raise(ERR_LIB_SSL, SSL_R_UNINITIALIZED);
-        return -1;
+        ret = -1;
+        goto end;
     }
 
     if (!SSL_in_init(s)) {
@@ -2796,14 +2881,18 @@ int SSL_shutdown(SSL *s)
             args.type = OTHERFUNC;
             args.f.func_other = s->method->ssl_shutdown;
 
-            return ssl_start_async_job(s, &args, ssl_io_intern);
+            ret = ssl_start_async_job(s, &args, ssl_io_intern);
         } else {
-            return s->method->ssl_shutdown(s);
+            ret = s->method->ssl_shutdown(s);
         }
     } else {
         ERR_raise(ERR_LIB_SSL, SSL_R_SHUTDOWN_WHILE_IN_INIT);
-        return -1;
+        ret = -1;
     }
+
+end:
+    ssl_check_error_stack(s);
+    return ret;
 }
 
 int SSL_key_update(SSL *s, int updatetype)
@@ -4858,7 +4947,6 @@ int SSL_get_error(const SSL *s, int i)
 int ossl_ssl_get_error(const SSL *s, int i, int check_err)
 {
     int reason;
-    unsigned long l;
     BIO *bio;
     const SSL_CONNECTION *sc = SSL_CONNECTION_FROM_CONST_SSL(s);
 
@@ -4876,15 +4964,11 @@ int ossl_ssl_get_error(const SSL *s, int i, int check_err)
     if (sc == NULL)
         return SSL_ERROR_SSL;
 
-    /*
-     * Make things return SSL_ERROR_SYSCALL when doing SSL_do_handshake etc,
-     * where we do encode the error
-     */
-    if (check_err && (l = ERR_peek_error()) != 0) {
-        if (ERR_GET_LIB(l) == ERR_LIB_SYS)
-            return SSL_ERROR_SYSCALL;
-        else
+    if (check_err != 0) {
+        if (sc->statem.error_state == ERROR_STATE_SSL)
             return SSL_ERROR_SSL;
+        if (sc->statem.error_state == ERROR_STATE_SYSCALL)
+            return SSL_ERROR_SYSCALL;
     }
 
 #ifndef OPENSSL_NO_QUIC
@@ -4976,21 +5060,30 @@ int SSL_do_handshake(SSL *s)
     int ret = 1;
     SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(s);
 
+    ssl_clear_error_state(s);
+
 #ifndef OPENSSL_NO_QUIC
-    if (IS_QUIC(s))
-        return ossl_quic_do_handshake(s);
+    if (IS_QUIC(s)) {
+        ret = ossl_quic_do_handshake(s);
+        goto end;
+    }
 #endif
 
-    if (sc == NULL)
-        return -1;
+    if (sc == NULL) {
+        ret = -1;
+        goto end;
+    }
 
     if (sc->handshake_func == NULL) {
         ERR_raise(ERR_LIB_SSL, SSL_R_CONNECTION_TYPE_NOT_SET);
-        return -1;
+        ret = -1;
+        goto end;
     }
 
-    if (!ossl_statem_check_finish_init(sc, -1))
-        return -1;
+    if (!ossl_statem_check_finish_init(sc, -1)) {
+        ret = -1;
+        goto end;
+    }
 
     s->method->ssl_renegotiate_check(s, 0);
 
@@ -5007,6 +5100,8 @@ int SSL_do_handshake(SSL *s)
         }
     }
 
+end:
+    ssl_check_error_stack(s);
     return ret;
 }
 

diff --git a/ssl/statem/statem.c b/ssl/statem/statem.c
index d649e38474009a78f1bfe6338f871d052075a57b..8f24979ececfaec1912ec9b8c14b4287697b1629 100644 (file)
--- a/ssl/statem/statem.c
+++ b/ssl/statem/statem.c
@@ -131,6 +131,7 @@ void ossl_statem_clear(SSL_CONNECTION *s)
 {
     s->statem.state = MSG_FLOW_UNINITED;
     s->statem.hand_state = TLS_ST_BEFORE;
+    s->statem.error_state = ERROR_STATE_NOERROR;
     ossl_statem_set_in_init(s, 1);
     s->statem.no_cert_verify = 0;
 }
@@ -288,6 +289,7 @@ void ossl_statem_set_hello_verify_done(SSL_CONNECTION *s)
      * sensible.
      */
     s->statem.hand_state = TLS_ST_SR_CLNT_HELLO;
+    s->statem.error_state = ERROR_STATE_NOERROR;
 }
 
 int ossl_statem_connect(SSL *s)

diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 2f6cef3343b29f12c9f82f012432920775ecd1c7..63b1278575d5839c4031e3296b1ac88fd3490a41 100644 (file)
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1199,7 +1199,17 @@ WORK_STATE ossl_statem_server_post_work(SSL_CONNECTION *s, WORK_STATE wst)
 
     case TLS_ST_SW_SESSION_TICKET:
         clear_sys_error();
+        ERR_set_mark();
+        st->error_state = ERROR_STATE_NOERROR;
         if (SSL_CONNECTION_IS_TLS13(s) && statem_flush(s) != 1) {
+            if (ERR_count_to_mark() > 0) {
+                unsigned long l = ERR_peek_error();
+                if (ERR_GET_LIB(l) == ERR_LIB_SYS)
+                    st->error_state = ERROR_STATE_SYSCALL;
+                else
+                    st->error_state = ERROR_STATE_SSL;
+            }
+            ERR_clear_last_mark();
             if (SSL_get_error(ssl, 0) == SSL_ERROR_SYSCALL
                 && conn_is_closed()) {
                 /*
@@ -1215,6 +1225,7 @@ WORK_STATE ossl_statem_server_post_work(SSL_CONNECTION *s, WORK_STATE wst)
 
             return WORK_MORE_A;
         }
+        ERR_clear_last_mark();
         break;
     }
 

diff --git a/test/helpers/handshake.c b/test/helpers/handshake.c
index 5e5606056f7f82289fbc03f39c23b7aa70adaae2..98d46a4e540a3e5317118ce704ca3064fd820ec3 100644 (file)
--- a/test/helpers/handshake.c
+++ b/test/helpers/handshake.c
@@ -833,6 +833,7 @@ typedef enum {
     PEER_SUCCESS,
     PEER_RETRY,
     PEER_ERROR,
+    PEER_FINAL_ERROR,
     PEER_WAITING,
     PEER_TEST_FAILURE
 } peer_status_t;
@@ -1028,6 +1029,10 @@ static void do_reneg_setup_step(const SSL_TEST_CTX *test_ctx, PEER *peer)
              */
             if (SSL_is_server(peer->ssl)) {
                 ret = SSL_renegotiate(peer->ssl);
+                if (!ret) {
+                    peer->status = PEER_FINAL_ERROR;
+                    return;
+                }
             } else {
                 int full_reneg = 0;
 
@@ -1047,10 +1052,10 @@ static void do_reneg_setup_step(const SSL_TEST_CTX *test_ctx, PEER *peer)
                     ret = SSL_renegotiate(peer->ssl);
                 else
                     ret = SSL_renegotiate_abbreviated(peer->ssl);
-            }
-            if (!ret) {
-                peer->status = PEER_ERROR;
-                return;
+                if (!ret) {
+                    peer->status = PEER_FINAL_ERROR;
+                    return;
+                }
             }
             do_handshake_step(peer);
             /*
@@ -1311,6 +1316,7 @@ static handshake_status_t handshake_status(peer_status_t last_status,
             /* Let the first peer finish. */
             return HANDSHAKE_RETRY;
         case PEER_ERROR:
+        case PEER_FINAL_ERROR:
             /*
              * Second peer succeeded despite the fact that the first peer
              * already errored. This shouldn't happen.
@@ -1322,6 +1328,9 @@ static handshake_status_t handshake_status(peer_status_t last_status,
     case PEER_RETRY:
         return HANDSHAKE_RETRY;
 
+    case PEER_FINAL_ERROR:
+        return client_spoke_last ? CLIENT_ERROR : SERVER_ERROR;
+
     case PEER_ERROR:
         switch (previous_status) {
         case PEER_TEST_FAILURE:
@@ -1336,6 +1345,7 @@ static handshake_status_t handshake_status(peer_status_t last_status,
             /* We errored; let the peer finish. */
             return HANDSHAKE_RETRY;
         case PEER_ERROR:
+        case PEER_FINAL_ERROR:
             /* Both peers errored. Return the one that errored first. */
             return client_spoke_last ? SERVER_ERROR : CLIENT_ERROR;
         }

diff --git a/test/quicapitest.c b/test/quicapitest.c
index d311ec65da30a6ba794cedc184c8ad0e97eacf09..5afd571b8b47373b0203f38a95faf1de30bed44b 100644 (file)
--- a/test/quicapitest.c
+++ b/test/quicapitest.c
@@ -3489,7 +3489,7 @@ int setup_tests(void)
         goto err;
 
     cprivkey = test_mk_file_path(certsdir, "ee-key.pem");
-    if (privkey == NULL)
+    if (cprivkey == NULL)
         goto err;
 
     ADD_ALL_TESTS(test_quic_write_read, 3);

diff --git a/test/recipes/70-test_renegotiation.t b/test/recipes/70-test_renegotiation.t
index d2ff9a8ab6cbd80200c0d7c0d53ed3fc113d1dc7..cc48f0be7ce2f0bca8c69dcdf63dad39fce6f41d 100644 (file)
--- a/test/recipes/70-test_renegotiation.t
+++ b/test/recipes/70-test_renegotiation.t
@@ -39,12 +39,23 @@ my $proxy = TLSProxy::Proxy->new(
     (!$ENV{HARNESS_ACTIVE} || $ENV{HARNESS_VERBOSE})
 );
 
+sub success_or_closenotify
+{
+    return 1 if TLSProxy::Message->success();
+
+    my $alert = TLSProxy::Message->alert();
+    return 0 unless defined $alert;
+
+    return ($alert->level() == TLSProxy::Message::AL_LEVEL_WARN()
+            && $alert->description() == TLSProxy::Message::AL_DESC_CLOSE_NOTIFY());
+}
+
 #Test 1: A basic renegotiation test
 $proxy->clientflags("-no_tls1_3");
 $proxy->serverflags("-client_renegotiation");
 $proxy->reneg(1);
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
-ok(TLSProxy::Message->success(), "Basic renegotiation");
+ok(success_or_closenotify(), "Basic renegotiation");
 
 #Test 2: Seclevel 0 client does not send the Reneg SCSV. Reneg should fail
 $proxy->clear();
@@ -97,7 +108,7 @@ SKIP: {
             }
         }
     }
-    ok(TLSProxy::Message->success() && $chmatch,
+    ok(success_or_closenotify() && $chmatch,
        "Check ClientHello version is the same");
 }