Skip to content

Commit 88fe725

Browse files
committed
fix(http): stop HTTPS connection-permit leak that silently killed TLS
The HTTPS accept loop bounds concurrency with a 1024-permit semaphore and held each permit for the whole connection, but the only timeout was a 30s cap on the TLS handshake -- nothing bounded an idle keep-alive or slow-loris connection once established. Scanner and abandoned connections accumulated until all 1024 permits were held, after which try_acquire_owned() failed for every new connection and the loop silently `continue`d, refusing it before the handshake. The result was a total HTTPS outage -- connection resets, "no peer certificate available" -- while the process, HTTP:80 and DNS all stayed healthy and the container reported healthy, so nothing surfaced the failure. - Add header_read_timeout so an idle or slow connection releases its permit instead of pinning it; this is what stops the pool draining. - Release the permit explicitly when serving completes. For a WebSocket, serve_connection resolves at the with_upgrades() handoff and the socket continues in its own task, so an established WebSocket no longer holds one of the bounded HTTPS slots for its lifetime. - Replace the silent drop on pool exhaustion with a rate-limited warning (at most one line per 5s) so this condition can never again be invisible.
1 parent 91edc03 commit 88fe725

1 file changed

Lines changed: 53 additions & 6 deletions

File tree

src/src/http/mod.rs

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use std::borrow::Cow;
2626
use std::net::SocketAddr;
2727
use std::sync::Arc;
2828
use std::task::{Context, Poll};
29-
use std::time::Duration;
29+
use std::time::{Duration, Instant};
3030
use tokio::net::TcpListener;
3131
use tokio::sync::{broadcast, Semaphore};
3232
use tower::{Layer, Service};
@@ -40,6 +40,16 @@ use crate::utils::config::CONFIG;
4040

4141
const HTTPS_MAX_CONCURRENT_CONNECTIONS: usize = 1024;
4242
const TLS_HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(30);
43+
/// Maximum time a connection may spend waiting for request headers, applied to the first
44+
/// request and again between keep-alive requests. Without it an idle keep-alive or
45+
/// slow-loris connection pins its concurrency permit indefinitely; enough of them drain
46+
/// HTTPS_MAX_CONCURRENT_CONNECTIONS, after which every new connection is refused before the
47+
/// TLS handshake. That presents as a total HTTPS outage (resets, "no peer certificate")
48+
/// even though the process, HTTP, and DNS all stay healthy.
49+
const HTTP_HEADER_READ_TIMEOUT: Duration = Duration::from_secs(30);
50+
/// Minimum spacing between "connection pool exhausted" log lines, so a sustained flood
51+
/// cannot itself become a log flood while never letting the condition go silent.
52+
const PERMIT_EXHAUSTION_LOG_INTERVAL: Duration = Duration::from_secs(5);
4353

4454
fn sanitized_sentry_path(path: &str) -> Cow<'_, str> {
4555
const SHARED_REQUEST_PREFIX: &str = "/api/v2/requests/shared/";
@@ -264,6 +274,11 @@ impl HttpsServer {
264274
let listener = TcpListener::bind(addr).await?;
265275
let connection_limit = Arc::new(Semaphore::new(HTTPS_MAX_CONCURRENT_CONNECTIONS));
266276

277+
// Rate-limited reporting of pool-exhaustion drops. Both are only touched from this
278+
// single accept loop, so plain locals suffice — no synchronization needed.
279+
let mut permit_exhausted_drops: u64 = 0;
280+
let mut last_exhaustion_log: Option<Instant> = None;
281+
267282
loop {
268283
let (stream, remote_addr) = match listener.accept().await {
269284
Ok(conn) => conn,
@@ -282,11 +297,31 @@ impl HttpsServer {
282297
}
283298
};
284299

285-
// Refuse excess connections before allocating a TLS task. The permit is
286-
// held for the entire HTTP connection, including keep-alive time.
300+
// Refuse excess connections before allocating a TLS task. The permit is held
301+
// through the TLS handshake and the HTTP-serving phase, then released (see the
302+
// explicit drop after serve_connection). header_read_timeout caps idle
303+
// keep-alive time so a permit can't be pinned indefinitely.
287304
let connection_permit = match connection_limit.clone().try_acquire_owned() {
288305
Ok(permit) => permit,
289-
Err(_) => continue,
306+
Err(_) => {
307+
// Pool exhausted — refuse this connection. Log at most once per
308+
// PERMIT_EXHAUSTION_LOG_INTERVAL so the drop is never silent (an
309+
// exhausted pool means HTTPS is effectively down) without letting a
310+
// flood of refusals turn into a flood of log lines.
311+
permit_exhausted_drops += 1;
312+
let due = last_exhaustion_log
313+
.map(|t| t.elapsed() >= PERMIT_EXHAUSTION_LOG_INTERVAL)
314+
.unwrap_or(true);
315+
if due {
316+
warn!(
317+
"HTTPS connection pool exhausted ({} slots): refused {} connection(s) since last report",
318+
HTTPS_MAX_CONCURRENT_CONNECTIONS, permit_exhausted_drops
319+
);
320+
permit_exhausted_drops = 0;
321+
last_exhaustion_log = Some(Instant::now());
322+
}
323+
continue;
324+
}
290325
};
291326

292327
// Clone state for the spawned task
@@ -298,7 +333,8 @@ impl HttpsServer {
298333
};
299334

300335
tokio::spawn(async move {
301-
let _connection_permit = connection_permit;
336+
// `connection_permit` (moved in above) is held for the HTTP-serving phase
337+
// and released explicitly once serving/upgrade completes, below.
302338

303339
// Perform TLS handshake
304340
let tls_stream = match tokio::time::timeout(
@@ -324,8 +360,11 @@ impl HttpsServer {
324360

325361
let io = TokioIo::new(tls_stream);
326362

327-
// Use http1 builder with upgrades enabled for WebSocket support
363+
// http1 with upgrades enabled for WebSocket support. header_read_timeout
364+
// bounds how long an idle or slow-loris connection can hold its permit
365+
// between requests, which is what stops the pool from leaking to zero.
328366
if let Err(e) = hyper::server::conn::http1::Builder::new()
367+
.header_read_timeout(HTTP_HEADER_READ_TIMEOUT)
329368
.serve_connection(io, service)
330369
.with_upgrades()
331370
.await
@@ -335,6 +374,14 @@ impl HttpsServer {
335374
error!("Error serving HTTPS connection: {}", e);
336375
}
337376
}
377+
378+
// Free the connection slot the instant the HTTP phase ends. For a WebSocket
379+
// this future resolves at the `with_upgrades()` handoff, after which the
380+
// socket lives in its own task (see websocket handler) — so an established
381+
// WebSocket does not occupy one of the HTTPS_MAX_CONCURRENT_CONNECTIONS
382+
// slots for its lifetime. Dropping explicitly keeps that guarantee from
383+
// depending on where the enclosing scope happens to end.
384+
drop(connection_permit);
338385
});
339386
}
340387
}

0 commit comments

Comments
 (0)