A request whose body has no length of its own — Transfer-Encoding: chunked, or
HTTP/2 and HTTP/3 requests sent without the header — cannot be forwarded over
FastCGI until it has been buffered in full, because CGI/1.1 requires
CONTENT_LENGTH and php-fpm hangs when it is absent or wrong and the body is not
empty. When request_buffers is too small to hold the whole body, the length
stays unknown and the request is refused with 411.
The refusal said none of that. RoundTrip passes r.ContentLength to Post, which
is -1 for such a request, so the operator got a bare "411 Length Required" with
either no error at all or strconv's "invalid syntax" on a value they never
wrote. On #7386 that sent two people looking for the fault in their backend.
The 411 now carries the reason and the remedy, and the unknown-length case is
told apart from a genuinely malformed value. ParseUint becomes ParseInt plus an
explicit negative check, which is strictly tighter: values above MaxInt64 used
to be accepted and are unreachable anyway, since CONTENT_LENGTH is always
FormatInt of an int64 by the time Do sees it.
No status code changes. Every request that was refused before is still refused;
raising request_buffers is still what makes a large chunked body work.
Tests: the reachable path through Post, the unusable CONTENT_LENGTH values Do
itself guards against, an integration test driving a chunked body through a
fastcgi reverse_proxy over a unix socket at each side of the buffer boundary
(including request_buffers -1, which succeeds at any size and is the remedy),
and a boundary test recording that a body exactly the size of the buffer counts
as partial.
That last one is deliberately not changed here. Telling "exactly the limit"
apart from "more to come" costs a read that a paused stream may never answer,
and bufferedBody also backs response_buffers: measured against a body that
delivers exactly the limit and stops, the current code hands the buffered
prefix on immediately, while peeking one byte first delivers nothing at all.
Co-authored-by: Aditya <205600203+Rohilalala@users.noreply.github.com>
Co-authored-by: Zen Dodd <mail@steadytao.com>
* Implement HTTPoxy mitigation in FastCGI
Added HTTPoxy mitigation to prevent trusting client-supplied Proxy header for HTTP_PROXY environment variable.
* Implement test for HTTPoxy vulnerability protection
Add test to ensure HTTPoxy vulnerability is mitigated by dropping client-supplied Proxy headers.
* gofmt: format code
* revert unrelated gofmt change to replacer_test.go
Caddy's underscore header filter (GHSA-f59h-q822-g45g) only checked
for `_`. PHP folds `.` to `_` when registering $_SERVER keys the same
way CGI/FastCGI folds `-` to `_`, so a dotted alias (e.g. Remote.User)
survived the filter and collided with the legitimate hyphenated
header once it reached a PHP/FastCGI backend, bypassing forward_auth
copy_headers the same way the underscore alias did.
Extends the filter to drop `.` symmetrically, adds an
`expected_dot_headers` allowlist mirroring `expected_underscore_headers`,
and handles header names containing both separators (only an exact
allowlist entry is honored there, since a prefix glob's free-form
suffix can't be vetted for an embedded second separator).
Root cause identified by @iliaal in the FrankenPHP advisory
GHSA-49wc-4hcv-v58q.
* feat(fastcgi): populate SERVER_ADDR by default
Populate the SERVER_ADDR FastCGI environment variable using the local socket address of the incoming connection from the request context. This improves out-of-the-box compatibility for PHP applications that rely on $_SERVER['SERVER_ADDR'] (such as legacy frameworks and custom telemetry packages) when migrating from Nginx or Apache.
Co-developed-by: Gemini AI <renich+gemini@woralelandia.com>
Signed-off-by: Rénich Bon Ćirić <renich@woralelandia.com>
* fix(fastcgi): validate SERVER_ADDR IP format and add test coverage
Validate that SERVER_ADDR is only populated when the local address parses as a valid IP address, preventing Unix socket paths from being assigned if SplitHostPort fails. Add test cases covering IPv4, IPv6, missing context, Unix sockets, and explicit overrides.
Co-developed-by: Gemini AI <renich+gemini@woralelandia.com>
Signed-off-by: Rénich Bon Ćirić <renich@woralelandia.com>
* feat: drop headers with underscore in their names
* feat: Caddyfile binding and tests for underscore-in-header drop
Add the `allow_underscore_in_headers` global server option, refine the
doc comment, and cover the filter end-to-end: server-level unit tests
(drop, opt-out, debug log, RFC-7230 space rejection), a fastcgi unit
test for the trimmed header name replacer, and forward_auth integration
tests for both the default-drop and opt-out paths.
* remove allow_underscore_in_headers option for now
Both fallbacks in splitPos relied on golang.org/x/text/search with
search.IgnoreCase, which performs Unicode equivalence matching far beyond
ASCII case folding. Combined with the validated-ASCII guarantee on every
SplitPath entry, that fallback turned non-PHP filenames into PHP scripts:
- when the inner loop hit a non-ASCII byte and the IndexString fallback
returned -1, the loop broke without resetting match=false, so a stale
match=true caused a non-existent .php to be reported (PoC:
"/name.<U+00A1>.txt").
- search.IgnoreCase folded fullwidth, mathematical and circled letters
onto ASCII, so "/shell.<math sans-serif php>",
"/shell.<fullwidth p>hp", "/shell.<circled php>" were all detected as
".php" files.
Replace the fallback with strict byte-level ASCII case-insensitive
matching: any byte >= utf8.RuneSelf in the path can never be part of a
match, since SplitPath entries are validated ASCII-only and lower-cased
in Provision(). This keeps the hot path branch-light and removes the
x/text/search dependency from the main module.
Reported against FrankenPHP as GHSA-3g8v-8r37-cgjm and
GHSA-v4h7-cj44-8fc8. The vulnerable function in this module was adapted
from the same FrankenPHP code.
* use gofmput to format code
* use gci to format imports
* reconfigure gci
* linter autofixes
* rearrange imports a little
* export GOOS=windows golangci-lint run ./... --fix
Ideally I'd just remove the parameter to caddy.Context.Logger(), but
this would break most Caddy plugins.
Instead, I'm making it variadic and marking it as partially deprecated.
In the future, I might completely remove the parameter once most
plugins have updated.
* break up code and use lazy reading and pool bufio.Writer
* close underlying connection when operation failed
* allocate bufWriter and streamWriter only once
* refactor record writing
* rebase from master
* handle err
* Fix type assertion
Also reduce some duplication
* Refactor client and clientCloser for logging
Should reduce allocations
* Minor cosmetic adjustments; apply Apache license
* Appease the linter
Co-authored-by: Matthew Holt <mholt@users.noreply.github.com>
* fastcgi: Fix a TODO, prevent zap using reflection for logging env
* Update modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go
Co-authored-by: Mohammed Al Sahaf <msaa1990@gmail.com>
Co-authored-by: Mohammed Al Sahaf <msaa1990@gmail.com>
* reverseproxy: Adjust defaults, document defaults
Related to some of the issues in https://github.com/caddyserver/caddy/issues/4245, a complaint about the proxy transport defaults not being properly documented in https://caddy.community/t/default-values-for-directives/14254/6.
- Dug into the stdlib to find the actual defaults for some of the timeouts and buffer limits, documenting them in godoc so the JSON docs get them next release.
- Moved the keep-alive and dial-timeout defaults from `reverseproxy.go` to `httptransport.go`. It doesn't make sense to set defaults in the proxy, because then any time the transport is configured with non-defaults, the keep-alive and dial-timeout defaults are lost!
- Sped up the dial timeout from 10s to 3s, in practice it rarely makes sense to wait a whole 10s for dialing. A shorter timeout helps a lot with the load balancer retries, so using something lower helps with user experience.
* reverseproxy: Make keepalive interval configurable via Caddyfile
* fastcgi: DialTimeout default for fastcgi transport too
* reverseproxy: Add `handle_response` blocks to `reverse_proxy` (#3710)
* reverseproxy: complete handle_response test
* reverseproxy: Change handle_response matchers to use named matchers
reverseproxy: Add support for changing status code
* fastcgi: Remove obsolete TODO
We already have d.Err("transport already specified") in the reverse_proxy parsing code which covers this case
* reverseproxy: Fix support for "4xx" type status codes
* Apply suggestions from code review
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
* caddyhttp: Reorganize response matchers
* reverseproxy: Reintroduce caddyfile.Unmarshaler
* reverseproxy: Add comment mentioning Finalize should be called
Co-authored-by: Maxime Soulé <btik-git@scoubidou.com>
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
