caddyhttp: enforce bounded host metric labels on HTTPS

With per_host metrics, HTTPS servers automatically observed catch-all
hosts, and port/case variants of configured hosts produced distinct
labels. Require explicit observe_catchall_hosts for unconfigured hosts,
normalize the host label, and also collect hosts from subroutes, error
routes and named routes (scanned after handlers are provisioned).

Behavior change: unconfigured hosts on HTTPS servers are now labeled
"_other" unless observe_catchall_hosts is enabled.

Addresses scan report 457dcf090b421495.

Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

modules/caddyhttp/app.go

@@ -216,8 +216,6 @@ func (app *App) Provision(ctx caddy.Context) error {
 	if app.Metrics != nil {
 		app.Metrics.init = sync.Once{}
 		app.Metrics.httpMetrics = &httpMetrics{}
-		// Scan config for allowed hosts to prevent cardinality explosion
-		app.Metrics.scanConfigForHosts(app)
 		if err := app.Metrics.provisionOTLP(ctx); err != nil {
 			return err
 		}
@@ -419,6 +417,9 @@ func (app *App) Provision(ctx caddy.Context) error {
 			srv.MaxWriteChunk = DefaultMaxWriteChunk
 		}
 	}
+	if app.Metrics != nil {
+		app.Metrics.scanConfigForHosts(app)
+	}
 	ctx.Context = oldContext
 	return nil
 }

modules/caddyhttp/metrics.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net"
 	"net/http"
 	"strings"
 	"sync"
