Skip to content

Commit 2eda5d4

Browse files
MagicalTuxclaude
andcommitted
Harden the thread-pool runtime against connection-exhaustion DoS
Build on the EMFILE backoff with the structural fixes that keep a public listener solid under load and abuse: - Bound the connection queue: replace the unbounded mpsc channel with a sync_channel sized to workers*64 (clamped 256..4096). Each queued stream holds an fd, so an unbounded queue let a burst exhaust the descriptor limit; the cap is a generous safety ceiling that stays clear of normal and ACME-issuance concurrency, with the kernel backlog shedding overflow. - Add read/write socket timeouts (30s) to every accepted stream. A peer that connects and stalls (slowloris, dead keep-alive, a client that stops reading) now releases its worker instead of pinning it forever. TLS rides the same raw stream, so this covers the handshake too. - Cap the plain-HTTP redirect listener at 256 concurrent connections so a flood can't spawn unbounded threads; excess is shed by closing. - Wrap per-connection handling in catch_unwind so one panicking request can't kill a worker and slowly drain the pool. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 4a3174f commit 2eda5d4

2 files changed

Lines changed: 62 additions & 14 deletions

File tree

src/rt/common.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//! Pieces shared across the blocking runtimes.
22
33
use std::io::{self, Read, Write};
4+
use std::net::TcpStream;
45
use std::sync::Mutex;
56
use std::time::{Duration, Instant};
67

@@ -10,6 +11,20 @@ use crate::session::Session;
1011
/// Read buffer size used by the blocking drive loop.
1112
pub(crate) const READ_BUF: usize = 16 * 1024;
1213

14+
/// Per-operation inactivity timeout for blocking connections. A read or write
15+
/// that makes no progress within this window fails, so a peer that connects and
16+
/// then stalls (slowloris, dead keep-alive, a client that stops reading our
17+
/// response) releases its worker thread instead of pinning it forever.
18+
pub(crate) const IO_TIMEOUT: Duration = Duration::from_secs(30);
19+
20+
/// Apply [`IO_TIMEOUT`] to a freshly accepted stream. Errors are ignored: a
21+
/// socket that won't take a timeout will still be bounded by the read/write
22+
/// loop, and failing the connection here would be worse than serving it.
23+
pub(crate) fn apply_timeouts(stream: &TcpStream) {
24+
let _ = stream.set_read_timeout(Some(IO_TIMEOUT));
25+
let _ = stream.set_write_timeout(Some(IO_TIMEOUT));
26+
}
27+
1328
/// Backoff applied after a descriptor-exhaustion `accept()` error.
1429
pub(crate) const ACCEPT_BACKOFF: Duration = Duration::from_millis(50);
1530

src/rt/threadpool.rs

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@
66
//! served concurrently. There is no async runtime involved.
77
88
use std::net::{TcpListener, TcpStream};
9-
use std::sync::mpsc::{Receiver, Sender};
9+
use std::sync::atomic::{AtomicUsize, Ordering};
10+
use std::sync::mpsc::{Receiver, SyncSender};
1011
use std::sync::{Arc, Mutex};
1112
use std::thread;
1213

1314
use crate::error::Result;
1415
use crate::rt::TlsMode;
15-
use crate::rt::common::serve_blocking;
16+
use crate::rt::common::{self, serve_blocking};
1617
use crate::rt::redirect::{self, HttpCtx};
1718
use crate::session::{Session, SessionConfig};
1819

