Harden gRPC transport - #266
Conversation
Limit concurrent handshakes and apply a deadline so unauthenticated connections cannot retain sockets and tasks without bound. This commit was created with assistance from Codex.
Limit HTTP/2 streams and concurrent body collection so unauthenticated clients cannot multiply the per-request body allocation without bound. Reject requests that omit authentication before reading their bodies. This commit was created with assistance from Codex.
Omit oversized encoded error messages from response headers and trailers so attacker-controlled paths cannot cause large response allocations. This commit was created with assistance from Codex.
|
👋 Thanks for assigning @joostjager as a reviewer! |
Mark a stream terminal after an unrecoverable framing, decoding, or transport error so later reads cannot repeat the same failure forever. This commit was created with assistance from Codex.
joostjager
left a comment
joostjager
left a comment
There was a problem hiding this comment.
I raised the risk of hand-rolling previously in PR #220. This PR reinforces that. I think we should reconsider this path and consider migrating to tonic.
| runtime.spawn(async move { | ||
| match acceptor.accept(stream).await { | ||
| Ok(tls_stream) => { | ||
| let _handshake_permit = handshake_permit; |
There was a problem hiding this comment.
[P1] Release this permit once the TLS handshake completes. Because _handshake_permit remains in scope across serve_connection(...).await, it is held for the entire HTTP/2 connection. An unauthenticated peer can complete 64 TLS handshakes, keep those connections idle, and cause every subsequent connection to be rejected indefinitely.
| let shutdown_rx = self.shutdown_rx.clone(); | ||
| let (request_parts, request_body) = req.into_parts(); | ||
| let future: Self::Future = Box::pin(async move { | ||
| let body_permit = match REQUEST_BODY_SEMAPHORE.try_acquire() { |
There was a problem hiding this comment.
[P1] Add a mandatory server-side deadline for request-body collection. This global permit is held while read_request_body(...).await waits without a timeout, and only the presence of x-auth has been checked. One unauthenticated client can leave eight HTTP/2 bodies unfinished and force every legitimate RPC to fail with UNAVAILABLE.
3 participants
Various things found by project loupe. Nothing critical but all worth doing.
This PR was created with assistance from Codex and Claude Code.