fix(api): bound how long a client may take to send request headers - #29500
Draft
nishantbkl3345-ship-it wants to merge 1 commit into
Draft
Conversation
The API service's http.Server sets only IdleTimeout, which covers the
wait between requests on a keep-alive connection. Nothing bounds the read
of the request headers themselves, so a client that connects and sends a
partial header block - "GET /_ping HTTP/1.1\r\nHost: x\r\n" with no
terminating CRLF - is never disconnected.
Each connection held this way pins a file descriptor and a goroutine in
the service for as long as it runs, and nothing limits how many of them a
client may open; against a server configured exactly as this one is, 100
out of 100 half-open connections were still held after the API idle
window had passed. With "podman system service --time=0" they are held
for the lifetime of the process, until it runs out of descriptors and
stops serving legitimate clients. This is reachable by anyone who can
open a connection to the service, which matters most when it is bound to
TCP ("podman system service tcp://...") rather than to a unix socket.
Set ReadHeaderTimeout to 30 seconds. ReadTimeout and WriteTimeout stay
unset on purpose: they cover the whole request and would break attach,
exec, events, logs and image push/pull, which stream for as long as the
client wants them to. ReadHeaderTimeout does not have that problem,
because net/http clears the read deadline once the headers have been read
and again when a handler hijacks the connection. It bounds HTTP/1.x
header reads.
Move the http.Server construction into newHTTPServer() so this can be
tested without a libpod runtime. The tests cover a connection stalled
mid-header (now closed), the same server with no ReadHeaderTimeout (still
held - the regression this guards against), and a hijacked connection
that streams well after the header timeout has elapsed (unaffected).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: nishantbkl3345-ship-it <nishantbkl3345-ship-it@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The API service's http.Server sets only IdleTimeout, which covers the wait between requests on a keep-alive connection. Nothing bounds the read of the request headers themselves, so a client that connects and sends a partial header block - "GET /_ping HTTP/1.1\r\nHost: x\r\n" with no terminating CRLF - is never disconnected.
Each connection held this way pins a file descriptor and a goroutine in the service for as long as it runs, and nothing limits how many of them a client may open; against a server configured exactly as this one is, 100 out of 100 half-open connections were still held after the API idle window had passed. With "podman system service --time=0" they are held for the lifetime of the process, until it runs out of descriptors and stops serving legitimate clients. This is reachable by anyone who can open a connection to the service, which matters most when it is bound to TCP ("podman system service tcp://...") rather than to a unix socket.
Set ReadHeaderTimeout to 30 seconds. ReadTimeout and WriteTimeout stay unset on purpose: they cover the whole request and would break attach, exec, events, logs and image push/pull, which stream for as long as the client wants them to. ReadHeaderTimeout does not have that problem, because net/http clears the read deadline once the headers have been read and again when a handler hijacks the connection. It bounds HTTP/1.x header reads.
Move the http.Server construction into newHTTPServer() so this can be tested without a libpod runtime. The tests cover a connection stalled mid-header (now closed), the same server with no ReadHeaderTimeout (still held - the regression this guards against), and a hijacked connection that streams well after the header timeout has elapsed (unaffected).
Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?