Skip to content

Commit ae05ce6

Browse files
authored
fix(cluster-agent): gate pprof/expvar debug endpoints to loopback (#54508)
### What does this PR do? The Cluster Agent's metrics server (`0.0.0.0:metrics_port`, default `5000`) served the entire `http.DefaultServeMux`, exposing the `pprof` and `expvar` debug endpoints (registered via blank imports in `cmd/cluster-agent/main.go`) unauthenticated to anything that can reach the pod. This routes only `/metrics` on the public mux and gates everything under `/debug/` to loopback callers, returning `404` to non-loopback requests. ### Motivation Solves #CONTP-1771 and #VULN-87907. `/metrics` must stay reachable off-pod so the node Agent can scrape Cluster Agent telemetry, so a blanket localhost bind (like the core Agent's expvar server) isn't viable — hence a per-path loopback gate. This removes an unauthenticated, network-reachable debug/DoS surface while preserving telemetry scraping and local flare tooling. ### Describe how you validated your changes - Unit tests (`TestLoopbackOnly`, `TestMetricsMuxRouting`) covering loopback vs off-host for `/metrics` and `/debug/*` — 16/16 pass. - `dda inv cluster-agent.build` links clean; `dda inv linter.go` clean; release-note lint clean. - Generated a flare. - Deployed the built cluster-agent image to a local minikube cluster and curled the live DCA pod: - **In-pod** (loopback `127.0.0.1:5000`): `/metrics`, `/debug/vars`, `/debug/pprof/*` all `200` — flare/profiler unaffected. - **Off-pod** (from a separate pod → DCA pod IP): `/debug/*` = `404`, `/metrics` = `200`. - Verified nothing else is registered on the DCA's `DefaultServeMux` (only `pprof`/`expvar`), so no routes are dropped. ### Additional Notes Brings the DCA in line with the core Agent, which already binds its equivalent `pprof`/`expvar` server to `127.0.0.1`, while keeping `/metrics` public. Co-authored-by: wassim.dhif <wassim.dhif@datadoghq.com>
1 parent 0b134fc commit ae05ce6

4 files changed

Lines changed: 149 additions & 3 deletions

File tree

cmd/cluster-agent/subcommands/start/BUILD.bazel

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2163,66 +2163,79 @@ dd_agent_go_test(
21632163
"@rules_go//go/platform:aix": [
21642164
"//cmd/cluster-agent/command",
21652165
"//pkg/util/fxutil",
2166+
"@com_github_stretchr_testify//assert",
21662167
"@com_github_stretchr_testify//require",
21672168
],
21682169
"@rules_go//go/platform:android": [
21692170
"//cmd/cluster-agent/command",
21702171
"//pkg/util/fxutil",
2172+
"@com_github_stretchr_testify//assert",
21712173
"@com_github_stretchr_testify//require",
21722174
],
21732175
"@rules_go//go/platform:dragonfly": [
21742176
"//cmd/cluster-agent/command",
21752177
"//pkg/util/fxutil",
2178+
"@com_github_stretchr_testify//assert",
21762179
"@com_github_stretchr_testify//require",
21772180
],
21782181
"@rules_go//go/platform:freebsd": [
21792182
"//cmd/cluster-agent/command",
21802183
"//pkg/util/fxutil",
2184+
"@com_github_stretchr_testify//assert",
21812185
"@com_github_stretchr_testify//require",
21822186
],
21832187
"@rules_go//go/platform:illumos": [
21842188
"//cmd/cluster-agent/command",
21852189
"//pkg/util/fxutil",
2190+
"@com_github_stretchr_testify//assert",
21862191
"@com_github_stretchr_testify//require",
21872192
],
21882193
"@rules_go//go/platform:js": [
21892194
"//cmd/cluster-agent/command",
21902195
"//pkg/util/fxutil",
2196+
"@com_github_stretchr_testify//assert",
21912197
"@com_github_stretchr_testify//require",
21922198
],
21932199
"@rules_go//go/platform:linux": [
21942200
"//cmd/cluster-agent/command",
21952201
"//pkg/util/fxutil",
2202+
"@com_github_stretchr_testify//assert",
21962203
"@com_github_stretchr_testify//require",
21972204
],
21982205
"@rules_go//go/platform:netbsd": [
21992206
"//cmd/cluster-agent/command",
22002207
"//pkg/util/fxutil",
2208+
"@com_github_stretchr_testify//assert",
22012209
"@com_github_stretchr_testify//require",
22022210
],
22032211
"@rules_go//go/platform:openbsd": [
22042212
"//cmd/cluster-agent/command",
22052213
"//pkg/util/fxutil",
2214+
"@com_github_stretchr_testify//assert",
22062215
"@com_github_stretchr_testify//require",
22072216
],
22082217
"@rules_go//go/platform:osx": [
22092218
"//cmd/cluster-agent/command",
22102219
"//pkg/util/fxutil",
2220+
"@com_github_stretchr_testify//assert",
22112221
"@com_github_stretchr_testify//require",
22122222
],
22132223
"@rules_go//go/platform:plan9": [
22142224
"//cmd/cluster-agent/command",
22152225
"//pkg/util/fxutil",
2226+
"@com_github_stretchr_testify//assert",
22162227
"@com_github_stretchr_testify//require",
22172228
],
22182229
"@rules_go//go/platform:qnx": [
22192230
"//cmd/cluster-agent/command",
22202231
"//pkg/util/fxutil",
2232+
"@com_github_stretchr_testify//assert",
22212233
"@com_github_stretchr_testify//require",
22222234
],
22232235
"@rules_go//go/platform:solaris": [
22242236
"//cmd/cluster-agent/command",
22252237
"//pkg/util/fxutil",
2238+
"@com_github_stretchr_testify//assert",
22262239
"@com_github_stretchr_testify//require",
22272240
],
22282241
"//conditions:default": [],

cmd/cluster-agent/subcommands/start/command.go

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"context"
1313
"errors"
1414
"fmt"
15+
"net"
1516
"net/http"
1617
"os"
1718
"os/signal"
@@ -332,12 +333,19 @@ func start(log log.Component,
332333
return errors.New("no API key configured, exiting")
333334
}
334335

335-
// Expose the registered metrics via HTTP.
336-
http.Handle("/metrics", telemetry.Handler())
336+
// Expose the registered metrics via HTTP. /metrics must stay reachable on
337+
// all interfaces so the node agent can scrape cluster-agent telemetry, but
338+
// the pprof/expvar handlers that main.go registers on http.DefaultServeMux
339+
// must not: gate everything under /debug/ to loopback callers only.
340+
metricsMux := http.NewServeMux()
341+
metricsMux.Handle("/metrics", telemetry.Handler())
342+
debugHandler := loopbackOnly(http.DefaultServeMux)
343+
metricsMux.Handle("/debug", debugHandler)
344+
metricsMux.Handle("/debug/", debugHandler)
337345
metricsPort := config.GetInt("metrics_port")
338346
metricsServer := &http.Server{
339347
Addr: fmt.Sprintf("0.0.0.0:%d", metricsPort),
340-
Handler: http.DefaultServeMux,
348+
Handler: metricsMux,
341349
}
342350

343351
go func() {
@@ -776,6 +784,25 @@ func start(log log.Component,
776784
return nil
777785
}
778786

787+
// loopbackOnly serves h only for requests originating from a loopback address;
788+
// any other client receives a 404. The check uses the transport-level
789+
// RemoteAddr, not forwardable headers, so it cannot be spoofed by an off-host
790+
// caller. This keeps the pprof/expvar debug handlers reachable for local
791+
// tooling (e.g. the cluster-agent flare) while hiding them on the network.
792+
func loopbackOnly(h http.Handler) http.HandlerFunc {
793+
return func(w http.ResponseWriter, r *http.Request) {
794+
host, _, err := net.SplitHostPort(r.RemoteAddr)
795+
if err != nil {
796+
host = r.RemoteAddr
797+
}
798+
if ip := net.ParseIP(host); ip == nil || !ip.IsLoopback() {
799+
http.NotFound(w, r)
800+
return
801+
}
802+
h.ServeHTTP(w, r)
803+
}
804+
}
805+
779806
func setupInstrumentationCRDHandler(le *leaderelection.LeaderEngine, ac autodiscovery.Component, serviceTemplateStore *instrumentationhandlers.ServiceCheckTemplateStore) []instrumentation.Handler {
780807
checkStore := instrumentationhandlers.NewCheckStore()
781808
instrHandlers := instrumentationhandlers.DefaultHandlers(&instrumentationhandlers.Deps{

cmd/cluster-agent/subcommands/start/command_test.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,14 @@
88
package start
99

1010
import (
11+
"io"
12+
"net/http"
13+
"net/http/httptest"
1114
"os"
1215
"path"
1316
"testing"
1417

18+
"github.com/stretchr/testify/assert"
1519
"github.com/stretchr/testify/require"
1620

1721
"github.com/DataDog/datadog-agent/cmd/cluster-agent/command"
@@ -38,3 +42,93 @@ func newGlobalParamsTest(t *testing.T) *command.GlobalParams {
3842
ConfFilePath: config,
3943
}
4044
}
45+
46+
func TestLoopbackOnly(t *testing.T) {
47+
handler := loopbackOnly(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
48+
_, _ = io.WriteString(w, "served")
49+
}))
50+
51+
tests := []struct {
52+
name string
53+
remoteAddr string
54+
wantServed bool
55+
}{
56+
{"IPv4 loopback", "127.0.0.1:34567", true},
57+
{"IPv4 loopback range", "127.0.0.5:34567", true},
58+
{"IPv6 loopback", "[::1]:34567", true},
59+
{"RemoteAddr without port", "127.0.0.1", true},
60+
{"private IPv4 off-pod", "10.0.0.5:34567", false},
61+
{"docker bridge gateway", "172.17.0.1:34567", false},
62+
{"pod IP", "192.168.49.2:34567", false},
63+
{"public IPv4", "8.8.8.8:34567", false},
64+
{"unparseable RemoteAddr", "garbage", false},
65+
}
66+
67+
for _, tc := range tests {
68+
t.Run(tc.name, func(t *testing.T) {
69+
req := httptest.NewRequest(http.MethodGet, "/debug/vars", nil)
70+
req.RemoteAddr = tc.remoteAddr
71+
rec := httptest.NewRecorder()
72+
handler.ServeHTTP(rec, req)
73+
if tc.wantServed {
74+
assert.Equal(t, http.StatusOK, rec.Code)
75+
assert.Equal(t, "served", rec.Body.String())
76+
} else {
77+
assert.Equal(t, http.StatusNotFound, rec.Code)
78+
assert.NotContains(t, rec.Body.String(), "served")
79+
}
80+
})
81+
}
82+
}
83+
84+
// TestMetricsMuxRouting mirrors the mux wired in start(): /metrics is public on
85+
// all interfaces, everything under /debug/ is loopback-only.
86+
func TestMetricsMuxRouting(t *testing.T) {
87+
debug := http.NewServeMux()
88+
debug.HandleFunc("/debug/vars", func(w http.ResponseWriter, _ *http.Request) {
89+
_, _ = io.WriteString(w, "vars")
90+
})
91+
debug.HandleFunc("/debug/pprof/", func(w http.ResponseWriter, _ *http.Request) {
92+
_, _ = io.WriteString(w, "pprof")
93+
})
94+
95+
mux := http.NewServeMux()
96+
mux.HandleFunc("/metrics", func(w http.ResponseWriter, _ *http.Request) {
97+
_, _ = io.WriteString(w, "metrics")
98+
})
99+
debugHandler := loopbackOnly(debug)
100+
mux.Handle("/debug", debugHandler)
101+
mux.Handle("/debug/", debugHandler)
102+
103+
const offPod = "10.0.0.5:41000"
104+
const loopback = "127.0.0.1:41000"
105+
106+
tests := []struct {
107+
name string
108+
path string
109+
remoteAddr string
110+
wantCode int
111+
wantBody string
112+
}{
113+
{"metrics reachable off-pod", "/metrics", offPod, http.StatusOK, "metrics"},
114+
{"metrics reachable loopback", "/metrics", loopback, http.StatusOK, "metrics"},
115+
{"debug root hidden off-pod", "/debug", offPod, http.StatusNotFound, ""},
116+
{"expvar hidden off-pod", "/debug/vars", offPod, http.StatusNotFound, ""},
117+
{"expvar served loopback", "/debug/vars", loopback, http.StatusOK, "vars"},
118+
{"pprof hidden off-pod", "/debug/pprof/heap", offPod, http.StatusNotFound, ""},
119+
{"pprof served loopback", "/debug/pprof/heap", loopback, http.StatusOK, "pprof"},
120+
}
121+
122+
for _, tc := range tests {
123+
t.Run(tc.name, func(t *testing.T) {
124+
req := httptest.NewRequest(http.MethodGet, tc.path, nil)
125+
req.RemoteAddr = tc.remoteAddr
126+
rec := httptest.NewRecorder()
127+
mux.ServeHTTP(rec, req)
128+
assert.Equal(t, tc.wantCode, rec.Code)
129+
if tc.wantBody != "" {
130+
assert.Equal(t, tc.wantBody, rec.Body.String())
131+
}
132+
})
133+
}
134+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
security:
3+
- |
4+
The Cluster Agent no longer exposes the Go ``pprof`` profiling and
5+
``expvar`` debug endpoints on its metrics port (``metrics_port``, default
6+
``5000``) to remote callers. These ``/debug/`` endpoints were previously
7+
served on all network interfaces without authentication; they are now
8+
restricted to loopback callers, and requests originating from any other
9+
address receive a ``404``. The ``/metrics`` endpoint is unchanged and
10+
remains reachable off-host so the node Agent can continue to scrape
11+
Cluster Agent telemetry. Local tooling such as the Cluster Agent flare,
12+
which connects over loopback, is unaffected.

0 commit comments

Comments
 (0)