From 855c843d9c0ef328712223adffca2f91f73940ce Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Fri, 28 Aug 2026 10:57:46 -0400 Subject: [PATCH 1/7] Fix swallowed non-blocking connect errors (CWE-390) Use the SO_ERROR result for retry classification and error reporting instead of a stale thread-local socket error. Add a regression test covering the false-success sequence. --- crypto/bio/bio_socket_test.cc | 78 +++++++++++++++++++++++++++++++++++ crypto/bio/connect.c | 10 +++++ crypto/bio/internal.h | 6 ++- crypto/bio/socket_helper.c | 11 ++++- 4 files changed, 103 insertions(+), 2 deletions(-) diff --git a/crypto/bio/bio_socket_test.cc b/crypto/bio/bio_socket_test.cc index fbdfc7b1f4e..1f6f4032755 100644 --- a/crypto/bio/bio_socket_test.cc +++ b/crypto/bio/bio_socket_test.cc @@ -43,6 +43,7 @@ using Socket = int; #define INVALID_SOCKET (-1) static int closesocket(const int sock) { return close(sock); } static std::string LastSocketError() { return strerror(errno); } +static void SetLastSocketError(int error) { errno = error; } #else using Socket = SOCKET; static std::string LastSocketError() { @@ -50,6 +51,7 @@ static std::string LastSocketError() { snprintf(buf, sizeof(buf), "%d", WSAGetLastError()); return buf; } +static void SetLastSocketError(int error) { WSASetLastError(error); } #endif struct SockaddrStorage { @@ -413,6 +415,27 @@ static bool WaitForSocket(Socket sock, WaitType wait_type) { #endif } +static bool WaitForConnect(Socket sock) { + static constexpr int kTimeoutSeconds = 5; +#if defined(OPENSSL_WINDOWS) + fd_set write_set, error_set; + FD_ZERO(&write_set); + FD_ZERO(&error_set); + FD_SET(sock, &write_set); + FD_SET(sock, &error_set); + timeval timeout = {kTimeoutSeconds, 0}; + if (select(0 /* unused on Windows */, nullptr, &write_set, &error_set, + &timeout) <= 0) { + return false; + } + return FD_ISSET(sock, &write_set) || FD_ISSET(sock, &error_set); +#else + pollfd fd = {.fd = sock, .events = POLLOUT, .revents = 0}; + return poll(&fd, 1, kTimeoutSeconds * 1000) == 1 && + (fd.revents & (POLLOUT | POLLERR | POLLHUP)); +#endif +} + TEST(BIOTest, SocketConnect) { static constexpr char kTestMessage[] = "test"; const OwnedSocket listening_sock = ListenLoopback(SOCK_STREAM); @@ -454,6 +477,61 @@ TEST(BIOTest, SocketConnect) { ASSERT_EQ(Bytes(kTestMessage, sizeof(kTestMessage)), Bytes(buf, sizeof(buf))); } +TEST(BIOTest, SocketNonBlockingConnectFailure) { + sockaddr_in sin; + OPENSSL_cleanse(&sin, sizeof(sin)); + sin.sin_family = AF_INET; + ASSERT_EQ(1, inet_pton(AF_INET, "127.0.0.1", &sin.sin_addr)); + + // Find an unused loopback port for the connection attempt. + OwnedSocket bound_sock(Bind(AF_INET, SOCK_STREAM, + reinterpret_cast(&sin), + sizeof(sin))); + ASSERT_TRUE(bound_sock.is_valid()) << LastSocketError(); + + SockaddrStorage addr; + ASSERT_EQ(0, getsockname(bound_sock.get(), addr.addr_mut(), &addr.len)) + << LastSocketError(); + + char hostname[80]; + snprintf(hostname, sizeof(hostname), "127.0.0.1:%d", + ntohs(addr.ToIPv4().sin_port)); + bound_sock.reset(); + + const bssl::UniquePtr bio(BIO_new_connect(hostname)); + ASSERT_TRUE(bio); + ASSERT_TRUE(BIO_set_nbio(bio.get(), 1)); + + ASSERT_EQ(-1, BIO_do_connect(bio.get())); + ASSERT_TRUE(BIO_should_retry(bio.get())); + + const int fd = BIO_get_fd(bio.get(), nullptr); + ASSERT_NE(-1, fd); + ASSERT_TRUE(WaitForConnect(fd)) << LastSocketError(); + + // A successful readiness call need not clear the retryable socket error left + // by connect. Ensure the connect BIO uses SO_ERROR, not this stale value. +#if defined(OPENSSL_WINDOWS) + SetLastSocketError(WSAEWOULDBLOCK); +#else + SetLastSocketError(EINPROGRESS); +#endif + ERR_clear_error(); + + EXPECT_EQ(0, BIO_do_connect(bio.get())); + EXPECT_FALSE(BIO_should_retry(bio.get())); + + uint32_t error = ERR_get_error(); + EXPECT_EQ(ERR_LIB_SYS, ERR_GET_LIB(error)); +#if !defined(OPENSSL_WINDOWS) + EXPECT_EQ(ECONNREFUSED, ERR_GET_REASON(error)); +#endif + error = ERR_get_error(); + EXPECT_EQ(ERR_LIB_BIO, ERR_GET_LIB(error)); + EXPECT_EQ(BIO_R_NBIO_CONNECT_ERROR, ERR_GET_REASON(error)); + EXPECT_EQ(0u, ERR_get_error()); +} + TEST(BIOTest, SocketNonBlocking) { OwnedSocket listening_sock = ListenLoopback(SOCK_STREAM); ASSERT_TRUE(listening_sock.is_valid()) << LastSocketError(); diff --git a/crypto/bio/connect.c b/crypto/bio/connect.c index b7f6c546bbd..a80ca93fe83 100644 --- a/crypto/bio/connect.c +++ b/crypto/bio/connect.c @@ -198,7 +198,17 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) { case BIO_CONN_S_BLOCKED_CONNECT: i = bio_sock_error_get_and_clear(bio->num); + if (i < 0) { + BIO_clear_retry_flags(bio); + OPENSSL_PUT_SYSTEM_ERROR(); + OPENSSL_PUT_ERROR(BIO, BIO_R_NBIO_CONNECT_ERROR); + ERR_add_error_data(4, "host=", c->param_hostname, ":", + c->param_port); + ret = 0; + goto exit_loop; + } if (i) { + bio_socket_set_error(i); if (bio_socket_should_retry(ret)) { BIO_set_flags(bio, (BIO_FLAGS_IO_SPECIAL | BIO_FLAGS_SHOULD_RETRY)); c->state = BIO_CONN_S_BLOCKED_CONNECT; diff --git a/crypto/bio/internal.h b/crypto/bio/internal.h index 4f25a44be2d..df758f24f87 100644 --- a/crypto/bio/internal.h +++ b/crypto/bio/internal.h @@ -52,9 +52,13 @@ int bio_socket_nbio(int sock, int on); // bio_clear_socket_error clears the last socket error on |sock|. void bio_clear_socket_error(int sock); -// bio_sock_error_get_and_clear clears and returns the last socket error on |sock|. +// bio_sock_error_get_and_clear clears and returns the last socket error on +// |sock|, or -1 if querying the socket error failed. int bio_sock_error_get_and_clear(int sock); +// bio_socket_set_error sets the last socket error for the current thread. +void bio_socket_set_error(int error); + // bio_socket_should_retry returns non-zero if |return_value| indicates an error // and the last socket error indicates that it's non-fatal. int bio_socket_should_retry(int return_value); diff --git a/crypto/bio/socket_helper.c b/crypto/bio/socket_helper.c index a7ff9d3505f..d1a88ce6ef5 100644 --- a/crypto/bio/socket_helper.c +++ b/crypto/bio/socket_helper.c @@ -11,6 +11,7 @@ #if !defined(OPENSSL_NO_SOCK) +#include #include #include #include @@ -108,11 +109,19 @@ int bio_sock_error_get_and_clear(int sock) { socklen_t error_size = sizeof(error); // Get and clear the pending socket error. The SO_ERROR option is read-only. if (getsockopt(sock, SOL_SOCKET, SO_ERROR, (char *)&error, &error_size) < 0) { - return 1; + return -1; } return error; } +void bio_socket_set_error(int error) { +#if defined(OPENSSL_WINDOWS) + WSASetLastError(error); +#else + errno = error; +#endif +} + int bio_socket_should_retry(int return_value) { #if defined(OPENSSL_WINDOWS) return return_value == -1 && (WSAGetLastError() == WSAEWOULDBLOCK); From 9fb19e397e41e4e2c84c1ea73c0f4276411b2bf3 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Mon, 31 Aug 2026 11:32:20 -0400 Subject: [PATCH 2/7] Clarify connect BIO error classification and tidy the new test No functional change. Pass the -1 explicitly to bio_socket_should_retry and document that it classifies the thread's last socket error rather than its argument, so the preceding bio_socket_set_error call is clearly load-bearing. Fold WaitForConnect into WaitForSocket as a kConnect wait type, and give that wait a longer timeout: Windows takes ~2s to report a refused loopback connection, which left little margin against the previous 5s. Explain why the SYS reason code is only checked off Windows, and sort the socket_helper.c include block. --- crypto/bio/bio_socket_test.cc | 53 +++++++++++++++-------------------- crypto/bio/connect.c | 7 +++-- crypto/bio/socket_helper.c | 2 +- 3 files changed, 28 insertions(+), 34 deletions(-) diff --git a/crypto/bio/bio_socket_test.cc b/crypto/bio/bio_socket_test.cc index 1f6f4032755..76993a540c9 100644 --- a/crypto/bio/bio_socket_test.cc +++ b/crypto/bio/bio_socket_test.cc @@ -390,49 +390,40 @@ static bool SocketSetNonBlocking(Socket sock) { #endif } -enum class WaitType { kRead, kWrite }; +enum class WaitType { kRead, kWrite, kConnect }; static bool WaitForSocket(Socket sock, WaitType wait_type) { - // Use an arbitrary 5-second timeout, so the test doesn't hang indefinitely if - // there's an issue. - static constexpr int kTimeoutSeconds = 5; + // Arbitrary timeouts, so the test cannot hang. |kConnect| waits on a full + // connection attempt, which Windows can take a couple of seconds to refuse. + const int timeout_seconds = wait_type == WaitType::kConnect ? 30 : 5; + // A failed connection attempt can surface as an error, not writability. + const bool watch_errors = wait_type == WaitType::kConnect; #if defined(OPENSSL_WINDOWS) - fd_set read_set, write_set; + fd_set read_set, write_set, error_set; FD_ZERO(&read_set); FD_ZERO(&write_set); + FD_ZERO(&error_set); fd_set *wait_set = wait_type == WaitType::kRead ? &read_set : &write_set; FD_SET(sock, wait_set); - timeval timeout = {kTimeoutSeconds, 0}; - if (select(0 /* unused on Windows */, &read_set, &write_set, nullptr, - &timeout) <= 0) { + if (watch_errors) { + FD_SET(sock, &error_set); + } + timeval timeout = {timeout_seconds, 0}; + if (select(0 /* unused on Windows */, &read_set, &write_set, + watch_errors ? &error_set : nullptr, &timeout) <= 0) { return false; } - return FD_ISSET(sock, wait_set); + return FD_ISSET(sock, wait_set) || + (watch_errors && FD_ISSET(sock, &error_set)); #else const short events = wait_type == WaitType::kRead ? POLLIN : POLLOUT; pollfd fd = {.fd = sock, .events = events, .revents = 0}; - return poll(&fd, 1, kTimeoutSeconds * 1000) == 1 && (fd.revents & events); -#endif -} - -static bool WaitForConnect(Socket sock) { - static constexpr int kTimeoutSeconds = 5; -#if defined(OPENSSL_WINDOWS) - fd_set write_set, error_set; - FD_ZERO(&write_set); - FD_ZERO(&error_set); - FD_SET(sock, &write_set); - FD_SET(sock, &error_set); - timeval timeout = {kTimeoutSeconds, 0}; - if (select(0 /* unused on Windows */, nullptr, &write_set, &error_set, - &timeout) <= 0) { + if (poll(&fd, 1, timeout_seconds * 1000) != 1) { return false; } - return FD_ISSET(sock, &write_set) || FD_ISSET(sock, &error_set); -#else - pollfd fd = {.fd = sock, .events = POLLOUT, .revents = 0}; - return poll(&fd, 1, kTimeoutSeconds * 1000) == 1 && - (fd.revents & (POLLOUT | POLLERR | POLLHUP)); + // POLLERR and POLLHUP are reported regardless of |events|. + const int accepted = watch_errors ? (events | POLLERR | POLLHUP) : events; + return (fd.revents & accepted) != 0; #endif } @@ -507,7 +498,7 @@ TEST(BIOTest, SocketNonBlockingConnectFailure) { const int fd = BIO_get_fd(bio.get(), nullptr); ASSERT_NE(-1, fd); - ASSERT_TRUE(WaitForConnect(fd)) << LastSocketError(); + ASSERT_TRUE(WaitForSocket(fd, WaitType::kConnect)) << LastSocketError(); // A successful readiness call need not clear the retryable socket error left // by connect. Ensure the connect BIO uses SO_ERROR, not this stale value. @@ -523,6 +514,8 @@ TEST(BIOTest, SocketNonBlockingConnectFailure) { uint32_t error = ERR_get_error(); EXPECT_EQ(ERR_LIB_SYS, ERR_GET_LIB(error)); + // |ERR_GET_REASON| truncates to 12 bits, so Winsock codes (10000 and up) + // cannot round-trip. Only assert the reason where it is representable. #if !defined(OPENSSL_WINDOWS) EXPECT_EQ(ECONNREFUSED, ERR_GET_REASON(error)); #endif diff --git a/crypto/bio/connect.c b/crypto/bio/connect.c index a80ca93fe83..00184029f20 100644 --- a/crypto/bio/connect.c +++ b/crypto/bio/connect.c @@ -202,14 +202,15 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) { BIO_clear_retry_flags(bio); OPENSSL_PUT_SYSTEM_ERROR(); OPENSSL_PUT_ERROR(BIO, BIO_R_NBIO_CONNECT_ERROR); - ERR_add_error_data(4, "host=", c->param_hostname, ":", - c->param_port); + ERR_add_error_data(4, "host=", c->param_hostname, ":", c->param_port); ret = 0; goto exit_loop; } if (i) { + // |bio_socket_should_retry| classifies the thread's last socket + // error, not its argument, so publish |i| first. bio_socket_set_error(i); - if (bio_socket_should_retry(ret)) { + if (bio_socket_should_retry(-1)) { BIO_set_flags(bio, (BIO_FLAGS_IO_SPECIAL | BIO_FLAGS_SHOULD_RETRY)); c->state = BIO_CONN_S_BLOCKED_CONNECT; bio->retry_reason = BIO_RR_CONNECT; diff --git a/crypto/bio/socket_helper.c b/crypto/bio/socket_helper.c index d1a88ce6ef5..f47f06f13f5 100644 --- a/crypto/bio/socket_helper.c +++ b/crypto/bio/socket_helper.c @@ -12,10 +12,10 @@ #if !defined(OPENSSL_NO_SOCK) #include -#include #include #include #include +#include #if !defined(OPENSSL_WINDOWS) #include From 1a526d045575b6b57276c1cac8ded631b8475c07 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Mon, 31 Aug 2026 11:35:31 -0400 Subject: [PATCH 3/7] Stop reporting success after a failed non-blocking connect Reading SO_ERROR to diagnose the failure also clears it, so the connect state machine stayed in BIO_CONN_S_BLOCKED_CONNECT and the next BIO_do_connect saw a clear socket error and transitioned to BIO_CONN_S_OK. A caller polling BIO_do_connect would see 0 once and then 1, on a connection that never completed and with an empty error queue. Add a terminal BIO_CONN_S_ERROR state so the failure sticks until BIO_CTRL_RESET. Repeat attempts, and reads and writes, now keep reporting failure. No system error is pushed on the repeat path because errno no longer describes the original failure. Extend the regression test to cover the repeated call and a subsequent write. --- crypto/bio/bio_socket_test.cc | 13 +++++++++++++ crypto/bio/connect.c | 14 ++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/crypto/bio/bio_socket_test.cc b/crypto/bio/bio_socket_test.cc index 76993a540c9..1dbe4873784 100644 --- a/crypto/bio/bio_socket_test.cc +++ b/crypto/bio/bio_socket_test.cc @@ -523,6 +523,19 @@ TEST(BIOTest, SocketNonBlockingConnectFailure) { EXPECT_EQ(ERR_LIB_BIO, ERR_GET_LIB(error)); EXPECT_EQ(BIO_R_NBIO_CONNECT_ERROR, ERR_GET_REASON(error)); EXPECT_EQ(0u, ERR_get_error()); + + // Reading SO_ERROR above cleared it, so a further attempt must keep failing + // rather than read the now-clear error as a completed connection. + EXPECT_EQ(0, BIO_do_connect(bio.get())); + EXPECT_FALSE(BIO_should_retry(bio.get())); + error = ERR_get_error(); + EXPECT_EQ(ERR_LIB_BIO, ERR_GET_LIB(error)); + EXPECT_EQ(BIO_R_NBIO_CONNECT_ERROR, ERR_GET_REASON(error)); + EXPECT_EQ(0u, ERR_get_error()); + + // Writing to a BIO whose connection never completed must fail too. + EXPECT_GE(0, BIO_write(bio.get(), "test", 4)); + EXPECT_FALSE(BIO_should_retry(bio.get())); } TEST(BIOTest, SocketNonBlocking) { diff --git a/crypto/bio/connect.c b/crypto/bio/connect.c index 00184029f20..3885c5025cb 100644 --- a/crypto/bio/connect.c +++ b/crypto/bio/connect.c @@ -32,6 +32,9 @@ enum { BIO_CONN_S_BEFORE, BIO_CONN_S_BLOCKED_CONNECT, BIO_CONN_S_OK, + // BIO_CONN_S_ERROR is terminal: the connect attempt failed and its socket + // error was already consumed. Keep last; |bio_info_cb| sees these values. + BIO_CONN_S_ERROR, }; typedef struct bio_connect_st { @@ -203,6 +206,7 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) { OPENSSL_PUT_SYSTEM_ERROR(); OPENSSL_PUT_ERROR(BIO, BIO_R_NBIO_CONNECT_ERROR); ERR_add_error_data(4, "host=", c->param_hostname, ":", c->param_port); + c->state = BIO_CONN_S_ERROR; ret = 0; goto exit_loop; } @@ -220,6 +224,7 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) { OPENSSL_PUT_SYSTEM_ERROR(); OPENSSL_PUT_ERROR(BIO, BIO_R_NBIO_CONNECT_ERROR); ERR_add_error_data(4, "host=", c->param_hostname, ":", c->param_port); + c->state = BIO_CONN_S_ERROR; ret = 0; } goto exit_loop; @@ -228,6 +233,15 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) { } break; + case BIO_CONN_S_ERROR: + // |SO_ERROR| was cleared when this failure was first reported, so + // re-reading it would look like success. |errno| is stale too. + BIO_clear_retry_flags(bio); + OPENSSL_PUT_ERROR(BIO, BIO_R_NBIO_CONNECT_ERROR); + ERR_add_error_data(4, "host=", c->param_hostname, ":", c->param_port); + ret = 0; + goto exit_loop; + case BIO_CONN_S_OK: ret = 1; goto exit_loop; From 198716655e43f20b7e86dd1059dff668b51e5838 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Tue, 22 Sep 2026 10:21:08 -0400 Subject: [PATCH 4/7] Refine connect BIO error classification and failure states Classify socket error codes directly without changing thread-local error state, preserving platform-specific retry behavior. Make immediate connect failures terminal until BIO_reset. Cover blocking and non-blocking failures, repeated I/O, reset recovery, and the callback's terminal state. --- crypto/bio/bio_socket_test.cc | 70 +++++++++++++++++++++++++---------- crypto/bio/bio_test.cc | 2 +- crypto/bio/connect.c | 19 +++++----- crypto/bio/errno.c | 20 +++++----- crypto/bio/internal.h | 8 +++- crypto/bio/socket_helper.c | 15 +++++--- 6 files changed, 86 insertions(+), 48 deletions(-) diff --git a/crypto/bio/bio_socket_test.cc b/crypto/bio/bio_socket_test.cc index 1dbe4873784..254d6387bd5 100644 --- a/crypto/bio/bio_socket_test.cc +++ b/crypto/bio/bio_socket_test.cc @@ -468,7 +468,7 @@ TEST(BIOTest, SocketConnect) { ASSERT_EQ(Bytes(kTestMessage, sizeof(kTestMessage)), Bytes(buf, sizeof(buf))); } -TEST(BIOTest, SocketNonBlockingConnectFailure) { +static void TestSocketConnectFailure(bool non_blocking) { sockaddr_in sin; OPENSSL_cleanse(&sin, sizeof(sin)); sin.sin_family = AF_INET; @@ -491,25 +491,27 @@ TEST(BIOTest, SocketNonBlockingConnectFailure) { const bssl::UniquePtr bio(BIO_new_connect(hostname)); ASSERT_TRUE(bio); - ASSERT_TRUE(BIO_set_nbio(bio.get(), 1)); + ASSERT_TRUE(BIO_set_nbio(bio.get(), non_blocking)); + ERR_clear_error(); ASSERT_EQ(-1, BIO_do_connect(bio.get())); - ASSERT_TRUE(BIO_should_retry(bio.get())); - const int fd = BIO_get_fd(bio.get(), nullptr); ASSERT_NE(-1, fd); - ASSERT_TRUE(WaitForSocket(fd, WaitType::kConnect)) << LastSocketError(); - // A successful readiness call need not clear the retryable socket error left - // by connect. Ensure the connect BIO uses SO_ERROR, not this stale value. + if (non_blocking) { + ASSERT_TRUE(BIO_should_retry(bio.get())); + ASSERT_TRUE(WaitForSocket(fd, WaitType::kConnect)) << LastSocketError(); + + // A successful readiness call need not clear the retryable socket error left + // by connect. Ensure the connect BIO uses SO_ERROR, not this stale value. #if defined(OPENSSL_WINDOWS) - SetLastSocketError(WSAEWOULDBLOCK); + SetLastSocketError(WSAEWOULDBLOCK); #else - SetLastSocketError(EINPROGRESS); + SetLastSocketError(EINPROGRESS); #endif - ERR_clear_error(); - - EXPECT_EQ(0, BIO_do_connect(bio.get())); + ERR_clear_error(); + EXPECT_EQ(0, BIO_do_connect(bio.get())); + } EXPECT_FALSE(BIO_should_retry(bio.get())); uint32_t error = ERR_get_error(); @@ -519,25 +521,55 @@ TEST(BIOTest, SocketNonBlockingConnectFailure) { #if !defined(OPENSSL_WINDOWS) EXPECT_EQ(ECONNREFUSED, ERR_GET_REASON(error)); #endif + const int reason = + non_blocking ? BIO_R_NBIO_CONNECT_ERROR : BIO_R_CONNECT_ERROR; + error = ERR_get_error(); + EXPECT_EQ(ERR_LIB_BIO, ERR_GET_LIB(error)); + EXPECT_EQ(reason, ERR_GET_REASON(error)); + EXPECT_EQ(0u, ERR_get_error()); + + // Even if the peer starts listening, the failed BIO must not silently create + // a new socket or interpret a cleared SO_ERROR as a successful connection. + bound_sock = Bind(AF_INET, SOCK_STREAM, addr.addr(), addr.len); + ASSERT_TRUE(bound_sock.is_valid()) << LastSocketError(); + ASSERT_EQ(0, listen(bound_sock.get(), 1)) << LastSocketError(); + ASSERT_EQ(0, BIO_do_connect(bio.get())); + EXPECT_FALSE(BIO_should_retry(bio.get())); + EXPECT_EQ(fd, BIO_get_fd(bio.get(), nullptr)); + error = ERR_get_error(); + EXPECT_EQ(ERR_LIB_BIO, ERR_GET_LIB(error)); + EXPECT_EQ(reason, ERR_GET_REASON(error)); + EXPECT_EQ(0u, ERR_get_error()); + + // Reading and writing must also fail without reporting a stale system error. + char buf[4]; + EXPECT_EQ(0, BIO_read(bio.get(), buf, sizeof(buf))); + EXPECT_FALSE(BIO_should_retry(bio.get())); error = ERR_get_error(); EXPECT_EQ(ERR_LIB_BIO, ERR_GET_LIB(error)); - EXPECT_EQ(BIO_R_NBIO_CONNECT_ERROR, ERR_GET_REASON(error)); + EXPECT_EQ(reason, ERR_GET_REASON(error)); EXPECT_EQ(0u, ERR_get_error()); - // Reading SO_ERROR above cleared it, so a further attempt must keep failing - // rather than read the now-clear error as a completed connection. - EXPECT_EQ(0, BIO_do_connect(bio.get())); + EXPECT_EQ(0, BIO_write(bio.get(), "test", 4)); EXPECT_FALSE(BIO_should_retry(bio.get())); error = ERR_get_error(); EXPECT_EQ(ERR_LIB_BIO, ERR_GET_LIB(error)); - EXPECT_EQ(BIO_R_NBIO_CONNECT_ERROR, ERR_GET_REASON(error)); + EXPECT_EQ(reason, ERR_GET_REASON(error)); EXPECT_EQ(0u, ERR_get_error()); - // Writing to a BIO whose connection never completed must fail too. - EXPECT_GE(0, BIO_write(bio.get(), "test", 4)); + // An explicit reset closes the failed socket and permits a fresh connection. + ASSERT_EQ(0, BIO_reset(bio.get())); + EXPECT_EQ(-1, BIO_get_fd(bio.get(), nullptr)); + ASSERT_TRUE(BIO_set_nbio(bio.get(), 0)); + ASSERT_EQ(1, BIO_do_connect(bio.get())) << LastSocketError(); EXPECT_FALSE(BIO_should_retry(bio.get())); + EXPECT_EQ(0u, ERR_get_error()); } +TEST(BIOTest, SocketConnectFailure) { TestSocketConnectFailure(false); } + +TEST(BIOTest, SocketNonBlockingConnectFailure) { TestSocketConnectFailure(true); } + TEST(BIOTest, SocketNonBlocking) { OwnedSocket listening_sock = ListenLoopback(SOCK_STREAM); ASSERT_TRUE(listening_sock.is_valid()) << LastSocketError(); diff --git a/crypto/bio/bio_test.cc b/crypto/bio/bio_test.cc index 44e67f0fa44..e24fe43da93 100644 --- a/crypto/bio/bio_test.cc +++ b/crypto/bio/bio_test.cc @@ -1052,7 +1052,7 @@ static int callback_invoked = 0; static long callback(BIO *b, int state, int res) { callback_invoked = 1; - EXPECT_EQ(state, 0); + EXPECT_EQ(state, 3 /* BIO_CONN_S_ERROR */); EXPECT_EQ(res, -1); return 0; } diff --git a/crypto/bio/connect.c b/crypto/bio/connect.c index 3885c5025cb..8b066f22088 100644 --- a/crypto/bio/connect.c +++ b/crypto/bio/connect.c @@ -32,8 +32,8 @@ enum { BIO_CONN_S_BEFORE, BIO_CONN_S_BLOCKED_CONNECT, BIO_CONN_S_OK, - // BIO_CONN_S_ERROR is terminal: the connect attempt failed and its socket - // error was already consumed. Keep last; |bio_info_cb| sees these values. + // BIO_CONN_S_ERROR is terminal: the connect attempt failed and its error + // was already reported. Keep last; |bio_info_cb| sees these values. BIO_CONN_S_ERROR, }; @@ -192,6 +192,7 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) { OPENSSL_PUT_ERROR(BIO, BIO_R_CONNECT_ERROR); ERR_add_error_data(4, "host=", c->param_hostname, ":", c->param_port); + c->state = BIO_CONN_S_ERROR; } goto exit_loop; } else { @@ -211,17 +212,14 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) { goto exit_loop; } if (i) { - // |bio_socket_should_retry| classifies the thread's last socket - // error, not its argument, so publish |i| first. - bio_socket_set_error(i); - if (bio_socket_should_retry(-1)) { + if (bio_socket_error_is_retryable(i)) { BIO_set_flags(bio, (BIO_FLAGS_IO_SPECIAL | BIO_FLAGS_SHOULD_RETRY)); c->state = BIO_CONN_S_BLOCKED_CONNECT; bio->retry_reason = BIO_RR_CONNECT; ret = -1; } else { BIO_clear_retry_flags(bio); - OPENSSL_PUT_SYSTEM_ERROR(); + OPENSSL_PUT_ERROR(SYS, i); OPENSSL_PUT_ERROR(BIO, BIO_R_NBIO_CONNECT_ERROR); ERR_add_error_data(4, "host=", c->param_hostname, ":", c->param_port); c->state = BIO_CONN_S_ERROR; @@ -234,10 +232,11 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) { break; case BIO_CONN_S_ERROR: - // |SO_ERROR| was cleared when this failure was first reported, so - // re-reading it would look like success. |errno| is stale too. + // The original error was already reported. Do not start a new connection + // or re-read |SO_ERROR|, which may have been cleared. |errno| is stale too. BIO_clear_retry_flags(bio); - OPENSSL_PUT_ERROR(BIO, BIO_R_NBIO_CONNECT_ERROR); + OPENSSL_PUT_ERROR(BIO, c->nbio ? BIO_R_NBIO_CONNECT_ERROR + : BIO_R_CONNECT_ERROR); ERR_add_error_data(4, "host=", c->param_hostname, ":", c->param_port); ret = 0; goto exit_loop; diff --git a/crypto/bio/errno.c b/crypto/bio/errno.c index 8a07b369338..8ec81dc2d39 100644 --- a/crypto/bio/errno.c +++ b/crypto/bio/errno.c @@ -9,31 +9,31 @@ int bio_errno_should_retry(int return_value) { - if (return_value != -1) { - return 0; - } + return return_value == -1 && bio_errno_is_retryable(errno); +} +int bio_errno_is_retryable(int error) { return #ifdef EWOULDBLOCK - errno == EWOULDBLOCK || + error == EWOULDBLOCK || #endif #ifdef ENOTCONN - errno == ENOTCONN || + error == ENOTCONN || #endif #ifdef EINTR - errno == EINTR || + error == EINTR || #endif #ifdef EAGAIN - errno == EAGAIN || + error == EAGAIN || #endif #ifdef EPROTO - errno == EPROTO || + error == EPROTO || #endif #ifdef EINPROGRESS - errno == EINPROGRESS || + error == EINPROGRESS || #endif #ifdef EALREADY - errno == EALREADY || + error == EALREADY || #endif 0; } diff --git a/crypto/bio/internal.h b/crypto/bio/internal.h index df758f24f87..92736e32bfb 100644 --- a/crypto/bio/internal.h +++ b/crypto/bio/internal.h @@ -56,8 +56,9 @@ void bio_clear_socket_error(int sock); // |sock|, or -1 if querying the socket error failed. int bio_sock_error_get_and_clear(int sock); -// bio_socket_set_error sets the last socket error for the current thread. -void bio_socket_set_error(int error); +// bio_socket_error_is_retryable returns non-zero if |error| is a non-fatal socket +// error. |error| is a Winsock error on Windows and an errno value elsewhere. +int bio_socket_error_is_retryable(int error); // bio_socket_should_retry returns non-zero if |return_value| indicates an error // and the last socket error indicates that it's non-fatal. @@ -88,6 +89,9 @@ union bio_addr_st { // and |errno| indicates that it's non-fatal. int bio_errno_should_retry(int return_value); +// bio_errno_is_retryable returns non-zero if |error| is a non-fatal errno value. +int bio_errno_is_retryable(int error); + #if defined(__cplusplus) } // extern C #endif diff --git a/crypto/bio/socket_helper.c b/crypto/bio/socket_helper.c index f47f06f13f5..5d0b7cb5117 100644 --- a/crypto/bio/socket_helper.c +++ b/crypto/bio/socket_helper.c @@ -114,20 +114,23 @@ int bio_sock_error_get_and_clear(int sock) { return error; } -void bio_socket_set_error(int error) { +int bio_socket_error_is_retryable(int error) { #if defined(OPENSSL_WINDOWS) - WSASetLastError(error); + return error == WSAEWOULDBLOCK; #else - errno = error; + // On POSIX platforms, sockets and fds are the same. + return bio_errno_is_retryable(error); #endif } int bio_socket_should_retry(int return_value) { + if (return_value != -1) { + return 0; + } #if defined(OPENSSL_WINDOWS) - return return_value == -1 && (WSAGetLastError() == WSAEWOULDBLOCK); + return bio_socket_error_is_retryable(WSAGetLastError()); #else - // On POSIX platforms, sockets and fds are the same. - return bio_errno_should_retry(return_value); + return bio_socket_error_is_retryable(errno); #endif } From dc6c3c593798451076db01b7ff1e6ab4f2b54203 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Wed, 23 Sep 2026 12:53:13 -0400 Subject: [PATCH 5/7] Report connect BIO errors by failure path, not NBIO mode A non-blocking connect can be refused immediately (e.g. to loopback on BSDs). Record whether the failure was immediate or deferred and report that reason on later operations, and let the test accept either path. --- crypto/bio/bio_socket_test.cc | 10 ++++++---- crypto/bio/connect.c | 9 +++++++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/crypto/bio/bio_socket_test.cc b/crypto/bio/bio_socket_test.cc index 254d6387bd5..e502e0f0b8c 100644 --- a/crypto/bio/bio_socket_test.cc +++ b/crypto/bio/bio_socket_test.cc @@ -498,8 +498,10 @@ static void TestSocketConnectFailure(bool non_blocking) { const int fd = BIO_get_fd(bio.get(), nullptr); ASSERT_NE(-1, fd); - if (non_blocking) { - ASSERT_TRUE(BIO_should_retry(bio.get())); + // Some platforms (e.g. BSDs) refuse a non-blocking loopback connect + // immediately. + const bool blocked = non_blocking && BIO_should_retry(bio.get()); + if (blocked) { ASSERT_TRUE(WaitForSocket(fd, WaitType::kConnect)) << LastSocketError(); // A successful readiness call need not clear the retryable socket error left @@ -521,8 +523,8 @@ static void TestSocketConnectFailure(bool non_blocking) { #if !defined(OPENSSL_WINDOWS) EXPECT_EQ(ECONNREFUSED, ERR_GET_REASON(error)); #endif - const int reason = - non_blocking ? BIO_R_NBIO_CONNECT_ERROR : BIO_R_CONNECT_ERROR; + // The reason depends on how the connect failed, not the NBIO setting. + const int reason = blocked ? BIO_R_NBIO_CONNECT_ERROR : BIO_R_CONNECT_ERROR; error = ERR_get_error(); EXPECT_EQ(ERR_LIB_BIO, ERR_GET_LIB(error)); EXPECT_EQ(reason, ERR_GET_REASON(error)); diff --git a/crypto/bio/connect.c b/crypto/bio/connect.c index 8b066f22088..c18175235e5 100644 --- a/crypto/bio/connect.c +++ b/crypto/bio/connect.c @@ -44,6 +44,9 @@ typedef struct bio_connect_st { char *param_port; int nbio; + // error_reason is the |BIO_R_*| reason reported in |BIO_CONN_S_ERROR|. + int error_reason; + unsigned short port; struct sockaddr_storage them; @@ -193,6 +196,7 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) { ERR_add_error_data(4, "host=", c->param_hostname, ":", c->param_port); c->state = BIO_CONN_S_ERROR; + c->error_reason = BIO_R_CONNECT_ERROR; } goto exit_loop; } else { @@ -208,6 +212,7 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) { OPENSSL_PUT_ERROR(BIO, BIO_R_NBIO_CONNECT_ERROR); ERR_add_error_data(4, "host=", c->param_hostname, ":", c->param_port); c->state = BIO_CONN_S_ERROR; + c->error_reason = BIO_R_NBIO_CONNECT_ERROR; ret = 0; goto exit_loop; } @@ -223,6 +228,7 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) { OPENSSL_PUT_ERROR(BIO, BIO_R_NBIO_CONNECT_ERROR); ERR_add_error_data(4, "host=", c->param_hostname, ":", c->param_port); c->state = BIO_CONN_S_ERROR; + c->error_reason = BIO_R_NBIO_CONNECT_ERROR; ret = 0; } goto exit_loop; @@ -235,8 +241,7 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) { // The original error was already reported. Do not start a new connection // or re-read |SO_ERROR|, which may have been cleared. |errno| is stale too. BIO_clear_retry_flags(bio); - OPENSSL_PUT_ERROR(BIO, c->nbio ? BIO_R_NBIO_CONNECT_ERROR - : BIO_R_CONNECT_ERROR); + OPENSSL_PUT_ERROR(BIO, c->error_reason); ERR_add_error_data(4, "host=", c->param_hostname, ":", c->param_port); ret = 0; goto exit_loop; From c26d00b2570f8c26b77980f7ff43f8ecfd116f91 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Thu, 24 Sep 2026 15:46:04 -0400 Subject: [PATCH 6/7] Skip connect callback test on WASM --- crypto/bio/bio_test.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crypto/bio/bio_test.cc b/crypto/bio/bio_test.cc index e24fe43da93..1a6f44ce770 100644 --- a/crypto/bio/bio_test.cc +++ b/crypto/bio/bio_test.cc @@ -1060,6 +1060,8 @@ static long callback(BIO *b, int state, int res) { TEST(BIOTest, InvokeConnectCallback) { #if defined(TARGET_OS_IPHONE) && TARGET_OS_IPHONE GTEST_SKIP() << "InvokeConnectCallback does not run on iOS"; +#elif defined(OPENSSL_WASM) + GTEST_SKIP() << "InvokeConnectCallback requires socket support"; #endif ASSERT_EQ(callback_invoked, 0); From 9bbcc93c3b461ae901b745589a52252e0df43364 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Thu, 24 Sep 2026 16:49:15 -0400 Subject: [PATCH 7/7] Restore legacy-compatible socket header order --- crypto/bio/socket_helper.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crypto/bio/socket_helper.c b/crypto/bio/socket_helper.c index 5d0b7cb5117..db25b04f294 100644 --- a/crypto/bio/socket_helper.c +++ b/crypto/bio/socket_helper.c @@ -12,10 +12,12 @@ #if !defined(OPENSSL_NO_SOCK) #include +// Older glibc versions require before so struct timespec is +// complete when includes . +#include #include #include #include -#include #if !defined(OPENSSL_WINDOWS) #include