From 754d5bcf3e7b595f09bbfd072b580ee77a65847e Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Tue, 29 Sep 2026 13:59:44 +0000 Subject: [PATCH] fix(lib): a stale OpenSSL error no longer kills a healthy connection (bug-1871) SSL_get_error() reports SSL_ERROR_SSL whenever the calling thread's OpenSSL error queue is non-empty. io_worker serves every endpoint from one thread, so an error left by another connection -- a ctrlstatus client's EOF queues 'unexpected eof while reading' in OpenSSL 3 -- turned the next plain would-block SSL_read on a healthy connection into a fatal error. handle_error() then tore that connection down; mid snapshot transfer this left a rejoining standby with a truncated snap.part until its liveness timeout forced a reconnect and a fresh transfer. Clear the queue before each SSL_read/SSL_write/SSL_do_handshake so the classification is about that call only. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/lib/meson.build | 1 + src/lib/netio.c | 9 + src/lib/testsuite/test.c | 2 + src/lib/testsuite/test_netio_err_queue.c | 206 +++++++++++++++++++++++ 4 files changed, 218 insertions(+) create mode 100644 src/lib/testsuite/test_netio_err_queue.c diff --git a/src/lib/meson.build b/src/lib/meson.build index 6781f49c..4e2981cd 100644 --- a/src/lib/meson.build +++ b/src/lib/meson.build @@ -220,6 +220,7 @@ libxce_test_src = ['testsuite/test.c', 'testsuite/test_netio_close_propagation.c', 'testsuite/test_netio_bounded_connect.c', 'testsuite/test_netio_batch_dispatch_uaf.c', + 'testsuite/test_netio_err_queue.c', 'testsuite/test_request_send_propagation.c', 'testsuite/test_raft.c', 'testsuite/test_wal_wire.c', diff --git a/src/lib/netio.c b/src/lib/netio.c index 06a41021..971e241b 100644 --- a/src/lib/netio.c +++ b/src/lib/netio.c @@ -116,6 +116,14 @@ static int io_fsm_do(io_fsm_t* fsm, char* data, size_t len) count = len - fsm->iocount; + /* bug-1871: SSL_get_error() reports SSL_ERROR_SSL whenever this thread's + * OpenSSL error queue is non-empty. io_worker serves every endpoint, so + * an error left by another connection (e.g. a client's EOF) turned a + * plain would-block here into a fatal error and tore down a healthy + * connection -- mid snapshot transfer. Start each call with a clean + * queue so the classification below is about this call only. */ + ERR_clear_error(); + switch (fsm->op) { case FSM_IO_READ: rc = SSL_read(ep->ssl, data + fsm->iocount, count); @@ -545,6 +553,7 @@ static int handle_ssl_handshake(endpoint_t* ep) int err; int ecode = 0; + ERR_clear_error(); /* bug-1871: see io_fsm_do() */ rc = SSL_do_handshake(ep->ssl); if (rc == 1) { ep->ssl_handshake_done = 1; diff --git a/src/lib/testsuite/test.c b/src/lib/testsuite/test.c index 04c21473..b30c6641 100644 --- a/src/lib/testsuite/test.c +++ b/src/lib/testsuite/test.c @@ -49,6 +49,7 @@ extern void test_netio_arm_failure(void); extern void test_netio_close_propagation(void); extern void test_netio_bounded_connect(void); extern void test_netio_batch_dispatch_uaf(void); +extern void test_netio_err_queue(void); extern void test_request_send_propagation(void); extern void test_raft(void); extern void test_wal_wire(void); @@ -176,6 +177,7 @@ int main(int argc, char* argv[]) TEST(test_netio_close_propagation); TEST(test_netio_bounded_connect); TEST(test_netio_batch_dispatch_uaf); + TEST(test_netio_err_queue); TEST(test_request_send_propagation); TEST(test_raft); TEST(test_wal_wire); diff --git a/src/lib/testsuite/test_netio_err_queue.c b/src/lib/testsuite/test_netio_err_queue.c new file mode 100644 index 00000000..0cd9588a --- /dev/null +++ b/src/lib/testsuite/test_netio_err_queue.c @@ -0,0 +1,206 @@ +/* + * test_netio_err_queue.c — bug-1871: a stale entry on the io_worker thread's + * OpenSSL error queue must not turn an ordinary would-block into a fatal + * endpoint error. + * + * OpenSSL's error queue is per THREAD, and SSL_get_error() reports + * SSL_ERROR_SSL whenever that queue is non-empty, whatever the SSL call it is + * asked about actually did. io_worker serves every endpoint from one thread: + * a short-lived client closing its connection (OpenSSL 3 queues "unexpected + * eof while reading") left an entry behind, and the next SSL_read that merely + * found no data on an unrelated, healthy connection — the HA snapshot stream + * mid-transfer — was classified as a protocol error, so handle_error() tore + * that connection down. The standby then waited for its liveness timeout + * before reconnecting and asking for the snapshot again. + * + * The test drives the real io_handler (via io_worker_dispatch_batch_for_test) + * on an endpoint whose SSL_read genuinely would-block: an ESTABLISHED client + * TLS session on a non-blocking socketpair whose peer sends nothing. It must + * be established: the handshake state machine clears the error queue itself, + * the application-data read path does not. + */ + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#include "netio.h" + +static int g_err_calls = 0; + +static int count_err_handler(void* p) +{ + (void)p; + g_err_calls++; + return 0; +} + +static int unused_msg_handler(void* p) +{ + (void)p; + return 0; +} + +/* A server context with a throwaway self-signed P-256 certificate. */ +static SSL_CTX* make_server_ctx(void) +{ + SSL_CTX* ctx = SSL_CTX_new(TLS_server_method()); + EVP_PKEY* key = EVP_EC_gen("P-256"); + X509* cert = X509_new(); + + TEST_ASSERT_NOT_NULL(ctx); + TEST_ASSERT_NOT_NULL(key); + TEST_ASSERT_NOT_NULL(cert); + + ASN1_INTEGER_set(X509_get_serialNumber(cert), 1); + X509_gmtime_adj(X509_getm_notBefore(cert), 0); + X509_gmtime_adj(X509_getm_notAfter(cert), 3600); + X509_set_pubkey(cert, key); + X509_NAME_add_entry_by_txt(X509_get_subject_name(cert), "CN", MBSTRING_ASC, + (const unsigned char*)"localhost", -1, -1, 0); + X509_set_issuer_name(cert, X509_get_subject_name(cert)); + TEST_ASSERT_TRUE(X509_sign(cert, key, EVP_sha256()) > 0); + + TEST_ASSERT_EQUAL_INT(1, SSL_CTX_use_certificate(ctx, cert)); + TEST_ASSERT_EQUAL_INT(1, SSL_CTX_use_PrivateKey(ctx, key)); + + X509_free(cert); + EVP_PKEY_free(key); + return ctx; +} + +/* An endpoint holding the client end of an established TLS session over a + * non-blocking socketpair. The server end (*server) never writes application + * data, so the endpoint's next SSL_read would block. */ +static endpoint_t* make_blocked_reader(SSL_CTX* cctx, SSL_CTX* sctx, + SSL** server) +{ + int sv[2]; + int i; + int done_c = 0; + int done_s = 0; + endpoint_t* ep; + char buf[256]; + + TEST_ASSERT_EQUAL_INT(0, socketpair(AF_UNIX, SOCK_STREAM, 0, sv)); + fcntl(sv[0], F_SETFL, fcntl(sv[0], F_GETFL) | O_NONBLOCK); + fcntl(sv[1], F_SETFL, fcntl(sv[1], F_GETFL) | O_NONBLOCK); + + ep = endpoint_get(); + TEST_ASSERT_NOT_NULL(ep); + + ep->fd = sv[0]; + ep->ssl = SSL_new(cctx); + TEST_ASSERT_NOT_NULL(ep->ssl); + SSL_set_connect_state(ep->ssl); + SSL_set_fd(ep->ssl, sv[0]); + + *server = SSL_new(sctx); + TEST_ASSERT_NOT_NULL(*server); + SSL_set_accept_state(*server); + SSL_set_fd(*server, sv[1]); + + for (i = 0; (i < 1000) && !(done_c && done_s); i++) { + if (!done_c) { + done_c = (SSL_do_handshake(ep->ssl) == 1); + } + if (!done_s) { + done_s = (SSL_do_handshake(*server) == 1); + } + } + TEST_ASSERT_TRUE_MESSAGE(done_c && done_s, "TLS handshake did not finish"); + + /* Consume the server's post-handshake session tickets (TLS 1.3) so the + * endpoint's read below finds no data at all. */ + (void)SSL_read(ep->ssl, buf, sizeof(buf)); + ERR_clear_error(); + + ep->ssl_handshake_done = 1; + ep->msg_handler = unused_msg_handler; + ep->err_handler = count_err_handler; + + return ep; +} + +static void dispatch_input(endpoint_t* ep) +{ + struct epoll_event ev; + + memset(&ev, 0, sizeof(ev)); + ev.events = EPOLLIN; + ev.data.ptr = ep; + io_worker_dispatch_batch_for_test(&ev, 1); +} + +/* Baseline: with a clean error queue a would-block read is not an error. */ +static void test_would_block_read_is_not_an_error(void) +{ + SSL_CTX* cctx = SSL_CTX_new(TLS_client_method()); + SSL_CTX* sctx = make_server_ctx(); + SSL* server; + endpoint_t* ep = make_blocked_reader(cctx, sctx, &server); + + g_err_calls = 0; + ERR_clear_error(); + + dispatch_input(ep); + + TEST_ASSERT_EQUAL_INT(0, g_err_calls); + TEST_ASSERT_NOT_EQUAL(FSM_STATE_ERROR, ep->fsm_in.state); + + endpoint_unref(ep); + close(SSL_get_fd(server)); + SSL_free(server); + SSL_CTX_free(sctx); + SSL_CTX_free(cctx); + printf("test_would_block_read_is_not_an_error: PASS\n"); +} + +/* THE BUG: an error left on this thread's queue by an earlier, unrelated + * connection must not make the same would-block read fatal. */ +static void test_stale_thread_error_does_not_kill_endpoint(void) +{ + SSL_CTX* cctx = SSL_CTX_new(TLS_client_method()); + SSL_CTX* sctx = make_server_ctx(); + SSL* server; + endpoint_t* ep = make_blocked_reader(cctx, sctx, &server); + + g_err_calls = 0; + ERR_clear_error(); + /* What a peer's close left behind on this thread (OpenSSL 3). */ + ERR_raise(ERR_LIB_SSL, SSL_R_UNEXPECTED_EOF_WHILE_READING); + + dispatch_input(ep); + + TEST_ASSERT_EQUAL_INT_MESSAGE(0, g_err_calls, + "bug-1871: a would-block read was reported as a connection error " + "because of another connection's stale OpenSSL error"); + TEST_ASSERT_NOT_EQUAL(FSM_STATE_ERROR, ep->fsm_in.state); + + ERR_clear_error(); + endpoint_unref(ep); + close(SSL_get_fd(server)); + SSL_free(server); + SSL_CTX_free(sctx); + SSL_CTX_free(cctx); + printf("test_stale_thread_error_does_not_kill_endpoint: PASS\n"); +} + +void test_netio_err_queue(void) +{ + printf("\n>> netio per-thread OpenSSL error queue tests (bug-1871)\n\n"); + + RUN_TEST(test_would_block_read_is_not_an_error); + RUN_TEST(test_stale_thread_error_does_not_kill_endpoint); +}