@@ -57,17 +58,17 @@ type Metrics struct {
 	// CARDINALITY PROTECTION: To prevent unbounded cardinality attacks,
 	// only explicitly configured hosts (via host matchers) are allowed
 	// by default. Other hosts are aggregated under the "_other" label.
-	// See AllowCatchAllHosts to change this behavior.
+	// See ObserveCatchallHosts to change this behavior. This also applies to
+	// HTTPS, including On-Demand TLS sites, which do not have host matchers
+	// for every name they serve. Host labels never include the port.
 	PerHost bool `json:"per_host,omitempty"`
 
 	// Allow metrics for catch-all hosts (hosts without explicit configuration).
 	// When false (default), only hosts explicitly configured via host matchers
 	// will get individual metrics labels. All other hosts will be aggregated
-	// under the "_other" label to prevent cardinality explosion.
-	//
-	// This is automatically enabled for HTTPS servers (since certificates provide
-	// some protection against unbounded cardinality), but disabled for HTTP servers
-	// by default to prevent cardinality attacks from arbitrary Host headers.
+	// under the "_other" label to prevent cardinality explosion. HTTPS
+	// requests are not exempt: the Host header is not bound to the TLS
+	// certificate, so it can be any value.
 	//
 	// Set to true to allow all hosts to get individual metrics (NOT RECOMMENDED
 	// for production environments exposed to the internet).
@@ -82,11 +83,10 @@ type Metrics struct {
 	// keep this field false to disable OTLP export.
 	OTLP bool `json:"otlp,omitempty"`
 
-	init           sync.Once
-	httpMetrics    *httpMetrics
-	allowedHosts   map[string]struct{}
-	hasHTTPSServer bool
-	meterProvider  *sdkmetric.MeterProvider
+	init          sync.Once
+	httpMetrics   *httpMetrics
+	allowedHosts  map[string]struct{}
+	meterProvider *sdkmetric.MeterProvider
 }
 
 type httpMetrics struct {
@@ -235,44 +235,50 @@ func (m *Metrics) scanConfigForHosts(app *App) {
 	}
 
 	m.allowedHosts = make(map[string]struct{})
-	m.hasHTTPSServer = false
-
-	for _, srv := range app.Servers {
-		// Check if this server has TLS enabled
-		serverHasTLS := len(srv.TLSConnPolicies) > 0
-		if serverHasTLS {
-			m.hasHTTPSServer = true
-		}
-
-		// Collect hosts from route matchers
-		for _, route := range srv.Routes {
+	var scanRoutes func(RouteList)
+	scanRoutes = func(routes RouteList) {
+		for _, route := range routes {
 			for _, matcherSet := range route.MatcherSets {
 				for _, matcher := range matcherSet {
 					if hm, ok := matcher.(*MatchHost); ok {
 						for _, host := range *hm {
 							// Only allow non-fuzzy hosts to prevent unbounded cardinality
 							if !hm.fuzzy(host) {
-								m.allowedHosts[strings.ToLower(host)] = struct{}{}
+								m.allowedHosts[metricsHost(host)] = struct{}{}
 							}
 						}
 					}
 				}
 			}
+			for _, handler := range route.Handlers {
+				if sr, ok := handler.(*Subroute); ok {
+					scanRoutes(sr.Routes)
+					if sr.Errors != nil {
+						scanRoutes(sr.Errors.Routes)
+					}
+				}
+			}
+		}
+	}
+	for _, srv := range app.Servers {
+		scanRoutes(srv.Routes)
+		if srv.Errors != nil {
+			scanRoutes(srv.Errors.Routes)
+		}
+		for _, route := range srv.NamedRoutes {
+			scanRoutes(RouteList{*route})
 		}
 	}
 }
 
 // shouldAllowHostMetrics determines if metrics should be collected for the given host.
-// This implements the cardinality protection by only allowing metrics for:
-// 1. Explicitly configured hosts
-// 2. Catch-all requests on HTTPS servers (if AllowCatchAllHosts is true or auto-enabled)
-// 3. Catch-all requests on HTTP servers only if explicitly allowed
-func (m *Metrics) shouldAllowHostMetrics(host string, isHTTPS bool) bool {
+// Only explicitly configured hosts are allowed unless catch-all observation is enabled.
+func (m *Metrics) shouldAllowHostMetrics(host string) bool {
 	if !m.PerHost {
 		return true // host won't be used in labels anyway
 	}
 
-	normalizedHost := strings.ToLower(host)
+	normalizedHost := metricsHost(host)
 
 	// Always allow explicitly configured hosts
 	if _, exists := m.allowedHosts[normalizedHost]; exists {
@@ -280,8 +286,14 @@ func (m *Metrics) shouldAllowHostMetrics(host string, isHTTPS bool) bool {
 	}
 
 	// For catch-all requests (not in allowed hosts)
-	allowCatchAll := m.ObserveCatchallHosts || (isHTTPS && m.hasHTTPSServer)
-	return allowCatchAll
+	return m.ObserveCatchallHosts
+}
+
+func metricsHost(host string) string {
+	if hostname, _, err := net.SplitHostPort(host); err == nil {
+		host = hostname
+	}
+	return strings.ToLower(strings.Trim(host, "[]"))
 }
 
 // serverNameFromContext extracts the current server name from the context.
@@ -319,14 +331,12 @@ func (h *metricsInstrumentedRoute) ServeHTTP(w http.ResponseWriter, r *http.Requ
 	// of a panic
 	statusLabels := prometheus.Labels{"server": server, "handler": h.handler, "method": method, "code": ""}
 
-	// Determine if this is an HTTPS request
-	isHTTPS := r.TLS != nil
-
 	if h.metrics.PerHost {
 		// Apply cardinality protection for host metrics
-		if h.metrics.shouldAllowHostMetrics(r.Host, isHTTPS) {
-			labels["host"] = strings.ToLower(r.Host)
-			statusLabels["host"] = strings.ToLower(r.Host)
+		host := metricsHost(r.Host)
+		if h.metrics.shouldAllowHostMetrics(host) {
+			labels["host"] = host
+			statusLabels["host"] = host
 		} else {
 			// Use a catch-all label for unallowed hosts to prevent cardinality explosion
 			labels["host"] = "_other"

modules/caddyhttp/metrics_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/tls"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -434,11 +435,10 @@ func TestMetricsCardinalityProtection(t *testing.T) {
 func TestMetricsHTTPSCatchAll(t *testing.T) {
 	ctx, _ := caddy.NewContext(caddy.Context{Context: context.Background()})
 
-	// Test that HTTPS requests allow catch-all even when AllowCatchAllHosts is false
+	// HTTPS must not implicitly disable cardinality protection.
 	metrics := &Metrics{
 		PerHost:              true,
 		ObserveCatchallHosts: false,
-		hasHTTPSServer:       true, // Simulate having HTTPS servers
 		init:                 sync.Once{},
 		httpMetrics:          &httpMetrics{},
 		allowedHosts:         make(map[string]struct{}), // Empty - no explicitly allowed hosts
@@ -451,7 +451,7 @@ func TestMetricsHTTPSCatchAll(t *testing.T) {
 
 	ih := newMetricsInstrumentedRoute(ctx, "test", h, metrics)
 
-	// Test HTTPS request (should be allowed even though not in allowedHosts)
+	// Test HTTPS request (should be mapped to "_other").
 	r1 := httptest.NewRequest("GET", "https://unknown.com/", nil)
 	r1.Host = "unknown.com"
 	r1.TLS = &tls.ConnectionState{} // Mark as TLS/HTTPS
@@ -465,12 +465,11 @@ func TestMetricsHTTPSCatchAll(t *testing.T) {
 	w2 := httptest.NewRecorder()
 	ih.ServeHTTP(w2, r2)
 
-	// Check that HTTPS request gets real host, HTTP gets "_other"
+	// Both requests should be aggregated.
 	expected := `
 	# HELP caddy_http_requests_total Counter of HTTP(S) requests made.
 	# TYPE caddy_http_requests_total counter
-	caddy_http_requests_total{handler="test",host="_other",server="UNKNOWN"} 1
-	caddy_http_requests_total{handler="test",host="unknown.com",server="UNKNOWN"} 1
+	caddy_http_requests_total{handler="test",host="_other",server="UNKNOWN"} 2
 	`
 
 	if err := testutil.GatherAndCompare(ctx.GetMetricsRegistry(), strings.NewReader(expected),
@@ -480,6 +479,126 @@ func TestMetricsHTTPSCatchAll(t *testing.T) {
 	}
 }
 
+func TestMetricsHostCardinality(t *testing.T) {
+	for _, scheme := range []string{"http", "https"} {
+		for _, catchall := range []bool{false, true} {
+			t.Run(fmt.Sprintf("%s/catchall=%t", scheme, catchall), func(t *testing.T) {
+				ctx, cancel := caddy.NewContext(caddy.Context{Context: context.Background()})
+				defer cancel()
+				m := &Metrics{
+					PerHost:              true,
+					ObserveCatchallHosts: catchall,
+					httpMetrics:          &httpMetrics{},
+					allowedHosts:         map[string]struct{}{"site.test": {}},
+				}
+				h := newMetricsInstrumentedRoute(ctx, "test", HandlerFunc(func(w http.ResponseWriter, _ *http.Request) error {
+					w.WriteHeader(http.StatusOK)
+					return nil
+				}), m)
+				for i := range 80 {
+					for _, host := range []string{
+						fmt.Sprintf("SITE.test:%d", 20000+i),
+						fmt.Sprintf("metric-%d.site.test", i),
+						fmt.Sprintf("unrelated-%d.test", i),
+					} {
+						req := httptest.NewRequest(http.MethodGet, scheme+"://site.test/", nil)
+						req.Host = host
+						if req.TLS != nil {
+							req.TLS.ServerName = "site.test"
+						}
+						if err := h.ServeHTTP(httptest.NewRecorder(), req); err != nil {
+							t.Fatal(err)
+						}
+					}
+				}
+				families, err := ctx.GetMetricsRegistry().Gather()
+				if err != nil {
+					t.Fatal(err)
+				}
+				wantCount := 2
+				if catchall {
+					wantCount = 161
+				}
+				found := false
+				for _, family := range families {
+					if family.GetName() != "caddy_http_requests_total" {
+						continue
+					}
+					found = true
+					if len(family.Metric) != wantCount {
+						t.Fatalf("got %d series, want %d", len(family.Metric), wantCount)
+					}
+					for _, metric := range family.Metric {
+						for _, label := range metric.Label {
+							if label.GetName() != "host" {
+								continue
+							}
+							host := label.GetValue()
+							if strings.Contains(host, ":") {
+								t.Errorf("port persisted in host label %q", host)
+							}
+							if !catchall && host != "site.test" && host != "_other" {
+								t.Errorf("unbounded host label %q", host)
+							}
+							if host == "site.test" && metric.Counter.GetValue() != 80 {
+								t.Errorf("port variants not aggregated: %v", metric.Counter.GetValue())
+							}
+						}
+					}
+				}
+				if !found {
+					t.Fatal("request counter not collected")
+				}
+			})
+		}
+	}
+}
+
+func TestMetricsScanNestedHosts(t *testing.T) {
+	hostRoute := func(hosts ...string) Route {
+		matcher := MatchHost(hosts)
+		return Route{MatcherSets: MatcherSets{{&matcher}}}
+	}
+	named := hostRoute("NAMED.test")
+	m := &Metrics{PerHost: true}
+	m.scanConfigForHosts(&App{Servers: map[string]*Server{
+		"test": {
+			Routes: RouteList{
+				hostRoute("TOP.test", "*.wild.test", "{dynamic}.test"),
+				{Handlers: []MiddlewareHandler{&Subroute{
+					Routes: RouteList{hostRoute("nested.test")},
+					Errors: &HTTPErrorConfig{Routes: RouteList{hostRoute("nested-error.test")}},
+				}}},
+			},
+			Errors:      &HTTPErrorConfig{Routes: RouteList{hostRoute("error.test")}},
+			NamedRoutes: map[string]*Route{"named": &named},
+		},
+	}})
+	for _, host := range []string{"top.test", "NESTED.test:443", "nested-error.test", "error.test", "named.test"} {
+		if !m.shouldAllowHostMetrics(host) {
+			t.Errorf("configured host %q was not collected", host)
+		}
+	}
+	if len(m.allowedHosts) != 5 {
+		t.Fatalf("unexpected allowlist: %v", m.allowedHosts)
+	}
+	for _, host := range []string{"random.wild.test", "unknown.test", "attacker.test:999999999"} {
+		if m.shouldAllowHostMetrics(host) {
+			t.Errorf("unconfigured host %q admitted", host)
+		}
+	}
+	for _, tc := range []struct{ input, want string }{
+		{"EXAMPLE.test:999999999999999999", "example.test"},
+		{"[::1]:443", "::1"},
+		{"[::1]", "::1"},
+		{"::1", "::1"},
+	} {
+		if got := metricsHost(tc.input); got != tc.want {
+			t.Errorf("metricsHost(%q) = %q, want %q", tc.input, got, tc.want)
+		}
+	}
+}
+
 func TestMetricsInstrumentedRoute(t *testing.T) {
 	ctx, _ := caddy.NewContext(caddy.Context{Context: context.Background()})
 	m := &Metrics{