diff --git a/admin.go b/admin.go index 97af846ef9c..3e87a8c5504 100644 --- a/admin.go +++ b/admin.go @@ -364,6 +364,23 @@ func (admin AdminConfig) allowedOrigins(addr NetworkAddress) []*url.URL { return allowed } +// LocalAdminAddress returns the configured local admin API listen address +// and whether it is enabled. +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 // according to the relevant configuration in cfg. If no configuration // for the admin endpoint exists in cfg, a default one is used, so 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..481e81ffb84 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(ctx.cfg, 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 6031f98e495..33a6e013f87 100644 --- a/listeners.go +++ b/listeners.go @@ -255,6 +255,34 @@ func (na NetworkAddress) PortRangeSize() uint { return (na.EndPort - na.StartPort) + 1 } +// 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() || other.IsUnixNetwork() { + return na.IsUnixNetwork() && other.IsUnixNetwork() && na.bindPath() == other.bindPath() + } + if na.IsFdNetwork() || other.IsFdNetwork() { + return na.IsFdNetwork() && other.IsFdNetwork() && na.Host == other.Host + } + return na.Network == other.Network && + na.Host == other.Host && + na.StartPort <= other.EndPort && + other.StartPort <= na.EndPort +} + +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 +} + func (na NetworkAddress) isLoopback() bool { if na.IsUnixNetwork() || na.IsFdNetwork() { return true diff --git a/listeners_test.go b/listeners_test.go index 7bbaca1f9b3..1db32fac742 100644 --- a/listeners_test.go +++ b/listeners_test.go @@ -710,3 +710,93 @@ func TestSplitUnixSocketPermissionsBits(t *testing.T) { } } } + +func TestOverlapsWith(t *testing.T) { + for i, tc := range []struct { + a, b NetworkAddress + expect bool + }{ + { + a: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2019, EndPort: 2019}, + expect: true, + }, + { + a: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2019, EndPort: 2020}, + b: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2020, EndPort: 2021}, + expect: true, + }, + { + a: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2020, EndPort: 2020}, + expect: false, + }, + { + a: NetworkAddress{Network: "tcp", Host: "127.0.0.1"}, + b: NetworkAddress{Network: "tcp", Host: "127.0.0.1"}, + expect: true, + }, + { + a: NetworkAddress{Network: "tcp", Host: "localhost", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "tcp", Host: "", StartPort: 2019, EndPort: 2019}, + expect: false, + }, + { + 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: "plugin", Host: "same", StartPort: 2019, EndPort: 2019}, + b: NetworkAddress{Network: "plugin6", Host: "same", StartPort: 2019, EndPort: 2019}, + expect: false, + }, + { + 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: "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, + }, + { + 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, + }, + { + a: NetworkAddress{Network: "unix", Host: "/run/caddy/admin.sock|0222"}, + 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) + } +} + +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.go b/modules/caddyhttp/app.go index d9d9b9f8425..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") @@ -418,14 +426,18 @@ func (app *App) Validate() error { if err != nil { return fmt.Errorf("invalid listener address '%s': %v", addr, err) } + 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 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 + 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[addr] = srvName + lnAddrs[joinedAddr] = srvName } } diff --git a/modules/caddyhttp/app_test.go b/modules/caddyhttp/app_test.go new file mode 100644 index 00000000000..322a5a3a856 --- /dev/null +++ b/modules/caddyhttp/app_test.go @@ -0,0 +1,187 @@ +package caddyhttp + +import ( + "encoding/json" + "fmt" + "net" + "net/http" + "strings" + "testing" + "time" + + "github.com/caddyserver/caddy/v2" +) + +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:2019", + httpListen: "localhost:2019", + wantOverlapErr: true, + }, + { + 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) { + err := caddy.Validate(&caddy.Config{ + Admin: &caddy.AdminConfig{Listen: tc.adminAddr, Disabled: tc.adminDisabled}, + AppsRaw: map[string]json.RawMessage{ + "http": httpListenConfig(t, tc.httpListen), + }, + }) + 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 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() + 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 ln2.Close() + return uint(ln1.Addr().(*net.TCPAddr).Port), uint(ln2.Addr().(*net.TCPAddr).Port) +} + +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 { + t.Fatal(err) + } + return raw +}