From 4367f1a5d408ebbe7a6e495097d237945e9f2542 Mon Sep 17 00:00:00 2001 From: Ebrahim Date: Thu, 9 Jul 2026 10:58:37 +0200 Subject: [PATCH 1/6] caddyhttp: reject HTTP listeners that use admin API port Expose LocalAdminPort() so the HTTP app can detect listener addresses that conflict with the local admin endpoint during validation. When admin is disabled, the check is skipped so HTTP servers may use the default admin port. When admin listens on a custom port, only that port is protected. Fixes: #7053 Signed-off-by: Ebrahim Nejati --- admin.go | 15 +++++++++++++++ modules/caddyhttp/app.go | 13 +++++++++---- modules/caddyhttp/app_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 modules/caddyhttp/app_test.go diff --git a/admin.go b/admin.go index 97af846ef9c..38f03d31566 100644 --- a/admin.go +++ b/admin.go @@ -364,6 +364,14 @@ func (admin AdminConfig) allowedOrigins(addr NetworkAddress) []*url.URL { return allowed } +// LocalAdminPort returns the port of the local admin API endpoint +// and whether it is enabled. +func LocalAdminPort() (uint, bool) { + serverMu.Lock() + defer serverMu.Unlock() + return localAdminPort, localAdminEnabled +} + // replaceLocalAdminServer replaces the running local admin server // according to the relevant configuration in cfg. If no configuration // for the admin endpoint exists in cfg, a default one is used, so @@ -403,6 +411,9 @@ func replaceLocalAdminServer(cfg *Config, ctx Context) error { // if new admin endpoint is to be disabled, we're done if cfg.Admin.Disabled { + serverMu.Lock() + localAdminEnabled = false + serverMu.Unlock() Log().Named("admin").Warn("admin endpoint disabled") return nil } @@ -424,6 +435,8 @@ func replaceLocalAdminServer(cfg *Config, ctx Context) error { } serverMu.Lock() + localAdminPort = addr.StartPort + localAdminEnabled = true localAdminServer = &http.Server{ Addr: addr.String(), // for logging purposes only Handler: handler, @@ -1480,6 +1493,8 @@ var bufPool = sync.Pool{ // keep a reference to admin endpoint singletons while they're active var ( serverMu sync.Mutex + localAdminPort uint + localAdminEnabled bool localAdminServer, remoteAdminServer *http.Server identityCertCache *certmagic.Cache ) diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go index d9d9b9f8425..a6ef58a5f6f 100644 --- a/modules/caddyhttp/app.go +++ b/modules/caddyhttp/app.go @@ -410,6 +410,7 @@ func (app *App) Provision(ctx caddy.Context) error { // Validate ensures the app's configuration is valid. func (app *App) Validate() error { lnAddrs := make(map[string]string) + adminPort, checkAdmin := caddy.LocalAdminPort() for srvName, srv := range app.Servers { // each server must use distinct listener addresses @@ -421,11 +422,15 @@ func (app *App) Validate() error { // check that every address in the port range is unique to this server; // we do not use <= here because PortRangeSize() adds 1 to EndPort for us for i := uint(0); i < listenAddr.PortRangeSize(); i++ { - addr := caddy.JoinNetworkAddress(listenAddr.Network, listenAddr.Host, strconv.FormatUint(uint64(listenAddr.StartPort+i), 10)) - if sn, ok := lnAddrs[addr]; ok { - return fmt.Errorf("server %s: listener address repeated: %s (already claimed by server '%s')", srvName, addr, sn) + port := listenAddr.StartPort + i + if checkAdmin && port == adminPort { + return fmt.Errorf("server %s: listener %q uses admin API port %d (use another port or 'admin off')", srvName, addr, adminPort) } - lnAddrs[addr] = srvName + joinedAddr := caddy.JoinNetworkAddress(listenAddr.Network, listenAddr.Host, strconv.FormatUint(uint64(port), 10)) + if sn, ok := lnAddrs[joinedAddr]; ok { + return fmt.Errorf("server %s: listener address repeated: %s (already claimed by server '%s')", srvName, joinedAddr, sn) + } + lnAddrs[joinedAddr] = srvName } } diff --git a/modules/caddyhttp/app_test.go b/modules/caddyhttp/app_test.go new file mode 100644 index 00000000000..26850af8ec6 --- /dev/null +++ b/modules/caddyhttp/app_test.go @@ -0,0 +1,25 @@ +package caddyhttp + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/caddyserver/caddy/v2" +) + +func TestAppRejectsAdminPort(t *testing.T) { + t.Cleanup(func() { _ = caddy.Stop() }) + + err := caddy.Run(&caddy.Config{ + AppsRaw: map[string]json.RawMessage{ + "http": []byte(`{"servers":{"srv0":{"listen":[":2019"]}}}`), + }, + }) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "admin API port") { + t.Fatalf("unexpected error: %v", err) + } +} From 2c6ac7811f8a866ef756ef4f2bc8947bdc6024e3 Mon Sep 17 00:00:00 2001 From: Ebrahim Date: Fri, 10 Jul 2026 14:09:34 +0200 Subject: [PATCH 2/6] caddyhttp: refactor admin API address handling Adjusted related logic in the app validation to check for overlapping listener addresses with the admin API. Added a new test to ensure non-overlapping admin addresses are allowed. Fixes: #7053 Signed-off-by: Ebrahim Nejati --- admin.go | 10 +++---- listeners.go | 55 +++++++++++++++++++++++++++++++++++ listeners_test.go | 47 ++++++++++++++++++++++++++++++ modules/caddyhttp/app.go | 8 ++--- modules/caddyhttp/app_test.go | 16 +++++++++- 5 files changed, 126 insertions(+), 10 deletions(-) diff --git a/admin.go b/admin.go index 38f03d31566..761e896125c 100644 --- a/admin.go +++ b/admin.go @@ -364,12 +364,12 @@ func (admin AdminConfig) allowedOrigins(addr NetworkAddress) []*url.URL { return allowed } -// LocalAdminPort returns the port of the local admin API endpoint +// LocalAdminAddress returns the local admin API listen address // and whether it is enabled. -func LocalAdminPort() (uint, bool) { +func LocalAdminAddress() (NetworkAddress, bool) { serverMu.Lock() defer serverMu.Unlock() - return localAdminPort, localAdminEnabled + return localAdminAddr, localAdminEnabled } // replaceLocalAdminServer replaces the running local admin server @@ -435,7 +435,7 @@ func replaceLocalAdminServer(cfg *Config, ctx Context) error { } serverMu.Lock() - localAdminPort = addr.StartPort + localAdminAddr = addr localAdminEnabled = true localAdminServer = &http.Server{ Addr: addr.String(), // for logging purposes only @@ -1493,7 +1493,7 @@ var bufPool = sync.Pool{ // keep a reference to admin endpoint singletons while they're active var ( serverMu sync.Mutex - localAdminPort uint + localAdminAddr NetworkAddress localAdminEnabled bool localAdminServer, remoteAdminServer *http.Server identityCertCache *certmagic.Cache diff --git a/listeners.go b/listeners.go index 6031f98e495..351fce4b14d 100644 --- a/listeners.go +++ b/listeners.go @@ -15,6 +15,7 @@ package caddy import ( + "cmp" "context" "crypto/tls" "errors" @@ -255,6 +256,60 @@ func (na NetworkAddress) PortRangeSize() uint { return (na.EndPort - na.StartPort) + 1 } +// OverlapsWith reports whether na and other could bind to the same socket and +// thus conflict. For unix and fd sockets, only the network and host (socket +// path) are compared. For IP-based networks, it accounts for intersecting port +// ranges, IP families (e.g. tcp4 and tcp6 never overlap), wildcard interfaces +// (empty host or 0.0.0.0/::), and the localhost alias. +func (na NetworkAddress) OverlapsWith(other NetworkAddress) bool { + if na.IsUnixNetwork() || na.IsFdNetwork() || other.IsUnixNetwork() || other.IsFdNetwork() { + return na.Network == other.Network && na.Host == other.Host + } + + // port ranges must intersect + if na.EndPort < other.StartPort || other.EndPort < na.StartPort { + return false + } + + // listeners with distinct IP families (e.g. tcp4 vs tcp6) never overlap. + // an empty or "tcp" network is family-agnostic and may overlap either + n1, n2 := cmp.Or(na.Network, "tcp"), cmp.Or(other.Network, "tcp") + if n1 != n2 && n1 != "tcp" && n2 != "tcp" { + return false + } + + return na.hostsOverlap(other) +} + +// hostsOverlap reports whether the hosts of na and other could resolve to a +// common listening interface. Callers must ensure neither is a unix/fd socket. +func (na NetworkAddress) hostsOverlap(other NetworkAddress) bool { + // an empty host means "all interfaces" + if na.Host == "" || other.Host == "" || na.Host == other.Host { + return true + } + + // Use == "localhost" instead of isLoopback() to avoid family-aware comparison. + if na.Host == "localhost" { + return other.isLoopback() + } + if other.Host == "localhost" { + return na.isLoopback() + } + + ip1, err1 := netip.ParseAddr(na.Host) + ip2, err2 := netip.ParseAddr(other.Host) + if err1 != nil || err2 != nil { + return false // unresolved names, assume distinct + } + + // a wildcard overlaps any address of the same family + if ip1.IsUnspecified() || ip2.IsUnspecified() { + return ip1.Is6() == ip2.Is6() + } + return ip1 == ip2 +} + func (na NetworkAddress) isLoopback() bool { if na.IsUnixNetwork() || na.IsFdNetwork() { return true diff --git a/listeners_test.go b/listeners_test.go index 7bbaca1f9b3..9d45362ac1a 100644 --- a/listeners_test.go +++ b/listeners_test.go @@ -710,3 +710,50 @@ func TestSplitUnixSocketPermissionsBits(t *testing.T) { } } } + +func TestOverlapsWith(t *testing.T) { + for i, tc := range []struct { + a, b NetworkAddress + expect bool + }{ + { + a: NetworkAddress{Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + expect: true, + }, + { + a: NetworkAddress{Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Host: "127.0.0.2", StartPort: 2019, EndPort: 2019}, + expect: false, + }, + { + a: NetworkAddress{Host: "localhost", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + expect: true, + }, + { + a: NetworkAddress{Host: "", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Host: "192.168.1.1", StartPort: 2019, EndPort: 2019}, + expect: true, + }, + { + a: NetworkAddress{Host: "0.0.0.0", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Host: "::1", StartPort: 2019, EndPort: 2019}, + expect: false, + }, + { + a: NetworkAddress{Network: "unix", Host: "/run/caddy/admin.sock"}, + b: NetworkAddress{Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + expect: false, + }, + { + a: NetworkAddress{Network: "unix", Host: "/run/caddy/admin.sock"}, + b: NetworkAddress{Network: "unix", Host: "/run/caddy/admin.sock"}, + expect: true, + }, + } { + if tc.a.OverlapsWith(tc.b) != tc.expect { + t.Errorf("Test %d: expected %v for %v vs %v", i, tc.expect, tc.a, tc.b) + } + } +} diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go index a6ef58a5f6f..72c1397085b 100644 --- a/modules/caddyhttp/app.go +++ b/modules/caddyhttp/app.go @@ -410,7 +410,7 @@ func (app *App) Provision(ctx caddy.Context) error { // Validate ensures the app's configuration is valid. func (app *App) Validate() error { lnAddrs := make(map[string]string) - adminPort, checkAdmin := caddy.LocalAdminPort() + adminAddr, checkAdmin := caddy.LocalAdminAddress() for srvName, srv := range app.Servers { // each server must use distinct listener addresses @@ -419,13 +419,13 @@ func (app *App) Validate() error { if err != nil { return fmt.Errorf("invalid listener address '%s': %v", addr, err) } + if checkAdmin && listenAddr.OverlapsWith(adminAddr) { + return fmt.Errorf("server %s: listener %q overlaps with admin API address %s (use another address or 'admin off')", srvName, addr, adminAddr) + } // check that every address in the port range is unique to this server; // we do not use <= here because PortRangeSize() adds 1 to EndPort for us for i := uint(0); i < listenAddr.PortRangeSize(); i++ { port := listenAddr.StartPort + i - if checkAdmin && port == adminPort { - return fmt.Errorf("server %s: listener %q uses admin API port %d (use another port or 'admin off')", srvName, addr, adminPort) - } joinedAddr := caddy.JoinNetworkAddress(listenAddr.Network, listenAddr.Host, strconv.FormatUint(uint64(port), 10)) if sn, ok := lnAddrs[joinedAddr]; ok { return fmt.Errorf("server %s: listener address repeated: %s (already claimed by server '%s')", srvName, joinedAddr, sn) diff --git a/modules/caddyhttp/app_test.go b/modules/caddyhttp/app_test.go index 26850af8ec6..0bcbc147e99 100644 --- a/modules/caddyhttp/app_test.go +++ b/modules/caddyhttp/app_test.go @@ -19,7 +19,21 @@ func TestAppRejectsAdminPort(t *testing.T) { if err == nil { t.Fatal("expected error") } - if !strings.Contains(err.Error(), "admin API port") { + if !strings.Contains(err.Error(), "overlaps with admin API address") { t.Fatalf("unexpected error: %v", err) } } + +func TestAppAllowsNonOverlappingAdminAddress(t *testing.T) { + t.Cleanup(func() { _ = caddy.Stop() }) + + err := caddy.Run(&caddy.Config{ + Admin: &caddy.AdminConfig{Listen: "127.0.0.1:2019"}, + AppsRaw: map[string]json.RawMessage{ + "http": []byte(`{"servers":{"srv0":{"listen":["127.0.0.2:2019"]}}}`), + }, + }) + if err != nil && strings.Contains(err.Error(), "overlaps with admin API address") { + t.Fatalf("unexpected admin overlap error: %v", err) + } +} From 2c58801918d63dfe5f90637c3681160d61a382d7 Mon Sep 17 00:00:00 2001 From: Ebrahim Date: Mon, 13 Jul 2026 10:11:54 +0200 Subject: [PATCH 3/6] caddyhttp: enhance NetworkAddress overlap checks and improve tests Refactored the OverlapsWith method in NetworkAddress. Added new test cases to validate overlapping and non-overlapping scenarios. Signed-off-by: Ebrahim Nejati --- listeners.go | 64 ++++++++++++++++++++------- listeners_test.go | 54 +++++++++++++++++++++-- modules/caddyhttp/app_test.go | 82 ++++++++++++++++++++++++++--------- 3 files changed, 161 insertions(+), 39 deletions(-) diff --git a/listeners.go b/listeners.go index 351fce4b14d..c576fbf9e38 100644 --- a/listeners.go +++ b/listeners.go @@ -256,14 +256,13 @@ func (na NetworkAddress) PortRangeSize() uint { return (na.EndPort - na.StartPort) + 1 } -// OverlapsWith reports whether na and other could bind to the same socket and -// thus conflict. For unix and fd sockets, only the network and host (socket -// path) are compared. For IP-based networks, it accounts for intersecting port -// ranges, IP families (e.g. tcp4 and tcp6 never overlap), wildcard interfaces -// (empty host or 0.0.0.0/::), and the localhost alias. +// Reports whether na and other could bind to the same socket and thus conflict. +// For unix and fd sockets, only the network and host (socket path) are compared. +// For IP-based networks, it accounts for intersecting port ranges, transport +// IP families, wildcard interfaces, and the localhost alias. func (na NetworkAddress) OverlapsWith(other NetworkAddress) bool { if na.IsUnixNetwork() || na.IsFdNetwork() || other.IsUnixNetwork() || other.IsFdNetwork() { - return na.Network == other.Network && na.Host == other.Host + return na.Network == other.Network && na.bindPath() == other.bindPath() } // port ranges must intersect @@ -271,16 +270,43 @@ func (na NetworkAddress) OverlapsWith(other NetworkAddress) bool { return false } - // listeners with distinct IP families (e.g. tcp4 vs tcp6) never overlap. - // an empty or "tcp" network is family-agnostic and may overlap either - n1, n2 := cmp.Or(na.Network, "tcp"), cmp.Or(other.Network, "tcp") - if n1 != n2 && n1 != "tcp" && n2 != "tcp" { + // transport and IP family must be compatible + if !networksOverlap(na.Network, other.Network) { return false } return na.hostsOverlap(other) } +func (na NetworkAddress) bindPath() string { + if !na.IsUnixNetwork() { + return na.Host + } + path, _, err := internal.SplitUnixSocketPermissionsBits(na.Host) + if err != nil { + return na.Host + } + return path +} + +// Reports whether two network names could bind the same socket. +func networksOverlap(n1, n2 string) bool { + t1, f1 := networkParts(n1) + t2, f2 := networkParts(n2) + return t1 == t2 && (f1 == 0 || f2 == 0 || f1 == f2) +} + +func networkParts(network string) (transport string, family int) { + network = cmp.Or(network, "tcp") // default to tcp + if base, ok := strings.CutSuffix(network, "4"); ok && base != "" { + return base, 4 + } + if base, ok := strings.CutSuffix(network, "6"); ok && base != "" { + return base, 6 + } + return network, 0 +} + // hostsOverlap reports whether the hosts of na and other could resolve to a // common listening interface. Callers must ensure neither is a unix/fd socket. func (na NetworkAddress) hostsOverlap(other NetworkAddress) bool { @@ -289,12 +315,8 @@ func (na NetworkAddress) hostsOverlap(other NetworkAddress) bool { return true } - // Use == "localhost" instead of isLoopback() to avoid family-aware comparison. - if na.Host == "localhost" { - return other.isLoopback() - } - if other.Host == "localhost" { - return na.isLoopback() + if na.Host == "localhost" || other.Host == "localhost" { + return bindsLikeLocalhost(na.Host) && bindsLikeLocalhost(other.Host) } ip1, err1 := netip.ParseAddr(na.Host) @@ -310,6 +332,16 @@ func (na NetworkAddress) hostsOverlap(other NetworkAddress) bool { return ip1 == ip2 } +// Reports whether host could bind the same socket as localhost +func bindsLikeLocalhost(host string) bool { + switch host { + case "", "localhost": + return true + } + ip, err := netip.ParseAddr(host) + return err == nil && (ip.IsUnspecified() || ip == netip.MustParseAddr("127.0.0.1") || ip == netip.MustParseAddr("::1")) +} + func (na NetworkAddress) isLoopback() bool { if na.IsUnixNetwork() || na.IsFdNetwork() { return true diff --git a/listeners_test.go b/listeners_test.go index 9d45362ac1a..766063c71be 100644 --- a/listeners_test.go +++ b/listeners_test.go @@ -731,6 +731,21 @@ func TestOverlapsWith(t *testing.T) { b: NetworkAddress{Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, expect: true, }, + { + a: NetworkAddress{Host: "localhost", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Host: "127.0.0.2", StartPort: 2019, EndPort: 2019}, + expect: false, + }, + { + a: NetworkAddress{Host: "localhost", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Host: "0.0.0.0", StartPort: 2019, EndPort: 2019}, + expect: true, + }, + { + a: NetworkAddress{Host: "localhost", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Host: "::1", StartPort: 2019, EndPort: 2019}, + expect: true, + }, { a: NetworkAddress{Host: "", StartPort: 2019, EndPort: 2019}, b: NetworkAddress{Host: "192.168.1.1", StartPort: 2019, EndPort: 2019}, @@ -741,6 +756,26 @@ func TestOverlapsWith(t *testing.T) { b: NetworkAddress{Host: "::1", StartPort: 2019, EndPort: 2019}, expect: false, }, + { + a: NetworkAddress{Network: "tcp", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "udp", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + expect: false, + }, + { + a: NetworkAddress{Network: "udp4", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "udp6", Host: "::1", StartPort: 2019, EndPort: 2019}, + expect: false, + }, + { + a: NetworkAddress{Network: "udp", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "udp4", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + expect: true, + }, + { + a: NetworkAddress{Network: "tcp4", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "tcp6", Host: "::1", StartPort: 2019, EndPort: 2019}, + expect: false, + }, { a: NetworkAddress{Network: "unix", Host: "/run/caddy/admin.sock"}, b: NetworkAddress{Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, @@ -751,9 +786,22 @@ func TestOverlapsWith(t *testing.T) { b: NetworkAddress{Network: "unix", Host: "/run/caddy/admin.sock"}, expect: true, }, + { + a: NetworkAddress{Network: "unix", Host: "/run/caddy/admin.sock|0222"}, + b: NetworkAddress{Network: "unix", Host: "/run/caddy/admin.sock|0777"}, + expect: true, + }, } { - if tc.a.OverlapsWith(tc.b) != tc.expect { - t.Errorf("Test %d: expected %v for %v vs %v", i, tc.expect, tc.a, tc.b) - } + assertOverlap(t, i, tc.a, tc.b, tc.expect) + } +} + +func assertOverlap(t *testing.T, i int, a, b NetworkAddress, expect bool) { + t.Helper() + if got := a.OverlapsWith(b); got != expect { + t.Errorf("Test %d: expected %v for %v vs %v, got %v", i, expect, a, b, got) + } + if got := b.OverlapsWith(a); got != expect { + t.Errorf("Test %d: expected %v (symmetric) for %v vs %v, got %v", i, expect, b, a, got) } } diff --git a/modules/caddyhttp/app_test.go b/modules/caddyhttp/app_test.go index 0bcbc147e99..ce169545be8 100644 --- a/modules/caddyhttp/app_test.go +++ b/modules/caddyhttp/app_test.go @@ -2,38 +2,80 @@ package caddyhttp import ( "encoding/json" + "fmt" + "net" "strings" "testing" "github.com/caddyserver/caddy/v2" ) -func TestAppRejectsAdminPort(t *testing.T) { - t.Cleanup(func() { _ = caddy.Stop() }) - - err := caddy.Run(&caddy.Config{ - AppsRaw: map[string]json.RawMessage{ - "http": []byte(`{"servers":{"srv0":{"listen":[":2019"]}}}`), +func TestAppAdminAddressOverlap(t *testing.T) { + for _, tc := range []struct { + name string + adminAddr string + httpListen string + wantOverlapErr bool + }{ + { + name: "rejects overlapping listener", + adminAddr: "localhost:%d", + httpListen: ":%d", + wantOverlapErr: true, }, - }) - if err == nil { - t.Fatal("expected error") - } - if !strings.Contains(err.Error(), "overlaps with admin API address") { - t.Fatalf("unexpected error: %v", err) + { + name: "allows non-overlapping loopback addresses", + adminAddr: "127.0.0.1:%d", + httpListen: "127.0.0.2:%d", + wantOverlapErr: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + _ = caddy.Stop() + t.Cleanup(func() { _ = caddy.Stop() }) + + port := freeTCPPort(t) + err := caddy.Run(&caddy.Config{ + Admin: &caddy.AdminConfig{Listen: fmt.Sprintf(tc.adminAddr, port)}, + AppsRaw: map[string]json.RawMessage{ + "http": httpListenConfig(t, fmt.Sprintf(tc.httpListen, port)), + }, + }) + if tc.wantOverlapErr { + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "overlaps with admin API address") { + t.Fatalf("unexpected error: %v", err) + } + return + } + if err != nil { + t.Fatalf("expected no error, got: %v", err) + } + }) } } -func TestAppAllowsNonOverlappingAdminAddress(t *testing.T) { - t.Cleanup(func() { _ = caddy.Stop() }) +func freeTCPPort(t *testing.T) uint { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + return uint(ln.Addr().(*net.TCPAddr).Port) +} - err := caddy.Run(&caddy.Config{ - Admin: &caddy.AdminConfig{Listen: "127.0.0.1:2019"}, - AppsRaw: map[string]json.RawMessage{ - "http": []byte(`{"servers":{"srv0":{"listen":["127.0.0.2:2019"]}}}`), +func httpListenConfig(t *testing.T, addrs ...string) json.RawMessage { + t.Helper() + raw, err := json.Marshal(map[string]any{ + "servers": map[string]any{ + "srv0": map[string]any{"listen": addrs}, }, }) - if err != nil && strings.Contains(err.Error(), "overlaps with admin API address") { - t.Fatalf("unexpected admin overlap error: %v", err) + if err != nil { + t.Fatal(err) } + return raw } From 6d55435b9be11e67118621ba54433acd563ae2d8 Mon Sep 17 00:00:00 2001 From: Ebrahim Date: Thu, 16 Jul 2026 16:09:12 +0200 Subject: [PATCH 4/6] caddyhttp: enhance overlap logic for ipv6 Improved the hostsOverlap method to handle unspecified addresses more accurately. Signed-off-by: Ebrahim Nejati --- listeners.go | 8 +++++++- listeners_test.go | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/listeners.go b/listeners.go index c576fbf9e38..e254b680bc2 100644 --- a/listeners.go +++ b/listeners.go @@ -327,7 +327,13 @@ func (na NetworkAddress) hostsOverlap(other NetworkAddress) bool { // a wildcard overlaps any address of the same family if ip1.IsUnspecified() || ip2.IsUnspecified() { - return ip1.Is6() == ip2.Is6() + if ip1.Is6() == ip2.Is6() { + return true + } + _, f1 := networkParts(na.Network) + _, f2 := networkParts(other.Network) + return (ip1.IsUnspecified() && ip1.Is6() && f1 == 0) || + (ip2.IsUnspecified() && ip2.Is6() && f2 == 0) } return ip1 == ip2 } diff --git a/listeners_test.go b/listeners_test.go index 766063c71be..3b3f8295a5e 100644 --- a/listeners_test.go +++ b/listeners_test.go @@ -756,6 +756,21 @@ func TestOverlapsWith(t *testing.T) { b: NetworkAddress{Host: "::1", StartPort: 2019, EndPort: 2019}, expect: false, }, + { + a: NetworkAddress{Host: "0.0.0.0", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Host: "::", StartPort: 2019, EndPort: 2019}, + expect: true, // tcp/[::] may be dual-stack + }, + { + a: NetworkAddress{Network: "tcp6", Host: "::", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Host: "0.0.0.0", StartPort: 2019, EndPort: 2019}, + expect: false, // tcp6/[::] is IPv6-only + }, + { + a: NetworkAddress{Host: "::", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Host: "192.168.1.1", StartPort: 2019, EndPort: 2019}, + expect: true, // dual-stack [::] covers IPv4 + }, { a: NetworkAddress{Network: "tcp", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, b: NetworkAddress{Network: "udp", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, From 799c9d4c70b281f958a0e83bdeb0af62dfe358c0 Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Fri, 17 Jul 2026 18:49:10 +1000 Subject: [PATCH 5/6] caddyhttp: fix admin listener overlap validation --- admin.go | 26 ++++--- admin_test.go | 55 +++++++++++++ caddy.go | 69 +++++++++-------- context.go | 9 ++- listeners.go | 89 +++------------------ listeners_test.go | 78 +++++++------------ modules/caddyhttp/app.go | 13 +++- modules/caddyhttp/app_test.go | 140 +++++++++++++++++++++++++++++----- 8 files changed, 289 insertions(+), 190 deletions(-) diff --git a/admin.go b/admin.go index 761e896125c..3e87a8c5504 100644 --- a/admin.go +++ b/admin.go @@ -364,12 +364,21 @@ func (admin AdminConfig) allowedOrigins(addr NetworkAddress) []*url.URL { return allowed } -// LocalAdminAddress returns the local admin API listen address +// LocalAdminAddress returns the configured local admin API listen address // and whether it is enabled. -func LocalAdminAddress() (NetworkAddress, bool) { - serverMu.Lock() - defer serverMu.Unlock() - return localAdminAddr, localAdminEnabled +func (ctx Context) LocalAdminAddress() (NetworkAddress, bool, error) { + if !ctx.validateAdmin { + return NetworkAddress{}, false, nil + } + if ctx.cfg != nil && ctx.cfg.Admin != nil && ctx.cfg.Admin.Disabled { + return NetworkAddress{}, false, nil + } + listen := DefaultAdminListen + if ctx.cfg != nil && ctx.cfg.Admin != nil { + listen = ctx.cfg.Admin.Listen + } + addr, err := parseAdminListenAddr(listen, DefaultAdminListen) + return addr, true, err } // replaceLocalAdminServer replaces the running local admin server @@ -411,9 +420,6 @@ func replaceLocalAdminServer(cfg *Config, ctx Context) error { // if new admin endpoint is to be disabled, we're done if cfg.Admin.Disabled { - serverMu.Lock() - localAdminEnabled = false - serverMu.Unlock() Log().Named("admin").Warn("admin endpoint disabled") return nil } @@ -435,8 +441,6 @@ func replaceLocalAdminServer(cfg *Config, ctx Context) error { } serverMu.Lock() - localAdminAddr = addr - localAdminEnabled = true localAdminServer = &http.Server{ Addr: addr.String(), // for logging purposes only Handler: handler, @@ -1493,8 +1497,6 @@ var bufPool = sync.Pool{ // keep a reference to admin endpoint singletons while they're active var ( serverMu sync.Mutex - localAdminAddr NetworkAddress - localAdminEnabled bool localAdminServer, remoteAdminServer *http.Server identityCertCache *certmagic.Cache ) diff --git a/admin_test.go b/admin_test.go index dda06a9e90e..ccb6ca97480 100644 --- a/admin_test.go +++ b/admin_test.go @@ -57,6 +57,61 @@ var testCfg = []byte(`{ } `) +func TestContextLocalAdminAddress(t *testing.T) { + for _, tc := range []struct { + name string + ctx Context + wantAddress string + wantEnabled bool + }{ + { + name: "provision-only context does not validate admin address", + ctx: Context{cfg: &Config{}}, + }, + { + name: "validation context uses default admin address", + ctx: Context{cfg: &Config{}, validateAdmin: true}, + wantAddress: DefaultAdminListen, + wantEnabled: true, + }, + { + name: "disabled admin is not validated", + ctx: Context{ + cfg: &Config{Admin: &AdminConfig{Disabled: true}}, + validateAdmin: true, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + addr, enabled, err := tc.ctx.LocalAdminAddress() + if err != nil { + t.Fatal(err) + } + if enabled != tc.wantEnabled { + t.Fatalf("enabled: got %t, want %t", enabled, tc.wantEnabled) + } + if enabled && addr.String() != tc.wantAddress { + t.Fatalf("address: got %q, want %q", addr, tc.wantAddress) + } + }) + } +} + +func TestDerivedContextPreservesAdminValidation(t *testing.T) { + ctx := Context{Context: context.Background(), cfg: &Config{}, validateAdmin: true} + + child, cancel := NewContext(ctx) + cancel() + if _, enabled, err := child.LocalAdminAddress(); err != nil || !enabled { + t.Fatalf("NewContext did not preserve admin validation: enabled=%t, err=%v", enabled, err) + } + + withValue := ctx.WithValue("test-key", "test-value") + if _, enabled, err := withValue.LocalAdminAddress(); err != nil || !enabled { + t.Fatalf("WithValue did not preserve admin validation: enabled=%t, err=%v", enabled, err) + } +} + type testAdminPublicKey string func (k testAdminPublicKey) Equal(x crypto.PublicKey) bool { diff --git a/caddy.go b/caddy.go index 8799594a942..58acb84289a 100644 --- a/caddy.go +++ b/caddy.go @@ -417,7 +417,7 @@ func unsyncedDecodeAndRun(cfgJSON []byte, allowPersist bool) error { // will want to use Run instead, which also // updates the config's raw state. func run(newCfg *Config, start bool) (Context, error) { - ctx, err := provisionContext(newCfg, start) + ctx, err := provisionContext(newCfg, true) if err != nil { globalMetrics.configSuccess.Set(0) return ctx, err @@ -441,28 +441,35 @@ func run(newCfg *Config, start bool) (Context, error) { }() // Start - err = func() error { - started := make([]string, 0, len(ctx.cfg.apps)) - for name, a := range ctx.cfg.apps { - err := a.Start() - if err != nil { - // an app failed to start, so we need to stop - // all other apps that were already started - for _, otherAppName := range started { - err2 := ctx.cfg.apps[otherAppName].Stop() - if err2 != nil { - err = fmt.Errorf("%v; additionally, aborting app %s: %v", - err, otherAppName, err2) - } + started := make([]string, 0, len(ctx.cfg.apps)) + for name, a := range ctx.cfg.apps { + err = a.Start() + if err != nil { + // an app failed to start, so we need to stop + // all other apps that were already started + for _, otherAppName := range started { + err2 := ctx.cfg.apps[otherAppName].Stop() + if err2 != nil { + err = fmt.Errorf("%v; additionally, aborting app %s: %v", + err, otherAppName, err2) } - return fmt.Errorf("%s app module: start: %v", name, err) } - started = append(started, name) + return ctx, fmt.Errorf("%s app module: start: %v", name, err) } - return nil - }() + started = append(started, name) + } + + // replace the local admin endpoint only after every app has started; + // otherwise a rejected config could replace the admin endpoint while the + // previous app config remains active + err = replaceLocalAdminServer(newCfg, ctx) if err != nil { - return ctx, err + for _, appName := range started { + if stopErr := ctx.cfg.apps[appName].Stop(); stopErr != nil { + err = fmt.Errorf("%v; additionally, aborting app %s: %v", err, appName, stopErr) + } + } + return ctx, fmt.Errorf("starting caddy administration endpoint: %v", err) } globalMetrics.configSuccess.Set(1) globalMetrics.configSuccessTime.SetToCurrentTime() @@ -479,9 +486,9 @@ func run(newCfg *Config, start bool) (Context, error) { // provisionContext creates a new context from the given configuration and provisions // storage and apps. // If `newCfg` is nil a new empty configuration will be created. -// If `replaceAdminServer` is true any currently active admin server will be replaced -// with a new admin server based on the provided configuration. -func provisionContext(newCfg *Config, replaceAdminServer bool) (Context, error) { +// If `validateAdmin` is true apps may validate their listeners against the +// configured admin listener. +func provisionContext(newCfg *Config, validateAdmin bool) (Context, error) { // because we will need to roll back any state // modifications if this function errors, we // keep a single error value and scope all @@ -501,7 +508,11 @@ func provisionContext(newCfg *Config, replaceAdminServer bool) (Context, error) // cleanup occurs when we return if there // was an error; if no error, it will get // cleaned up on next config cycle - ctx, cancelCause := NewContextWithCause(Context{Context: context.Background(), cfg: newCfg}) + ctx, cancelCause := NewContextWithCause(Context{ + Context: context.Background(), + cfg: newCfg, + validateAdmin: validateAdmin, + }) defer func() { if err != nil { globalMetrics.configSuccess.Set(0) @@ -561,14 +572,6 @@ func provisionContext(newCfg *Config, replaceAdminServer bool) (Context, error) return ctx, err } - // start the admin endpoint (and stop any prior one) - if replaceAdminServer { - err = replaceLocalAdminServer(newCfg, ctx) - if err != nil { - return ctx, fmt.Errorf("starting caddy administration endpoint: %v", err) - } - } - // Load and Provision each app and their submodules err = func() error { for appName := range newCfg.AppsRaw { @@ -578,6 +581,10 @@ func provisionContext(newCfg *Config, replaceAdminServer bool) (Context, error) } return nil }() + if err != nil { + return ctx, err + } + return ctx, err } diff --git a/context.go b/context.go index 7c06d26c071..6d3f465304b 100644 --- a/context.go +++ b/context.go @@ -48,6 +48,7 @@ type Context struct { moduleInstances map[string][]Module cfg *Config + validateAdmin bool ancestry []Module cleanupFuncs []func() // invoked at every config unload exitFuncs []func(context.Context) // invoked at config unload ONLY IF the process is exiting (EXPERIMENTAL) @@ -70,7 +71,12 @@ func NewContext(ctx Context) (Context, context.CancelFunc) { // NewContextWithCause is like NewContext but returns a context.CancelCauseFunc. // EXPERIMENTAL: This API is subject to change. func NewContextWithCause(ctx Context) (Context, context.CancelCauseFunc) { - newCtx := Context{moduleInstances: make(map[string][]Module), cfg: ctx.cfg, metricsRegistry: prometheus.NewPedanticRegistry()} + newCtx := Context{ + moduleInstances: make(map[string][]Module), + cfg: ctx.cfg, + validateAdmin: ctx.validateAdmin, + metricsRegistry: prometheus.NewPedanticRegistry(), + } c, cancel := context.WithCancelCause(ctx.Context) wrappedCancel := func(cause error) { cancel(cause) @@ -673,6 +679,7 @@ func (ctx *Context) WithValue(key, value any) Context { Context: context.WithValue(ctx.Context, key, value), moduleInstances: ctx.moduleInstances, cfg: ctx.cfg, + validateAdmin: ctx.validateAdmin, ancestry: ctx.ancestry, cleanupFuncs: ctx.cleanupFuncs, exitFuncs: ctx.exitFuncs, diff --git a/listeners.go b/listeners.go index e254b680bc2..33a6e013f87 100644 --- a/listeners.go +++ b/listeners.go @@ -15,7 +15,6 @@ package caddy import ( - "cmp" "context" "crypto/tls" "errors" @@ -256,26 +255,21 @@ func (na NetworkAddress) PortRangeSize() uint { return (na.EndPort - na.StartPort) + 1 } -// Reports whether na and other could bind to the same socket and thus conflict. -// For unix and fd sockets, only the network and host (socket path) are compared. -// For IP-based networks, it accounts for intersecting port ranges, transport -// IP families, wildcard interfaces, and the localhost alias. +// OverlapsWith reports whether na and other use the same Caddy listener. +// It compares configured listener identity rather than resolving hostnames or +// predicting platform-specific IP wildcard behaviour. Unix socket types share +// the same path namespace, so equal normalized paths overlap across types. func (na NetworkAddress) OverlapsWith(other NetworkAddress) bool { - if na.IsUnixNetwork() || na.IsFdNetwork() || other.IsUnixNetwork() || other.IsFdNetwork() { - return na.Network == other.Network && na.bindPath() == other.bindPath() + if na.IsUnixNetwork() || other.IsUnixNetwork() { + return na.IsUnixNetwork() && other.IsUnixNetwork() && na.bindPath() == other.bindPath() } - - // port ranges must intersect - if na.EndPort < other.StartPort || other.EndPort < na.StartPort { - return false - } - - // transport and IP family must be compatible - if !networksOverlap(na.Network, other.Network) { - return false + if na.IsFdNetwork() || other.IsFdNetwork() { + return na.IsFdNetwork() && other.IsFdNetwork() && na.Host == other.Host } - - return na.hostsOverlap(other) + return na.Network == other.Network && + na.Host == other.Host && + na.StartPort <= other.EndPort && + other.StartPort <= na.EndPort } func (na NetworkAddress) bindPath() string { @@ -289,65 +283,6 @@ func (na NetworkAddress) bindPath() string { return path } -// Reports whether two network names could bind the same socket. -func networksOverlap(n1, n2 string) bool { - t1, f1 := networkParts(n1) - t2, f2 := networkParts(n2) - return t1 == t2 && (f1 == 0 || f2 == 0 || f1 == f2) -} - -func networkParts(network string) (transport string, family int) { - network = cmp.Or(network, "tcp") // default to tcp - if base, ok := strings.CutSuffix(network, "4"); ok && base != "" { - return base, 4 - } - if base, ok := strings.CutSuffix(network, "6"); ok && base != "" { - return base, 6 - } - return network, 0 -} - -// hostsOverlap reports whether the hosts of na and other could resolve to a -// common listening interface. Callers must ensure neither is a unix/fd socket. -func (na NetworkAddress) hostsOverlap(other NetworkAddress) bool { - // an empty host means "all interfaces" - if na.Host == "" || other.Host == "" || na.Host == other.Host { - return true - } - - if na.Host == "localhost" || other.Host == "localhost" { - return bindsLikeLocalhost(na.Host) && bindsLikeLocalhost(other.Host) - } - - ip1, err1 := netip.ParseAddr(na.Host) - ip2, err2 := netip.ParseAddr(other.Host) - if err1 != nil || err2 != nil { - return false // unresolved names, assume distinct - } - - // a wildcard overlaps any address of the same family - if ip1.IsUnspecified() || ip2.IsUnspecified() { - if ip1.Is6() == ip2.Is6() { - return true - } - _, f1 := networkParts(na.Network) - _, f2 := networkParts(other.Network) - return (ip1.IsUnspecified() && ip1.Is6() && f1 == 0) || - (ip2.IsUnspecified() && ip2.Is6() && f2 == 0) - } - return ip1 == ip2 -} - -// Reports whether host could bind the same socket as localhost -func bindsLikeLocalhost(host string) bool { - switch host { - case "", "localhost": - return true - } - ip, err := netip.ParseAddr(host) - return err == nil && (ip.IsUnspecified() || ip == netip.MustParseAddr("127.0.0.1") || ip == netip.MustParseAddr("::1")) -} - func (na NetworkAddress) isLoopback() bool { if na.IsUnixNetwork() || na.IsFdNetwork() { return true diff --git a/listeners_test.go b/listeners_test.go index 3b3f8295a5e..1db32fac742 100644 --- a/listeners_test.go +++ b/listeners_test.go @@ -717,78 +717,48 @@ func TestOverlapsWith(t *testing.T) { expect bool }{ { - a: NetworkAddress{Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + a: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2019, EndPort: 2019}, expect: true, }, { - a: NetworkAddress{Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Host: "127.0.0.2", StartPort: 2019, EndPort: 2019}, - expect: false, - }, - { - a: NetworkAddress{Host: "localhost", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + a: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2019, EndPort: 2020}, + b: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2020, EndPort: 2021}, expect: true, }, { - a: NetworkAddress{Host: "localhost", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Host: "127.0.0.2", StartPort: 2019, EndPort: 2019}, + a: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2020, EndPort: 2020}, expect: false, }, { - a: NetworkAddress{Host: "localhost", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Host: "0.0.0.0", StartPort: 2019, EndPort: 2019}, + a: NetworkAddress{Network: "tcp", Host: "127.0.0.1"}, + b: NetworkAddress{Network: "tcp", Host: "127.0.0.1"}, expect: true, }, { - a: NetworkAddress{Host: "localhost", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Host: "::1", StartPort: 2019, EndPort: 2019}, - expect: true, - }, - { - a: NetworkAddress{Host: "", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Host: "192.168.1.1", StartPort: 2019, EndPort: 2019}, - expect: true, - }, - { - a: NetworkAddress{Host: "0.0.0.0", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Host: "::1", StartPort: 2019, EndPort: 2019}, + a: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "tcp", Host: "", StartPort: 2019, EndPort: 2019}, expect: false, }, { - a: NetworkAddress{Host: "0.0.0.0", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Host: "::", StartPort: 2019, EndPort: 2019}, - expect: true, // tcp/[::] may be dual-stack - }, - { - a: NetworkAddress{Network: "tcp6", Host: "::", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Host: "0.0.0.0", StartPort: 2019, EndPort: 2019}, - expect: false, // tcp6/[::] is IPv6-only - }, - { - a: NetworkAddress{Host: "::", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Host: "192.168.1.1", StartPort: 2019, EndPort: 2019}, - expect: true, // dual-stack [::] covers IPv4 - }, - { - a: NetworkAddress{Network: "tcp", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Network: "udp", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + a: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "tcp", Host: "::1", StartPort: 2019, EndPort: 2019}, expect: false, }, { - a: NetworkAddress{Network: "udp4", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Network: "udp6", Host: "::1", StartPort: 2019, EndPort: 2019}, + a: NetworkAddress{Network: "plugin", Host: "same", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "plugin6", Host: "same", StartPort: 2019, EndPort: 2019}, expect: false, }, { - a: NetworkAddress{Network: "udp", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Network: "udp4", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, - expect: true, + a: NetworkAddress{Network: "tcp4", Host: "::1", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "tcp", Host: "::1", StartPort: 2019, EndPort: 2019}, + expect: false, }, { - a: NetworkAddress{Network: "tcp4", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, - b: NetworkAddress{Network: "tcp6", Host: "::1", StartPort: 2019, EndPort: 2019}, + a: NetworkAddress{Network: "tcp6", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "tcp", Host: "127.0.0.1", StartPort: 2019, EndPort: 2019}, expect: false, }, { @@ -806,6 +776,16 @@ func TestOverlapsWith(t *testing.T) { b: NetworkAddress{Network: "unix", Host: "/run/caddy/admin.sock|0777"}, expect: true, }, + { + a: NetworkAddress{Network: "unix", Host: "/run/caddy/admin.sock"}, + b: NetworkAddress{Network: "unixpacket", Host: "/run/caddy/admin.sock"}, + expect: true, + }, + { + a: NetworkAddress{Network: "fd", Host: "3"}, + b: NetworkAddress{Network: "fdgram", Host: "3"}, + expect: true, + }, } { assertOverlap(t, i, tc.a, tc.b, tc.expect) } diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go index 72c1397085b..426ceeef2a9 100644 --- a/modules/caddyhttp/app.go +++ b/modules/caddyhttp/app.go @@ -165,6 +165,9 @@ type App struct { logger *zap.Logger tlsApp *caddytls.TLS + adminAddr caddy.NetworkAddress + checkAdmin bool + // stopped indicates whether the app has stopped // It can only happen if it has started successfully in the first place. // Otherwise, Cleanup will call Stop to clean up resources. @@ -187,6 +190,11 @@ func (app *App) Provision(ctx caddy.Context) error { // store some references app.logger = ctx.Logger() app.ctx = ctx + var err error + app.adminAddr, app.checkAdmin, err = ctx.LocalAdminAddress() + if err != nil { + return fmt.Errorf("parsing local admin address: %v", err) + } // provision TLS and events apps tlsAppIface, err := ctx.App("tls") @@ -410,7 +418,6 @@ func (app *App) Provision(ctx caddy.Context) error { // Validate ensures the app's configuration is valid. func (app *App) Validate() error { lnAddrs := make(map[string]string) - adminAddr, checkAdmin := caddy.LocalAdminAddress() for srvName, srv := range app.Servers { // each server must use distinct listener addresses @@ -419,8 +426,8 @@ func (app *App) Validate() error { if err != nil { return fmt.Errorf("invalid listener address '%s': %v", addr, err) } - if checkAdmin && listenAddr.OverlapsWith(adminAddr) { - return fmt.Errorf("server %s: listener %q overlaps with admin API address %s (use another address or 'admin off')", srvName, addr, adminAddr) + if app.checkAdmin && listenAddr.OverlapsWith(app.adminAddr) { + return fmt.Errorf("server %s: listener %q overlaps with admin API address %s (use another address or 'admin off')", srvName, addr, app.adminAddr) } // check that every address in the port range is unique to this server; // we do not use <= here because PortRangeSize() adds 1 to EndPort for us diff --git a/modules/caddyhttp/app_test.go b/modules/caddyhttp/app_test.go index ce169545be8..322a5a3a856 100644 --- a/modules/caddyhttp/app_test.go +++ b/modules/caddyhttp/app_test.go @@ -4,41 +4,51 @@ import ( "encoding/json" "fmt" "net" + "net/http" "strings" "testing" + "time" "github.com/caddyserver/caddy/v2" ) -func TestAppAdminAddressOverlap(t *testing.T) { +func TestValidateAdminAddressOverlap(t *testing.T) { for _, tc := range []struct { name string adminAddr string httpListen string + adminDisabled bool wantOverlapErr bool }{ { name: "rejects overlapping listener", - adminAddr: "localhost:%d", - httpListen: ":%d", + adminAddr: "localhost:2019", + httpListen: "localhost:2019", wantOverlapErr: true, }, { - name: "allows non-overlapping loopback addresses", - adminAddr: "127.0.0.1:%d", - httpListen: "127.0.0.2:%d", + name: "rejects identical ephemeral listener", + adminAddr: "127.0.0.1:0", + httpListen: "127.0.0.1:0", + wantOverlapErr: true, + }, + { + name: "allows distinct configured listeners", + adminAddr: "localhost:2019", + httpListen: ":2019", + }, + { + name: "allows listener when admin is disabled", + adminDisabled: true, + httpListen: "localhost:2019", wantOverlapErr: false, }, } { t.Run(tc.name, func(t *testing.T) { - _ = caddy.Stop() - t.Cleanup(func() { _ = caddy.Stop() }) - - port := freeTCPPort(t) - err := caddy.Run(&caddy.Config{ - Admin: &caddy.AdminConfig{Listen: fmt.Sprintf(tc.adminAddr, port)}, + err := caddy.Validate(&caddy.Config{ + Admin: &caddy.AdminConfig{Listen: tc.adminAddr, Disabled: tc.adminDisabled}, AppsRaw: map[string]json.RawMessage{ - "http": httpListenConfig(t, fmt.Sprintf(tc.httpListen, port)), + "http": httpListenConfig(t, tc.httpListen), }, }) if tc.wantOverlapErr { @@ -57,14 +67,110 @@ func TestAppAdminAddressOverlap(t *testing.T) { } } -func freeTCPPort(t *testing.T) uint { +func TestRunOverlapDoesNotReplaceAdminServer(t *testing.T) { + oldPort, newPort := freeTCPPorts(t) + oldAddr := fmt.Sprintf("127.0.0.1:%d", oldPort) + newAddr := fmt.Sprintf("127.0.0.1:%d", newPort) + + if err := caddy.Run(&caddy.Config{Admin: &caddy.AdminConfig{Disabled: true}}); err != nil { + t.Fatalf("disabling initial admin server: %v", err) + } + + assertOverlapRunRejected(t, newAddr) + assertNotListening(t, newAddr) + + if err := caddy.Run(&caddy.Config{Admin: &caddy.AdminConfig{Listen: oldAddr}}); err != nil { + t.Fatalf("starting initial admin server: %v", err) + } + t.Cleanup(func() { + if err := caddy.Run(&caddy.Config{Admin: &caddy.AdminConfig{Disabled: true}}); err != nil { + t.Errorf("stopping admin server: %v", err) + } + }) + + assertOverlapRunRejected(t, newAddr) + + assertAdminAvailable(t, oldAddr) + assertNotListening(t, newAddr) + + busy := busyTCPListenerExcept(t, newAddr) + defer busy.Close() + busyAddr := busy.Addr().String() + err := caddy.Run(&caddy.Config{ + Admin: &caddy.AdminConfig{Listen: newAddr}, + AppsRaw: map[string]json.RawMessage{ + "http": httpListenConfig(t, busyAddr), + }, + }) + if err == nil || !strings.Contains(err.Error(), "bind") { + t.Fatalf("expected listener bind error, got: %v", err) + } + assertAdminAvailable(t, oldAddr) + assertNotListening(t, newAddr) +} + +func busyTCPListenerExcept(t *testing.T, excludedAddr string) net.Listener { t.Helper() - ln, err := net.Listen("tcp", "127.0.0.1:0") + for { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + if ln.Addr().String() != excludedAddr { + return ln + } + ln.Close() + } +} + +func assertOverlapRunRejected(t *testing.T, addr string) { + t.Helper() + err := caddy.Run(&caddy.Config{ + Admin: &caddy.AdminConfig{Listen: addr}, + AppsRaw: map[string]json.RawMessage{ + "http": httpListenConfig(t, addr), + }, + }) + if err == nil || !strings.Contains(err.Error(), "overlaps with admin API address") { + t.Fatalf("expected overlap error, got: %v", err) + } +} + +func assertNotListening(t *testing.T, addr string) { + t.Helper() + conn, err := net.DialTimeout("tcp", addr, 200*time.Millisecond) + if err == nil { + conn.Close() + t.Fatalf("rejected admin server is listening on %s", addr) + } +} + +func assertAdminAvailable(t *testing.T, addr string) { + t.Helper() + client := &http.Client{Timeout: 2 * time.Second} + resp, err := client.Get("http://" + addr + "/config/") + if err != nil { + t.Fatalf("previous admin server is not available: %v", err) + } + resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("previous admin server returned %s", resp.Status) + } +} + +func freeTCPPorts(t *testing.T) (uint, uint) { + t.Helper() + ln1, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln1.Close() + ln2, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { t.Fatal(err) } - defer ln.Close() - return uint(ln.Addr().(*net.TCPAddr).Port) + defer ln2.Close() + return uint(ln1.Addr().(*net.TCPAddr).Port), uint(ln2.Addr().(*net.TCPAddr).Port) } func httpListenConfig(t *testing.T, addrs ...string) json.RawMessage { From 51000f481658725490e8fab64b50ebfbfc03c5af Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Fri, 17 Jul 2026 18:56:11 +1000 Subject: [PATCH 6/6] chore: fix Cfg because I am stupid --- caddy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/caddy.go b/caddy.go index 58acb84289a..481e81ffb84 100644 --- a/caddy.go +++ b/caddy.go @@ -462,7 +462,7 @@ func run(newCfg *Config, start bool) (Context, error) { // replace the local admin endpoint only after every app has started; // otherwise a rejected config could replace the admin endpoint while the // previous app config remains active - err = replaceLocalAdminServer(newCfg, ctx) + err = replaceLocalAdminServer(ctx.cfg, ctx) if err != nil { for _, appName := range started { if stopErr := ctx.cfg.apps[appName].Stop(); stopErr != nil {