fix(tls): stop a peer that closed first from killing the process

OpenSSL's socket BIO writes with write(2) rather than send(2), so unlike
PlainStream it cannot pass MSG_NOSIGNAL. Writing to a peer that is gone
therefore raised SIGPIPE, and with the default disposition that takes the
whole process down.

This is not an edge case. It fires on any teardown where the far side
closed first, because SSL_shutdown still tries to put a close_notify on
the wire — which is exactly what ShouldSendRecieveHTTPS1 does when it
drops a client whose certificate check failed. The test died on SIGPIPE
with every assertion passing.

Installing a process-wide SIG_IGN would fix it by changing how the
caller's own writes report failure, which a library has no business
doing. SIGPIPE from write(2) is delivered to the writing thread, so block
it for that thread across each OpenSSL call instead and drain any pending
instance before unblocking. A caller who already blocks SIGPIPE is left
untouched — a pending signal there may be theirs to consume.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
catbot 2026-07-28 20:18:51 +00:00
commit b7fe39871b

View file

@ -4,6 +4,8 @@
module; module;
#include <poll.h> #include <poll.h>
#include <arpa/inet.h> #include <arpa/inet.h>
#include <signal.h>
#include <pthread.h>
#include <cerrno> #include <cerrno>
#include <climits> #include <climits>
@ -23,6 +25,48 @@ import std;
using namespace Crafter; using namespace Crafter;
namespace { namespace {
// ── SIGPIPE ──────────────────────────────────────────────────────────
// OpenSSL's socket BIO writes with write(2), not send(2), so it cannot
// pass MSG_NOSIGNAL the way PlainStream does. Writing to a peer that has
// gone therefore raises SIGPIPE and — with the default disposition — kills
// the process. That is not an edge case: it happens on every teardown
// where the far side closed first, because SSL_shutdown still tries to put
// a close_notify on the wire.
//
// A library must not install a process-wide SIG_IGN on its caller's
// behalf; that would silently change how the caller's own writes behave.
// SIGPIPE from write(2) is delivered to the thread that wrote, so block it
// for this thread across the call instead, and drain any instance that
// went pending while it was blocked so it cannot fire on unblock.
class SigPipeGuard {
public:
SigPipeGuard() {
sigset_t pipeOnly;
sigemptyset(&pipeOnly);
sigaddset(&pipeOnly, SIGPIPE);
sigset_t previous;
// If the caller already blocks SIGPIPE, leave everything alone —
// any pending instance may be theirs to consume, not ours.
blocked = pthread_sigmask(SIG_BLOCK, &pipeOnly, &previous) == 0
&& sigismember(&previous, SIGPIPE) == 0;
restore = previous;
}
~SigPipeGuard() {
if (!blocked) return;
sigset_t pipeOnly;
sigemptyset(&pipeOnly);
sigaddset(&pipeOnly, SIGPIPE);
const timespec immediately{ .tv_sec = 0, .tv_nsec = 0 };
while (sigtimedwait(&pipeOnly, nullptr, &immediately) >= 0) {}
pthread_sigmask(SIG_SETMASK, &restore, nullptr);
}
SigPipeGuard(const SigPipeGuard&) = delete;
private:
sigset_t restore{};
bool blocked = false;
};
// ── OpenSSL plumbing ───────────────────────────────────────────────── // ── OpenSSL plumbing ─────────────────────────────────────────────────
template <typename T, void (*Release)(T*)> template <typename T, void (*Release)(T*)>
struct Releaser { struct Releaser {
@ -424,6 +468,7 @@ struct TLSStream::Impl {
// handshake, not each poll, so a peer that dribbles records cannot // handshake, not each poll, so a peer that dribbles records cannot
// stretch it indefinitely. // stretch it indefinitely.
void Handshake(bool client, std::chrono::milliseconds timeout) { void Handshake(bool client, std::chrono::milliseconds timeout) {
const SigPipeGuard noSigPipe;
const auto deadline = std::chrono::steady_clock::now() + timeout; const auto deadline = std::chrono::steady_clock::now() + timeout;
for (;;) { for (;;) {
ERR_clear_error(); ERR_clear_error();
@ -546,6 +591,9 @@ StreamStatus TLSStream::ReadSome(char* buffer, std::size_t size,
std::size_t& read) { std::size_t& read) {
read = 0; read = 0;
if (size == 0) return StreamStatus::Data; if (size == 0) return StreamStatus::Data;
// A read can put bytes on the wire too — a TLS 1.3 key update, or an alert
// in response to something we refuse.
const SigPipeGuard noSigPipe;
const auto deadline = std::chrono::steady_clock::now() + timeout; const auto deadline = std::chrono::steady_clock::now() + timeout;
const int wanted = static_cast<int>(std::min<std::size_t>(size, INT_MAX)); const int wanted = static_cast<int>(std::min<std::size_t>(size, INT_MAX));
for (;;) { for (;;) {
@ -586,6 +634,7 @@ StreamStatus TLSStream::ReadSome(char* buffer, std::size_t size,
void TLSStream::Write(const void* buffer, std::size_t size, void TLSStream::Write(const void* buffer, std::size_t size,
std::chrono::milliseconds timeout) { std::chrono::milliseconds timeout) {
const SigPipeGuard noSigPipe;
const auto deadline = std::chrono::steady_clock::now() + timeout; const auto deadline = std::chrono::steady_clock::now() + timeout;
const char* data = reinterpret_cast<const char*>(buffer); const char* data = reinterpret_cast<const char*>(buffer);
std::size_t sent = 0; std::size_t sent = 0;
@ -625,6 +674,9 @@ void TLSStream::Write(const void* buffer, std::size_t size,
void TLSStream::Shutdown() noexcept { void TLSStream::Shutdown() noexcept {
if (!impl || impl->ssl == nullptr || impl->shutdownSent) return; if (!impl || impl->ssl == nullptr || impl->shutdownSent) return;
impl->shutdownSent = true; impl->shutdownSent = true;
// The likeliest SIGPIPE of the lot: by the time a connection is being torn
// down the peer has often gone already.
const SigPipeGuard noSigPipe;
// One attempt only: close_notify goes out, and we deliberately do not // One attempt only: close_notify goes out, and we deliberately do not
// wait for the peer's. Waiting means blocking a teardown path on a peer // wait for the peer's. Waiting means blocking a teardown path on a peer
// that may never answer, and every framing decision has already been made // that may never answer, and every framing decision has already been made