From 2a3ed96f8cbf0c72ce4621de84939e8828726d59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Sat, 25 Apr 2026 12:52:08 +0200 Subject: [PATCH 01/23] metrics: Implement pushing via OLTP (#7664) --- caddyconfig/httpcaddyfile/options.go | 2 + .../metrics_otlp.caddyfiletest | 35 ++++++++ go.mod | 4 +- modules/caddyhttp/app.go | 9 ++ modules/caddyhttp/metrics.go | 88 ++++++++++++++++++- modules/caddyhttp/metrics_test.go | 50 +++++++++++ modules/caddyhttp/tracing/tracer.go | 3 +- 7 files changed, 183 insertions(+), 8 deletions(-) create mode 100644 caddytest/integration/caddyfile_adapt/metrics_otlp.caddyfiletest diff --git a/caddyconfig/httpcaddyfile/options.go b/caddyconfig/httpcaddyfile/options.go index ffe43ff7e..0b4ee5402 100644 --- a/caddyconfig/httpcaddyfile/options.go +++ b/caddyconfig/httpcaddyfile/options.go @@ -484,6 +484,8 @@ func unmarshalCaddyfileMetricsOptions(d *caddyfile.Dispenser) (any, error) { metrics.PerHost = true case "observe_catchall_hosts": metrics.ObserveCatchallHosts = true + case "otlp": + metrics.OTLP = true default: return nil, d.Errf("unrecognized servers option '%s'", d.Val()) } diff --git a/caddytest/integration/caddyfile_adapt/metrics_otlp.caddyfiletest b/caddytest/integration/caddyfile_adapt/metrics_otlp.caddyfiletest new file mode 100644 index 000000000..551c2f2ec --- /dev/null +++ b/caddytest/integration/caddyfile_adapt/metrics_otlp.caddyfiletest @@ -0,0 +1,35 @@ +{ + metrics { + otlp + } +} +:80 { + respond "Hello" +} +---------- +{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":80" + ], + "routes": [ + { + "handle": [ + { + "body": "Hello", + "handler": "static_response" + } + ] + } + ] + } + }, + "metrics": { + "otlp": true + } + } + } +} diff --git a/go.mod b/go.mod index 6e7a81b29..93b73f3f6 100644 --- a/go.mod +++ b/go.mod @@ -30,11 +30,13 @@ require ( github.com/tailscale/tscert v0.0.0-20251216020129-aea342f6d747 github.com/yuin/goldmark v1.8.2 github.com/yuin/goldmark-highlighting/v2 v2.0.0-20230729083705-37449abec8cc + go.opentelemetry.io/contrib/bridges/prometheus v0.68.0 go.opentelemetry.io/contrib/exporters/autoexport v0.65.0 go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.65.0 go.opentelemetry.io/contrib/propagators/autoprop v0.65.0 go.opentelemetry.io/otel v1.43.0 go.opentelemetry.io/otel/sdk v1.43.0 + go.opentelemetry.io/otel/sdk/metric v1.43.0 go.step.sm/crypto v0.77.1 go.uber.org/automaxprocs v1.6.0 go.uber.org/zap v1.27.1 @@ -87,7 +89,6 @@ require ( github.com/x448/float16 v0.8.4 // indirect github.com/zeebo/blake3 v0.2.4 // indirect go.opentelemetry.io/auto/sdk v1.2.1 // indirect - go.opentelemetry.io/contrib/bridges/prometheus v0.68.0 // indirect go.opentelemetry.io/contrib/propagators/aws v1.43.0 // indirect go.opentelemetry.io/contrib/propagators/b3 v1.43.0 // indirect go.opentelemetry.io/contrib/propagators/jaeger v1.43.0 // indirect @@ -104,7 +105,6 @@ require ( go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.43.0 // indirect go.opentelemetry.io/otel/log v0.19.0 // indirect go.opentelemetry.io/otel/sdk/log v0.19.0 // indirect - go.opentelemetry.io/otel/sdk/metric v1.43.0 // indirect go.yaml.in/yaml/v2 v2.4.4 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/exp v0.0.0-20251023183803-a4bb9ffd2546 // indirect diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go index a3b71836d..571ac496e 100644 --- a/modules/caddyhttp/app.go +++ b/modules/caddyhttp/app.go @@ -208,6 +208,9 @@ func (app *App) Provision(ctx caddy.Context) error { 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 + } } // prepare each server oldContext := ctx.Context @@ -817,6 +820,12 @@ func (app *App) Stop() error { } } + // flush and shut down the OTLP metrics exporter (if configured) so any + // last data point reaches the collector before the process exits + if err := app.Metrics.shutdown(ctx); err != nil { + app.logger.Error("shutting down OTLP metrics", zap.Error(err)) + } + app.stopped = true return nil } diff --git a/modules/caddyhttp/metrics.go b/modules/caddyhttp/metrics.go index b212bbfb8..8d20e01b6 100644 --- a/modules/caddyhttp/metrics.go +++ b/modules/caddyhttp/metrics.go @@ -3,6 +3,7 @@ package caddyhttp import ( "context" "errors" + "fmt" "net/http" "strings" "sync" @@ -10,9 +11,14 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" + otelprom "go.opentelemetry.io/contrib/bridges/prometheus" + "go.opentelemetry.io/contrib/exporters/autoexport" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/resource" + semconv "go.opentelemetry.io/otel/semconv/v1.26.0" "github.com/caddyserver/caddy/v2" - "github.com/caddyserver/caddy/v2/internal/metrics" + caddymetrics "github.com/caddyserver/caddy/v2/internal/metrics" ) // Metrics configures metrics observations. @@ -67,10 +73,20 @@ type Metrics struct { // for production environments exposed to the internet). ObserveCatchallHosts bool `json:"observe_catchall_hosts,omitempty"` + // Enable pushing metrics via OTLP in addition to the existing Prometheus + // scrape endpoints. When set, a PeriodicReader is attached to the shared + // Prometheus registry (via a Prometheus -> OpenTelemetry bridge), and the + // exporter is autoconfigured from the standard OTEL_* environment + // variables (OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_PROTOCOL, + // OTEL_METRICS_EXPORTER, ...). Set OTEL_METRICS_EXPORTER=none or simply + // 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 } type httpMetrics struct { @@ -147,6 +163,70 @@ func initHTTPMetrics(ctx caddy.Context, metrics *Metrics) { }, httpLabels) } +// provisionOTLP wires a MeterProvider that periodically reads the process-wide +// Prometheus registry and pushes the result via OTLP. The exporter and reader +// are autoconfigured from the standard OTEL_* environment variables, matching +// the ergonomics of the existing `tracing` directive. It is a no-op when +// m.OTLP is false, and honors OTEL_METRICS_EXPORTER=none (autoexport +// short-circuits to a no-op reader in that case). +func (m *Metrics) provisionOTLP(ctx caddy.Context) error { + if !m.OTLP { + return nil + } + + // Register a Prometheus -> OpenTelemetry bridge against the process-wide + // Prometheus registry as the *default* source the NewMetricReader below + // will read from. + // + // NB: despite the "With*" naming, autoexport.WithFallbackMetricProducer is + // a package-level setter (it returns nothing) — it mutates autoexport's + // internal producer registry and takes effect on the very next call to + // NewMetricReader. It is NOT a MetricOption and must not be passed as one. + // Users can still override the source by setting OTEL_METRICS_PRODUCERS. + reg := ctx.GetMetricsRegistry() + autoexport.WithFallbackMetricProducer(func(context.Context) (sdkmetric.Producer, error) { + return otelprom.NewMetricProducer(otelprom.WithGatherer(reg)), nil + }) + + reader, err := autoexport.NewMetricReader(ctx) + if err != nil { + return fmt.Errorf("creating OTLP metric reader: %w", err) + } + + version, _ := caddy.Version() + res, err := resource.Merge(resource.Default(), resource.NewSchemaless( + semconv.WebEngineName(ServerHeader), + semconv.WebEngineVersion(version), + )) + if err != nil { + return fmt.Errorf("building OTLP metrics resource: %w", err) + } + + m.meterProvider = sdkmetric.NewMeterProvider( + sdkmetric.WithResource(res), + sdkmetric.WithReader(reader), + ) + + return nil +} + +// shutdown flushes and tears down the OTLP MeterProvider if one was provisioned. +// Both ForceFlush and Shutdown are always attempted so that a flush failure +// does not prevent the reader goroutines from being stopped; errors from both +// are returned joined. +func (m *Metrics) shutdown(ctx context.Context) error { + if m == nil || m.meterProvider == nil { + return nil + } + + // ForceFlush gives the final collection a chance to reach the collector + // before the reader goroutine is stopped by Shutdown. + return errors.Join( + m.meterProvider.ForceFlush(ctx), + m.meterProvider.Shutdown(ctx), + ) +} + // scanConfigForHosts scans the HTTP app configuration to build a set of allowed hosts // for metrics collection, similar to how auto-HTTPS scans for domain names. func (m *Metrics) scanConfigForHosts(app *App) { @@ -234,7 +314,7 @@ func newMetricsInstrumentedRoute(ctx caddy.Context, handler string, next Handler func (h *metricsInstrumentedRoute) ServeHTTP(w http.ResponseWriter, r *http.Request) error { server := serverNameFromContext(r.Context()) labels := prometheus.Labels{"server": server, "handler": h.handler} - method := metrics.SanitizeMethod(r.Method) + method := caddymetrics.SanitizeMethod(r.Method) // the "code" value is set later, but initialized here to eliminate the possibility // of a panic statusLabels := prometheus.Labels{"server": server, "handler": h.handler, "method": method, "code": ""} @@ -264,7 +344,7 @@ func (h *metricsInstrumentedRoute) ServeHTTP(w http.ResponseWriter, r *http.Requ // being called when the headers are written. // Effectively the same behaviour as promhttp.InstrumentHandlerTimeToWriteHeader. writeHeaderRecorder := ShouldBufferFunc(func(status int, header http.Header) bool { - statusLabels["code"] = metrics.SanitizeCode(status) + statusLabels["code"] = caddymetrics.SanitizeCode(status) ttfb := time.Since(start).Seconds() h.metrics.httpMetrics.responseDuration.With(statusLabels).Observe(ttfb) return false @@ -280,7 +360,7 @@ func (h *metricsInstrumentedRoute) ServeHTTP(w http.ResponseWriter, r *http.Requ if statusLabels["code"] == "" { // we still sanitize it, even though it's likely to be 0. A 200 is // returned on fallthrough so we want to reflect that. - statusLabels["code"] = metrics.SanitizeCode(status) + statusLabels["code"] = caddymetrics.SanitizeCode(status) } h.metrics.httpMetrics.requestDuration.With(statusLabels).Observe(dur) diff --git a/modules/caddyhttp/metrics_test.go b/modules/caddyhttp/metrics_test.go index 987b3f342..d75b3cae1 100644 --- a/modules/caddyhttp/metrics_test.go +++ b/modules/caddyhttp/metrics_test.go @@ -523,6 +523,56 @@ func TestMetricsInstrumentedRoute(t *testing.T) { } } +func TestMetricsProvisionOTLPDisabled(t *testing.T) { + ctx, _ := caddy.NewContext(caddy.Context{Context: context.Background()}) + + m := &Metrics{OTLP: false} + + if err := m.provisionOTLP(ctx); err != nil { + t.Fatalf("provisionOTLP returned unexpected error: %v", err) + } + if m.meterProvider != nil { + t.Fatalf("meterProvider should remain nil when OTLP is disabled") + } + + // shutdown must be safe on a never-provisioned Metrics. + if err := m.shutdown(context.Background()); err != nil { + t.Fatalf("shutdown returned unexpected error: %v", err) + } +} + +func TestMetricsProvisionOTLPNoopExporter(t *testing.T) { + // OTEL_METRICS_EXPORTER=none makes autoexport return its built-in + // no-op reader, which avoids any network I/O while still exercising + // the full provisionOTLP -> shutdown lifecycle. + t.Setenv("OTEL_METRICS_EXPORTER", "none") + + ctx, _ := caddy.NewContext(caddy.Context{Context: context.Background()}) + + m := &Metrics{OTLP: true} + + if err := m.provisionOTLP(ctx); err != nil { + t.Fatalf("provisionOTLP returned unexpected error: %v", err) + } + if m.meterProvider == nil { + t.Fatalf("provisionOTLP did not create a MeterProvider") + } + + if err := m.shutdown(context.Background()); err != nil { + t.Fatalf("shutdown returned unexpected error: %v", err) + } +} + +// shutdown on a nil receiver is a convenience so App.Stop can call it +// without guarding against app.Metrics being unset. +func TestMetricsShutdownNilReceiver(t *testing.T) { + var m *Metrics + + if err := m.shutdown(context.Background()); err != nil { + t.Fatalf("shutdown on nil Metrics returned unexpected error: %v", err) + } +} + func BenchmarkMetricsInstrumentedRoute(b *testing.B) { ctx, _ := caddy.NewContext(caddy.Context{Context: context.Background()}) m := &Metrics{ diff --git a/modules/caddyhttp/tracing/tracer.go b/modules/caddyhttp/tracing/tracer.go index bb0f81fc3..5d71059ed 100644 --- a/modules/caddyhttp/tracing/tracer.go +++ b/modules/caddyhttp/tracing/tracer.go @@ -21,7 +21,6 @@ import ( ) const ( - webEngineName = "Caddy" defaultSpanName = "handler" nextCallCtxKey caddy.CtxKey = "nextCall" ) @@ -58,7 +57,7 @@ func newOpenTelemetryWrapper( } version, _ := caddy.Version() - res, err := ot.newResource(webEngineName, version) + res, err := ot.newResource(caddyhttp.ServerHeader, version) if err != nil { return ot, fmt.Errorf("creating resource error: %w", err) } From fdbef2a6ef698efe414076836b631daf8f791111 Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Sun, 26 Apr 2026 23:30:44 +1000 Subject: [PATCH 02/23] logging: add regression coverage for rotated file mode (#7620) --- modules/logging/filewriter_test.go | 41 ++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/modules/logging/filewriter_test.go b/modules/logging/filewriter_test.go index 915784b53..de46891fa 100644 --- a/modules/logging/filewriter_test.go +++ b/modules/logging/filewriter_test.go @@ -174,6 +174,47 @@ func TestFileRotationPreserveMode(t *testing.T) { } } +func TestFileRotationPreserveModeWithUmask(t *testing.T) { + m := syscall.Umask(0o022) + defer syscall.Umask(m) + + dir, err := os.MkdirTemp("", "caddytest") + if err != nil { + t.Fatalf("failed to create tempdir: %v", err) + } + defer os.RemoveAll(dir) + + fpath := path.Join(dir, "test.log") + + roll := true + mode := fileMode(0o660) + fw := FileWriter{ + Filename: fpath, + Mode: mode, + Roll: &roll, + RollSizeMB: 1, + } + + logger, err := fw.OpenWriter() + if err != nil { + t.Fatalf("failed to create file: %v", err) + } + defer logger.Close() + + b := make([]byte, 1024*1024-1000) + logger.Write(b) + logger.Write(b[0:2000]) + + st, err := os.Stat(fpath) + if err != nil { + t.Fatalf("failed to check file permissions: %v", err) + } + + if got := st.Mode().Perm(); got != os.FileMode(mode) { + t.Errorf("file mode after rotation is %v, want %v", got, mode) + } +} + func TestFileModeConfig(t *testing.T) { tests := []struct { name string From c1918ff1ad6fd411d828c8ea82ad535c5c793219 Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Sun, 26 Apr 2026 23:39:57 +1000 Subject: [PATCH 03/23] httpcaddyfile: inherit global ACME issuer settings in tls shortcuts (#7617) --- caddyconfig/httpcaddyfile/builtins.go | 23 +- caddyconfig/httpcaddyfile/options_test.go | 125 ++++++++++ caddyconfig/httpcaddyfile/tlsapp.go | 283 ++++++++++++++++++++++ 3 files changed, 412 insertions(+), 19 deletions(-) diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go index 6d6b71fa8..311a29e02 100644 --- a/caddyconfig/httpcaddyfile/builtins.go +++ b/caddyconfig/httpcaddyfile/builtins.go @@ -550,26 +550,11 @@ func parseTLS(h Helper) ([]ConfigValue, error) { } case acmeIssuer != nil: - // implicit ACME issuers (from various subdirectives) - use defaults; there might be more than one - defaultIssuers := caddytls.DefaultIssuers(acmeIssuer.Email) - - // if an ACME CA endpoint was set, the user expects to use that specific one, - // not any others that may be defaults, so replace all defaults with that ACME CA - if acmeIssuer.CA != "" { - defaultIssuers = []certmagic.Issuer{acmeIssuer} - } - + // implicit ACME issuers (from various subdirectives) should inherit from + // any globally-configured ACME issuer templates, then apply the local + // shortcut settings as overrides. + defaultIssuers := implicitACMEIssuers(h, acmeIssuer) for _, issuer := range defaultIssuers { - // apply settings from the implicitly-configured ACMEIssuer to any - // default ACMEIssuers, but preserve each default issuer's CA endpoint, - // because, for example, if you configure the DNS challenge, it should - // apply to any of the default ACMEIssuers, but you don't want to trample - // out their unique CA endpoints - if iss, ok := issuer.(*caddytls.ACMEIssuer); ok && iss != nil { - acmeCopy := *acmeIssuer - acmeCopy.CA = iss.CA - issuer = &acmeCopy - } configVals = append(configVals, ConfigValue{ Class: "tls.cert_issuer", Value: issuer, diff --git a/caddyconfig/httpcaddyfile/options_test.go b/caddyconfig/httpcaddyfile/options_test.go index 524187f30..50b431d3e 100644 --- a/caddyconfig/httpcaddyfile/options_test.go +++ b/caddyconfig/httpcaddyfile/options_test.go @@ -3,7 +3,9 @@ package httpcaddyfile import ( "encoding/json" "testing" + "time" + "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/caddyserver/caddy/v2/modules/caddytls" _ "github.com/caddyserver/caddy/v2/modules/logging" @@ -166,3 +168,126 @@ func TestGlobalResolversOption(t *testing.T) { }) } } + +func TestGlobalCertIssuerAppliesToImplicitACMEIssuer(t *testing.T) { + adapter := caddyfile.Adapter{ + ServerType: ServerType{}, + } + + input := `{ + cert_issuer acme { + disable_tlsalpn_challenge + } + } + report.company.intern { + tls { + ca https://deglacme01.company.intern/acme/acme/directory + ca_root /etc/certs/company_root2.crt + } + respond "ok" + }` + + out, _, err := adapter.Adapt([]byte(input), nil) + if err != nil { + t.Fatalf("adapting caddyfile: %v", err) + } + + var config struct { + Apps struct { + TLS *caddytls.TLS `json:"tls"` + } `json:"apps"` + } + if err := json.Unmarshal(out, &config); err != nil { + t.Fatalf("unmarshaling adapted config: %v", err) + } + if config.Apps.TLS == nil || config.Apps.TLS.Automation == nil { + t.Fatal("expected tls automation config") + } + + var subjectPolicy *caddytls.AutomationPolicy + for _, ap := range config.Apps.TLS.Automation.Policies { + if len(ap.SubjectsRaw) == 1 && ap.SubjectsRaw[0] == "report.company.intern" { + subjectPolicy = ap + break + } + } + if subjectPolicy == nil { + t.Fatal("expected subject-specific automation policy") + } + if len(subjectPolicy.IssuersRaw) != 1 { + t.Fatalf("expected one issuer for subject-specific policy, got %d", len(subjectPolicy.IssuersRaw)) + } + + var issuer caddytls.ACMEIssuer + if err := json.Unmarshal(subjectPolicy.IssuersRaw[0], &issuer); err != nil { + t.Fatalf("unmarshaling issuer: %v", err) + } + if issuer.CA != "https://deglacme01.company.intern/acme/acme/directory" { + t.Fatalf("expected custom ACME CA, got %q", issuer.CA) + } + if len(issuer.TrustedRootsPEMFiles) != 1 || issuer.TrustedRootsPEMFiles[0] != "/etc/certs/company_root2.crt" { + t.Fatalf("expected trusted roots to include site CA root, got %v", issuer.TrustedRootsPEMFiles) + } + if issuer.Challenges == nil || issuer.Challenges.TLSALPN == nil || !issuer.Challenges.TLSALPN.Disabled { + t.Fatalf("expected tls-alpn challenge to be disabled, got %#v", issuer.Challenges) + } +} + +func TestMergeACMEIssuers(t *testing.T) { + base := &caddytls.ACMEIssuer{ + Email: "ops@example.com", + Challenges: &caddytls.ChallengesConfig{ + HTTP: &caddytls.HTTPChallengeConfig{ + AlternatePort: 8080, + }, + TLSALPN: &caddytls.TLSALPNChallengeConfig{ + Disabled: true, + AlternatePort: 8443, + }, + DNS: &caddytls.DNSChallengeConfig{ + Resolvers: []string{"1.1.1.1"}, + OverrideDomain: "_acme-challenge.example.net", + }, + }, + TrustedRootsPEMFiles: []string{"global.pem"}, + } + overrides := &caddytls.ACMEIssuer{ + CA: "https://deglacme01.company.intern/acme/acme/directory", + Challenges: &caddytls.ChallengesConfig{ + HTTP: &caddytls.HTTPChallengeConfig{ + Disabled: true, + }, + DNS: &caddytls.DNSChallengeConfig{ + PropagationTimeout: caddy.Duration(time.Minute), + }, + }, + TrustedRootsPEMFiles: []string{"site.pem"}, + } + + merged := mergeACMEIssuers(base, overrides) + if merged.CA != overrides.CA { + t.Fatalf("expected merged CA %q, got %q", overrides.CA, merged.CA) + } + if merged.Email != base.Email { + t.Fatalf("expected merged email %q, got %q", base.Email, merged.Email) + } + if len(merged.TrustedRootsPEMFiles) != 2 || merged.TrustedRootsPEMFiles[0] != "global.pem" || merged.TrustedRootsPEMFiles[1] != "site.pem" { + t.Fatalf("expected merged roots [global.pem site.pem], got %v", merged.TrustedRootsPEMFiles) + } + if merged.Challenges == nil || merged.Challenges.HTTP == nil || !merged.Challenges.HTTP.Disabled || merged.Challenges.HTTP.AlternatePort != 8080 { + t.Fatalf("expected merged HTTP challenge config to preserve alternate port and apply disable flag, got %#v", merged.Challenges) + } + if merged.Challenges.TLSALPN == nil || !merged.Challenges.TLSALPN.Disabled || merged.Challenges.TLSALPN.AlternatePort != 8443 { + t.Fatalf("expected merged TLS-ALPN challenge config to preserve global settings, got %#v", merged.Challenges) + } + if merged.Challenges.DNS == nil || merged.Challenges.DNS.PropagationTimeout != caddy.Duration(time.Minute) || len(merged.Challenges.DNS.Resolvers) != 1 || merged.Challenges.DNS.Resolvers[0] != "1.1.1.1" || merged.Challenges.DNS.OverrideDomain != "_acme-challenge.example.net" { + t.Fatalf("expected merged DNS challenge config to preserve global values and apply overrides, got %#v", merged.Challenges) + } + + if base.CA != "" { + t.Fatalf("expected base issuer to remain unchanged, got CA %q", base.CA) + } + if len(base.TrustedRootsPEMFiles) != 1 || base.TrustedRootsPEMFiles[0] != "global.pem" { + t.Fatalf("expected base roots to remain unchanged, got %v", base.TrustedRootsPEMFiles) + } +} diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go index 22bc22816..7a72cd6fb 100644 --- a/caddyconfig/httpcaddyfile/tlsapp.go +++ b/caddyconfig/httpcaddyfile/tlsapp.go @@ -612,6 +612,289 @@ func fillInGlobalACMEDefaults(issuer certmagic.Issuer, options map[string]any) e return nil } +// implicitACMEIssuers returns the issuers to use for ACME-related tls +// shortcuts such as ca, ca_root, and dns. If any global cert_issuer options +// configure ACME issuers, those become the templates for the local shortcut +// configuration; otherwise, default ACME issuers are used. +func implicitACMEIssuers(h Helper, acmeIssuer *caddytls.ACMEIssuer) []certmagic.Issuer { + globalIssuers, _ := h.Option("cert_issuer").([]certmagic.Issuer) + + var implicitIssuers []certmagic.Issuer + for _, issuer := range globalIssuers { + acmeWrapper, ok := issuer.(acmeCapable) + if !ok { + continue + } + baseIssuer := acmeWrapper.GetACMEIssuer() + if baseIssuer == nil { + continue + } + implicitIssuers = append(implicitIssuers, mergeACMEIssuers(baseIssuer, acmeIssuer)) + } + if len(implicitIssuers) > 0 { + return implicitIssuers + } + + // If an ACME CA endpoint was set locally, the user expects to use only that + // CA rather than the usual default fallback issuers. + defaultIssuers := caddytls.DefaultIssuers(acmeIssuer.Email) + if acmeIssuer.CA != "" { + defaultIssuers = []certmagic.Issuer{new(caddytls.ACMEIssuer)} + } + + implicitIssuers = make([]certmagic.Issuer, 0, len(defaultIssuers)) + for _, issuer := range defaultIssuers { + acmeWrapper, ok := issuer.(acmeCapable) + if !ok { + implicitIssuers = append(implicitIssuers, issuer) + continue + } + baseIssuer := acmeWrapper.GetACMEIssuer() + if baseIssuer == nil { + implicitIssuers = append(implicitIssuers, issuer) + continue + } + implicitIssuers = append(implicitIssuers, mergeACMEIssuers(baseIssuer, acmeIssuer)) + } + return implicitIssuers +} + +func mergeACMEIssuers(base, overrides *caddytls.ACMEIssuer) *caddytls.ACMEIssuer { + if base == nil { + return cloneACMEIssuer(overrides) + } + + merged := cloneACMEIssuer(base) + if overrides == nil { + return merged + } + + if overrides.CA != "" { + merged.CA = overrides.CA + } + if overrides.TestCA != "" { + merged.TestCA = overrides.TestCA + } + if overrides.Email != "" { + merged.Email = overrides.Email + } + if overrides.Profile != "" { + merged.Profile = overrides.Profile + } + if overrides.AccountKey != "" { + merged.AccountKey = overrides.AccountKey + } + if overrides.ExternalAccount != nil { + merged.ExternalAccount = cloneACMEEAB(overrides.ExternalAccount) + } + if overrides.ACMETimeout != 0 { + merged.ACMETimeout = overrides.ACMETimeout + } + if len(overrides.TrustedRootsPEMFiles) > 0 { + merged.TrustedRootsPEMFiles = appendUniqueStrings(merged.TrustedRootsPEMFiles, overrides.TrustedRootsPEMFiles...) + } + if overrides.PreferredChains != nil { + merged.PreferredChains = cloneChainPreference(overrides.PreferredChains) + } + if overrides.CertificateLifetime != 0 { + merged.CertificateLifetime = overrides.CertificateLifetime + } + if len(overrides.NetworkProxyRaw) > 0 { + merged.NetworkProxyRaw = slices.Clone(overrides.NetworkProxyRaw) + } + merged.Challenges = mergeChallengesConfig(merged.Challenges, overrides.Challenges) + + return merged +} + +func mergeChallengesConfig(base, overrides *caddytls.ChallengesConfig) *caddytls.ChallengesConfig { + if base == nil { + return cloneChallengesConfig(overrides) + } + merged := cloneChallengesConfig(base) + if overrides == nil { + return merged + } + + merged.HTTP = mergeHTTPChallengeConfig(merged.HTTP, overrides.HTTP) + merged.TLSALPN = mergeTLSALPNChallengeConfig(merged.TLSALPN, overrides.TLSALPN) + merged.DNS = mergeDNSChallengeConfig(merged.DNS, overrides.DNS) + if overrides.BindHost != "" { + merged.BindHost = overrides.BindHost + } + if overrides.Distributed != nil { + value := *overrides.Distributed + merged.Distributed = &value + } + + return merged +} + +func mergeHTTPChallengeConfig(base, overrides *caddytls.HTTPChallengeConfig) *caddytls.HTTPChallengeConfig { + if base == nil { + return cloneHTTPChallengeConfig(overrides) + } + merged := cloneHTTPChallengeConfig(base) + if overrides == nil { + return merged + } + + if overrides.Disabled { + merged.Disabled = true + } + if overrides.AlternatePort != 0 { + merged.AlternatePort = overrides.AlternatePort + } + + return merged +} + +func mergeTLSALPNChallengeConfig(base, overrides *caddytls.TLSALPNChallengeConfig) *caddytls.TLSALPNChallengeConfig { + if base == nil { + return cloneTLSALPNChallengeConfig(overrides) + } + merged := cloneTLSALPNChallengeConfig(base) + if overrides == nil { + return merged + } + + if overrides.Disabled { + merged.Disabled = true + } + if overrides.AlternatePort != 0 { + merged.AlternatePort = overrides.AlternatePort + } + + return merged +} + +func mergeDNSChallengeConfig(base, overrides *caddytls.DNSChallengeConfig) *caddytls.DNSChallengeConfig { + if base == nil { + return cloneDNSChallengeConfig(overrides) + } + merged := cloneDNSChallengeConfig(base) + if overrides == nil { + return merged + } + + if len(overrides.ProviderRaw) > 0 { + merged.ProviderRaw = slices.Clone(overrides.ProviderRaw) + } + if overrides.PropagationDelay != 0 { + merged.PropagationDelay = overrides.PropagationDelay + } + if overrides.PropagationTimeout != 0 { + merged.PropagationTimeout = overrides.PropagationTimeout + } + if overrides.Resolvers != nil { + merged.Resolvers = slices.Clone(overrides.Resolvers) + } + if overrides.OverrideDomain != "" { + merged.OverrideDomain = overrides.OverrideDomain + } + if overrides.TTL != 0 { + merged.TTL = overrides.TTL + } + + return merged +} + +func cloneACMEIssuer(iss *caddytls.ACMEIssuer) *caddytls.ACMEIssuer { + if iss == nil { + return nil + } + + cloned := *iss + cloned.Challenges = cloneChallengesConfig(iss.Challenges) + cloned.ExternalAccount = cloneACMEEAB(iss.ExternalAccount) + cloned.TrustedRootsPEMFiles = slices.Clone(iss.TrustedRootsPEMFiles) + cloned.PreferredChains = cloneChainPreference(iss.PreferredChains) + cloned.NetworkProxyRaw = slices.Clone(iss.NetworkProxyRaw) + + return &cloned +} + +func cloneChallengesConfig(cfg *caddytls.ChallengesConfig) *caddytls.ChallengesConfig { + if cfg == nil { + return nil + } + + cloned := *cfg + cloned.HTTP = cloneHTTPChallengeConfig(cfg.HTTP) + cloned.TLSALPN = cloneTLSALPNChallengeConfig(cfg.TLSALPN) + cloned.DNS = cloneDNSChallengeConfig(cfg.DNS) + if cfg.Distributed != nil { + value := *cfg.Distributed + cloned.Distributed = &value + } + + return &cloned +} + +func cloneHTTPChallengeConfig(cfg *caddytls.HTTPChallengeConfig) *caddytls.HTTPChallengeConfig { + if cfg == nil { + return nil + } + + cloned := *cfg + return &cloned +} + +func cloneTLSALPNChallengeConfig(cfg *caddytls.TLSALPNChallengeConfig) *caddytls.TLSALPNChallengeConfig { + if cfg == nil { + return nil + } + + cloned := *cfg + return &cloned +} + +func cloneDNSChallengeConfig(cfg *caddytls.DNSChallengeConfig) *caddytls.DNSChallengeConfig { + if cfg == nil { + return nil + } + + cloned := *cfg + cloned.ProviderRaw = slices.Clone(cfg.ProviderRaw) + cloned.Resolvers = slices.Clone(cfg.Resolvers) + + return &cloned +} + +func cloneACMEEAB(eab *acme.EAB) *acme.EAB { + if eab == nil { + return nil + } + + cloned := *eab + return &cloned +} + +func cloneChainPreference(pref *caddytls.ChainPreference) *caddytls.ChainPreference { + if pref == nil { + return nil + } + + cloned := *pref + cloned.RootCommonName = slices.Clone(pref.RootCommonName) + cloned.AnyCommonName = slices.Clone(pref.AnyCommonName) + if pref.Smallest != nil { + value := *pref.Smallest + cloned.Smallest = &value + } + + return &cloned +} + +func appendUniqueStrings(existing []string, additions ...string) []string { + for _, value := range additions { + if !slices.Contains(existing, value) { + existing = append(existing, value) + } + } + return existing +} + // newBaseAutomationPolicy returns a new TLS automation policy that gets // its values from the global options map. It should be used as the base // for any other automation policies. A nil policy (and no error) will be From c653e7d61a24ce854d3ff6410dc8ea19ffd04131 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 26 Apr 2026 23:51:26 +1000 Subject: [PATCH 04/23] build(deps): bump github.com/jackc/pgx/v5 from 5.9.0 to 5.9.2 (#7668) Bumps [github.com/jackc/pgx/v5](https://github.com/jackc/pgx) from 5.9.0 to 5.9.2. - [Changelog](https://github.com/jackc/pgx/blob/master/CHANGELOG.md) - [Commits](https://github.com/jackc/pgx/compare/v5.9.0...v5.9.2) --- updated-dependencies: - dependency-name: github.com/jackc/pgx/v5 dependency-version: 5.9.2 dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 93b73f3f6..b00a03c2b 100644 --- a/go.mod +++ b/go.mod @@ -72,7 +72,7 @@ require ( github.com/googleapis/enterprise-certificate-proxy v0.3.14 // indirect github.com/googleapis/gax-go/v2 v2.18.0 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0 // indirect - github.com/jackc/pgx/v5 v5.9.0 // indirect + github.com/jackc/pgx/v5 v5.9.2 // indirect github.com/jackc/puddle/v2 v2.2.2 // indirect github.com/kylelemons/godebug v1.1.0 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect diff --git a/go.sum b/go.sum index 50fb903b3..5cae77fe0 100644 --- a/go.sum +++ b/go.sum @@ -205,8 +205,8 @@ github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsI github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg= github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7UlwOQYpKFhBabPMi4aNAfoODPEFNiAnClxo= github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM= -github.com/jackc/pgx/v5 v5.9.0 h1:T/dI+2TvmI2H8s/KH1/lXIbz1CUFk3gn5oTjr0/mBsE= -github.com/jackc/pgx/v5 v5.9.0/go.mod h1:mal1tBGAFfLHvZzaYh77YS/eC6IX9OWbRV1QIIM0Jn4= +github.com/jackc/pgx/v5 v5.9.2 h1:3ZhOzMWnR4yJ+RW1XImIPsD1aNSz4T4fyP7zlQb56hw= +github.com/jackc/pgx/v5 v5.9.2/go.mod h1:mal1tBGAFfLHvZzaYh77YS/eC6IX9OWbRV1QIIM0Jn4= github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo= github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4= github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI= From 2d332714829aa3b530424d47ae092074b09a2268 Mon Sep 17 00:00:00 2001 From: Amemoyoi Date: Mon, 27 Apr 2026 23:43:39 +0900 Subject: [PATCH 05/23] admin: require path segment boundary in remote access control (#7673) --- admin.go | 19 ++++++++-- admin_test.go | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 3 deletions(-) diff --git a/admin.go b/admin.go index 766d8506a..fcf8ab6cb 100644 --- a/admin.go +++ b/admin.go @@ -212,8 +212,8 @@ type AdminAccess struct { // AdminPermissions specifies what kinds of requests are allowed // to be made to the admin endpoint. type AdminPermissions struct { - // The API paths allowed. Paths are simple prefix matches. - // Any subpath of the specified paths will be allowed. + // The API paths allowed. A request path must either equal an + // allowed path or be a subpath with a path-segment boundary. Paths []string `json:"paths,omitempty"` // The HTTP methods allowed for the given paths. @@ -718,7 +718,7 @@ func (remote RemoteAdmin) enforceAccessControls(r *http.Request) error { // verify path pathFound := accessPerm.Paths == nil for _, allowedPath := range accessPerm.Paths { - if strings.HasPrefix(r.URL.Path, allowedPath) { + if adminPathAllowed(r.URL.Path, allowedPath) { pathFound = true break } @@ -747,6 +747,19 @@ func (remote RemoteAdmin) enforceAccessControls(r *http.Request) error { } } +func adminPathAllowed(reqPath, allowedPath string) bool { + if allowedPath == "" || allowedPath == "/" { + return strings.HasPrefix(reqPath, allowedPath) + } + if reqPath == allowedPath { + return true + } + if strings.HasSuffix(allowedPath, "/") { + return strings.HasPrefix(reqPath, allowedPath) + } + return strings.HasPrefix(reqPath, allowedPath+"/") +} + func stopAdminServer(srv *http.Server) error { if srv == nil { return fmt.Errorf("no admin server") diff --git a/admin_test.go b/admin_test.go index 3801c301a..db6d6c45a 100644 --- a/admin_test.go +++ b/admin_test.go @@ -16,8 +16,11 @@ package caddy import ( "context" + "crypto" + "crypto/tls" "crypto/x509" "encoding/json" + "errors" "fmt" "maps" "net/http" @@ -53,6 +56,13 @@ var testCfg = []byte(`{ } `) +type testAdminPublicKey string + +func (k testAdminPublicKey) Equal(x crypto.PublicKey) bool { + other, ok := x.(testAdminPublicKey) + return ok && k == other +} + func TestUnsyncedConfigAccess(t *testing.T) { // each test is performed in sequence, so // each change builds on the previous ones; @@ -651,6 +661,99 @@ func TestAllowedOriginsUnixSocket(t *testing.T) { } } +func TestRemoteAdminAccessControlPathSegmentMatching(t *testing.T) { + const authorizedKey testAdminPublicKey = "authorized" + peerCert := &x509.Certificate{PublicKey: authorizedKey} + + tests := []struct { + name string + allowedPath string + requestPath string + wantErr bool + }{ + { + name: "exact path", + allowedPath: "/pki/ca/prod", + requestPath: "/pki/ca/prod", + wantErr: false, + }, + { + name: "subpath", + allowedPath: "/pki/ca/prod", + requestPath: "/pki/ca/prod/certificates", + wantErr: false, + }, + { + name: "trailing slash subpath", + allowedPath: "/pki/ca/prod/", + requestPath: "/pki/ca/prod/certificates", + wantErr: false, + }, + { + name: "sibling with shared prefix", + allowedPath: "/pki/ca/prod", + requestPath: "/pki/ca/prod-backup", + wantErr: true, + }, + { + name: "same segment plus digit", + allowedPath: "/pki/ca/prod", + requestPath: "/pki/ca/prod1", + wantErr: true, + }, + { + name: "root path", + allowedPath: "/", + requestPath: "/pki/ca/prod", + wantErr: false, + }, + } + + for i, test := range tests { + t.Run(test.name, func(t *testing.T) { + remote := RemoteAdmin{ + AccessControl: []*AdminAccess{ + { + Permissions: []AdminPermissions{ + { + Methods: []string{http.MethodGet}, + Paths: []string{test.allowedPath}, + }, + }, + publicKeys: []crypto.PublicKey{authorizedKey}, + }, + }, + } + + req := httptest.NewRequest(http.MethodGet, "https://localhost:2021"+test.requestPath, nil) + req.TLS = &tls.ConnectionState{ + VerifiedChains: [][]*x509.Certificate{{peerCert}}, + } + + err := remote.enforceAccessControls(req) + if test.wantErr { + if err == nil { + t.Errorf("test %d (%s): allowed path %q, request path %q: expected forbidden error, got nil", i, test.name, test.allowedPath, test.requestPath) + return + } + var apiErr APIError + if !errors.As(err, &apiErr) { + t.Errorf("test %d (%s): allowed path %q, request path %q: expected APIError with HTTP status %d, got %T: %v", i, test.name, test.allowedPath, test.requestPath, http.StatusForbidden, err, err) + return + } + if apiErr.HTTPStatus != http.StatusForbidden { + t.Errorf("test %d (%s): allowed path %q, request path %q: expected HTTP status %d, got %d", i, test.name, test.allowedPath, test.requestPath, http.StatusForbidden, apiErr.HTTPStatus) + } + return + } + + if err != nil { + t.Errorf("test %d (%s): allowed path %q, request path %q: expected no error, got %v", i, test.name, test.allowedPath, test.requestPath, err) + } + }) + } +} + func TestReplaceRemoteAdminServer(t *testing.T) { const testCert = `MIIDCTCCAfGgAwIBAgIUXsqJ1mY8pKlHQtI3HJ23x2eZPqwwDQYJKoZIhvcNAQEL BQAwFDESMBAGA1UEAwwJbG9jYWxob3N0MB4XDTIzMDEwMTAwMDAwMFoXDTI0MDEw From 4d6945769d205f2a60acf13eefb5af0a2eb428fc Mon Sep 17 00:00:00 2001 From: Matt Holt Date: Tue, 28 Apr 2026 09:16:18 -0600 Subject: [PATCH 06/23] reverseproxy: Add ability to clear dynamic upstreams cache during retries (#7662) * reverseproxy: Add ability to clear dynamic upstreams cache during retries This is an optional interface for dynamic upstream modules to implement if they cache results. TODO: More documentation; this is an experiment. * Add some godoc * Export interface; update godoc --- .../caddyhttp/reverseproxy/reverseproxy.go | 29 +++++++++++++++++++ modules/caddyhttp/reverseproxy/upstreams.go | 21 +++++++++++--- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 52d2b1ab3..cefe645ee 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -574,6 +574,17 @@ func (h *Handler) proxyLoopIteration(r *http.Request, origReq *http.Request, w h // get the updated list of upstreams upstreams := h.Upstreams if h.DynamicUpstreams != nil { + if retries > 0 { + // after a failure (and thus during a retry), give dynamic upstream modules an opportunity + // to purge their relevant cache entries so we don't keep retrying bad upstreams + if cachingDynamicUpstreams, ok := h.DynamicUpstreams.(CachingUpstreamSource); ok { + if err := cachingDynamicUpstreams.ResetCache(r); err != nil { + if c := h.logger.Check(zapcore.ErrorLevel, "failed clearing dynamic upstream source's cache"); c != nil { + c.Write(zap.Error(err)) + } + } + } + } dUpstreams, err := h.DynamicUpstreams.GetUpstreams(r) if err != nil { if c := h.logger.Check(zapcore.ErrorLevel, "failed getting dynamic upstreams; falling back to static upstreams"); c != nil { @@ -1640,10 +1651,28 @@ type Selector interface { // may be called during each retry, multiple times per request, and as // such, needs to be instantaneous. The returned slice will not be // modified. +// +// For upstream sources that cache results, implement the +// [CachingUpstreamSource] interface for optimal performance. type UpstreamSource interface { GetUpstreams(*http.Request) ([]*Upstream, error) } +// CachingUpstreamSource is an upstream source that caches its upstreams. +// The relevant cache entry can be cleared/reset for a given request during +// retries if a request fails. This can help ensure that failing backends +// are not retried. +// +// EXPERIMENTAL: Subject to change. +type CachingUpstreamSource interface { + UpstreamSource + + // ResetCache clears any cache entry related to the given request. + // The next time GetUpstreams is called, it should have new upstream + // information for the given request. + ResetCache(*http.Request) error +} + // Hop-by-hop headers. These are removed when sent to the backend. // As of RFC 7230, hop-by-hop headers are required to appear in the // Connection header field. These are the headers defined by the diff --git a/modules/caddyhttp/reverseproxy/upstreams.go b/modules/caddyhttp/reverseproxy/upstreams.go index e9120725a..f7077ce78 100644 --- a/modules/caddyhttp/reverseproxy/upstreams.go +++ b/modules/caddyhttp/reverseproxy/upstreams.go @@ -119,6 +119,18 @@ func (su *SRVUpstreams) Provision(ctx caddy.Context) error { return nil } +func (su *SRVUpstreams) ResetCache(r *http.Request) error { + srvsMu.Lock() + if r == nil { + srvs = make(map[string]srvLookup) + } else { + suAddr, _, _, _ := su.expandedAddr(r) + delete(srvs, suAddr) + } + srvsMu.Unlock() + return nil +} + func (su SRVUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) { suAddr, service, proto, name := su.expandedAddr(r) @@ -554,8 +566,9 @@ var ( // Interface guards var ( - _ caddy.Provisioner = (*SRVUpstreams)(nil) - _ UpstreamSource = (*SRVUpstreams)(nil) - _ caddy.Provisioner = (*AUpstreams)(nil) - _ UpstreamSource = (*AUpstreams)(nil) + _ caddy.Provisioner = (*SRVUpstreams)(nil) + _ UpstreamSource = (*SRVUpstreams)(nil) + _ CachingUpstreamSource = (*SRVUpstreams)(nil) + _ caddy.Provisioner = (*AUpstreams)(nil) + _ UpstreamSource = (*AUpstreams)(nil) ) From 6a64bb2ce55b9a0aaf89f0372a7381aefdebd530 Mon Sep 17 00:00:00 2001 From: mfrischknecht Date: Wed, 29 Apr 2026 13:52:04 +0200 Subject: [PATCH 07/23] listeners: clean up stale Unix socket files on Windows (#7676) * Delete old unix domain socket files on Windows While Windows doesn't have the need to reuse a socket file descriptor by dup()ing it on config reloads, there still is a valid need for an equivalent to the `syscall.Unlink()` call in listen_unix.go (also in `reuseUnixSocket`). If a previous Caddy instance didn't terminate properly, the chances it will leave behind a socket file are very high, breaking all subsequent starting attempts. Other than for regular files, Windows seemingly has no way for a process to flag a UNIX domain socket file with `FILE_DELETE_ON_CLOSE`, which means this scenario can never be avoided entirely (e.g. in the case of crashes). For the long comment on `isAbstractUnixSocket`: the logic itself is likely of dubious value, but I thought it better to explicitly reference the issue, as I have just spent half an hour searching the web to figure out whether abstract names will work or not on Windows. At least, the logic as-is should now do the sensible thing if these are ever implemented properly (and it matches what the Golang standard library does internally). * Add a dial attempt to check for active server processes As @steadytao pointed out (thanks!), the previous code didn't have solid proof that an existing unix socket file had really been orphaned, as it's also possible that there's another server process (still running). This would still give the Windows implementation parity with the unix one (as that one also unlinks the socket file without further checks), but I've performed a couple of small tests and found this way of handling socket files still problematic at least problematic if Caddy is used as a reverse proxy in real world scenarios. In tests with a simple Caddyfile that only declares an admin socket, starting two caddy instances with the same Caddyfile works and behaves like one would expect: the second instance removes the first instance's socket file and "wins" the race. When Caddy is used as a reverse proxy, though, what'll happen is more complicated: While the second instance wins the race for the admin socket, as long as the Caddyfile specifies a TCP downstream socket, the second process will not be able to take this one over from the first (also to be expected, that's how socket binding usually works). This results in a rather broken state: The first process still holds on to its TCP listening sockets, the second process fails to start because of the error in its listening attempt, leaving an orphaned admin socket file in the file system. Afterwards, the second process won't be running and the first _will_ be running but unable to be controlled because its admin socket has been replaced. This leaves the system in another state that is bad from an ops perspective. With this new change, we try first to connect to any unix socket that isn't already covered by our current process (with a very low timeout) and can easily decide if the socket is still in use by another process: - If the connection is accepted, there's obviously a server process. - If Windows returns WSACONNREFUSED [^1], there is either no active server process for the socket file anymore, or the socket file does not exist. - Any other errors are likely a sign that there still is a server process (e.g. a timeout would indicate that it's just slow in accepting new connection attempts). [^1]: https://learn.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2#wsaeconnrefused * chore: tidy Windows unix socket reuse helper --------- Co-authored-by: Zen Dodd --- listen.go | 4 -- listen_reuseUnixSocket.go | 21 ++++++++ listen_reuseUnixSocket_windows.go | 89 +++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 listen_reuseUnixSocket.go create mode 100644 listen_reuseUnixSocket_windows.go diff --git a/listen.go b/listen.go index 03b63c1e2..21df13ff4 100644 --- a/listen.go +++ b/listen.go @@ -30,10 +30,6 @@ import ( "go.uber.org/zap" ) -func reuseUnixSocket(_, _ string) (any, error) { - return nil, nil -} - func listenReusable(ctx context.Context, lnKey string, network, address string, config net.ListenConfig) (any, error) { var socketFile *os.File diff --git a/listen_reuseUnixSocket.go b/listen_reuseUnixSocket.go new file mode 100644 index 000000000..006610edc --- /dev/null +++ b/listen_reuseUnixSocket.go @@ -0,0 +1,21 @@ +// Copyright 2015 Matthew Holt and The Caddy Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build (!unix || solaris) && !windows + +package caddy + +func reuseUnixSocket(_, _ string) (any, error) { + return nil, nil +} diff --git a/listen_reuseUnixSocket_windows.go b/listen_reuseUnixSocket_windows.go new file mode 100644 index 000000000..9c547933e --- /dev/null +++ b/listen_reuseUnixSocket_windows.go @@ -0,0 +1,89 @@ +// Copyright 2015 Matthew Holt and The Caddy Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +package caddy + +import ( + "errors" + "fmt" + "io/fs" + "net" + "os" + "strings" + "syscall" + "time" +) + +var errUnixSocketAlreadyInUse = errors.New("unix socket is already in use by another process") + +func reuseUnixSocket(network, addr string) (any, error) { + if !IsUnixNetwork(network) { + return nil, nil + } + + // Note: This is here mainly for proper compatibility, because Unix sockets with abstract names are in an interesting limbo state on Windows: + // Go already translates `@` characters to `\0` for Windows: https://github.com/golang/go/blob/65d5c5f6dd8aa7b221cff6ec3f5101ea2e5f3efa/src/syscall/syscall_windows.go#L910 + // ...but there still is an open issue about the fact that this is not properly supported: https://github.com/microsoft/WSL/issues/4240#issuecomment-620805115 + // The main issue is that the original announcement proclaimed support for this feature, but it was (apparently) never implemented: https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/ + isAbstractUnixSocket := strings.HasPrefix(addr, "@") + + if isAbstractUnixSocket { + // Abstract Unix sockets do not require us to remove stale socket files. + return nil, nil + } + + // On Windows, we're using the `fakeCloseListener` wrappers around a single, ever-living listener. + // So, if there's an active listener entry in the pool, we're the current owner of the Unix socket file. + _, socketBelongsToCurrentProcess := listenerPool.References(listenerKey(network, addr)) + + if socketBelongsToCurrentProcess { + // Reuse/cleanup is entirely handled by the refcounting mechanism in `listenerPool`. + return nil, nil + } + + // If the socket file does not exist or has no backing server process, this will fail instantly. + connection, err := net.DialTimeout("unix", addr, 10*time.Millisecond) + + if err == nil { + connection.Close() + return nil, fmt.Errorf("cannot reuse socket %v: %w", addr, errUnixSocketAlreadyInUse) + } + + // Windows returns this error code both if the socket file does not exist and if it isn't backed by a server process anymore. + // See: https://learn.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2#wsaeconnrefused + const WSAECONNREFUSED syscall.Errno = 10061 + + var errno syscall.Errno + hasNoListeningServerProcess := errors.As(err, &errno) && errno == WSAECONNREFUSED + + if !hasNoListeningServerProcess { + return nil, fmt.Errorf("cannot reuse socket %v: %w", addr, errUnixSocketAlreadyInUse) + } + + // If the socket file exists, it hasn't been created by our process, and it seemingly + // isn't backed by a server process anymore. Try to delete it so we can bind to it later. + err = os.Remove(addr) + + if err == nil { + return nil, nil + } else if errors.Is(err, fs.ErrNotExist) { + // Either the file didn't exist in the first place, or it was deleted before we were able to. + return nil, nil + } else { + // We failed to delete the file. Likely, it belongs to another (active) process. + return nil, err + } +} From 18ab0f955fc1075d7727c7658dbfb734c673a5c9 Mon Sep 17 00:00:00 2001 From: Amemoyoi Date: Fri, 1 May 2026 00:39:57 +0900 Subject: [PATCH 08/23] admin: reject non-canonical config array indices (#7592) * admin: reject non-canonical config array indices * admin: expand canonical array index test coverage * Update admin.go Co-authored-by: Matt Holt * Update admin.go Co-authored-by: Matt Holt * admin: improve canonical array index test diagnostics --------- Co-authored-by: Matt Holt --- admin.go | 19 +++++++++++++++++-- admin_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/admin.go b/admin.go index fcf8ab6cb..08465a923 100644 --- a/admin.go +++ b/admin.go @@ -1161,6 +1161,20 @@ func handleStop(w http.ResponseWriter, r *http.Request) error { return nil } +func parseCanonicalArrayIndex(idx string) (int, error) { + if idx == "" { + return 0, fmt.Errorf("empty index") + } + i, err := strconv.Atoi(idx) + if err != nil { + return 0, err + } + if strconv.Itoa(i) != idx { + return 0, fmt.Errorf("non-canonical array index") + } + return i, nil +} + // unsyncedConfigAccess traverses into the current config and performs // the operation at path according to method, using body and out as // needed. This is a low-level, unsynchronized function; most callers @@ -1222,11 +1236,12 @@ traverseLoop: var idx int if method != http.MethodPost { idxStr := parts[len(parts)-1] - idx, err = strconv.Atoi(idxStr) + idx, err = parseCanonicalArrayIndex(idxStr) if err != nil { return fmt.Errorf("[%s] invalid array index '%s': %v", path, idxStr, err) } + if idx < 0 || (method != http.MethodPut && idx >= len(arr)) || idx > len(arr) { return fmt.Errorf("[%s] array index out of bounds: %s", path, idxStr) } @@ -1326,7 +1341,7 @@ traverseLoop: } case []any: - partInt, err := strconv.Atoi(part) + partInt, err := parseCanonicalArrayIndex(part) if err != nil { return fmt.Errorf("[/%s] invalid array index '%s': %v", strings.Join(parts[:i+1], "/"), part, err) diff --git a/admin_test.go b/admin_test.go index db6d6c45a..19cc6ae7c 100644 --- a/admin_test.go +++ b/admin_test.go @@ -15,6 +15,7 @@ package caddy import ( + "bytes" "context" "crypto" "crypto/tls" @@ -1106,3 +1107,47 @@ MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDRS0LmTwUT0iwP }) } } + +func TestUnsyncedConfigAccessCanonicalArrayIndices(t *testing.T) { + rawCfg = map[string]any{ + rawConfigKey: map[string]any{ + "list": []any{"zero", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten"}, + }, + } + + tests := []struct { + name string + path string + wantOutput string + wantErr bool + }{ + {name: "allow zero", path: "/" + rawConfigKey + "/list/0", wantOutput: "\"zero\"\n"}, + {name: "allow one", path: "/" + rawConfigKey + "/list/1", wantOutput: "\"one\"\n"}, + {name: "allow ten", path: "/" + rawConfigKey + "/list/10", wantOutput: "\"ten\"\n"}, + {name: "reject leading zero", path: "/" + rawConfigKey + "/list/01", wantErr: true}, + {name: "reject multiple leading zeros", path: "/" + rawConfigKey + "/list/002", wantErr: true}, + {name: "reject plus sign", path: "/" + rawConfigKey + "/list/+1", wantErr: true}, + {name: "reject negative zero", path: "/" + rawConfigKey + "/list/-0", wantErr: true}, + } + + for i, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var gotOutput bytes.Buffer + err := unsyncedConfigAccess(http.MethodGet, tc.path, nil, &gotOutput) + + if tc.wantErr { + if err == nil { + t.Errorf("test %d (%s): input path %q: expected error, got nil with output %q", i, tc.name, tc.path, gotOutput.String()) + } + return + } + + if err != nil { + t.Errorf("test %d (%s): input path %q: expected no error with output %q, got error %v with output %q", i, tc.name, tc.path, tc.wantOutput, err, gotOutput.String()) + } + if gotOutput.String() != tc.wantOutput { + t.Errorf("test %d (%s): input path %q: expected output %q, got %q", i, tc.name, tc.path, tc.wantOutput, gotOutput.String()) + } + }) + } +} From ef496e58ef9e7844cf8f1831030713ee5e9b354b Mon Sep 17 00:00:00 2001 From: Felix Eckhofer Date: Sat, 2 May 2026 23:13:57 +0200 Subject: [PATCH 09/23] caddytls: Expand ACME credentials (#7554) * caddytls: Expand ACME credentials This allows using global placeholders such as {file./run/secrets/key_id} when setting up the tls configuration. * chore(formatting): gofmt on acmeissuer_test --- modules/caddytls/acmeissuer.go | 36 ++++++++++++++++++++++++ modules/caddytls/acmeissuer_test.go | 43 +++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 modules/caddytls/acmeissuer_test.go diff --git a/modules/caddytls/acmeissuer.go b/modules/caddytls/acmeissuer.go index b511346b5..193c8bfc7 100644 --- a/modules/caddytls/acmeissuer.go +++ b/modules/caddytls/acmeissuer.go @@ -140,6 +140,42 @@ func (iss *ACMEIssuer) Provision(ctx caddy.Context) error { iss.Email = email } + // expand CA endpoint, if non-empty + if iss.CA != "" { + ca, err := repl.ReplaceOrErr(iss.CA, true, true) + if err != nil { + return fmt.Errorf("expanding CA endpoint '%s': %v", iss.CA, err) + } + iss.CA = ca + } + + // expand TestCA endpoint, if non-empty + if iss.TestCA != "" { + testca, err := repl.ReplaceOrErr(iss.TestCA, true, true) + if err != nil { + return fmt.Errorf("expanding TestCA endpoint '%s': %v", iss.TestCA, err) + } + iss.TestCA = testca + } + + // expand EAB credentials, if non-empty + if iss.ExternalAccount != nil { + if iss.ExternalAccount.KeyID != "" { + keyID, err := repl.ReplaceOrErr(iss.ExternalAccount.KeyID, true, true) + if err != nil { + return fmt.Errorf("expanding EAB key ID '%s': %v", iss.ExternalAccount.KeyID, err) + } + iss.ExternalAccount.KeyID = keyID + } + if iss.ExternalAccount.MACKey != "" { + macKey, err := repl.ReplaceOrErr(iss.ExternalAccount.MACKey, true, true) + if err != nil { + return fmt.Errorf("expanding EAB MAC key (redacted): %v", err) + } + iss.ExternalAccount.MACKey = macKey + } + } + // expand account key, if non-empty if iss.AccountKey != "" { accountKey, err := repl.ReplaceOrErr(iss.AccountKey, true, true) diff --git a/modules/caddytls/acmeissuer_test.go b/modules/caddytls/acmeissuer_test.go new file mode 100644 index 000000000..661f7b9e5 --- /dev/null +++ b/modules/caddytls/acmeissuer_test.go @@ -0,0 +1,43 @@ +package caddytls + +import ( + "github.com/caddyserver/caddy/v2" + "github.com/mholt/acmez/v3/acme" + "testing" +) + +func TestACMEIssuerExpandPlaceholders(t *testing.T) { + t.Setenv("CADDY_TEST_CA_URL", "https://acme.example.com/directory") + t.Setenv("CADDY_TEST_TEST_CA_URL", "https://acme2.example.com/directory") + t.Setenv("CADDY_TEST_EAB_KEY_ID", "example-key-id") + t.Setenv("CADDY_TEST_EAB_MAC_KEY", "example-mac-key") + + caddyCtx, cancel := caddy.NewContext(caddy.Context{Context: t.Context()}) + defer cancel() + + iss := &ACMEIssuer{ + CA: "{env.CADDY_TEST_CA_URL}", + TestCA: "{env.CADDY_TEST_TEST_CA_URL}", + ExternalAccount: &acme.EAB{ + KeyID: "{env.CADDY_TEST_EAB_KEY_ID}", + MACKey: "{env.CADDY_TEST_EAB_MAC_KEY}", + }, + } + + if err := iss.Provision(caddyCtx); err != nil { + t.Fatalf("Provision() returned unexpected error: %v", err) + } + + if want := "https://acme.example.com/directory"; iss.CA != want { + t.Errorf("CA: got %q, want %q", iss.CA, want) + } + if want := "https://acme2.example.com/directory"; iss.TestCA != want { + t.Errorf("TestCA: got %q, want %q", iss.TestCA, want) + } + if want := "example-key-id"; iss.ExternalAccount.KeyID != want { + t.Errorf("ExternalAccount.KeyID: got %q, want %q", iss.ExternalAccount.KeyID, want) + } + if want := "example-mac-key"; iss.ExternalAccount.MACKey != want { + t.Errorf("ExternalAccount.MACKey: got %q, want %q", iss.ExternalAccount.MACKey, want) + } +} From 7e77eec0ae824d86f71f410acd78a0f938baeda7 Mon Sep 17 00:00:00 2001 From: Rayan Salhab Date: Sun, 3 May 2026 06:40:11 +0300 Subject: [PATCH 10/23] caddyauth: set user placeholders before auth rejection (#7685) * caddyauth: set user placeholders before auth rejection * docs: update auth placeholder comment --- modules/caddyhttp/caddyauth/caddyauth.go | 15 ++-- modules/caddyhttp/caddyauth/caddyauth_test.go | 82 +++++++++++++++++++ 2 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 modules/caddyhttp/caddyauth/caddyauth_test.go diff --git a/modules/caddyhttp/caddyauth/caddyauth.go b/modules/caddyhttp/caddyauth/caddyauth.go index 792c198ee..de8e13f89 100644 --- a/modules/caddyhttp/caddyauth/caddyauth.go +++ b/modules/caddyhttp/caddyauth/caddyauth.go @@ -32,10 +32,10 @@ func init() { // Authentication is a middleware which provides user authentication. // Rejects requests with HTTP 401 if the request is not authenticated. // -// After a successful authentication, the placeholder +// When an authentication provider returns user information, the placeholder // `{http.auth.user.id}` will be set to the username, and also // `{http.auth.user.*}` placeholders may be set for any authentication -// modules that provide user metadata. +// modules that provide user metadata, even if authentication is rejected. // // In case of an error, the placeholder `{http.auth..error}` // will be set to the error message returned by the authentication @@ -91,6 +91,12 @@ func (a Authentication) ServeHTTP(w http.ResponseWriter, r *http.Request, next c repl.Set("http.auth."+provName+".error", err.Error()) continue } + if authed || user.ID != "" || len(user.Metadata) > 0 { + repl.Set("http.auth.user.id", user.ID) + for k, v := range user.Metadata { + repl.Set("http.auth.user."+k, v) + } + } if authed { break } @@ -99,11 +105,6 @@ func (a Authentication) ServeHTTP(w http.ResponseWriter, r *http.Request, next c return caddyhttp.Error(http.StatusUnauthorized, fmt.Errorf("not authenticated")) } - repl.Set("http.auth.user.id", user.ID) - for k, v := range user.Metadata { - repl.Set("http.auth.user."+k, v) - } - return next.ServeHTTP(w, r) } diff --git a/modules/caddyhttp/caddyauth/caddyauth_test.go b/modules/caddyhttp/caddyauth/caddyauth_test.go new file mode 100644 index 000000000..1f7211137 --- /dev/null +++ b/modules/caddyhttp/caddyauth/caddyauth_test.go @@ -0,0 +1,82 @@ +// Copyright 2015 Matthew Holt and The Caddy Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package caddyauth + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" +) + +func TestAuthenticationSetsUserPlaceholdersOnUnauthorized(t *testing.T) { + repl := caddy.NewReplacer() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req = req.WithContext(context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl)) + + a := Authentication{ + Providers: map[string]Authenticator{ + "test": staticAuthenticator{ + user: User{ + ID: "alice", + Metadata: map[string]string{ + "role": "admin", + }, + }, + }, + }, + } + + nextCalled := false + err := a.ServeHTTP(httptest.NewRecorder(), req, caddyhttp.HandlerFunc(func(http.ResponseWriter, *http.Request) error { + nextCalled = true + return nil + })) + if err == nil { + t.Fatal("expected unauthorized error") + } + + var handlerErr caddyhttp.HandlerError + if !errors.As(err, &handlerErr) { + t.Fatalf("expected caddyhttp.HandlerError, got %T", err) + } + if handlerErr.StatusCode != http.StatusUnauthorized { + t.Fatalf("expected status %d, got %d", http.StatusUnauthorized, handlerErr.StatusCode) + } + if nextCalled { + t.Fatal("next handler was called") + } + + if got, ok := repl.GetString("http.auth.user.id"); !ok || got != "alice" { + t.Fatalf("expected http.auth.user.id to be alice, got %q (ok=%v)", got, ok) + } + if got, ok := repl.GetString("http.auth.user.role"); !ok || got != "admin" { + t.Fatalf("expected http.auth.user.role to be admin, got %q (ok=%v)", got, ok) + } +} + +type staticAuthenticator struct { + user User + authed bool + err error +} + +func (s staticAuthenticator) Authenticate(http.ResponseWriter, *http.Request) (User, bool, error) { + return s.user, s.authed, s.err +} From c7c9f3108a4200a8099ae41175b8aa356b14109f Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Wed, 6 May 2026 01:12:46 +1000 Subject: [PATCH 11/23] caddyauth: Revert "set user placeholders before auth rejection (#7685)" (#7688) This reverts commit 7e77eec0ae824d86f71f410acd78a0f938baeda7. --- modules/caddyhttp/caddyauth/caddyauth.go | 15 ++-- modules/caddyhttp/caddyauth/caddyauth_test.go | 82 ------------------- 2 files changed, 7 insertions(+), 90 deletions(-) delete mode 100644 modules/caddyhttp/caddyauth/caddyauth_test.go diff --git a/modules/caddyhttp/caddyauth/caddyauth.go b/modules/caddyhttp/caddyauth/caddyauth.go index de8e13f89..792c198ee 100644 --- a/modules/caddyhttp/caddyauth/caddyauth.go +++ b/modules/caddyhttp/caddyauth/caddyauth.go @@ -32,10 +32,10 @@ func init() { // Authentication is a middleware which provides user authentication. // Rejects requests with HTTP 401 if the request is not authenticated. // -// When an authentication provider returns user information, the placeholder +// After a successful authentication, the placeholder // `{http.auth.user.id}` will be set to the username, and also // `{http.auth.user.*}` placeholders may be set for any authentication -// modules that provide user metadata, even if authentication is rejected. +// modules that provide user metadata. // // In case of an error, the placeholder `{http.auth..error}` // will be set to the error message returned by the authentication @@ -91,12 +91,6 @@ func (a Authentication) ServeHTTP(w http.ResponseWriter, r *http.Request, next c repl.Set("http.auth."+provName+".error", err.Error()) continue } - if authed || user.ID != "" || len(user.Metadata) > 0 { - repl.Set("http.auth.user.id", user.ID) - for k, v := range user.Metadata { - repl.Set("http.auth.user."+k, v) - } - } if authed { break } @@ -105,6 +99,11 @@ func (a Authentication) ServeHTTP(w http.ResponseWriter, r *http.Request, next c return caddyhttp.Error(http.StatusUnauthorized, fmt.Errorf("not authenticated")) } + repl.Set("http.auth.user.id", user.ID) + for k, v := range user.Metadata { + repl.Set("http.auth.user."+k, v) + } + return next.ServeHTTP(w, r) } diff --git a/modules/caddyhttp/caddyauth/caddyauth_test.go b/modules/caddyhttp/caddyauth/caddyauth_test.go deleted file mode 100644 index 1f7211137..000000000 --- a/modules/caddyhttp/caddyauth/caddyauth_test.go +++ /dev/null @@ -1,82 +0,0 @@ -// Copyright 2015 Matthew Holt and The Caddy Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package caddyauth - -import ( - "context" - "errors" - "net/http" - "net/http/httptest" - "testing" - - "github.com/caddyserver/caddy/v2" - "github.com/caddyserver/caddy/v2/modules/caddyhttp" -) - -func TestAuthenticationSetsUserPlaceholdersOnUnauthorized(t *testing.T) { - repl := caddy.NewReplacer() - req := httptest.NewRequest(http.MethodGet, "/", nil) - req = req.WithContext(context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl)) - - a := Authentication{ - Providers: map[string]Authenticator{ - "test": staticAuthenticator{ - user: User{ - ID: "alice", - Metadata: map[string]string{ - "role": "admin", - }, - }, - }, - }, - } - - nextCalled := false - err := a.ServeHTTP(httptest.NewRecorder(), req, caddyhttp.HandlerFunc(func(http.ResponseWriter, *http.Request) error { - nextCalled = true - return nil - })) - if err == nil { - t.Fatal("expected unauthorized error") - } - - var handlerErr caddyhttp.HandlerError - if !errors.As(err, &handlerErr) { - t.Fatalf("expected caddyhttp.HandlerError, got %T", err) - } - if handlerErr.StatusCode != http.StatusUnauthorized { - t.Fatalf("expected status %d, got %d", http.StatusUnauthorized, handlerErr.StatusCode) - } - if nextCalled { - t.Fatal("next handler was called") - } - - if got, ok := repl.GetString("http.auth.user.id"); !ok || got != "alice" { - t.Fatalf("expected http.auth.user.id to be alice, got %q (ok=%v)", got, ok) - } - if got, ok := repl.GetString("http.auth.user.role"); !ok || got != "admin" { - t.Fatalf("expected http.auth.user.role to be admin, got %q (ok=%v)", got, ok) - } -} - -type staticAuthenticator struct { - user User - authed bool - err error -} - -func (s staticAuthenticator) Authenticate(http.ResponseWriter, *http.Request) (User, bool, error) { - return s.user, s.authed, s.err -} From d2172bea61414635c55554b42714af94a3c9cefd Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Thu, 7 May 2026 17:40:26 +1000 Subject: [PATCH 12/23] chore: Fix golangci-lint 2.12.1 findings (#7690) --- cmd/packagesfuncs.go | 2 +- context.go | 2 +- modules/caddyhttp/fileserver/browse.go | 16 ++++++++++++++-- modules/caddyhttp/fileserver/staticfiles.go | 2 +- .../caddyhttp/reverseproxy/selectionpolicies.go | 10 ++++++---- modules/caddyhttp/routes.go | 9 +++++---- modules/caddyhttp/server.go | 6 +++--- 7 files changed, 31 insertions(+), 16 deletions(-) diff --git a/cmd/packagesfuncs.go b/cmd/packagesfuncs.go index 4d0ff0680..a26919922 100644 --- a/cmd/packagesfuncs.go +++ b/cmd/packagesfuncs.go @@ -234,7 +234,7 @@ func getModules() (standard, nonstandard, unknown []moduleInfo, err error) { // not sure why), and since New() should return a pointer // value, we need to dereference it first iface := any(modInfo.New()) - if rv := reflect.ValueOf(iface); rv.Kind() == reflect.Ptr { + if rv := reflect.ValueOf(iface); rv.Kind() == reflect.Pointer { iface = reflect.New(reflect.TypeOf(iface).Elem()).Elem().Interface() } modPkgPath := reflect.TypeOf(iface).PkgPath() diff --git a/context.go b/context.go index 980027275..f71d635e2 100644 --- a/context.go +++ b/context.go @@ -378,7 +378,7 @@ func (ctx Context) LoadModuleByID(id string, rawMsg json.RawMessage) (any, error // value must be a pointer for unmarshaling into concrete type, even if // the module's concrete type is a slice or map; New() *should* return // a pointer, otherwise unmarshaling errors or panics will occur - if rv := reflect.ValueOf(val); rv.Kind() != reflect.Ptr { + if rv := reflect.ValueOf(val); rv.Kind() != reflect.Pointer { log.Printf("[WARNING] ModuleInfo.New() for module '%s' did not return a pointer,"+ " so we are using reflection to make a pointer instead; please fix this by"+ " using new(Type) or &Type notation in your module's New() function.", id) diff --git a/modules/caddyhttp/fileserver/browse.go b/modules/caddyhttp/fileserver/browse.go index 304417009..3b97f2ff3 100644 --- a/modules/caddyhttp/fileserver/browse.go +++ b/modules/caddyhttp/fileserver/browse.go @@ -281,7 +281,13 @@ func (fsrv *FileServer) browseApplyQueryParams(w http.ResponseWriter, r *http.Re sortParam = sortCookie.Value } case sortByName, sortByNameDirFirst, sortBySize, sortByTime: - http.SetCookie(w, &http.Cookie{Name: "sort", Value: sortParam, Secure: r.TLS != nil}) + http.SetCookie(w, &http.Cookie{ //nolint:gosec // Secure depends on whether the request itself used TLS + Name: "sort", + Value: sortParam, + Secure: r.TLS != nil, + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + }) } // then figure out the order @@ -292,7 +298,13 @@ func (fsrv *FileServer) browseApplyQueryParams(w http.ResponseWriter, r *http.Re orderParam = orderCookie.Value } case sortOrderAsc, sortOrderDesc: - http.SetCookie(w, &http.Cookie{Name: "order", Value: orderParam, Secure: r.TLS != nil}) + http.SetCookie(w, &http.Cookie{ //nolint:gosec // Secure depends on whether the request itself used TLS + Name: "order", + Value: orderParam, + Secure: r.TLS != nil, + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + }) } // finally, apply the sorting and limiting diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go index dce40302d..507321ad6 100644 --- a/modules/caddyhttp/fileserver/staticfiles.go +++ b/modules/caddyhttp/fileserver/staticfiles.go @@ -785,7 +785,7 @@ func redirect(w http.ResponseWriter, r *http.Request, toPath string) error { if r.URL.RawQuery != "" { toPath += "?" + r.URL.RawQuery } - http.Redirect(w, r, toPath, http.StatusPermanentRedirect) + http.Redirect(w, r, toPath, http.StatusPermanentRedirect) //nolint:gosec // toPath is a same-origin path and leading // is stripped above return nil } diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies.go b/modules/caddyhttp/reverseproxy/selectionpolicies.go index 050a4f671..648edcf76 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies.go @@ -664,10 +664,12 @@ func (s CookieHashSelection) Select(pool UpstreamPool, req *http.Request, w http return upstream } cookie := &http.Cookie{ - Name: s.Name, - Value: sha, - Path: "/", - Secure: false, + Name: s.Name, + Value: sha, + Path: "/", + Secure: false, + HttpOnly: true, + SameSite: http.SameSiteLaxMode, } isProxyHttps := false if trusted, ok := caddyhttp.GetVar(req.Context(), caddyhttp.TrustedProxyVarKey).(bool); ok && trusted { diff --git a/modules/caddyhttp/routes.go b/modules/caddyhttp/routes.go index ce2287488..7cc6dd79d 100644 --- a/modules/caddyhttp/routes.go +++ b/modules/caddyhttp/routes.go @@ -18,6 +18,7 @@ import ( "encoding/json" "fmt" "net/http" + "slices" "strings" "github.com/caddyserver/caddy/v2" @@ -241,8 +242,8 @@ func (routes RouteList) Compile(next Handler) Handler { mid = append(mid, wrapRoute(route)) } stack := next - for i := len(mid) - 1; i >= 0; i-- { - stack = mid[i](stack) + for _, middleware := range slices.Backward(mid) { + stack = middleware(stack) } return stack } @@ -305,8 +306,8 @@ func wrapRoute(route Route) Middleware { } // compile this route's handler stack - for i := len(route.middleware) - 1; i >= 0; i-- { - nextCopy = route.middleware[i](nextCopy) + for _, middleware := range slices.Backward(route.middleware) { + nextCopy = middleware(nextCopy) } // Apply metrics instrumentation once for the entire route, diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go index 3005bc273..9aca53578 100644 --- a/modules/caddyhttp/server.go +++ b/modules/caddyhttp/server.go @@ -1085,11 +1085,11 @@ func strictUntrustedClientIp(r *http.Request, headers []string, trusted []netip. for _, headerName := range headers { parts := strings.Split(strings.Join(r.Header.Values(headerName), ","), ",") - for i := len(parts) - 1; i >= 0; i-- { + for _, part := range slices.Backward(parts) { // Some proxies may retain the port number, so split if possible - host, _, err := net.SplitHostPort(parts[i]) + host, _, err := net.SplitHostPort(part) if err != nil { - host = parts[i] + host = part } // Remove any zone identifier from the IP address From 0780d4489cc7199b87598b15aad3270e851ac138 Mon Sep 17 00:00:00 2001 From: tomholford <16504501+tomholford@users.noreply.github.com> Date: Thu, 7 May 2026 11:32:20 -0700 Subject: [PATCH 13/23] httpcaddyfile: accept duration strings for log sampling interval (#7694) Co-authored-by: tomholford --- caddyconfig/httpcaddyfile/builtins.go | 2 +- caddyconfig/httpcaddyfile/builtins_test.go | 4 ++-- .../caddyfile_adapt/global_options_log_sampling.caddyfiletest | 4 ++-- .../integration/caddyfile_adapt/log_sampling.caddyfiletest | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go index 311a29e02..da231fbd9 100644 --- a/caddyconfig/httpcaddyfile/builtins.go +++ b/caddyconfig/httpcaddyfile/builtins.go @@ -1053,7 +1053,7 @@ func parseLogHelper(h Helper, globalLogNames map[string]struct{}) ([]ConfigValue if !d.NextArg() { return nil, d.ArgErr() } - interval, err := time.ParseDuration(d.Val() + "ns") + interval, err := caddy.ParseDuration(d.Val()) if err != nil { return nil, d.Errf("failed to parse interval: %v", err) } diff --git a/caddyconfig/httpcaddyfile/builtins_test.go b/caddyconfig/httpcaddyfile/builtins_test.go index c23531f22..9cff29039 100644 --- a/caddyconfig/httpcaddyfile/builtins_test.go +++ b/caddyconfig/httpcaddyfile/builtins_test.go @@ -66,14 +66,14 @@ func TestLogDirectiveSyntax(t *testing.T) { input: `:8080 { log { sampling { - interval 2 + interval 2s first 3 thereafter 4 } } } `, - output: `{"logging":{"logs":{"default":{"exclude":["http.log.access.log0"]},"log0":{"sampling":{"interval":2,"first":3,"thereafter":4},"include":["http.log.access.log0"]}}},"apps":{"http":{"servers":{"srv0":{"listen":[":8080"],"logs":{"default_logger_name":"log0"}}}}}}`, + output: `{"logging":{"logs":{"default":{"exclude":["http.log.access.log0"]},"log0":{"sampling":{"interval":2000000000,"first":3,"thereafter":4},"include":["http.log.access.log0"]}}},"apps":{"http":{"servers":{"srv0":{"listen":[":8080"],"logs":{"default_logger_name":"log0"}}}}}}`, expectError: false, }, } { diff --git a/caddytest/integration/caddyfile_adapt/global_options_log_sampling.caddyfiletest b/caddytest/integration/caddyfile_adapt/global_options_log_sampling.caddyfiletest index 12b73b2b7..caa755a02 100644 --- a/caddytest/integration/caddyfile_adapt/global_options_log_sampling.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/global_options_log_sampling.caddyfiletest @@ -1,7 +1,7 @@ { log { sampling { - interval 300 + interval 5m first 50 thereafter 40 } @@ -13,7 +13,7 @@ "logs": { "default": { "sampling": { - "interval": 300, + "interval": 300000000000, "first": 50, "thereafter": 40 } diff --git a/caddytest/integration/caddyfile_adapt/log_sampling.caddyfiletest b/caddytest/integration/caddyfile_adapt/log_sampling.caddyfiletest index b58622572..fcda093a6 100644 --- a/caddytest/integration/caddyfile_adapt/log_sampling.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/log_sampling.caddyfiletest @@ -1,7 +1,7 @@ :80 { log { sampling { - interval 300 + interval 5m first 50 thereafter 40 } @@ -18,7 +18,7 @@ }, "log0": { "sampling": { - "interval": 300, + "interval": 300000000000, "first": 50, "thereafter": 40 }, From fb324331f40782ac7a48d83f591c2bb7615d7eed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Thu, 7 May 2026 21:59:42 +0200 Subject: [PATCH 14/23] Merge commit from fork 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..txt"). - search.IgnoreCase folded fullwidth, mathematical and circled letters onto ASCII, so "/shell.", "/shell.hp", "/shell." 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. --- go.mod | 2 +- .../caddyhttp/reverseproxy/fastcgi/fastcgi.go | 36 +++----- .../reverseproxy/fastcgi/fastcgi_test.go | 87 +++++++++++++++++++ 3 files changed, 101 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index b00a03c2b..bd44d644d 100644 --- a/go.mod +++ b/go.mod @@ -170,7 +170,7 @@ require ( go.uber.org/multierr v1.11.0 // indirect golang.org/x/mod v0.35.0 // indirect golang.org/x/sys v0.43.0 - golang.org/x/text v0.36.0 + golang.org/x/text v0.36.0 // indirect golang.org/x/tools v0.44.0 // indirect google.golang.org/grpc v1.80.0 // indirect google.golang.org/protobuf v1.36.11 // indirect diff --git a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go index c4279d9a0..3e0436062 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go @@ -28,8 +28,6 @@ import ( "go.uber.org/zap" "go.uber.org/zap/zapcore" - "golang.org/x/text/language" - "golang.org/x/text/search" "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/modules/caddyhttp" @@ -418,14 +416,19 @@ func (t Transport) buildEnv(r *http.Request) (envVars, error) { return env, nil } -var splitSearchNonASCII = search.New(language.Und, search.IgnoreCase) - // splitPos returns the index where path should // be split based on t.SplitPath. // // example: if splitPath is [".php"] // "/path/to/script.php/some/path": ("/path/to/script.php", "/some/path") // +// Matching is strictly ASCII case-insensitive. Bytes >= utf8.RuneSelf in path +// never match any split entry: split strings are validated ASCII-only and +// lower-cased in Provision(), so any Unicode equivalence (e.g. fullwidth or +// mathematical letters folding to ASCII) would let an attacker upload a file +// whose name contains such code points and have it served as PHP. See +// FrankenPHP advisories GHSA-3g8v-8r37-cgjm and GHSA-v4h7-cj44-8fc8. +// // Adapted from FrankenPHP's code (copyright 2026 Kévin Dunglas, MIT license) func (t Transport) splitPos(path string) int { // TODO: from v1... @@ -438,31 +441,18 @@ func (t Transport) splitPos(path string) int { pathLen := len(path) - // We are sure that split strings are all ASCII-only and lower-case because of validation and normalization in Provision(). for _, split := range t.SplitPath { splitLen := len(split) + if splitLen == 0 || splitLen > pathLen { + continue + } - for i := range pathLen { - if path[i] >= utf8.RuneSelf { - if _, end := splitSearchNonASCII.IndexString(path, split); end > -1 { - return end - } - - break - } - - if i+splitLen > pathLen { - continue - } - + for i := 0; i <= pathLen-splitLen; i++ { match := true - for j := range splitLen { + for j := 0; j < splitLen; j++ { c := path[i+j] - if c >= utf8.RuneSelf { - if _, end := splitSearchNonASCII.IndexString(path, split); end > -1 { - return end - } + match = false break } diff --git a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi_test.go b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi_test.go index 7097ff790..4977ae998 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi_test.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi_test.go @@ -191,6 +191,65 @@ func TestSplitPos(t *testing.T) { splitPath: []string{".php"}, wantPos: 9, }, + // Regression tests adapted from FrankenPHP advisories + // GHSA-3g8v-8r37-cgjm and GHSA-v4h7-cj44-8fc8: search.IgnoreCase + // matched Unicode equivalents of ASCII letters as ".php", and an + // inner non-ASCII byte path could leave the match flag stale. + { + name: "non-ascii byte after dot must not match", + path: "/PoC-match-unset.¡.txt", + splitPath: []string{".php"}, + wantPos: -1, + }, + { + name: "non-ascii byte mid-extension must not match", + path: "/script.p\xc2\xa1p", + splitPath: []string{".php"}, + wantPos: -1, + }, + { + name: "small full stop ﹒ in extension must not match", + path: "/shell﹒php", + splitPath: []string{".php"}, + wantPos: -1, + }, + { + name: "fullwidth full stop ． in extension must not match", + path: "/shell．php", + splitPath: []string{".php"}, + wantPos: -1, + }, + { + name: "fullwidth p in extension must not match", + path: "/shell.ｐhp", + splitPath: []string{".php"}, + wantPos: -1, + }, + { + name: "circled php must not match", + path: "/shell.ⓟⓗⓟ", + splitPath: []string{".php"}, + wantPos: -1, + }, + { + name: "mathematical sans-serif bold php must not match", + path: "/shell.\U0001D5FD\U0001D5F5\U0001D5FD", + splitPath: []string{".php"}, + wantPos: -1, + }, + { + name: "mathematical script php must not match", + path: "/shell.\U0001D4C5\U0001D4BD\U0001D4C5", + splitPath: []string{".php"}, + wantPos: -1, + }, + { + name: "circled php with later real php still picks the real one", + path: "/shell.ⓟⓗⓟ.anything-after-payload.php", + splitPath: []string{".php"}, + // "/shell." (7) + "ⓟⓗⓟ" (3*3 bytes) + ".anything-after-payload.php" (27) = 43 + wantPos: 43, + }, } for _, tt := range tests { @@ -244,3 +303,31 @@ func TestSplitPosUnicodeSecurityRegression(t *testing.T) { assert.Equal(t, ".txt.php", pathInfo, "path info should be the remainder after first .php") } } + +// TestSplitPosSecurityRegressionUnicodeBypass guards against the FrankenPHP +// advisories GHSA-3g8v-8r37-cgjm (uninitialized match flag on inner non-ASCII +// byte) and GHSA-v4h7-cj44-8fc8 (Unicode equivalence via search.IgnoreCase +// folding fullwidth/mathematical/circled letters onto ASCII). Every payload +// below produced a false positive in the vulnerable implementation; none +// must match here. +func TestSplitPosSecurityRegressionUnicodeBypass(t *testing.T) { + t.Parallel() + + tr := Transport{SplitPath: []string{".php"}} + payloads := []string{ + "/PoC-match-unset.¡.txt", // GHSA-3g8v: stale match=true on IndexString fallback + "/shell﹒php", // U+FE52 small full stop + "/shell．php", // U+FF0E fullwidth full stop + "/shell.ｐhp", // U+FF50 fullwidth p + "/shell.pｈp", // U+FF48 fullwidth h + "/shell.phｐ", // U+FF50 fullwidth p (trailing) + "/shell.\U0001D5C1\U0001D5B5\U0001D5C1", // mathematical sans-serif p/h + "/shell.\U0001D5FD\U0001D5F5\U0001D5FD", // mathematical sans-serif bold p/h + "/shell.\U0001D4C5\U0001D4BD\U0001D4C5", // mathematical script p/h + "/shell.ⓟⓗⓟ", // circled latin small + } + + for _, p := range payloads { + assert.Equalf(t, -1, tr.splitPos(p), "payload %q must not be detected as .php", p) + } +} From 9c78b97f9e79773d45cf3ad0326bfb5480861ac0 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 8 May 2026 10:46:28 -0600 Subject: [PATCH 15/23] fastcgi: Fix lint --- modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go index 3e0436062..f91394e58 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go @@ -449,7 +449,7 @@ func (t Transport) splitPos(path string) int { for i := 0; i <= pathLen-splitLen; i++ { match := true - for j := 0; j < splitLen; j++ { + for j := range splitLen { c := path[i+j] if c >= utf8.RuneSelf { match = false From 5e76b5ee43e8ec9e78d07665cdd75515b4c07acd Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Sun, 10 May 2026 13:10:29 +1000 Subject: [PATCH 16/23] tls: add alpn to managed HTTPS records (#7653) * tls: add alpn to managed HTTPS records * tls: centralise HTTPS RR ALPN defaults and registration Reuse shared protocol defaults instead of repeating the default HTTP protocol list, unify server name registration to carry ALPN in one experimental API and reuse the TLS default ALPN ordering for HTTPS RR publication * http: centralise effective protocol resolution for HTTPS RR ALPN --- modules/caddyhttp/app.go | 34 +-------- modules/caddyhttp/autohttps.go | 16 +++- modules/caddyhttp/autohttps_test.go | 63 ++++++++-------- modules/caddyhttp/server.go | 56 +++++++++++--- modules/caddytls/connpolicy.go | 4 +- modules/caddytls/ech.go | 33 +++++++-- modules/caddytls/ech_dns_test.go | 65 +++++++++++++++++ modules/caddytls/tls.go | 109 ++++++++++++++++++++++++---- 8 files changed, 286 insertions(+), 94 deletions(-) create mode 100644 modules/caddytls/ech_dns_test.go diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go index 571ac496e..bc2b896cd 100644 --- a/modules/caddyhttp/app.go +++ b/modules/caddyhttp/app.go @@ -20,7 +20,6 @@ import ( "crypto/tls" "errors" "fmt" - "maps" "net" "net/http" "strconv" @@ -241,12 +240,7 @@ func (app *App) Provision(ctx caddy.Context) error { // if no protocols configured explicitly, enable all except h2c if len(srv.Protocols) == 0 { - srv.Protocols = []string{"h1", "h2", "h3"} - } - - srvProtocolsUnique := map[string]struct{}{} - for _, srvProtocol := range srv.Protocols { - srvProtocolsUnique[srvProtocol] = struct{}{} + srv.Protocols = srv.protocolsWithDefaults() } if srv.ListenProtocols != nil { @@ -257,31 +251,7 @@ func (app *App) Provision(ctx caddy.Context) error { for i, lnProtocols := range srv.ListenProtocols { if lnProtocols != nil { - // populate empty listen protocols with server protocols - lnProtocolsDefault := false - var lnProtocolsInclude []string - srvProtocolsInclude := maps.Clone(srvProtocolsUnique) - - // keep existing listener protocols unless they are empty - for _, lnProtocol := range lnProtocols { - if lnProtocol == "" { - lnProtocolsDefault = true - } else { - lnProtocolsInclude = append(lnProtocolsInclude, lnProtocol) - delete(srvProtocolsInclude, lnProtocol) - } - } - - // append server protocols to listener protocols if any listener protocols were empty - if lnProtocolsDefault { - for _, srvProtocol := range srv.Protocols { - if _, ok := srvProtocolsInclude[srvProtocol]; ok { - lnProtocolsInclude = append(lnProtocolsInclude, srvProtocol) - } - } - } - - srv.ListenProtocols[i] = lnProtocolsInclude + srv.ListenProtocols[i] = srv.listenerProtocolsWithDefaults(lnProtocols) } } } diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go index 4d9759000..4e5b85f65 100644 --- a/modules/caddyhttp/autohttps.go +++ b/modules/caddyhttp/autohttps.go @@ -173,7 +173,7 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er for d := range serverDomainSet { echDomains = append(echDomains, d) } - app.tlsApp.RegisterServerNames(echDomains) + app.tlsApp.RegisterServerNames(echDomains, httpsRRALPNs(srv)) // nothing more to do here if there are no domains that qualify for // automatic HTTPS and there are no explicit TLS connection policies: @@ -574,6 +574,20 @@ func (app *App) makeRedirRoute(redirToPort uint, matcherSet MatcherSet) Route { } } +func httpsRRALPNs(srv *Server) []string { + alpn := make(map[string]struct{}, 3) + if srv.protocol("h3") { + alpn["h3"] = struct{}{} + } + if srv.protocol("h2") { + alpn["h2"] = struct{}{} + } + if srv.protocol("h1") { + alpn["http/1.1"] = struct{}{} + } + return caddytls.OrderedHTTPSRRALPN(alpn) +} + // createAutomationPolicies ensures that automated certificates for this // app are managed properly. This adds up to two automation policies: // one for the public names, and one for the internal names. If a catch-all diff --git a/modules/caddyhttp/autohttps_test.go b/modules/caddyhttp/autohttps_test.go index b5cc64d94..89843844d 100644 --- a/modules/caddyhttp/autohttps_test.go +++ b/modules/caddyhttp/autohttps_test.go @@ -1,44 +1,47 @@ package caddyhttp import ( + "reflect" "testing" - - "github.com/caddyserver/caddy/v2" ) -func TestRecordAutoHTTPSRedirectAddressPrefersHTTPSPort(t *testing.T) { - app := &App{HTTPSPort: 443} - redirDomains := make(map[string][]caddy.NetworkAddress) +func TestHTTPSRRALPNsDefaultProtocols(t *testing.T) { + srv := &Server{} - app.recordAutoHTTPSRedirectAddress(redirDomains, "example.com", caddy.NetworkAddress{Network: "tcp", StartPort: 2345, EndPort: 2345}) - app.recordAutoHTTPSRedirectAddress(redirDomains, "example.com", caddy.NetworkAddress{Network: "tcp", StartPort: 443, EndPort: 443}) - app.recordAutoHTTPSRedirectAddress(redirDomains, "example.com", caddy.NetworkAddress{Network: "tcp", StartPort: 8443, EndPort: 8443}) + got := httpsRRALPNs(srv) + want := []string{"h3", "h2", "http/1.1"} - got := redirDomains["example.com"] - if len(got) != 1 { - t.Fatalf("expected 1 redirect address, got %d: %#v", len(got), got) - } - if got[0].StartPort != 443 { - t.Fatalf("expected redirect to prefer HTTPS port 443, got %#v", got[0]) + if !reflect.DeepEqual(got, want) { + t.Fatalf("unexpected ALPN values: got %v want %v", got, want) } } -func TestRecordAutoHTTPSRedirectAddressKeepsAllBindAddressesOnWinningPort(t *testing.T) { - app := &App{HTTPSPort: 443} - redirDomains := make(map[string][]caddy.NetworkAddress) - - app.recordAutoHTTPSRedirectAddress(redirDomains, "example.com", caddy.NetworkAddress{Network: "tcp", Host: "10.0.0.189", StartPort: 8443, EndPort: 8443}) - app.recordAutoHTTPSRedirectAddress(redirDomains, "example.com", caddy.NetworkAddress{Network: "tcp", Host: "10.0.0.189", StartPort: 443, EndPort: 443}) - app.recordAutoHTTPSRedirectAddress(redirDomains, "example.com", caddy.NetworkAddress{Network: "tcp", Host: "2603:c024:8002:9500:9eb:e5d3:3975:d056", StartPort: 443, EndPort: 443}) - - got := redirDomains["example.com"] - if len(got) != 2 { - t.Fatalf("expected 2 redirect addresses for both bind addresses on the winning port, got %d: %#v", len(got), got) +func TestHTTPSRRALPNsListenProtocolOverrides(t *testing.T) { + srv := &Server{ + Protocols: []string{"h1", "h2"}, + ListenProtocols: [][]string{ + {"h1"}, + nil, + {}, + {"h3", ""}, + }, } - if got[0].StartPort != 443 || got[1].StartPort != 443 { - t.Fatalf("expected both redirect addresses to stay on HTTPS port 443, got %#v", got) - } - if got[0].Host != "10.0.0.189" || got[1].Host != "2603:c024:8002:9500:9eb:e5d3:3975:d056" { - t.Fatalf("expected both bind addresses to be preserved, got %#v", got) + + got := httpsRRALPNs(srv) + want := []string{"h3", "h2", "http/1.1"} + + if !reflect.DeepEqual(got, want) { + t.Fatalf("unexpected ALPN values: got %v want %v", got, want) + } +} + +func TestHTTPSRRALPNsIgnoresH2COnly(t *testing.T) { + srv := &Server{ + Protocols: []string{"h2c"}, + } + + got := httpsRRALPNs(srv) + if len(got) != 0 { + t.Fatalf("unexpected ALPN values: got %v want none", got) } } diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go index 9aca53578..66f93989b 100644 --- a/modules/caddyhttp/server.go +++ b/modules/caddyhttp/server.go @@ -300,6 +300,8 @@ type Server struct { onStopFuncs []func(context.Context) error // TODO: Experimental (Nov. 2023) } +var defaultProtocols = []string{"h1", "h2", "h3"} + var ( ServerHeader = "Caddy" serverHeader = []string{ServerHeader} @@ -899,22 +901,58 @@ func (s *Server) logRequest( // protocol returns true if the protocol proto is configured/enabled. func (s *Server) protocol(proto string) bool { if s.ListenProtocols == nil { - if slices.Contains(s.Protocols, proto) { + return slices.Contains(s.protocolsWithDefaults(), proto) + } + + for _, lnProtocols := range s.ListenProtocols { + if slices.Contains(s.listenerProtocolsWithDefaults(lnProtocols), proto) { return true } - } else { - for _, lnProtocols := range s.ListenProtocols { - for _, lnProtocol := range lnProtocols { - if lnProtocol == "" && slices.Contains(s.Protocols, proto) || lnProtocol == proto { - return true - } - } - } } return false } +func (s *Server) protocolsWithDefaults() []string { + if len(s.Protocols) == 0 { + return defaultProtocols + } + return s.Protocols +} + +func (s *Server) listenerProtocolsWithDefaults(lnProtocols []string) []string { + serverProtocols := s.protocolsWithDefaults() + if len(lnProtocols) == 0 { + return serverProtocols + } + + lnProtocolsDefault := false + lnProtocolsInclude := make([]string, 0, len(lnProtocols)+len(serverProtocols)) + srvProtocolsInclude := make(map[string]struct{}, len(serverProtocols)) + for _, srvProtocol := range serverProtocols { + srvProtocolsInclude[srvProtocol] = struct{}{} + } + + for _, lnProtocol := range lnProtocols { + if lnProtocol == "" { + lnProtocolsDefault = true + continue + } + lnProtocolsInclude = append(lnProtocolsInclude, lnProtocol) + delete(srvProtocolsInclude, lnProtocol) + } + + if lnProtocolsDefault { + for _, srvProtocol := range serverProtocols { + if _, ok := srvProtocolsInclude[srvProtocol]; ok { + lnProtocolsInclude = append(lnProtocolsInclude, srvProtocol) + } + } + } + + return lnProtocolsInclude +} + // Listeners returns the server's listeners. These are active listeners, // so calling Accept() or Close() on them will probably break things. // They are made available here for read-only purposes (e.g. Addr()) diff --git a/modules/caddytls/connpolicy.go b/modules/caddytls/connpolicy.go index c9258da48..9597af359 100644 --- a/modules/caddytls/connpolicy.go +++ b/modules/caddytls/connpolicy.go @@ -153,9 +153,9 @@ func (cp ConnectionPolicies) TLSConfig(ctx caddy.Context) *tls.Config { // in its config (remember, TLS connection policies are used by *other* apps to // run TLS servers) -- we skip names with placeholders if tlsApp.EncryptedClientHello.Publication == nil { - var echNames []string repl := caddy.NewReplacer() for _, p := range cp { + var echNames []string for _, m := range p.matchers { if sni, ok := m.(MatchServerName); ok { for _, name := range sni { @@ -164,8 +164,8 @@ func (cp ConnectionPolicies) TLSConfig(ctx caddy.Context) *tls.Config { } } } + tlsApp.RegisterServerNames(echNames, p.ALPN) } - tlsApp.RegisterServerNames(echNames) } tlsCfg.GetEncryptedClientHelloKeys = func(chi *tls.ClientHelloInfo) ([]tls.EncryptedClientHelloKey, error) { diff --git a/modules/caddytls/ech.go b/modules/caddytls/ech.go index b915fcfbe..4a48769d8 100644 --- a/modules/caddytls/ech.go +++ b/modules/caddytls/ech.go @@ -440,6 +440,10 @@ func (t *TLS) publishECHConfigs(logger *zap.Logger) error { zap.Strings("domains", dnsNamesToPublish), zap.Uint8s("config_ids", configIDs)) + if dnsPublisher, ok := publisher.(*ECHDNSPublisher); ok { + dnsPublisher.alpnByDomain = t.alpnValuesForServerNames(dnsNamesToPublish) + } + // publish this ECH config list with this publisher pubTime := time.Now() err := publisher.PublishECHConfigList(t.ctx, dnsNamesToPublish, echCfgListBin) @@ -776,7 +780,8 @@ type ECHDNSPublisher struct { ProviderRaw json.RawMessage `json:"provider,omitempty" caddy:"namespace=dns.providers inline_key=name"` provider ECHDNSProvider - logger *zap.Logger + alpnByDomain map[string][]string + logger *zap.Logger } // CaddyModule returns the Caddy module information. @@ -872,12 +877,7 @@ nextName: continue } params := httpsRec.Params - if params == nil { - params = make(libdns.SvcParams) - } - - // overwrite only the "ech" SvcParamKey - params["ech"] = []string{base64.StdEncoding.EncodeToString(configListBin)} + params = dnsPub.publishedSvcParams(domain, params, configListBin) // publish record _, err = dnsPub.provider.SetRecords(ctx, zone, []libdns.Record{ @@ -903,6 +903,25 @@ nextName: return nil } +func (dnsPub *ECHDNSPublisher) publishedSvcParams(domain string, existing libdns.SvcParams, configListBin []byte) libdns.SvcParams { + params := make(libdns.SvcParams, len(existing)+2) + for key, values := range existing { + params[key] = append([]string(nil), values...) + } + + params["ech"] = []string{base64.StdEncoding.EncodeToString(configListBin)} + + if len(dnsPub.alpnByDomain) == 0 { + return params + } + + if alpn := dnsPub.alpnByDomain[strings.ToLower(domain)]; len(alpn) > 0 { + params["alpn"] = append([]string(nil), alpn...) + } + + return params +} + // echConfig represents an ECHConfig from the specification, // [draft-ietf-tls-esni-22](https://www.ietf.org/archive/id/draft-ietf-tls-esni-22.html). type echConfig struct { diff --git a/modules/caddytls/ech_dns_test.go b/modules/caddytls/ech_dns_test.go new file mode 100644 index 000000000..7c337366e --- /dev/null +++ b/modules/caddytls/ech_dns_test.go @@ -0,0 +1,65 @@ +package caddytls + +import ( + "encoding/base64" + "reflect" + "sync" + "testing" + + "github.com/libdns/libdns" +) + +func TestRegisterServerNamesWithALPN(t *testing.T) { + tlsApp := &TLS{ + serverNames: make(map[string]serverNameRegistration), + serverNamesMu: new(sync.Mutex), + } + + tlsApp.RegisterServerNames([]string{ + "Example.com:443", + "example.com", + "127.0.0.1:443", + }, []string{"h2", "http/1.1"}) + tlsApp.RegisterServerNames([]string{"EXAMPLE.COM"}, []string{"h3"}) + + got := tlsApp.alpnValuesForServerNames([]string{"example.com:443", "127.0.0.1:443"}) + want := map[string][]string{ + "example.com": {"h3", "h2", "http/1.1"}, + } + + if !reflect.DeepEqual(got, want) { + t.Fatalf("unexpected ALPN values: got %#v want %#v", got, want) + } +} + +func TestECHDNSPublisherPublishedSvcParams(t *testing.T) { + dnsPub := &ECHDNSPublisher{ + alpnByDomain: map[string][]string{ + "example.com": {"h3", "h2", "http/1.1"}, + }, + } + + existing := libdns.SvcParams{ + "alpn": {"h2"}, + "ipv4hint": {"203.0.113.10"}, + } + + got := dnsPub.publishedSvcParams("Example.com", existing, []byte{0x01, 0x02, 0x03}) + + if !reflect.DeepEqual(existing["alpn"], []string{"h2"}) { + t.Fatalf("existing params mutated: got %v", existing["alpn"]) + } + + if !reflect.DeepEqual(got["alpn"], []string{"h3", "h2", "http/1.1"}) { + t.Fatalf("unexpected ALPN params: got %v", got["alpn"]) + } + + if !reflect.DeepEqual(got["ipv4hint"], []string{"203.0.113.10"}) { + t.Fatalf("unexpected preserved params: got %v", got["ipv4hint"]) + } + + wantECH := base64.StdEncoding.EncodeToString([]byte{0x01, 0x02, 0x03}) + if !reflect.DeepEqual(got["ech"], []string{wantECH}) { + t.Fatalf("unexpected ECH params: got %v want %v", got["ech"], wantECH) + } +} diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index 34ffbf62d..e5f6e6fc0 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -23,6 +23,7 @@ import ( "net" "net/http" "runtime/debug" + "slices" "strings" "sync" "time" @@ -140,7 +141,7 @@ type TLS struct { logger *zap.Logger events *caddyevents.App - serverNames map[string]struct{} + serverNames map[string]serverNameRegistration serverNamesMu *sync.Mutex // set of subjects with managed certificates, @@ -168,7 +169,7 @@ func (t *TLS) Provision(ctx caddy.Context) error { t.logger = ctx.Logger() repl := caddy.NewReplacer() t.managing, t.loaded = make(map[string]string), make(map[string]string) - t.serverNames = make(map[string]struct{}) + t.serverNames = make(map[string]serverNameRegistration) t.serverNamesMu = new(sync.Mutex) // set up default DNS module, if any, and make sure it implements all the @@ -648,27 +649,109 @@ func (t *TLS) managingWildcardFor(subj string, otherSubjsToManage map[string]str return false } -// RegisterServerNames registers the provided DNS names with the TLS app. -// This is currently used to auto-publish Encrypted ClientHello (ECH) -// configurations, if enabled. Use of this function by apps using the TLS -// app removes the need for the user to redundantly specify domain names -// in their configuration. This function separates hostname and port -// (keeping only the hotsname) and filters IP addresses, which can't be -// used with ECH. +// RegisterServerNames registers the provided DNS names with the TLS app and +// associates them with the given HTTPS RR ALPN values, if any. This is +// currently used to auto-publish Encrypted ClientHello (ECH) configurations, +// if enabled. Use of this function by apps using the TLS app removes the need +// for the user to redundantly specify domain names in their configuration. +// This function separates hostname and port, keeping only the hostname, and +// filters IP addresses which can't be used with ECH. // // EXPERIMENTAL: This function and its semantics/behavior are subject to change. -func (t *TLS) RegisterServerNames(dnsNames []string) { +func (t *TLS) RegisterServerNames(dnsNames, alpnValues []string) { t.serverNamesMu.Lock() + defer t.serverNamesMu.Unlock() + for _, name := range dnsNames { host, _, err := net.SplitHostPort(name) if err != nil { host = name } - if strings.TrimSpace(host) != "" && !certmagic.SubjectIsIP(host) { - t.serverNames[strings.ToLower(host)] = struct{}{} + host = strings.ToLower(strings.TrimSpace(host)) + if host == "" || certmagic.SubjectIsIP(host) { + continue + } + + registration := t.serverNames[host] + + if len(alpnValues) == 0 { + t.serverNames[host] = registration + continue + } + + if registration.alpnValues == nil { + registration.alpnValues = make(map[string]struct{}, len(alpnValues)) + } + for _, alpn := range alpnValues { + if alpn == "" { + continue + } + registration.alpnValues[alpn] = struct{}{} + } + t.serverNames[host] = registration + } +} + +func (t *TLS) alpnValuesForServerNames(dnsNames []string) map[string][]string { + t.serverNamesMu.Lock() + defer t.serverNamesMu.Unlock() + + result := make(map[string][]string, len(dnsNames)) + for _, name := range dnsNames { + host, _, err := net.SplitHostPort(name) + if err != nil { + host = name + } + host = strings.ToLower(strings.TrimSpace(host)) + if host == "" { + continue + } + + registration, ok := t.serverNames[host] + if !ok || len(registration.alpnValues) == 0 { + continue + } + result[host] = OrderedHTTPSRRALPN(registration.alpnValues) + } + + return result +} + +// OrderedHTTPSRRALPN returns the HTTPS RR ALPN values in preferred order. +func OrderedHTTPSRRALPN(alpnSet map[string]struct{}) []string { + if len(alpnSet) == 0 { + return nil + } + + knownOrder := append([]string{"h3"}, defaultALPN...) + ordered := make([]string, 0, len(alpnSet)) + seen := make(map[string]struct{}, len(alpnSet)) + + for _, alpn := range knownOrder { + if _, ok := alpnSet[alpn]; ok { + ordered = append(ordered, alpn) + seen[alpn] = struct{}{} } } - t.serverNamesMu.Unlock() + + if len(ordered) == len(alpnSet) { + return ordered + } + + var remaining []string + for alpn := range alpnSet { + if _, ok := seen[alpn]; ok { + continue + } + remaining = append(remaining, alpn) + } + slices.Sort(remaining) + + return append(ordered, remaining...) +} + +type serverNameRegistration struct { + alpnValues map[string]struct{} } // HandleHTTPChallenge ensures that the ACME HTTP challenge or ZeroSSL HTTP From 0fab9f0f7db04fdb0c1c3113d51e7c420d3c8f06 Mon Sep 17 00:00:00 2001 From: Rijul <31570722+Rijul-A@users.noreply.github.com> Date: Sun, 10 May 2026 19:38:40 +0530 Subject: [PATCH 17/23] caddytls: avoid duplicate automation for wildcard-covered hosts (#7697) * caddytls: Fix wildcard race in auto-HTTPS launch When evaluating whether to skip managing an individual subdomain due to an existing wildcard configuration, we now explicitly consult the automate loader. Because Caddy apps can start in any order, relying strictly on the TLS app's internal management state was non-deterministic if the HTTP app started first. Checking the automate loader guarantees predictable behavior since it is fully populated during the Provision phase, well before any apps are started. * respond to review comments 1. update requested comment 2. remove personal domain from test 3. add regression test * remove unnecessary mutex lock * refactor: -integration test, +explicit cases * refactor: remove redundant test, add comment * rename file and add header * update copyright year --- modules/caddytls/tls.go | 11 ++- modules/caddytls/tls_wildcard_test.go | 96 +++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 modules/caddytls/tls_wildcard_test.go diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index e5f6e6fc0..928e109e6 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -614,8 +614,8 @@ func (t *TLS) Manage(subjects map[string]struct{}) error { // managingWildcardFor returns true if the app is managing a certificate that covers that // subject name (including consideration of wildcards), either from its internal list of -// names that it IS managing certs for, or from the otherSubjsToManage which includes names -// that WILL be managed. +// names that it IS managing certs for, from the otherSubjsToManage which includes names +// that WILL be managed, or from names configured in the 'automate' loader. func (t *TLS) managingWildcardFor(subj string, otherSubjsToManage map[string]struct{}) bool { // TODO: we could also consider manually-loaded certs using t.HasCertificateForSubject(), // but that does not account for how manually-loaded certs may be restricted as to which @@ -630,7 +630,9 @@ func (t *TLS) managingWildcardFor(subj string, otherSubjsToManage map[string]str return managing } - // replace labels of the domain with wildcards until we get a match + // replace labels of the domain with wildcards until we get a match from names + // already being managed, those about to be managed in this batch, or those + // configured for automation labels := strings.Split(subj, ".") for i := range labels { if labels[i] == "*" { @@ -644,6 +646,9 @@ func (t *TLS) managingWildcardFor(subj string, otherSubjsToManage map[string]str if _, ok := otherSubjsToManage[candidate]; ok { return true } + if _, ok := t.automateNames[candidate]; ok { + return true + } } return false diff --git a/modules/caddytls/tls_wildcard_test.go b/modules/caddytls/tls_wildcard_test.go new file mode 100644 index 000000000..0151ca5dd --- /dev/null +++ b/modules/caddytls/tls_wildcard_test.go @@ -0,0 +1,96 @@ +// Copyright 2015 Matthew Holt and The Caddy Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package caddytls + +import ( + "encoding/json" + "testing" + + "github.com/caddyserver/caddy/v2" +) + +func TestAvoidDuplicateAutomation(t *testing.T) { + tests := []struct { + name string + automateNames []string + expectedToManage bool + }{ + { + name: "do not manage if wildcard is automated", + automateNames: []string{"*.example.com"}, + expectedToManage: false, + }, + { + name: "manage if no automation configured", + automateNames: []string{}, + expectedToManage: true, + }, + { + name: "manage if explicitly requested even when wildcard automated", + automateNames: []string{"*.example.com", "sub.example.com"}, + expectedToManage: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + automateJSON, err := json.Marshal(tc.automateNames) + if err != nil { + t.Fatal(err) + } + + tlsApp := &TLS{ + Automation: &AutomationConfig{ + Policies: []*AutomationPolicy{ + { + IssuersRaw: []json.RawMessage{ + []byte(`{"module": "internal"}`), + }, + }, + }, + }, + CertificatesRaw: map[string]json.RawMessage{ + "automate": automateJSON, + }, + } + + var cfg caddy.Config + ctx, err := caddy.ProvisionContext(&cfg) + if err != nil { + t.Fatal(err) + } + + if err := tlsApp.Provision(ctx); err != nil { + t.Fatal(err) + } + + // simulate a case wherein the HTTP app starts first and + // tells the TLS app about the following auto-HTTPS domains + httpDomains := map[string]struct{}{"sub.example.com": {}} + if err := tlsApp.Manage(httpDomains); err != nil { + t.Fatal(err) + } + + _, actuallyManaged := tlsApp.managing["sub.example.com"] + if actuallyManaged != tc.expectedToManage { + t.Errorf( + "expected sub.example.com individually managed: %v, got: %v", + tc.expectedToManage, + actuallyManaged, + ) + } + }) + } +} From 4ba16fe82c9dbf8affb670dc56f7c43f1296e3e9 Mon Sep 17 00:00:00 2001 From: Steffen Busch <37350514+steffenbusch@users.noreply.github.com> Date: Mon, 11 May 2026 20:23:58 +0200 Subject: [PATCH 18/23] docs: add documentation for fileExists and fileStat template functions (#7700) --- modules/caddyhttp/templates/templates.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/modules/caddyhttp/templates/templates.go b/modules/caddyhttp/templates/templates.go index 994beefab..f1f910857 100644 --- a/modules/caddyhttp/templates/templates.go +++ b/modules/caddyhttp/templates/templates.go @@ -162,6 +162,25 @@ func init() { // {{listFiles "/mydir"}} // ``` // +// ##### `fileExists` +// +// Returns true if the given file name, relative to the template context's file root, +// can be opened successfully. +// +// ``` +// {{fileExists "path/to/file.html"}} +// ``` +// +// ##### `fileStat` +// +// Returns [FileInfo](https://pkg.go.dev/io/fs#FileInfo) using [Stat](https://pkg.go.dev/io/fs#Stat) +// on the given file name, relative to the template context's file root. +// +// ``` +// {{$css := fileStat "css/style.css" -}} +// +// ``` +// // ##### `markdown` // // Renders the given Markdown text as HTML and returns it. This uses the From 761347aa635e14bf7a937e88d45a40e671d325c5 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Mon, 11 May 2026 16:45:49 -0600 Subject: [PATCH 19/23] templates: Explicitly warn about misconfigurations --- modules/caddyhttp/templates/templates.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/modules/caddyhttp/templates/templates.go b/modules/caddyhttp/templates/templates.go index f1f910857..caac85b8a 100644 --- a/modules/caddyhttp/templates/templates.go +++ b/modules/caddyhttp/templates/templates.go @@ -36,13 +36,22 @@ func init() { // Templates is a middleware which executes response bodies as Go templates. // The syntax is documented in the Go standard library's // [text/template package](https://golang.org/pkg/text/template/). +// Note that ANY response body that matches and qualifies may be evaluated, +// even if it comes from a proxied backend. // -// ⚠️ Template functions/actions are still experimental, so they are subject to change. +// ⚠️ Template functions/actions can access the environment, files on disk, +// and make HTTP requests. This is extremely useful, but you need to make +// sure templates are only evaluated on content that you trust, control, or +// at least sanitize properly. // -// Custom template functions can be registered by creating a plugin module under the `http.handlers.templates.functions.*` namespace that implements the `CustomFunctions` interface. +// ⚠️ Templates are still experimental, so they are subject to change. // // [All Sprig functions](https://masterminds.github.io/sprig/) are supported. // +// Custom template functions can be registered by creating a plugin module +// under the `http.handlers.templates.functions.*` namespace that implements +// the `CustomFunctions` interface. +// // In addition to the standard functions and the Sprig library, Caddy adds // extra functions and data that are available to a template: // From a4a38c3e88952a361c6e44c5dc3200bba1497e15 Mon Sep 17 00:00:00 2001 From: Rayan Salhab Date: Tue, 12 May 2026 02:16:33 +0300 Subject: [PATCH 20/23] rewrite: escape file matcher paths before rewriting (#7683) * fix: escape file matcher paths in rewrites Preserve matched file paths containing literal '?' or '%' when try_files rewrites to http.matchers.file.relative. * test: cover nested escaped try_files rewrite paths * test: cover encoded slash try_files rewrite paths * fix: assert file matcher placeholder as string --------- Co-authored-by: cyphercodes --- modules/caddyhttp/fileserver/matcher_test.go | 100 +++++++++++++++++++ modules/caddyhttp/rewrite/rewrite.go | 32 ++++-- 2 files changed, 126 insertions(+), 6 deletions(-) diff --git a/modules/caddyhttp/fileserver/matcher_test.go b/modules/caddyhttp/fileserver/matcher_test.go index 4342d5d31..b94b8444f 100644 --- a/modules/caddyhttp/fileserver/matcher_test.go +++ b/modules/caddyhttp/fileserver/matcher_test.go @@ -28,6 +28,7 @@ import ( "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/internal/filesystems" "github.com/caddyserver/caddy/v2/modules/caddyhttp" + "github.com/caddyserver/caddy/v2/modules/caddyhttp/rewrite" ) type testCase struct { @@ -188,6 +189,105 @@ func fileMatcherTest(t *testing.T, i int, tc testCase) { } } +func TestTryFilesRewriteEscapesMatchedPath(t *testing.T) { + root := t.TempDir() + + tests := []struct { + name string + requestTarget string + filename string + extraFiles []string + wantPath string + wantRequestURI string + skipWindows bool + }{ + { + name: "question mark in path", + requestTarget: "/%3F.html", + filename: "?.html", + wantPath: "/?.html", + wantRequestURI: "/%3F.html", + skipWindows: true, + }, + { + name: "percent in path", + requestTarget: "/%25.html", + filename: "%.html", + wantPath: "/%.html", + wantRequestURI: "/%25.html", + }, + { + name: "encoded question mark remains percent-encoded", + requestTarget: "/%253F.html", + filename: "%3F.html", + wantPath: "/%3F.html", + wantRequestURI: "/%253F.html", + }, + { + name: "question mark in nested path", + requestTarget: "/nested/%3F.html", + filename: filepath.Join("nested", "?.html"), + wantPath: "/nested/?.html", + wantRequestURI: "/nested/%3F.html", + skipWindows: true, + }, + { + name: "encoded slash in filename does not conflict with nesting", + requestTarget: "/nested%252Ffile.html", + filename: "nested%2Ffile.html", + extraFiles: []string{filepath.Join("nested", "file.html")}, + wantPath: "/nested%2Ffile.html", + wantRequestURI: "/nested%252Ffile.html", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if tc.skipWindows && runtime.GOOS == "windows" { + t.Skip("Windows file names cannot contain question marks") + } + + for _, name := range append([]string{tc.filename}, tc.extraFiles...) { + filename := filepath.Join(root, name) + if err := os.MkdirAll(filepath.Dir(filename), 0o700); err != nil { + t.Fatalf("creating test file parent directory: %v", err) + } + if err := os.WriteFile(filename, []byte(name), 0o600); err != nil { + t.Fatalf("writing test file: %v", err) + } + } + + m := &MatchFile{ + fsmap: &filesystems.FileSystemMap{}, + Root: root, + TryFiles: []string{"{http.request.uri.path}"}, + } + req := httptest.NewRequest(http.MethodGet, "http://example.com"+tc.requestTarget, nil) + repl := caddyhttp.NewTestReplacer(req) + + matched, err := m.MatchWithError(req) + if err != nil { + t.Fatalf("matching file: %v", err) + } + if !matched { + t.Fatalf("expected request %s to match %s", tc.requestTarget, tc.filename) + } + + rewrite.Rewrite{URI: "{http.matchers.file.relative}"}.Rewrite(req, repl) + + if req.URL.Path != tc.wantPath { + t.Errorf("rewritten path = %q, want %q", req.URL.Path, tc.wantPath) + } + if req.RequestURI != tc.wantRequestURI { + t.Errorf("rewritten request URI = %q, want %q", req.RequestURI, tc.wantRequestURI) + } + if req.URL.RawQuery != "" { + t.Errorf("rewritten raw query = %q, want empty", req.URL.RawQuery) + } + }) + } +} + func TestPHPFileMatcher(t *testing.T) { for i, tc := range []struct { path string diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go index ba2ea5407..3500028f9 100644 --- a/modules/caddyhttp/rewrite/rewrite.go +++ b/modules/caddyhttp/rewrite/rewrite.go @@ -211,12 +211,7 @@ func (rewr Rewrite) Rewrite(r *http.Request, repl *caddy.Replacer) bool { var newPath, newQuery, newFrag string if path != "" { - // replace the `path` placeholder to escaped path - pathPlaceholder := "{http.request.uri.path}" - if strings.Contains(path, pathPlaceholder) { - path = strings.ReplaceAll(path, pathPlaceholder, r.URL.EscapedPath()) - } - + path = escapePathPlaceholders(path, r, repl) newPath = repl.ReplaceAll(path, "") } @@ -300,6 +295,31 @@ func (rewr Rewrite) Rewrite(r *http.Request, repl *caddy.Replacer) bool { return r.Method != oldMethod || r.RequestURI != oldURI } +func escapePathPlaceholders(path string, r *http.Request, repl *caddy.Replacer) string { + // Replace path-valued placeholders in escaped form before the URI is parsed, + // otherwise literal '?' and '%' bytes from the path can be interpreted as URI + // delimiters or percent-escape sequences during the rewrite. + pathPlaceholder := "{http.request.uri.path}" + if strings.Contains(path, pathPlaceholder) { + path = strings.ReplaceAll(path, pathPlaceholder, r.URL.EscapedPath()) + } + + fileMatchRelativePlaceholder := "{http.matchers.file.relative}" + if strings.Contains(path, fileMatchRelativePlaceholder) { + if val, ok := repl.Get("http.matchers.file.relative"); ok { + if relativePath, ok := val.(string); ok { + path = strings.ReplaceAll(path, fileMatchRelativePlaceholder, escapePathPreservingSlashes(relativePath)) + } + } + } + + return path +} + +func escapePathPreservingSlashes(path string) string { + return strings.ReplaceAll(url.PathEscape(path), "%2F", "/") +} + // buildQueryString takes an input query string and // performs replacements on each component, returning // the resulting query string. This function appends From d80774cb3fe6abcd0fea9dd0c68812ee94c7cf2a Mon Sep 17 00:00:00 2001 From: Br1an <932039080@qq.com> Date: Tue, 12 May 2026 07:27:03 +0800 Subject: [PATCH 21/23] metrics: Add nil check for metricsHandler in AdminMetrics.serveHTTP (#7553) * metrics: Add nil check for metricsHandler in AdminMetrics.serveHTTP Prevents panic when the admin metrics endpoint is accessed before the module is fully provisioned. Returns a proper API error instead of crashing. * admin: provision router modules before registering routes Instead of adding a nil check for metricsHandler, address the root cause by provisioning admin router modules before calling Routes(). This ensures all handler state is initialized before routes are registered on the mux. Merge newAdminHandler and provisionAdminRouters into a single step, removing the two-phase setup where routes were registered first and modules provisioned later. The AdminConfig.routers field is no longer needed since provisioning happens inline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: go fmt admin.go --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- admin.go | 51 +++++++++++++-------------------------------------- admin_test.go | 24 +++++++++--------------- caddy.go | 7 ------- 3 files changed, 22 insertions(+), 60 deletions(-) diff --git a/admin.go b/admin.go index 08465a923..97af846ef 100644 --- a/admin.go +++ b/admin.go @@ -120,10 +120,6 @@ type AdminConfig struct { // // EXPERIMENTAL: This feature is subject to change. Remote *RemoteAdmin `json:"remote,omitempty"` - - // Holds onto the routers so that we can later provision them - // if they require provisioning. - routers []AdminRouter } // ConfigSettings configures the management of configuration. @@ -222,7 +218,7 @@ type AdminPermissions struct { // newAdminHandler reads admin's config and returns an http.Handler suitable // for use in an admin endpoint server, which will be listening on listenAddr. -func (admin *AdminConfig) newAdminHandler(addr NetworkAddress, remote bool, _ Context) adminHandler { +func (admin *AdminConfig) newAdminHandler(addr NetworkAddress, remote bool, ctx Context) (adminHandler, error) { muxWrap := adminHandler{mux: http.NewServeMux()} // secure the local or remote endpoint respectively @@ -279,34 +275,21 @@ func (admin *AdminConfig) newAdminHandler(addr NetworkAddress, remote bool, _ Co // register third-party module endpoints for _, m := range GetModules("admin.api") { router := m.New().(AdminRouter) + + // provision the router before registering its routes, so + // handlers have access to all provisioned state + if provisioner, ok := router.(Provisioner); ok { + if err := provisioner.Provision(ctx); err != nil { + return adminHandler{}, fmt.Errorf("provisioning admin router module %s: %v", m.ID, err) + } + } + for _, route := range router.Routes() { addRoute(route.Pattern, handlerLabel, route.Handler) } - admin.routers = append(admin.routers, router) } - return muxWrap -} - -// provisionAdminRouters provisions all the router modules -// in the admin.api namespace that need provisioning. -func (admin *AdminConfig) provisionAdminRouters(ctx Context) error { - for _, router := range admin.routers { - provisioner, ok := router.(Provisioner) - if !ok { - continue - } - - err := provisioner.Provision(ctx) - if err != nil { - return err - } - } - - // We no longer need the routers once provisioned, allow for GC - admin.routers = nil - - return nil + return muxWrap, nil } // allowedOrigins returns a list of origins that are allowed. @@ -430,11 +413,7 @@ func replaceLocalAdminServer(cfg *Config, ctx Context) error { return err } - handler := cfg.Admin.newAdminHandler(addr, false, ctx) - - // run the provisioners for loaded modules to make sure local - // state is properly re-initialized in the new admin server - err = cfg.Admin.provisionAdminRouters(ctx) + handler, err := cfg.Admin.newAdminHandler(addr, false, ctx) if err != nil { return err } @@ -558,11 +537,7 @@ func replaceRemoteAdminServer(ctx Context, cfg *Config) error { // make the HTTP handler but disable Host/Origin enforcement // because we are using TLS authentication instead - handler := cfg.Admin.newAdminHandler(addr, true, ctx) - - // run the provisioners for loaded modules to make sure local - // state is properly re-initialized in the new admin server - err = cfg.Admin.provisionAdminRouters(ctx) + handler, err := cfg.Admin.newAdminHandler(addr, true, ctx) if err != nil { return err } diff --git a/admin_test.go b/admin_test.go index 19cc6ae7c..dda06a9e9 100644 --- a/admin_test.go +++ b/admin_test.go @@ -340,7 +340,10 @@ func TestAdminHandlerBuiltinRouteErrors(t *testing.T) { if err != nil { t.Fatalf("Failed to parse address: %v", err) } - handler := cfg.Admin.newAdminHandler(addr, false, Context{}) + handler, err := cfg.Admin.newAdminHandler(addr, false, Context{}) + if err != nil { + t.Fatalf("Failed to create admin handler: %v", err) + } tests := []struct { name string @@ -461,7 +464,10 @@ func TestNewAdminHandlerRouterRegistration(t *testing.T) { admin := &AdminConfig{ EnforceOrigin: false, } - handler := admin.newAdminHandler(addr, false, Context{}) + handler, err := admin.newAdminHandler(addr, false, Context{}) + if err != nil { + t.Fatalf("Failed to create admin handler: %v", err) + } req := httptest.NewRequest("GET", "/mock", nil) req.Host = "localhost:2019" @@ -473,10 +479,6 @@ func TestNewAdminHandlerRouterRegistration(t *testing.T) { t.Errorf("Expected status code %d but got %d", http.StatusOK, rr.Code) t.Logf("Response body: %s", rr.Body.String()) } - - if len(admin.routers) != 1 { - t.Errorf("Expected 1 router to be stored, got %d", len(admin.routers)) - } } type mockProvisionableRouter struct { @@ -514,19 +516,16 @@ func TestAdminRouterProvisioning(t *testing.T) { name string provisionErr error wantErr bool - routersAfter int // expected number of routers after provisioning }{ { name: "successful provisioning", provisionErr: nil, wantErr: false, - routersAfter: 0, }, { name: "provisioning error", provisionErr: fmt.Errorf("provision failed"), wantErr: true, - routersAfter: 1, }, } @@ -562,8 +561,7 @@ func TestAdminRouterProvisioning(t *testing.T) { t.Fatalf("Failed to parse address: %v", err) } - _ = admin.newAdminHandler(addr, false, Context{}) - err = admin.provisionAdminRouters(Context{}) + _, err = admin.newAdminHandler(addr, false, Context{}) if test.wantErr { if err == nil { @@ -574,10 +572,6 @@ func TestAdminRouterProvisioning(t *testing.T) { t.Errorf("Expected no error but got: %v", err) } } - - if len(admin.routers) != test.routersAfter { - t.Errorf("Expected %d routers after provisioning, got %d", test.routersAfter, len(admin.routers)) - } }) } } diff --git a/caddy.go b/caddy.go index b3144299d..8799594a9 100644 --- a/caddy.go +++ b/caddy.go @@ -440,13 +440,6 @@ func run(newCfg *Config, start bool) (Context, error) { } }() - // Provision any admin routers which may need to access - // some of the other apps at runtime - err = ctx.cfg.Admin.provisionAdminRouters(ctx) - if err != nil { - return ctx, err - } - // Start err = func() error { started := make([]string, 0, len(ctx.cfg.apps)) From cc58caa1099240ef1a4c280b892260b380a85c86 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Mon, 11 May 2026 17:33:39 -0600 Subject: [PATCH 22/23] go.mod: Upgrade quic-go to v0.59.1 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index bd44d644d..be1789782 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/klauspost/cpuid/v2 v2.3.0 github.com/mholt/acmez/v3 v3.1.6 github.com/prometheus/client_golang v1.23.2 - github.com/quic-go/quic-go v0.59.0 + github.com/quic-go/quic-go v0.59.1 github.com/smallstep/certificates v0.30.2 github.com/smallstep/nosql v0.8.0 github.com/smallstep/truststore v0.13.0 diff --git a/go.sum b/go.sum index 5cae77fe0..0112afcc8 100644 --- a/go.sum +++ b/go.sum @@ -280,8 +280,8 @@ github.com/prometheus/procfs v0.20.1 h1:XwbrGOIplXW/AU3YhIhLODXMJYyC1isLFfYCsTEy github.com/prometheus/procfs v0.20.1/go.mod h1:o9EMBZGRyvDrSPH1RqdxhojkuXstoe4UlK79eF5TGGo= github.com/quic-go/qpack v0.6.0 h1:g7W+BMYynC1LbYLSqRt8PBg5Tgwxn214ZZR34VIOjz8= github.com/quic-go/qpack v0.6.0/go.mod h1:lUpLKChi8njB4ty2bFLX2x4gzDqXwUpaO1DP9qMDZII= -github.com/quic-go/quic-go v0.59.0 h1:OLJkp1Mlm/aS7dpKgTc6cnpynnD2Xg7C1pwL6vy/SAw= -github.com/quic-go/quic-go v0.59.0/go.mod h1:upnsH4Ju1YkqpLXC305eW3yDZ4NfnNbmQRCMWS58IKU= +github.com/quic-go/quic-go v0.59.1 h1:0Gmua0HW1Tv7ANR7hUYwRyD0MG5OJfgvYSZasGZzBic= +github.com/quic-go/quic-go v0.59.1/go.mod h1:upnsH4Ju1YkqpLXC305eW3yDZ4NfnNbmQRCMWS58IKU= github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/rs/xid v1.6.0 h1:fV591PaemRlL6JfRxGDEPl69wICngIQ3shQtzfy2gxU= From 77e9ce7404c4a76853e101a9f5687a929ee56654 Mon Sep 17 00:00:00 2001 From: James Hartig Date: Tue, 12 May 2026 13:05:50 -0500 Subject: [PATCH 23/23] reverseproxy: further prevent body closes from dial errors (#7715) --- .../caddyhttp/reverseproxy/retries_test.go | 55 +++++++++++++++++++ .../caddyhttp/reverseproxy/reverseproxy.go | 17 +++--- 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/modules/caddyhttp/reverseproxy/retries_test.go b/modules/caddyhttp/reverseproxy/retries_test.go index b0f78bac0..bfd70978c 100644 --- a/modules/caddyhttp/reverseproxy/retries_test.go +++ b/modules/caddyhttp/reverseproxy/retries_test.go @@ -730,3 +730,58 @@ func TestRetryMatchAllowsExpressionMixedWithOtherMatchers(t *testing.T) { }) } } + +// TestSubrouteErrorFallbackWithBody is similar to TestDialErrorBodyRetry but +// mimics Subroute's Error handler rather than testing retries specifically +func TestSubrouteErrorFallbackWithBody(t *testing.T) { + // Good upstream: echoes the request body with 200 OK. + goodServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, "read body: "+err.Error(), http.StatusInternalServerError) + return + } + w.WriteHeader(http.StatusOK) + _, err = w.Write(body) + if err != nil { + t.Errorf("error writing in good server: %v", err) + } + })) + t.Cleanup(goodServer.Close) + + // Handler which will dial error + badProxy := minimalHandler(0, &Upstream{Host: new(Host), Dial: deadUpstreamAddr(t)}) + + bodyReader := newCloseOnCloseReader("hello world") + req := httptest.NewRequest("POST", "http://localhost/", bodyReader) + // httptest.NewRequest wraps the reader in NopCloser; replace + // it with our close-aware reader so Close() is propagated. + req.Body = bodyReader + + req = prepareTestRequest(req) + rec := httptest.NewRecorder() + err := badProxy.ServeHTTP(rec, req, caddyhttp.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { + return nil + })) + if err == nil { + t.Fatalf("Expected error from badProxy.ServeHTTP") + } + + // Simulate the Subroute's Error handler by calling another handler with the + // same request and recorder + goodProxy := minimalHandler(0, &Upstream{Host: new(Host), Dial: goodServer.Listener.Addr().String()}) + err = goodProxy.ServeHTTP(rec, req, caddyhttp.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { + return nil + })) + + if err != nil { + t.Fatalf("Expected no error from goodProxy.ServeHTTP, got: %v", err) + } + if rec.Code != http.StatusOK { + t.Errorf("status: got %d, want %d", rec.Code, http.StatusOK) + } + expectedBody := "hello world" + if rec.Body.String() != expectedBody { + t.Errorf("body: got %q, want %q", rec.Body.String(), expectedBody) + } +} diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index cefe645ee..a11afcd79 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -488,20 +488,19 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht reqHost := clonedReq.Host reqHeader := clonedReq.Header - // When retries are configured and there is a body, wrap it in - // io.NopCloser to prevent Go's transport from closing it on dial - // errors. cloneRequest does a shallow copy, so clonedReq.Body and + // If the request contained a body, wrap it in io.NopCloser + // to prevent Go's transport from closing it on dial errors. + // cloneRequest does a shallow copy, so clonedReq.Body and // r.Body share the same io.ReadCloser — a dial-failure Close() - // would kill the original body for all subsequent retry attempts. - // The real body is closed by the HTTP server when the handler - // returns. + // would kill the original body for all subsequent retry + // attempts or subsequent handlers. The real body is closed by + // the HTTP server when the handler returns. // // If the body was already fully buffered (via request_buffers), // we also extract the buffer so the retry loop can replay it - // from the beginning on each attempt. (see #6259, #7546) + // from the beginning on each attempt. (see #6259, #7546, #7713) var bufferedReqBody *bytes.Buffer - if clonedReq.Body != nil && h.LoadBalancing != nil && - (h.LoadBalancing.Retries > 0 || h.LoadBalancing.TryDuration > 0) { + if clonedReq.Body != nil { if reqBodyBuf, ok := clonedReq.Body.(bodyReadCloser); ok && reqBodyBuf.body == nil && reqBodyBuf.buf != nil { bufferedReqBody = reqBodyBuf.buf reqBodyBuf.buf = nil