Merge branch 'master' into fix-websocket-header-normalization

Author: steadytao

.github/SECURITY.md

@@ -49,7 +49,7 @@ We'll need enough information to verify the bug and make a patch. To speed thing
 
 Please DO NOT use containers, VMs, cloud instances or services, or any other complex infrastructure in your steps. Always prefer `curl -v` instead of web browsers.
 
-We consider publicly-registered domain names to be public information. This necessary in order to maintain the integrity of certificate transparency, public DNS, and other public trust systems. Do not redact domain names from your reports. The actual content of your domain name affects Caddy's behavior, so we need the exact domain name(s) to reproduce with, or your report will be ignored.
+We consider publicly-registered domain names to be public information. This is necessary in order to maintain the integrity of certificate transparency, public DNS, and other public trust systems. Do not redact domain names from your reports. The actual content of your domain name affects Caddy's behavior, so we need the exact domain name(s) to reproduce with, or your report will be ignored.
 
 It will speed things up if you suggest a working patch, such as a code diff, and explain why and how it works. Reports that are not actionable, do not contain enough information, are too pushy/demanding, or are not able to convince us that it is a viable and practical attack on the web server itself may be deferred to a later time or possibly ignored, depending on available resources. Priority will be given to credible, responsible reports that are constructive, specific, and actionable. (We get a lot of invalid reports.) Thank you for understanding.
 

caddyconfig/httpcaddyfile/httptype.go

@@ -523,7 +523,11 @@ func (ServerType) extractNamedRoutes(
 			route.HandlersRaw = []json.RawMessage{caddyconfig.JSONModuleObject(handler, "handler", subroute.CaddyModule().ID.Name(), h.warnings)}
 		}
 
-		namedRoutes[sb.block.GetKeysText()[0]] = &route
+		key := sb.block.GetKeysText()[0]
+		if _, exists := namedRoutes[key]; exists {
+			return nil, fmt.Errorf("cannot have duplicate named_routes: %s", key)
+		}
+		namedRoutes[key] = &route
 	}
 	options["named_routes"] = namedRoutes
 

caddyconfig/httpcaddyfile/serveroptions.go