@@ -34,7 +35,17 @@ pub(crate) fn run(
3435
) -> Result<()> {
3536
let shared = Arc::new(Shared { cfg, tls });
3637
let workers = workers.max(1);
37-
let (tx, rx): (Sender<TcpStream>, Receiver<TcpStream>) = std::sync::mpsc::channel();
38+
// Bound the queue of accepted-but-unserved connections. Each queued stream
39+
// holds an open descriptor, so an unbounded queue lets a connection burst
40+
// exhaust the fd limit (which is exactly what melted the server once). The
41+
// cap is generous — a safety ceiling, not tight backpressure — so it stays
42+
// well clear of normal concurrency, including the ACME path where one worker
43+
// blocks on issuance while another answers the validation connection. When
44+
// the queue is full `send` blocks the accept loop; further connections wait
45+
// in the kernel backlog and are shed there if it too fills.
46+
let backlog = workers.saturating_mul(64).clamp(256, 4096);
47+
let (tx, rx): (SyncSender<TcpStream>, Receiver<TcpStream>) =
48+
std::sync::mpsc::sync_channel(backlog);
3849
let rx = Arc::new(Mutex::new(rx));
3950

4051
for _ in 0..workers {
@@ -51,8 +62,8 @@ pub(crate) fn run(
5162
}
5263
}
5364
Err(e) => {
54-
if crate::rt::common::note_accept_error("accept error", &e) {
55-
thread::sleep(crate::rt::common::ACCEPT_BACKOFF);
65+
if common::note_accept_error("accept error", &e) {
66+
thread::sleep(common::ACCEPT_BACKOFF);
5667
}
5768
}
5869
}
@@ -72,16 +83,25 @@ fn worker_loop(rx: Arc<Mutex<Receiver<TcpStream>>>, shared: Arc<Shared>) {
7283
let Ok(stream) = stream else {
7384
return;
7485
};
75-
if let Err(e) = handle(stream, &shared)
76-
&& cfg!(debug_assertions)
77-
{
78-
eprintln!("httpsd: connection ended: {e}");
86+
// Isolate each connection: a panic while serving one client must not
87+
// kill the worker thread, which would permanently shrink the pool and,
88+
// once every worker died, wedge the whole server.
89+
let result =
90+
std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| handle(stream, &shared)));
91+
match result {
92+
Ok(Ok(())) => {}
93+
Ok(Err(e)) if cfg!(debug_assertions) => {
94+
eprintln!("httpsd: connection ended: {e}");
95+
}
96+
Ok(Err(_)) => {}
97+
Err(_) => eprintln!("httpsd: worker recovered from a panic while serving a connection"),
7998
}
8099
}
81100
}
82101

83102
fn handle(mut stream: TcpStream, shared: &Shared) -> Result<()> {
84103
stream.set_nodelay(true).ok();
104+
common::apply_timeouts(&stream);
85105

86106
match &shared.tls {
87107
TlsMode::Plain => {
@@ -123,26 +143,39 @@ fn handle_acme(
123143
crate::rt::common::serve_blocking_prefed(&mut stream, &mut session, &initial)
124144
}
125145

126-
/// Accept loop for the plain-HTTP listener (redirects + ACME HTTP-01). One
127-
/// thread per connection — this listener carries little traffic.
146+
/// Accept loop for the plain-HTTP listener (redirects + ACME HTTP-01). It spawns
147+
/// a thread per connection — these are short-lived (read a request, reply, close)
148+
/// — but caps how many run at once so a flood can't spawn unbounded threads.
128149
pub(crate) fn run_http_redirect(listener: TcpListener, ctx: HttpCtx) {
150+
/// Maximum redirect connections served concurrently; excess is shed.
151+
const MAX_INFLIGHT: usize = 256;
152+
129153
let ctx = Arc::new(ctx);
154+
let inflight = Arc::new(AtomicUsize::new(0));
130155
for incoming in listener.incoming() {
131156
match incoming {
132157
Ok(mut stream) => {
158+
common::apply_timeouts(&stream);
159+
stream.set_nodelay(true).ok();
160+
// Shed load past the cap by closing the connection (drop).
161+
if inflight.fetch_add(1, Ordering::Relaxed) >= MAX_INFLIGHT {
162+
inflight.fetch_sub(1, Ordering::Relaxed);
163+
continue;
164+
}
133165
let ctx = Arc::clone(&ctx);
166+
let inflight = Arc::clone(&inflight);
134167
thread::spawn(move || {
135-
stream.set_nodelay(true).ok();
136168
if let Err(e) = redirect::serve(&mut stream, &ctx)
137169
&& cfg!(debug_assertions)
138170
{
139171
eprintln!("httpsd: http connection ended: {e}");
140172
}
173+
inflight.fetch_sub(1, Ordering::Relaxed);
141174
});
142175
}
143176
Err(e) => {
144-
if crate::rt::common::note_accept_error("http accept error", &e) {
145-
thread::sleep(crate::rt::common::ACCEPT_BACKOFF);
177+
if common::note_accept_error("http accept error", &e) {
178+
thread::sleep(common::ACCEPT_BACKOFF);
146179
}
147180
}
148181
}

0 commit comments

Comments
 (0)