Merge commit from fork
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.pull/7852/merge
Kévin Dunglas
GitHub
parent
63f4e387c3
commit
947087cadd
|
|
@ -49,6 +49,7 @@ type serverOptions struct {
|
|||
MaxHeaderBytes int
|
||||
EnableFullDuplex bool
|
||||
ExpectedUnderscoreHeaders []string
|
||||
ExpectedDotHeaders []string
|
||||
Protocols []string
|
||||
StrictSNIHost *bool
|
||||
TrustedProxiesRaw json.RawMessage
|
||||
|
|
@ -226,6 +227,13 @@ func unmarshalCaddyfileServerOptions(d *caddyfile.Dispenser) (any, error) {
|
|||
}
|
||||
serverOpts.ExpectedUnderscoreHeaders = args
|
||||
|
||||
case "expected_dot_headers":
|
||||
args := d.RemainingArgs()
|
||||
if len(args) == 0 {
|
||||
return nil, d.ArgErr()
|
||||
}
|
||||
serverOpts.ExpectedDotHeaders = args
|
||||
|
||||
case "log_credentials":
|
||||
if d.NextArg() {
|
||||
return nil, d.ArgErr()
|
||||
|
|
@ -389,6 +397,7 @@ func applyServerOptions(
|
|||
server.MaxHeaderBytes = opts.MaxHeaderBytes
|
||||
server.EnableFullDuplex = opts.EnableFullDuplex
|
||||
server.ExpectedUnderscoreHeaders = opts.ExpectedUnderscoreHeaders
|
||||
server.ExpectedDotHeaders = opts.ExpectedDotHeaders
|
||||
server.Protocols = opts.Protocols
|
||||
server.StrictSNIHost = opts.StrictSNIHost
|
||||
server.TrustedProxiesRaw = opts.TrustedProxiesRaw
|
||||
|
|
|
|||
|
|
@ -207,12 +207,12 @@ func TestForwardAuthCopyHeadersAuthResponseWins(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// TestForwardAuthCopyHeadersUnderscoreAlias guards GHSA-f59h-q822-g45g:
|
||||
// a client-supplied `Remote_user` alias of the copy_headers target
|
||||
// `Remote-User` must be stripped before the auth route runs, otherwise
|
||||
// a downstream CGI/FastCGI backend would fold both names into the same
|
||||
// HTTP_REMOTE_USER variable and the attacker would override the trusted
|
||||
// identity.
|
||||
// TestForwardAuthCopyHeadersUnderscoreAlias guards GHSA-f59h-q822-g45g and
|
||||
// GHSA-49wc-4hcv-v58q: client-supplied `Remote_user`/`Remote.user` aliases
|
||||
// of the copy_headers target `Remote-User` must be stripped before the
|
||||
// auth route runs, otherwise a downstream CGI/FastCGI/PHP backend would
|
||||
// fold all three names into the same HTTP_REMOTE_USER variable and the
|
||||
// attacker would override the trusted identity.
|
||||
func TestForwardAuthCopyHeadersUnderscoreAlias(t *testing.T) {
|
||||
const wantRemoteUser = "alice"
|
||||
|
||||
|
|
@ -223,7 +223,7 @@ func TestForwardAuthCopyHeadersUnderscoreAlias(t *testing.T) {
|
|||
t.Cleanup(authSrv.Close)
|
||||
|
||||
type received struct {
|
||||
remoteUserHyphen, remoteUserUnderscore string
|
||||
remoteUserHyphen, remoteUserUnderscore, remoteUserDot string
|
||||
}
|
||||
var (
|
||||
mu sync.Mutex
|
||||
|
|
@ -234,6 +234,7 @@ func TestForwardAuthCopyHeadersUnderscoreAlias(t *testing.T) {
|
|||
last = received{
|
||||
remoteUserHyphen: r.Header.Get("Remote-User"),
|
||||
remoteUserUnderscore: strings.Join(r.Header["Remote_user"], ","),
|
||||
remoteUserDot: strings.Join(r.Header["Remote.user"], ","),
|
||||
}
|
||||
mu.Unlock()
|
||||
fmt.Fprint(w, "ok")
|
||||
|
|
@ -259,13 +260,15 @@ func TestForwardAuthCopyHeadersUnderscoreAlias(t *testing.T) {
|
|||
`, strings.TrimPrefix(authSrv.URL, "http://"), strings.TrimPrefix(backendSrv.URL, "http://")), "caddyfile")
|
||||
|
||||
req, _ := http.NewRequest(http.MethodGet, "http://localhost:9080/", nil)
|
||||
// Set the underscore alias via raw map access to bypass http.Header
|
||||
// canonicalization, as an attacker would on the wire.
|
||||
// Set the underscore and dot aliases via raw map access to bypass
|
||||
// http.Header canonicalization, as an attacker would on the wire.
|
||||
req.Header["Remote_user"] = []string{"attacker"}
|
||||
req.Header["Remote.user"] = []string{"attacker"}
|
||||
tester.AssertResponse(req, http.StatusOK, "ok")
|
||||
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
assert.Equal(t, wantRemoteUser, last.remoteUserHyphen, "trusted Remote-User must reach the backend")
|
||||
assert.Empty(t, last.remoteUserUnderscore, "underscore alias must be dropped")
|
||||
assert.Empty(t, last.remoteUserDot, "dot alias must be dropped")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -49,9 +49,9 @@ func init() {
|
|||
// defined by adding a good, default TLS connection policy.
|
||||
//
|
||||
// Similar to how other popular web servers work, incoming request header fields
|
||||
// with underscores are ignored/dropped implicitly to mitigate security risks.
|
||||
// Specific headers to allow can be explicitly configured using
|
||||
// `expected_underscore_headers`.
|
||||
// with underscores or dots are ignored/dropped implicitly to mitigate security
|
||||
// risks. Specific headers to allow can be explicitly configured using
|
||||
// `expected_underscore_headers` and `expected_dot_headers`.
|
||||
//
|
||||
// ### Placeholders
|
||||
//
|
||||
|
|
@ -302,10 +302,13 @@ func (app *App) Provision(ctx caddy.Context) error {
|
|||
srv.ClientIPHeaders = []string{"X-Forwarded-For"}
|
||||
}
|
||||
|
||||
// precompute underscore header allowlist rules
|
||||
// precompute underscore and dot header allowlist rules
|
||||
if err := srv.provisionUnderscoreHeaders(); err != nil {
|
||||
return fmt.Errorf("server %s: %v", srvName, err)
|
||||
}
|
||||
if err := srv.provisionDotHeaders(); err != nil {
|
||||
return fmt.Errorf("server %s: %v", srvName, err)
|
||||
}
|
||||
|
||||
// process each listener address
|
||||
for i := range srv.Listen {
|
||||
|
|
|
|||
|
|
@ -310,10 +310,11 @@ func TestSplitPosUnicodeSecurityRegression(t *testing.T) {
|
|||
|
||||
// TestHeaderNameReplacer asserts the CGI header-to-env normalization rule:
|
||||
// hyphens are mapped to underscores while every other character (including
|
||||
// spaces) is passed through. Spaces are not RFC 7230 tokens, so they cannot
|
||||
// reach this function from the wire; the only header names that survive
|
||||
// untouched at the server layer are sanitized by the underscore filter in
|
||||
// caddyhttp.Server.serveHTTP (see GHSA-f59h-q822-g45g).
|
||||
// spaces and dots) is passed through. Spaces are not RFC 7230 tokens, so they
|
||||
// cannot reach this function from the wire; the only header names that
|
||||
// survive untouched at the server layer are sanitized by the underscore/dot
|
||||
// filter in caddyhttp.Server.serveHTTP (see GHSA-f59h-q822-g45g,
|
||||
// GHSA-49wc-4hcv-v58q).
|
||||
func TestHeaderNameReplacer(t *testing.T) {
|
||||
tests := []struct {
|
||||
in, want string
|
||||
|
|
@ -323,6 +324,9 @@ func TestHeaderNameReplacer(t *testing.T) {
|
|||
// Underscores are preserved (the server has already dropped any
|
||||
// underscore-named headers when the filter is on).
|
||||
{"Remote_User", "Remote_User"},
|
||||
// Dots are preserved by this replacer too; the server-layer filter is
|
||||
// what prevents a dotted alias (e.g. Remote.User) from reaching here.
|
||||
{"Remote.User", "Remote.User"},
|
||||
// Spaces are not rewritten because Go's HTTP parser rejects whitespace in
|
||||
// header field names.
|
||||
{"Foo Bar", "Foo Bar"},
|
||||
|
|
|
|||
|
|
@ -137,9 +137,52 @@ type Server struct {
|
|||
// multiple values (repeated field), all values are dropped
|
||||
// as a safeguard against header injection.
|
||||
//
|
||||
// If the same logical header name is also allowlisted (in dot form)
|
||||
// in ExpectedDotHeaders, both spellings are kept independently. Only
|
||||
// do this if you know your backend doesn't fold both forms onto the
|
||||
// same variable name; PHP/CGI-style backends do (see
|
||||
// ExpectedDotHeaders), so allowlisting both there reintroduces the
|
||||
// ambiguity this filter exists to remove.
|
||||
//
|
||||
// A header name containing both an underscore and a dot (e.g.
|
||||
// "webhook_user.id") is never matched by a prefix glob here, even
|
||||
// one whose prefix matches, because the free-form suffix a glob
|
||||
// allows can't be vetted for an embedded dot; only an exact (non-glob)
|
||||
// entry for that literal spelling is honored.
|
||||
//
|
||||
// TODO: This is an EXPERIMENTAL feature. Subject to change or removal.
|
||||
ExpectedUnderscoreHeaders []string `json:"expected_underscore_headers,omitempty"`
|
||||
|
||||
// A list of header field names containing dots that should be
|
||||
// preserved instead of being dropped. By default, Caddy drops ALL
|
||||
// headers with dots to prevent ambiguity with backends that fold
|
||||
// dots to underscores when registering CGI-style variables, e.g.
|
||||
// PHP's $_SERVER (GHSA-49wc-4hcv-v58q). When this list is
|
||||
// configured, only the specified headers are kept; their
|
||||
// hyphenated variants are actively dropped to prevent confusion.
|
||||
// Entries are case-insensitive. A trailing "*" acts as a prefix
|
||||
// glob (e.g., "webhook.*" matches any header starting with
|
||||
// "webhook."). If an allowlisted header arrives with multiple
|
||||
// values (repeated field), all values are dropped as a safeguard
|
||||
// against header injection.
|
||||
//
|
||||
// Dotted headers are legal HTTP tokens and some non-CGI backends
|
||||
// (e.g. Node.js, Go) use them as ordinary, unrelated header names.
|
||||
// If your backend is one of those, this poses no risk. Only
|
||||
// PHP/CGI/FastCGI-style backends fold '.', '_', and '-' onto the
|
||||
// same variable name; avoid allowlisting both the dot and
|
||||
// underscore form of the same logical name if such a backend is in
|
||||
// the request path, since Caddy will not stop you and the backend
|
||||
// may then see either value depending on map iteration order.
|
||||
//
|
||||
// A header name containing both a dot and an underscore is never
|
||||
// matched by a prefix glob here, for the same reason described on
|
||||
// ExpectedUnderscoreHeaders; only an exact (non-glob) entry for
|
||||
// that literal spelling is honored.
|
||||
//
|
||||
// TODO: This is an EXPERIMENTAL feature. Subject to change or removal.
|
||||
ExpectedDotHeaders []string `json:"expected_dot_headers,omitempty"`
|
||||
|
||||
// Routes describes how this server will handle requests.
|
||||
// Routes are executed sequentially. First a route's matchers
|
||||
// are evaluated, then its grouping. If it matches and has
|
||||
|
|
@ -312,7 +355,12 @@ type Server struct {
|
|||
// precomputed underscore header allowlist (built during provisioning)
|
||||
underscoreExactAllow map[string]struct{}
|
||||
underscoreExactDrop map[string]struct{}
|
||||
underscorePrefixRules []underscoreRule
|
||||
underscorePrefixRules []aliasPrefixRule
|
||||
|
||||
// precomputed dot header allowlist (built during provisioning)
|
||||
dotExactAllow map[string]struct{}
|
||||
dotExactDrop map[string]struct{}
|
||||
dotPrefixRules []aliasPrefixRule
|
||||
|
||||
// registered callback functions
|
||||
connStateFuncs []func(net.Conn, http.ConnState)
|
||||
|
|
@ -321,30 +369,29 @@ type Server struct {
|
|||
onStopFuncs []func(context.Context) error // TODO: Experimental (Nov. 2023)
|
||||
}
|
||||
|
||||
// underscoreRule pairs a canonical underscore prefix with its hyphenated
|
||||
// counterpart. Used for prefix-glob matching in the allowlist.
|
||||
type underscoreRule struct {
|
||||
allow string // canonical underscore form, e.g. "Webhook_"
|
||||
// aliasPrefixRule pairs a canonical allowed prefix (underscore- or
|
||||
// dot-named) with its hyphenated counterpart. Used for prefix-glob
|
||||
// matching in the underscore/dot allowlists.
|
||||
type aliasPrefixRule struct {
|
||||
allow string // canonical allowed form, e.g. "Webhook_" or "Webhook."
|
||||
drop string // canonical hyphenated form, e.g. "Webhook-"
|
||||
}
|
||||
|
||||
// provisionUnderscoreHeaders validates the ExpectedUnderscoreHeaders
|
||||
// entries and builds the precomputed maps and prefix rules used by
|
||||
// the hot-path filter in serveHTTP.
|
||||
func (s *Server) provisionUnderscoreHeaders() error {
|
||||
if len(s.ExpectedUnderscoreHeaders) == 0 {
|
||||
return nil
|
||||
}
|
||||
// provisionHeaderAliasAllowlist validates entries for a header-alias
|
||||
// allowlist (ExpectedUnderscoreHeaders or ExpectedDotHeaders) and builds
|
||||
// the precomputed maps and prefix rules used by the hot-path filter in
|
||||
// serveHTTP. sep is the separator the entries must contain ('_' or '.');
|
||||
// directive is the Caddyfile/JSON name used in error messages.
|
||||
func provisionHeaderAliasAllowlist(entries []string, sep rune, directive string) (exactAllow, exactDrop map[string]struct{}, prefixRules []aliasPrefixRule, err error) {
|
||||
exactAllow = make(map[string]struct{}, len(entries))
|
||||
exactDrop = make(map[string]struct{}, len(entries))
|
||||
|
||||
s.underscoreExactAllow = make(map[string]struct{}, len(s.ExpectedUnderscoreHeaders))
|
||||
s.underscoreExactDrop = make(map[string]struct{}, len(s.ExpectedUnderscoreHeaders))
|
||||
|
||||
for _, entry := range s.ExpectedUnderscoreHeaders {
|
||||
for _, entry := range entries {
|
||||
// Reject non-ASCII bytes: Go's HTTP parser returns 400 for
|
||||
// non-ASCII header names, so such entries can never match.
|
||||
for i := 0; i < len(entry); i++ {
|
||||
if entry[i] >= 0x80 {
|
||||
return fmt.Errorf("expected_underscore_headers: entry %q contains non-ASCII characters", entry)
|
||||
return nil, nil, nil, fmt.Errorf("%s: entry %q contains non-ASCII characters", directive, entry)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -356,29 +403,55 @@ func (s *Server) provisionUnderscoreHeaders() error {
|
|||
|
||||
// Reject entries with '*' not at the trailing position.
|
||||
if strings.ContainsRune(name, '*') {
|
||||
return fmt.Errorf("expected_underscore_headers: entry %q has '*' in an invalid position (only a trailing '*' is allowed)", entry)
|
||||
return nil, nil, nil, fmt.Errorf("%s: entry %q has '*' in an invalid position (only a trailing '*' is allowed)", directive, entry)
|
||||
}
|
||||
|
||||
// The name (without trailing '*') must contain at least one underscore.
|
||||
if !strings.ContainsRune(name, '_') {
|
||||
return fmt.Errorf("expected_underscore_headers: entry %q does not contain an underscore", entry)
|
||||
// The name (without trailing '*') must contain at least one separator.
|
||||
if !strings.ContainsRune(name, sep) {
|
||||
return nil, nil, nil, fmt.Errorf("%s: entry %q does not contain a %q", directive, entry, sep)
|
||||
}
|
||||
|
||||
canonAllow := http.CanonicalHeaderKey(name)
|
||||
canonDrop := http.CanonicalHeaderKey(strings.ReplaceAll(name, "_", "-"))
|
||||
canonDrop := http.CanonicalHeaderKey(strings.ReplaceAll(name, string(sep), "-"))
|
||||
|
||||
if isGlob {
|
||||
s.underscorePrefixRules = append(s.underscorePrefixRules, underscoreRule{
|
||||
prefixRules = append(prefixRules, aliasPrefixRule{
|
||||
allow: canonAllow,
|
||||
drop: canonDrop,
|
||||
})
|
||||
} else {
|
||||
s.underscoreExactAllow[canonAllow] = struct{}{}
|
||||
s.underscoreExactDrop[canonDrop] = struct{}{}
|
||||
exactAllow[canonAllow] = struct{}{}
|
||||
exactDrop[canonDrop] = struct{}{}
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
return exactAllow, exactDrop, prefixRules, nil
|
||||
}
|
||||
|
||||
// provisionUnderscoreHeaders validates the ExpectedUnderscoreHeaders
|
||||
// entries and builds the precomputed maps and prefix rules used by
|
||||
// the hot-path filter in serveHTTP.
|
||||
func (s *Server) provisionUnderscoreHeaders() error {
|
||||
if len(s.ExpectedUnderscoreHeaders) == 0 {
|
||||
return nil
|
||||
}
|
||||
var err error
|
||||
s.underscoreExactAllow, s.underscoreExactDrop, s.underscorePrefixRules, err =
|
||||
provisionHeaderAliasAllowlist(s.ExpectedUnderscoreHeaders, '_', "expected_underscore_headers")
|
||||
return err
|
||||
}
|
||||
|
||||
// provisionDotHeaders validates the ExpectedDotHeaders entries and
|
||||
// builds the precomputed maps and prefix rules used by the hot-path
|
||||
// filter in serveHTTP.
|
||||
func (s *Server) provisionDotHeaders() error {
|
||||
if len(s.ExpectedDotHeaders) == 0 {
|
||||
return nil
|
||||
}
|
||||
var err error
|
||||
s.dotExactAllow, s.dotExactDrop, s.dotPrefixRules, err =
|
||||
provisionHeaderAliasAllowlist(s.ExpectedDotHeaders, '.', "expected_dot_headers")
|
||||
return err
|
||||
}
|
||||
|
||||
// isAllowedUnderscoreHeader reports whether key (a canonical header
|
||||
|
|
@ -395,18 +468,41 @@ func (s *Server) isAllowedUnderscoreHeader(key string) bool {
|
|||
return false
|
||||
}
|
||||
|
||||
// isAllowedDotHeader reports whether key (a canonical header name
|
||||
// containing a dot) is permitted by the allowlist.
|
||||
func (s *Server) isAllowedDotHeader(key string) bool {
|
||||
if _, ok := s.dotExactAllow[key]; ok {
|
||||
return true
|
||||
}
|
||||
for _, rule := range s.dotPrefixRules {
|
||||
if strings.HasPrefix(key, rule.allow) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// isHyphenatedVariant reports whether key (a canonical header name
|
||||
// without underscores) is the hyphenated variant of an allowlisted
|
||||
// underscore header and should therefore be dropped.
|
||||
// containing neither an underscore nor a dot) is the hyphenated variant
|
||||
// of an allowlisted underscore or dot header and should therefore be
|
||||
// dropped.
|
||||
func (s *Server) isHyphenatedVariant(key string) bool {
|
||||
if _, ok := s.underscoreExactDrop[key]; ok {
|
||||
return true
|
||||
}
|
||||
if _, ok := s.dotExactDrop[key]; ok {
|
||||
return true
|
||||
}
|
||||
for _, rule := range s.underscorePrefixRules {
|
||||
if strings.HasPrefix(key, rule.drop) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
for _, rule := range s.dotPrefixRules {
|
||||
if strings.HasPrefix(key, rule.drop) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
|
|
@ -604,44 +700,79 @@ func (s *Server) serveHTTP(w http.ResponseWriter, r *http.Request) error {
|
|||
}
|
||||
}
|
||||
|
||||
// Drop headers whose names contain `_`: once FastCGI/CGI/FrankenPHP etc. rewrites `-` to
|
||||
// `_`, an underscore alias collides with the legitimate hyphenated header
|
||||
// and can bypass `forward_auth copy_headers` (GHSA-f59h-q822-g45g).
|
||||
// Drop headers whose names contain `_` or `.`: once FastCGI/CGI/FrankenPHP etc.
|
||||
// rewrites `-` to `_` (and PHP additionally folds `.` to `_` when registering
|
||||
// $_SERVER keys), an underscore or dot alias collides with the legitimate
|
||||
// hyphenated header and can bypass `forward_auth copy_headers`
|
||||
// (GHSA-f59h-q822-g45g, GHSA-49wc-4hcv-v58q).
|
||||
//
|
||||
// When an allowlist is configured, only the listed headers are kept and
|
||||
// their hyphenated variants are actively dropped to prevent ambiguity.
|
||||
if len(s.ExpectedUnderscoreHeaders) == 0 {
|
||||
for k := range r.Header {
|
||||
if strings.ContainsRune(k, '_') {
|
||||
// The two allowlists (ExpectedUnderscoreHeaders, ExpectedDotHeaders) are
|
||||
// otherwise independent: each only keeps headers spelled with its own
|
||||
// character, and either allowlist actively drops the plain-hyphenated
|
||||
// variant of its entries.
|
||||
for k := range r.Header {
|
||||
hasUnderscore := strings.ContainsRune(k, '_')
|
||||
hasDot := strings.ContainsRune(k, '.')
|
||||
|
||||
switch {
|
||||
case hasUnderscore && hasDot:
|
||||
// A name containing both separators still collides with the
|
||||
// hyphenated, underscore-only, and dot-only spellings once a
|
||||
// CGI/FastCGI/PHP backend folds them onto the same variable
|
||||
// name. A prefix glob can't vet its free-form suffix for an
|
||||
// embedded "other" separator, so only an exact allowlist entry
|
||||
// for this literal spelling is honored here.
|
||||
_, underscoreOK := s.underscoreExactAllow[k]
|
||||
_, dotOK := s.dotExactAllow[k]
|
||||
if !underscoreOK && !dotOK {
|
||||
delete(r.Header, k)
|
||||
|
||||
if c := s.logger.Check(zapcore.DebugLevel, "dropping header containing both underscore and dot"); c != nil {
|
||||
c.Write(zap.String("header", k))
|
||||
}
|
||||
} else if n := len(r.Header[k]); n > 1 {
|
||||
delete(r.Header, k)
|
||||
|
||||
if c := s.logger.Check(zapcore.WarnLevel, "dropping allowlisted underscore/dot header with repeated values (possible spoofing)"); c != nil {
|
||||
c.Write(zap.String("header", k), zap.Int("count", n))
|
||||
}
|
||||
}
|
||||
|
||||
case hasUnderscore:
|
||||
if len(s.ExpectedUnderscoreHeaders) == 0 || !s.isAllowedUnderscoreHeader(k) {
|
||||
delete(r.Header, k)
|
||||
|
||||
if c := s.logger.Check(zapcore.DebugLevel, "dropping header containing underscore"); c != nil {
|
||||
c.Write(zap.String("header", k))
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
for k := range r.Header {
|
||||
if strings.ContainsRune(k, '_') {
|
||||
if !s.isAllowedUnderscoreHeader(k) {
|
||||
delete(r.Header, k)
|
||||
|
||||
if c := s.logger.Check(zapcore.DebugLevel, "dropping header containing underscore"); c != nil {
|
||||
c.Write(zap.String("header", k))
|
||||
}
|
||||
} else if n := len(r.Header[k]); n > 1 {
|
||||
delete(r.Header, k)
|
||||
|
||||
if c := s.logger.Check(zapcore.WarnLevel, "dropping allowlisted underscore header with repeated values (possible spoofing)"); c != nil {
|
||||
c.Write(zap.String("header", k), zap.Int("count", n))
|
||||
}
|
||||
}
|
||||
} else if s.isHyphenatedVariant(k) {
|
||||
} else if n := len(r.Header[k]); n > 1 {
|
||||
delete(r.Header, k)
|
||||
|
||||
if c := s.logger.Check(zapcore.DebugLevel, "dropping hyphenated variant of expected underscore header"); c != nil {
|
||||
if c := s.logger.Check(zapcore.WarnLevel, "dropping allowlisted underscore header with repeated values (possible spoofing)"); c != nil {
|
||||
c.Write(zap.String("header", k), zap.Int("count", n))
|
||||
}
|
||||
}
|
||||
|
||||
case hasDot:
|
||||
if len(s.ExpectedDotHeaders) == 0 || !s.isAllowedDotHeader(k) {
|
||||
delete(r.Header, k)
|
||||
|
||||
if c := s.logger.Check(zapcore.DebugLevel, "dropping header containing dot"); c != nil {
|
||||
c.Write(zap.String("header", k))
|
||||
}
|
||||
} else if n := len(r.Header[k]); n > 1 {
|
||||
delete(r.Header, k)
|
||||
|
||||
if c := s.logger.Check(zapcore.WarnLevel, "dropping allowlisted dot header with repeated values (possible spoofing)"); c != nil {
|
||||
c.Write(zap.String("header", k), zap.Int("count", n))
|
||||
}
|
||||
}
|
||||
|
||||
case s.isHyphenatedVariant(k):
|
||||
delete(r.Header, k)
|
||||
|
||||
if c := s.logger.Check(zapcore.DebugLevel, "dropping hyphenated variant of expected underscore/dot header"); c != nil {
|
||||
c.Write(zap.String("header", k))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -527,6 +527,51 @@ func TestServer_serveHTTP_LogsDroppedUnderscoreHeader(t *testing.T) {
|
|||
assert.Contains(t, buf.String(), `"header":"Remote_user"`)
|
||||
}
|
||||
|
||||
// TestServer_serveHTTP_DropsDotHeader guards GHSA-49wc-4hcv-v58q: a
|
||||
// dot-named alias (e.g. `Remote.user`) of a hyphenated header must be
|
||||
// dropped too, since PHP's $_SERVER registration folds `.` to `_` just
|
||||
// like CGI/FastCGI folds `-` to `_`.
|
||||
func TestServer_serveHTTP_DropsDotHeader(t *testing.T) {
|
||||
got := &http.Header{}
|
||||
s := &Server{
|
||||
logger: zap.NewNop(),
|
||||
primaryHandlerChain: HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
|
||||
*got = r.Header.Clone()
|
||||
return nil
|
||||
}),
|
||||
}
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header["X-Real-Header"] = []string{"ok"}
|
||||
req.Header["Remote.user"] = []string{"attacker"}
|
||||
req.Header["Remote.groups"] = []string{"admin"}
|
||||
|
||||
require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
|
||||
assert.NotContains(t, *got, "Remote.user")
|
||||
assert.NotContains(t, *got, "Remote.groups")
|
||||
assert.Equal(t, "ok", got.Get("X-Real-Header"))
|
||||
}
|
||||
|
||||
// TestServer_serveHTTP_LogsDroppedDotHeader verifies each dropped dotted
|
||||
// header is emitted at debug level, same as the underscore case.
|
||||
func TestServer_serveHTTP_LogsDroppedDotHeader(t *testing.T) {
|
||||
var buf bytes.Buffer
|
||||
s := &Server{
|
||||
logger: testLogger(buf.Write),
|
||||
primaryHandlerChain: HandlerFunc(func(http.ResponseWriter, *http.Request) error {
|
||||
return nil
|
||||
}),
|
||||
}
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header["Remote.user"] = []string{"attacker"}
|
||||
|
||||
require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
|
||||
assert.Contains(t, buf.String(), `"level":"debug"`)
|
||||
assert.Contains(t, buf.String(), `"msg":"dropping header containing dot"`)
|
||||
assert.Contains(t, buf.String(), `"header":"Remote.user"`)
|
||||
}
|
||||
|
||||
// --- Allowlist: exact match ---
|
||||
|
||||
func TestServer_serveHTTP_AllowlistKeepsExactMatch(t *testing.T) {
|
||||
|
|
@ -777,6 +822,277 @@ func TestServer_serveHTTP_LiteralAsteriskInHeader(t *testing.T) {
|
|||
assert.Equal(t, "val", got.Get("Webhook_*"))
|
||||
}
|
||||
|
||||
// --- Allowlist: dot headers (ExpectedDotHeaders), independent of underscore ---
|
||||
|
||||
// TestServer_serveHTTP_DotAllowlistKeepsExactMatch verifies a dot-named
|
||||
// header explicitly allowlisted via ExpectedDotHeaders survives, mirroring
|
||||
// the underscore allowlist behavior.
|
||||
func TestServer_serveHTTP_DotAllowlistKeepsExactMatch(t *testing.T) {
|
||||
got := &http.Header{}
|
||||
s := &Server{
|
||||
logger: zap.NewNop(),
|
||||
ExpectedDotHeaders: []string{"user.id"},
|
||||
primaryHandlerChain: HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
|
||||
*got = r.Header.Clone()
|
||||
return nil
|
||||
}),
|
||||
}
|
||||
require.NoError(t, s.provisionDotHeaders())
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header["User.id"] = []string{"zeus"}
|
||||
|
||||
require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
|
||||
assert.Equal(t, "zeus", got.Get("User.id"))
|
||||
}
|
||||
|
||||
// TestServer_serveHTTP_DotAllowlistDropsHyphenatedVariant verifies the
|
||||
// hyphenated variant of an allowlisted dot header is dropped, same as for
|
||||
// the underscore allowlist.
|
||||
func TestServer_serveHTTP_DotAllowlistDropsHyphenatedVariant(t *testing.T) {
|
||||
got := &http.Header{}
|
||||
s := &Server{
|
||||
logger: zap.NewNop(),
|
||||
ExpectedDotHeaders: []string{"user.id"},
|
||||
primaryHandlerChain: HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
|
||||
*got = r.Header.Clone()
|
||||
return nil
|
||||
}),
|
||||
}
|
||||
require.NoError(t, s.provisionDotHeaders())
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header["User.id"] = []string{"zeus"}
|
||||
req.Header.Set("User-Id", "attacker")
|
||||
|
||||
require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
|
||||
assert.Equal(t, "zeus", got.Get("User.id"))
|
||||
assert.NotContains(t, *got, "User-Id")
|
||||
}
|
||||
|
||||
// TestServer_serveHTTP_DotAllowlistDropsUnderscoreVariant verifies that when
|
||||
// only the dot form is allowlisted, an underscore-form header of the same
|
||||
// logical name is NOT independently kept: ExpectedDotHeaders does not grant
|
||||
// any allowance to underscore-named headers, so it falls through to the
|
||||
// default underscore behavior (dropped, since ExpectedUnderscoreHeaders is
|
||||
// unset).
|
||||
func TestServer_serveHTTP_DotAllowlistDropsUnderscoreVariant(t *testing.T) {
|
||||
got := &http.Header{}
|
||||
s := &Server{
|
||||
logger: zap.NewNop(),
|
||||
ExpectedDotHeaders: []string{"user.id"},
|
||||
primaryHandlerChain: HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
|
||||
*got = r.Header.Clone()
|
||||
return nil
|
||||
}),
|
||||
}
|
||||
require.NoError(t, s.provisionDotHeaders())
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header["User.id"] = []string{"zeus"}
|
||||
req.Header["User_id"] = []string{"attacker"}
|
||||
|
||||
require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
|
||||
assert.Equal(t, "zeus", got.Get("User.id"))
|
||||
assert.NotContains(t, *got, "User_id")
|
||||
}
|
||||
|
||||
// TestServer_serveHTTP_DotAllowlistDropsUnlisted verifies an unlisted
|
||||
// dot-form header is dropped just like its underscore-form counterpart.
|
||||
func TestServer_serveHTTP_DotAllowlistDropsUnlisted(t *testing.T) {
|
||||
got := &http.Header{}
|
||||
s := &Server{
|
||||
logger: zap.NewNop(),
|
||||
ExpectedDotHeaders: []string{"user.id"},
|
||||
primaryHandlerChain: HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
|
||||
*got = r.Header.Clone()
|
||||
return nil
|
||||
}),
|
||||
}
|
||||
require.NoError(t, s.provisionDotHeaders())
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header["User.id"] = []string{"zeus"}
|
||||
req.Header["Remote.user"] = []string{"attacker"}
|
||||
|
||||
require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
|
||||
assert.Equal(t, "zeus", got.Get("User.id"))
|
||||
assert.NotContains(t, *got, "Remote.user")
|
||||
}
|
||||
|
||||
// TestServer_serveHTTP_DotPrefixGlobKeepsMatch verifies prefix-glob
|
||||
// matching works for dot headers the same way it does for underscore
|
||||
// headers.
|
||||
func TestServer_serveHTTP_DotPrefixGlobKeepsMatch(t *testing.T) {
|
||||
got := &http.Header{}
|
||||
s := &Server{
|
||||
logger: zap.NewNop(),
|
||||
ExpectedDotHeaders: []string{"webhook.*"},
|
||||
primaryHandlerChain: HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
|
||||
*got = r.Header.Clone()
|
||||
return nil
|
||||
}),
|
||||
}
|
||||
require.NoError(t, s.provisionDotHeaders())
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header["Webhook.event"] = []string{"push"}
|
||||
|
||||
require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
|
||||
assert.Equal(t, "push", got.Get("Webhook.event"))
|
||||
}
|
||||
|
||||
// --- Both allowlists configured together ---
|
||||
|
||||
// TestServer_serveHTTP_BothAllowlistsCoexistForDifferentNames verifies
|
||||
// unrelated underscore and dot allowlist entries can coexist without
|
||||
// interfering with each other.
|
||||
func TestServer_serveHTTP_BothAllowlistsCoexistForDifferentNames(t *testing.T) {
|
||||
got := &http.Header{}
|
||||
s := &Server{
|
||||
logger: zap.NewNop(),
|
||||
ExpectedUnderscoreHeaders: []string{"user_id"},
|
||||
ExpectedDotHeaders: []string{"webhook.event"},
|
||||
primaryHandlerChain: HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
|
||||
*got = r.Header.Clone()
|
||||
return nil
|
||||
}),
|
||||
}
|
||||
require.NoError(t, s.provisionUnderscoreHeaders())
|
||||
require.NoError(t, s.provisionDotHeaders())
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header["User_id"] = []string{"zeus"}
|
||||
req.Header["Webhook.event"] = []string{"push"}
|
||||
|
||||
require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
|
||||
assert.Equal(t, "zeus", got.Get("User_id"))
|
||||
assert.Equal(t, "push", got.Get("Webhook.event"))
|
||||
}
|
||||
|
||||
// TestServer_serveHTTP_BothAllowlistsPermitSameLogicalName verifies Caddy
|
||||
// does not enforce mutual exclusion between the two allowlists: an
|
||||
// operator whose backend doesn't fold '.' and '_' onto the same variable
|
||||
// (e.g. a Node.js or Go app behind reverse_proxy, not a CGI/FastCGI/PHP
|
||||
// backend) may legitimately want both spellings of the same name kept as
|
||||
// distinct headers. It's their responsibility to know whether that's safe
|
||||
// for their backend; see the doc comments on ExpectedUnderscoreHeaders and
|
||||
// ExpectedDotHeaders.
|
||||
func TestServer_serveHTTP_BothAllowlistsPermitSameLogicalName(t *testing.T) {
|
||||
got := &http.Header{}
|
||||
s := &Server{
|
||||
logger: zap.NewNop(),
|
||||
ExpectedUnderscoreHeaders: []string{"user_id"},
|
||||
ExpectedDotHeaders: []string{"user.id"},
|
||||
primaryHandlerChain: HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
|
||||
*got = r.Header.Clone()
|
||||
return nil
|
||||
}),
|
||||
}
|
||||
require.NoError(t, s.provisionUnderscoreHeaders())
|
||||
require.NoError(t, s.provisionDotHeaders())
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header["User_id"] = []string{"alice"}
|
||||
req.Header["User.id"] = []string{"bob"}
|
||||
|
||||
require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
|
||||
assert.Equal(t, "alice", got.Get("User_id"))
|
||||
assert.Equal(t, "bob", got.Get("User.id"))
|
||||
}
|
||||
|
||||
// --- Headers with both underscore and dot in the same name ---
|
||||
|
||||
// TestServer_serveHTTP_DropsMixedUnderscoreDotHeaderByDefault verifies a
|
||||
// header spelled with both an underscore and a dot (e.g. "Webhook_user.id")
|
||||
// is dropped by default, same as either separator alone.
|
||||
func TestServer_serveHTTP_DropsMixedUnderscoreDotHeaderByDefault(t *testing.T) {
|
||||
got := &http.Header{}
|
||||
s := &Server{
|
||||
logger: zap.NewNop(),
|
||||
primaryHandlerChain: HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
|
||||
*got = r.Header.Clone()
|
||||
return nil
|
||||
}),
|
||||
}
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header["Webhook_user.id"] = []string{"attacker"}
|
||||
|
||||
require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
|
||||
assert.NotContains(t, *got, "Webhook_user.id")
|
||||
}
|
||||
|
||||
// TestServer_serveHTTP_PrefixGlobDoesNotLeakMixedHeader guards against a
|
||||
// prefix glob unintentionally admitting a mixed-separator header: an
|
||||
// underscore prefix rule like "webhook_*" matches "Webhook_user.id" via
|
||||
// plain string prefix, but the embedded dot still collides at a
|
||||
// CGI/FastCGI/PHP backend the same way a bare dot-named header would, so
|
||||
// it must not slip through just because it also happens to start with an
|
||||
// allowlisted underscore prefix.
|
||||
func TestServer_serveHTTP_PrefixGlobDoesNotLeakMixedHeader(t *testing.T) {
|
||||
got := &http.Header{}
|
||||
s := &Server{
|
||||
logger: zap.NewNop(),
|
||||
ExpectedUnderscoreHeaders: []string{"webhook_*"},
|
||||
primaryHandlerChain: HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
|
||||
*got = r.Header.Clone()
|
||||
return nil
|
||||
}),
|
||||
}
|
||||
require.NoError(t, s.provisionUnderscoreHeaders())
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header["Webhook_user.id"] = []string{"attacker"}
|
||||
|
||||
require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
|
||||
assert.NotContains(t, *got, "Webhook_user.id")
|
||||
}
|
||||
|
||||
// TestServer_serveHTTP_ExactAllowlistPermitsMixedHeader verifies that an
|
||||
// operator who explicitly vetted the exact mixed-separator spelling (not a
|
||||
// glob) can still allowlist it.
|
||||
func TestServer_serveHTTP_ExactAllowlistPermitsMixedHeader(t *testing.T) {
|
||||
got := &http.Header{}
|
||||
s := &Server{
|
||||
logger: zap.NewNop(),
|
||||
ExpectedUnderscoreHeaders: []string{"webhook_user.id"},
|
||||
primaryHandlerChain: HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
|
||||
*got = r.Header.Clone()
|
||||
return nil
|
||||
}),
|
||||
}
|
||||
require.NoError(t, s.provisionUnderscoreHeaders())
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header["Webhook_user.id"] = []string{"ok"}
|
||||
|
||||
require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
|
||||
assert.Equal(t, "ok", got.Get("Webhook_user.id"))
|
||||
}
|
||||
|
||||
// TestServer_serveHTTP_ExactAllowlistDropsMixedHeaderRepeatedValues verifies
|
||||
// the repeated-value guard also applies to an exactly-allowlisted mixed
|
||||
// header.
|
||||
func TestServer_serveHTTP_ExactAllowlistDropsMixedHeaderRepeatedValues(t *testing.T) {
|
||||
got := &http.Header{}
|
||||
s := &Server{
|
||||
logger: zap.NewNop(),
|
||||
ExpectedUnderscoreHeaders: []string{"webhook_user.id"},
|
||||
primaryHandlerChain: HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
|
||||
*got = r.Header.Clone()
|
||||
return nil
|
||||
}),
|
||||
}
|
||||
require.NoError(t, s.provisionUnderscoreHeaders())
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
|
||||
req.Header["Webhook_user.id"] = []string{"ok", "injected"}
|
||||
|
||||
require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
|
||||
assert.NotContains(t, *got, "Webhook_user.id")
|
||||
}
|
||||
|
||||
// --- Combined allowlist ---
|
||||
|
||||
func TestServer_serveHTTP_ExactAndPrefixCoexist(t *testing.T) {
|
||||
|
|
@ -895,7 +1211,7 @@ func TestServer_serveHTTP_LogsHyphenatedVariantDrop(t *testing.T) {
|
|||
|
||||
require.NoError(t, s.serveHTTP(httptest.NewRecorder(), req))
|
||||
assert.Contains(t, buf.String(), `"level":"debug"`)
|
||||
assert.Contains(t, buf.String(), `"msg":"dropping hyphenated variant of expected underscore header"`)
|
||||
assert.Contains(t, buf.String(), `"msg":"dropping hyphenated variant of expected underscore/dot header"`)
|
||||
assert.Contains(t, buf.String(), `"header":"User-Id"`)
|
||||
}
|
||||
|
||||
|
|
@ -954,6 +1270,52 @@ func TestServer_provisionUnderscoreHeaders_DeduplicatesSilently(t *testing.T) {
|
|||
assert.Len(t, s.underscoreExactAllow, 1)
|
||||
}
|
||||
|
||||
func TestServer_provisionDotHeaders_EmptyListIsNoOp(t *testing.T) {
|
||||
s := &Server{ExpectedDotHeaders: []string{}}
|
||||
assert.NoError(t, s.provisionDotHeaders())
|
||||
}
|
||||
|
||||
func TestServer_provisionDotHeaders_RejectsNoDot(t *testing.T) {
|
||||
s := &Server{ExpectedDotHeaders: []string{"content-type"}}
|
||||
assert.Error(t, s.provisionDotHeaders())
|
||||
}
|
||||
|
||||
func TestServer_provisionDotHeaders_RejectsBareWildcard(t *testing.T) {
|
||||
s := &Server{ExpectedDotHeaders: []string{"*"}}
|
||||
assert.Error(t, s.provisionDotHeaders())
|
||||
}
|
||||
|
||||
func TestServer_provisionDotHeaders_RejectsMidGlob(t *testing.T) {
|
||||
s := &Server{ExpectedDotHeaders: []string{"f*oo.bar"}}
|
||||
assert.Error(t, s.provisionDotHeaders())
|
||||
}
|
||||
|
||||
func TestServer_provisionDotHeaders_RejectsLeadingGlob(t *testing.T) {
|
||||
s := &Server{ExpectedDotHeaders: []string{"*.foo"}}
|
||||
assert.Error(t, s.provisionDotHeaders())
|
||||
}
|
||||
|
||||
func TestServer_provisionDotHeaders_RejectsNonASCII(t *testing.T) {
|
||||
s := &Server{ExpectedDotHeaders: []string{"uşer.id"}}
|
||||
assert.Error(t, s.provisionDotHeaders())
|
||||
}
|
||||
|
||||
func TestServer_provisionDotHeaders_ValidExact(t *testing.T) {
|
||||
s := &Server{ExpectedDotHeaders: []string{"user.id"}}
|
||||
assert.NoError(t, s.provisionDotHeaders())
|
||||
}
|
||||
|
||||
func TestServer_provisionDotHeaders_ValidGlob(t *testing.T) {
|
||||
s := &Server{ExpectedDotHeaders: []string{"webhook.*"}}
|
||||
assert.NoError(t, s.provisionDotHeaders())
|
||||
}
|
||||
|
||||
func TestServer_provisionDotHeaders_DeduplicatesSilently(t *testing.T) {
|
||||
s := &Server{ExpectedDotHeaders: []string{"user.id", "user.id"}}
|
||||
require.NoError(t, s.provisionDotHeaders())
|
||||
assert.Len(t, s.dotExactAllow, 1)
|
||||
}
|
||||
|
||||
// TestServer_SpaceInHeaderNameReturnsBadRequest documents why the underscore
|
||||
// filter does not also strip space-named headers: Go's HTTP parser rejects a
|
||||
// space in a field name with 400 before any handler runs, so such a request
|
||||
|
|
|
|||
Loading…
Reference in New Issue