This is an automated email from the ASF dual-hosted git repository.
chenBright pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new f98d5d06 Fix HTTPS pooled client crash on unexpected SSL EOF (#3316)
f98d5d06 is described below
commit f98d5d0628483a7a730d2ec4db5e3bea89cf8f00
Author: Felix-Gong <[email protected]>
AuthorDate: Fri Jun 5 10:30:31 2026 +0800
Fix HTTPS pooled client crash on unexpected SSL EOF (#3316)
* Fix HTTPS pooled client crash on unexpected SSL EOF
When OpenSSL 3.x detects unexpected EOF (peer closed without
close_notify), SSL_read returns 0 with SSL_ERROR_SSL. The code
didn't return -1, causing error_code=0 to propagate to
Controller::SetFailed() which triggers CHECK(false).
Fix by returning -1 with errno=ESSL when SSL errors are detected
in DoRead(), instead of falling through and returning nr.
Discovered during RISC-V porting and integration testing.
Fixes #3307
Signed-off-by: Felix Gong <[email protected]>
* Add unit test for unexpected SSL EOF handling
Test that Socket::DoRead() returns -1 with errno=ESSL when the remote
side closes the TCP connection without sending close_notify, verifying
the fix in commit 7a1dfe7c.
---------
Signed-off-by: Felix Gong <[email protected]>
---
src/brpc/socket.cpp | 9 +++++-
test/brpc_ssl_unittest.cpp | 76 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/src/brpc/socket.cpp b/src/brpc/socket.cpp
index 0ca69504..aa349ec8 100644
--- a/src/brpc/socket.cpp
+++ b/src/brpc/socket.cpp
@@ -2133,7 +2133,14 @@ ssize_t Socket::DoRead(size_t size_hint) {
default: {
const unsigned long e = ERR_get_error();
if (nr == 0) {
- // Socket EOF or SSL session EOF
+ if (ssl_error != SSL_ERROR_ZERO_RETURN) {
+ // Unexpected EOF without proper SSL shutdown (close_notify)
+ LOG(WARNING) << "Fail to read from ssl_fd=" << fd()
+ << ": unexpected ssl_error=" << ssl_error;
+ errno = ESSL;
+ return -1;
+ }
+ // Clean SSL shutdown (close_notify received)
} else if (e != 0) {
LOG(WARNING) << "Fail to read from ssl_fd=" << fd()
<< ": " << SSLError(e);
diff --git a/test/brpc_ssl_unittest.cpp b/test/brpc_ssl_unittest.cpp
index a469a623..e7545a0c 100644
--- a/test/brpc_ssl_unittest.cpp
+++ b/test/brpc_ssl_unittest.cpp
@@ -497,3 +497,79 @@ TEST_F(SSLTest, ssl_perf) {
close(clifd);
close(servfd);
}
+
+struct AbruptCloseArgs { int listenfd; };
+
+static void* abrupt_close_server(void* arg) {
+ AbruptCloseArgs* a = (AbruptCloseArgs*)arg;
+ int connfd = accept(a->listenfd, NULL, NULL);
+ if (connfd < 0) return NULL;
+ SSL_CTX* ctx = brpc::CreateServerSSLContext(
+ "cert1.crt", "cert1.key", brpc::SSLOptions(), NULL, NULL);
+ SSL* ssl = brpc::CreateSSLSession(ctx, 0, connfd, true);
+ if (ssl) { SSL_do_handshake(ssl); SSL_free(ssl); }
+ close(connfd);
+ return NULL;
+}
+
+TEST_F(SSLTest, ssl_unexpected_eof) {
+ // Verify that Socket::DoRead() returns -1 with errno=ESSL when the
+ // remote side closes the TCP connection without sending close_notify.
+ // Without the fix, DoRead() returns 0, causing error_code=0 to
+ // propagate to Controller::SetFailed() which triggers CHECK(false).
+
+ const int port = 5962;
+ butil::EndPoint ep(butil::IP_ANY, port);
+ butil::fd_guard listenfd(butil::tcp_listen(ep));
+ ASSERT_GT(listenfd, 0);
+
+ AbruptCloseArgs server_args = { listenfd };
+ pthread_t server_tid;
+ ASSERT_EQ(0, pthread_create(&server_tid, NULL, abrupt_close_server,
+ &server_args));
+
+ brpc::Protocol dummy_protocol = {
+ brpc::policy::ParseRpcMessage, brpc::SerializeRequestDefault,
+ brpc::policy::PackRpcRequest, NULL, ProcessResponse,
+ NULL, NULL, NULL, brpc::CONNECTION_TYPE_ALL, "ssl_ut_eof"
+ };
+ ASSERT_EQ(0, RegisterProtocol((brpc::ProtocolType)31, dummy_protocol));
+
+ brpc::InputMessageHandler dummy_handler = {
+ dummy_protocol.parse, dummy_protocol.process_response,
+ NULL, NULL, dummy_protocol.name
+ };
+ brpc::InputMessenger messenger;
+ ASSERT_EQ(0, messenger.AddHandler(dummy_handler));
+
+ brpc::SocketOptions socket_options;
+ butil::EndPoint server_ep(butil::IP_ANY, port);
+ socket_options.remote_side = server_ep;
+ socket_options.connect_on_create = true;
+ // Do NOT set on_edge_triggered_events — we will call DoRead manually.
+ socket_options.user = &messenger;
+
+ brpc::ChannelSSLOptions ssl_options;
+ SSL_CTX* raw_ctx = brpc::CreateClientSSLContext(ssl_options);
+ ASSERT_NE(nullptr, raw_ctx);
+ std::shared_ptr<brpc::SocketSSLContext> ssl_ctx =
+ std::make_shared<brpc::SocketSSLContext>();
+ ssl_ctx->raw_ctx = raw_ctx;
+ socket_options.initial_ssl_ctx = ssl_ctx;
+
+ brpc::SocketId socket_id;
+ ASSERT_EQ(0, brpc::Socket::Create(socket_options, &socket_id));
+ brpc::SocketUniquePtr ptr;
+ ASSERT_EQ(0, brpc::Socket::Address(socket_id, &ptr));
+
+ // Wait for server to close the connection without close_notify.
+ pthread_join(server_tid, NULL);
+ usleep(50000);
+
+ // DoRead should detect the unexpected EOF and return -1 with errno=ESSL.
+ ssize_t nr = ptr->DoRead(1024);
+ EXPECT_EQ(-1, nr);
+ EXPECT_EQ(brpc::ESSL, errno);
+
+ ptr->SetFailed();
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
