From: Igor Ustinov Date: Sat, 7 Feb 2026 09:21:22 +0000 (+0100) Subject: SSL_get_error(): Do not depend on the state of the error stack X-Git-Tag: openssl-4.0.0-alpha1~293 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7b5ddfb157671b360d04175dcac656c6dc6b8edd;p=thirdparty%2Fopenssl.git 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 Reviewed-by: Tomas Mraz 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 a2621cb75ca..d30b96ace32 100644 --- 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 db750cc37fa..07a79d667aa 100644 --- 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. The value returned by that TLS/SSL I/O function must be passed to SSL_get_error() in parameter B. -In addition to B and B, 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. - =head1 NOTES Some TLS implementations do not send a close_notify alert on shutdown. @@ -195,6 +187,10 @@ L, 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 a41ed322703..aab3dcbab5d 100644 --- 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 f105408ecd0..4949fc9229c 100644 --- 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 d649e384740..8f24979ecec 100644 --- 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 2f6cef3343b..63b1278575d 100644 --- 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 5e5606056f7..98d46a4e540 100644 --- 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 d311ec65da3..5afd571b8b4 100644 --- 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 d2ff9a8ab6c..cc48f0be7ce 100644 --- 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"); }