@@ -36,27 +36,28 @@ type serverOptions struct {
 	ListenerAddress string
 
 	// These will all map 1:1 to the caddyhttp.Server struct
-	Name                  string
-	ListenerWrappersRaw   []json.RawMessage
-	PacketConnWrappersRaw []json.RawMessage
-	ReadTimeout           caddy.Duration
-	ReadHeaderTimeout     caddy.Duration
-	WriteTimeout          caddy.Duration
-	IdleTimeout           caddy.Duration
-	KeepAliveInterval     caddy.Duration
-	KeepAliveIdle         caddy.Duration
-	KeepAliveCount        int
-	MaxHeaderBytes        int
-	EnableFullDuplex      bool
-	Protocols             []string
-	StrictSNIHost         *bool
-	TrustedProxiesRaw     json.RawMessage
-	TrustedProxiesStrict  int
-	TrustedProxiesUnix    bool
-	ClientIPHeaders       []string
-	ShouldLogCredentials  bool
-	Metrics               *caddyhttp.Metrics
-	Trace                 bool // TODO: EXPERIMENTAL
+	Name                      string
+	ListenerWrappersRaw       []json.RawMessage
+	PacketConnWrappersRaw     []json.RawMessage
+	ReadTimeout               caddy.Duration
+	ReadHeaderTimeout         caddy.Duration
+	WriteTimeout              caddy.Duration
+	IdleTimeout               caddy.Duration
+	KeepAliveInterval         caddy.Duration
+	KeepAliveIdle             caddy.Duration
+	KeepAliveCount            int
+	MaxHeaderBytes            int
+	EnableFullDuplex          bool
+	ExpectedUnderscoreHeaders []string
+	Protocols                 []string
+	StrictSNIHost             *bool
+	TrustedProxiesRaw         json.RawMessage
+	TrustedProxiesStrict      int
+	TrustedProxiesUnix        bool
+	ClientIPHeaders           []string
+	ShouldLogCredentials      bool
+	Metrics                   *caddyhttp.Metrics
+	Trace                     bool // TODO: EXPERIMENTAL
 	// If set, overrides whether QUIC listeners allow 0-RTT (early data).
 	// If nil, the default behavior is used (currently allowed).
 	Allow0RTT *bool
@@ -218,6 +219,13 @@ func unmarshalCaddyfileServerOptions(d *caddyfile.Dispenser) (any, error) {
 			}
 			serverOpts.EnableFullDuplex = true
 
+		case "expected_underscore_headers":
+			args := d.RemainingArgs()
+			if len(args) == 0 {
+				return nil, d.ArgErr()
+			}
+			serverOpts.ExpectedUnderscoreHeaders = args
+
 		case "log_credentials":
 			if d.NextArg() {
 				return nil, d.ArgErr()
@@ -380,6 +388,7 @@ func applyServerOptions(
 		server.KeepAliveCount = opts.KeepAliveCount
 		server.MaxHeaderBytes = opts.MaxHeaderBytes
 		server.EnableFullDuplex = opts.EnableFullDuplex
+		server.ExpectedUnderscoreHeaders = opts.ExpectedUnderscoreHeaders
 		server.Protocols = opts.Protocols
 		server.StrictSNIHost = opts.StrictSNIHost
 		server.TrustedProxiesRaw = opts.TrustedProxiesRaw

caddytest/integration/caddyfile_adapt/duplicate_named_route_challenge.caddyfiletest

@@ -0,0 +1,16 @@
+&(api) {
+	header X-Version v1
+	respond "API v1"
+}
+
+&(api) {
+	header X-Version v2
+	respond "API v2"
+}
+
+localhost {
+	invoke api
+}
+
+----------
+cannot have duplicate named_routes: api

caddytest/integration/caddyfile_adapt/forward_auth_duplicate_uri.caddyfiletest

@@ -0,0 +1,12 @@
+localhost:9080 {
+	forward_auth :9091 {
+		uri /first
+		uri /second
+		copy_headers Remote-User
+	}
+
+	respond "ok" 200
+}
+
+----------
+parsing caddyfile tokens for 'forward_auth': cannot re-declare uri: /second

caddytest/integration/intercept_test.go

@@ -38,3 +38,72 @@ func TestIntercept(t *testing.T) {
 
 	tester.AssertGetResponse("http://localhost:9080/no-intercept", 200, "I'm not a teapot")
 }
+
+func TestInterceptReplaceStatusWithMatcher(t *testing.T) {
+	tester := caddytest.NewTester(t)
+	tester.InitServer(`{
+		skip_install_trust
+		admin localhost:2999
+		http_port     9080
+		https_port    9443
+		grace_period  1ns
+	}
+
+	localhost:9080 {
+		respond /error "boom" 500
+
+		intercept {
+			@err status 5xx
+			replace_status @err 200
+		}
+	}
+	`, "caddyfile")
+
+	tester.AssertGetResponse("http://localhost:9080/error", 200, "boom")
+}
+
+func TestInterceptReplaceStatusWithoutMatcher(t *testing.T) {
+	tester := caddytest.NewTester(t)
+	tester.InitServer(`{
+		skip_install_trust
+		admin localhost:2999
+		http_port     9080
+		https_port    9443
+		grace_period  1ns
+	}
+
+	localhost:9080 {
+		respond /forbidden "denied" 403
+
+		intercept {
+			replace_status 200
+		}
+	}
+	`, "caddyfile")
+
+	tester.AssertGetResponse("http://localhost:9080/forbidden", 200, "denied")
+}
+
+func TestInterceptReplaceStatusNotMatched(t *testing.T) {
+	tester := caddytest.NewTester(t)
+	tester.InitServer(`{
+		skip_install_trust
+		admin localhost:2999
+		http_port     9080
+		https_port    9443
+		grace_period  1ns
+	}
+
+	localhost:9080 {
+		respond /ok "all good" 200
+
+		intercept {
+			@err status 5xx
+			replace_status @err 503
+		}
+	}
+	`, "caddyfile")
+
+	// 200 does not match @err (5xx), so status should pass through unchanged
+	tester.AssertGetResponse("http://localhost:9080/ok", 200, "all good")
+}

caddytest/integration/reverseproxy_test.go

@@ -199,7 +199,7 @@ func TestReverseProxyWithPlaceholderDialAddress(t *testing.T) {
 								],
 								"handle": [
 									{
-	
+
 										"handler": "reverse_proxy",
 										"upstreams": [
 											{
@@ -293,7 +293,7 @@ func TestReverseProxyWithPlaceholderTCPDialAddress(t *testing.T) {
 								],
 								"handle": [
 									{
-	
+
 										"handler": "reverse_proxy",
 										"upstreams": [
 											{
@@ -374,7 +374,7 @@ func TestReverseProxyHealthCheck(t *testing.T) {
 	http://localhost:9080 {
 		reverse_proxy {
 			to localhost:2020
-	
+
 			health_uri /health
 			health_port 2021
 			health_interval 10ms
@@ -495,7 +495,7 @@ func TestReverseProxyHealthCheckUnixSocket(t *testing.T) {
 	http://localhost:9080 {
 		reverse_proxy {
 			to unix/%s
-	
+
 			health_uri /health
 			health_port 2021
 			health_interval 2s
@@ -553,7 +553,7 @@ func TestReverseProxyHealthCheckUnixSocketWithoutPort(t *testing.T) {
 	http://localhost:9080 {
 		reverse_proxy {
 			to unix/%s
-	
+
 			health_uri /health
 			health_interval 2s
 			health_timeout 5s
@@ -793,3 +793,103 @@ func TestReverseProxyRetryMatchIsTransportError(t *testing.T) {
 	// Transport error on broken upstream should be retried to good upstream
 	tester.AssertGetResponse("http://localhost:9080/", 200, "ok")
 }
+
+func TestReverseProxySNIPlaceHolder(t *testing.T) {
+	configTemplate := `
+	{
+        skip_install_trust
+        local_certs
+        admin localhost:2999
+        http_port 9080
+        https_port 9443
+        grace_period 1ns
+	}
+	localhost example.com {
+		@proxied header X-Transport caddy
+		respond @proxied {http.request.tls.server_name}
+		reverse_proxy 127.0.0.1:9443 {
+			header_up X-Transport caddy
+			header_up Host {host}
+			transport http {
+				versions %s
+				tls_server_name {header.X-SNI}
+				tls_insecure_skip_verify
+			}
+		}
+	}
+	`
+	for _, versions := range []string{"1.1 2", "3"} {
+		tester := caddytest.NewTester(t)
+		tester.InitServer(fmt.Sprintf(configTemplate, versions), "caddyfile")
+		req, err := http.NewRequest("GET", "https://localhost:9443", nil)
+		if err != nil {
+			t.Errorf("failed to create request %s", err)
+			return
+		}
+
+		req.Header.Set("X-SNI", "example.com")
+		tester.AssertResponse(req, 200, "example.com")
+	}
+}
+
+func TestWeightedRoundRobinSelectionValidation(t *testing.T) {
+	configTemplate := `
+	{
+		"apps": {
+			"http": {
+				"servers": {
+					"srv0": {
+						"listen": [":18080"],
+						"routes": [
+							{
+								"handle": [
+									{
+										"handler": "reverse_proxy",
+										"load_balancing": {
+											"selection_policy": {
+												"policy": "weighted_round_robin",
+												"weights": %s
+											}
+										},
+										"upstreams": [
+											{"dial": "localhost:18081"},
+											{"dial": "localhost:18082"}
+										]
+									}
+								]
+							}
+						]
+					}
+				}
+			}
+		}
+	}`
+
+	tests := []struct {
+		name    string
+		weights string
+		errMsg  string
+	}{
+		{
+			name:    "negative weight",
+			weights: "[-1, 2]",
+			errMsg:  "weight of an upstream cannot be negative",
+		},
+		{
+			name:    "zero total weight",
+			weights: "[0, 0]",
+			errMsg:  "requires at least one upstream with a positive weight",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			caddytest.AssertLoadError(
+				t,
+				fmt.Sprintf(configTemplate, tc.weights),
+				"json",
+				tc.errMsg,
+			)
+		})
+	}
+}

cmd/cobra.go

@@ -149,8 +149,14 @@ func caddyCmdToCobra(caddyCmd Command) *cobra.Command {
 func WrapCommandFuncForCobra(f CommandFunc) func(cmd *cobra.Command, _ []string) error {
 	return func(cmd *cobra.Command, _ []string) error {
 		status, err := f(Flags{cmd.Flags()})
-		if status > 1 {
+		if err != nil {
+			// Route the error through Caddy's logger so it receives the same
+			// colored, structured formatting as INFO/WARN output, rather than
+			// cobra's plain "Error: ..." line which lacks any highlighting.
+			caddy.Log().Error(err.Error())
 			cmd.SilenceErrors = true
+		}
+		if status > 1 {
 			return &exitError{ExitCode: status, Err: err}
 		}
 		return err

modules/caddyhttp/app.go

@@ -70,7 +70,8 @@ func init() {
 // `{http.request.orig_uri.query}` | The request's original query string (without `?`)
 // `{http.request.orig_uri.prefixed_query}` | The request's original query string with a `?` prefix, if non-empty
 // `{http.request.port}` | The port part of the request's Host header
-// `{http.request.proto}` | The protocol of the request
+// `{http.request.proto}` | The raw protocol of the request as returned by Go (e.g., HTTP/2.0 or HTTP/3.0)
+// `{http.request.proto_name}` | The spec-defined protocol of the request (e.g., HTTP/2 or HTTP/3)
 // `{http.request.local.host}` | The host (IP) part of the local address the connection arrived on
 // `{http.request.local.port}` | The port part of the local address the connection arrived on
 // `{http.request.local}` | The local address the connection arrived on
@@ -256,6 +257,12 @@ func (app *App) Provision(ctx caddy.Context) error {
 			}
 		}
 
+		// limit max header bytes to a more reasonable default than 1MB from Go std lib
+		// (see https://github.com/php/frankenphp/issues/2459#issuecomment-4655612909)
+		if srv.MaxHeaderBytes <= 0 {
+			srv.MaxHeaderBytes = 16 * 1024
+		}
+
 		// if not explicitly configured by the user, disallow TLS
 		// client auth bypass (domain fronting) which could
 		// otherwise be exploited by sending an unprotected SNI
@@ -286,6 +293,11 @@ func (app *App) Provision(ctx caddy.Context) error {
 			srv.ClientIPHeaders = []string{"X-Forwarded-For"}
 		}
 
+		// precompute underscore header allowlist rules
+		if err := srv.provisionUnderscoreHeaders(); err != nil {
+			return fmt.Errorf("server %s: %v", srvName, err)
+		}
+
 		// process each listener address
 		for i := range srv.Listen {
 			lnOut, err := repl.ReplaceOrErr(srv.Listen[i], true, true)