You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix(appsec/nginx): reject cross-namespace --configmap refs in pod mutation (#51635) ### What does this PR do?
Fixes a confused-deputy vulnerability in the Cluster Agent's AppSec ingress-nginx admission mutator. The webhook previously extracted the namespace from the pod's `--configmap=<ns>/<name>` argument and used it verbatim for ConfigMap `Get`/`Create`/`Update` calls. Combined with the DCA's cluster-wide `configmaps` permissions, a low-privileged tenant with `create pods` rights in one namespace could trigger writes to ConfigMaps in arbitrary namespaces.
Changes:
- **`pkg/clusteragent/appsec/nginx/sidecar.go`** — `findControllerConfigMapArg` now requires the `<ns>` portion to match the pod's own namespace (resolving `$(POD_NAMESPACE)` first) and rejects empty names. On rejection, `MutatePod` returns `(false, nil)` to preserve fail-open admission semantics — the pod is admitted unmodified.
- **`pkg/clusteragent/appsec/nginx/events.go`** — New `CrossNamespaceConfigMapRefused` warning event on the **pod** (not the IngressClass) so the diagnostic lands in the tenant's namespace where their operator can see it.
- **`pkg/clusteragent/appsec/nginx/configmap.go`** — Defense-in-depth `validateConfigMapTarget` (DNS-1123 validation) at the entry of `createOrUpdateDDConfigMap`, covering both webhook and reconciler paths.
- **Tests** — `TestMutatePod_CrossNamespaceConfigMapRefused` (the bisect anchor) asserts no API calls escape and the pod spec is unmodified. `TestFindControllerConfigMapArg` extended from 3 cases to 10 covering same-ns, foreign ns, `kube-system` reference, leading/trailing slash, multi-container priority.
- **Release note** — `releasenotes/notes/fix-appsec-nginx-configmap-confused-deputy-*.yaml` (security section).
The introducing change was PR #49318 (Agent 7.78.0). All releases ≥7.78.0 are affected; backports to `7.78.x`, `7.79.x`, `7.80.x` will follow.
### Motivation
Tracking: [APPSEC-68212](https://datadoghq.atlassian.net/browse/APPSEC-68212). Internal vulnerability report `clusteragent-appsec-nginx-configmap-confused-deputy` (severity High, threat model k8s-tenant). Full mitigation plan: `.sisyphus/plans/clusteragent-appsec-nginx-configmap-confused-deputy-mitigation.md`.
Pre-condition for exploitation: DCA with `cluster_agent.appsec.injector.enabled = true` (helm: `datadog.appsec.injector.enabled: true`), at least one ingress-nginx `IngressClass` (`controller: k8s.io/ingress-nginx`), and a tenant with `create pods` permission in any namespace.
### Describe how you validated your changes
**1. Automated tests (run in CI):**
```bash
dda inv test --targets=./pkg/clusteragent/appsec/nginx # 57/57 passed
dda inv test --targets=./pkg/clusteragent/admission/mutate/appsec # 35/35 passed
dda inv linter.go --targets=./pkg/clusteragent/appsec/nginx # 0 issues
dda inv linter.releasenote # passed
```
`TestMutatePod_CrossNamespaceConfigMapRefused` is the **bisect anchor**: it fails against the unpatched code (which accepted `--configmap=kube-system/coredns` verbatim) and passes against this patch.
**2. Live exploit reproduction (k3s, rancher-desktop, 7.78.0 + this patch):**
Setup the DCA with my patched binary:
```bash
# Overlay the patched binary on the 7.78.0 base image
cat > Dockerfile.overlay <<'DOCKERFILE'
FROM datadog/cluster-agent:7.78.0
COPY bin/datadog-cluster-agent/datadog-cluster-agent /opt/datadog-agent/bin/datadog-cluster-agent
DOCKERFILE
docker build --platform linux/arm64 -t datadog/cluster-agent:7.78.0-fix -f Dockerfile.overlay .
# Install DCA with AppSec ingress-nginx enabled
cat > values.yaml <<'YAML'
datadog:
apiKey: "0000000000000000000000000000000000000000"
appKey: "0000000000000000000000000000000000000000"
clusterName: confused-deputy-test
appsec:
injector:
enabled: true
autoDetect: false
proxies: [ingress-nginx]
clusterAgent:
image:
repository: datadog/cluster-agent
tag: 7.78.0-fix
pullPolicy: IfNotPresent
admissionController:
enabled: true
agents: { enabled: false }
clusterChecksRunner: { enabled: false }
YAML
helm install dd datadog/datadog -n datadog --create-namespace -f values.yaml
# Pre-condition: at least one ingress-nginx IngressClass
kubectl apply -f - <<'YAML'
apiVersion: networking.k8s.io/v1
kind: IngressClass
metadata: { name: nginx-test }
spec: { controller: k8s.io/ingress-nginx }
YAML
```
Apply the exploit pod from a low-privileged tenant namespace:
```yaml
# attacker-pod.yaml
apiVersion: v1
kind: Namespace
metadata: { name: attacker-ns }
---
apiVersion: v1
kind: Pod
metadata:
name: confused-deputy-poc
namespace: attacker-ns
labels:
app.kubernetes.io/name: ingress-nginx
app.kubernetes.io/component: controller
spec:
containers:
- name: c
image: registry.k8s.io/ingress-nginx/controller:v1.15.1
args:
- /nginx-ingress-controller
- --configmap=kube-system/coredns
- --election-id=test
```
**Expected (and observed) outcomes:**
| Assertion | Command | Result |
|---|---|---|
| Pod admitted (fail-open) | `kubectl get pod -n attacker-ns confused-deputy-poc` | ✅ admitted, no admission error |
| Args UNMODIFIED | `kubectl get pod -n attacker-ns confused-deputy-poc -o jsonpath='{.spec.containers[0].args}'` | ✅ `--configmap=kube-system/coredns` preserved |
| No init container injected | `kubectl get pod -n attacker-ns confused-deputy-poc -o jsonpath='{.spec.initContainers}'` | ✅ empty |
| No DD ConfigMap in `kube-system` | `kubectl get cm -n kube-system \| grep -i datadog-appsec` | ✅ none |
| Warning event on pod | `kubectl get events -n attacker-ns --field-selector involvedObject.name=confused-deputy-poc` | ✅ `Warning CrossNamespaceConfigMapRefused AppSec nginx mutation skipped: --configmap references a namespace different from the pod's namespace; refusing to mutate to avoid confused-deputy ConfigMap writes: pod attacker-ns/confused-deputy-poc, arg "--configmap=kube-system/coredns"` |
| DCA log line | `kubectl logs -n datadog deploy/dd-datadog-cluster-agent \| grep "AppSec mutation skipped"` | ✅ `WARN \| nginx AppSec mutation skipped for pod attacker-ns/confused-deputy-poc: --configmap references a namespace different from the pod's namespace` |
**3. Regression check — legitimate ingress-nginx deployments still work**
Pods using the upstream Helm default (`--configmap=$(POD_NAMESPACE)/ingress-nginx-controller`) and pods using a literal same-namespace ref (`--configmap=ingress-nginx/my-config` when the pod is in `ingress-nginx`) are accepted and mutated normally. Covered by `TestFindControllerConfigMapArg/standard_$(POD_NAMESPACE)_form_is_accepted` and `.../hardcoded_same_namespace_is_accepted`.
### Additional Notes
- **`qa/rc-required` is required** — admission webhook changes touch cross-component behavior (DCA ↔ kube-apiserver ↔ node agents) per `AGENTS.md` guidance.
- The fix is **fail-open**: rejection results in the pod being admitted unmodified with a warning event and log line — never a failed admission. Legitimate ingress-nginx deployments using `$(POD_NAMESPACE)/...` (the upstream Helm default) are unaffected.
- `createOrUpdateDDConfigMap` gains DNS-1123 validation as defense-in-depth. It is a no-op for the reconciler path (whose namespace/name come from a label-filtered informer watch and are already valid Kubernetes objects) and catches any future code path that bypasses `findControllerConfigMapArg`.
- Follow-ups deferred to separate Jira tickets per the plan:
- E2E test in `test/new-e2e/tests/clusteragent/appsec/` (§4.5)
- Owner-reference pre-check on ingress-nginx pods (§5.2)
- `ValidatingAdmissionPolicy` for ConfigMap creation scope (§6 Option C)
Co-authored-by: eliott.bouhana <eliott.bouhana@datadoghq.com>
(cherry picked from commit 9ae4dea)
___
Co-authored-by: Eliott B <47679741+eliottness@users.noreply.github.com>
Copy file name to clipboardExpand all lines: pkg/clusteragent/appsec/nginx/sidecar.go
+48-10Lines changed: 48 additions & 10 deletions
Original file line number
Diff line number
Diff line change
@@ -9,6 +9,7 @@ package nginx
9
9
10
10
import (
11
11
"context"
12
+
"errors"
12
13
"fmt"
13
14
"maps"
14
15
"slices"
@@ -25,6 +26,17 @@ import (
25
26
"k8s.io/utils/ptr"
26
27
)
27
28
29
+
// errCrossNamespaceConfigMap signals that the pod's --configmap arg references
30
+
// a namespace different from the pod's own. We must not act on this because
31
+
// the DCA service account holds cluster-wide ConfigMap permissions and the pod
32
+
// creator may be a low-privileged tenant.
33
+
varerrCrossNamespaceConfigMap=errors.New("--configmap references a namespace different from the pod's namespace; refusing to mutate to avoid confused-deputy ConfigMap writes")
34
+
35
+
// errEmptyConfigMapName signals that the pod's --configmap arg has an empty
36
+
// name after the slash (e.g. "--configmap=foo/"). This is a malformed arg
37
+
// and we refuse to act on it.
38
+
varerrEmptyConfigMapName=errors.New("--configmap has empty name after namespace separator")
39
+
28
40
const (
29
41
// mutateTimeout bounds ConfigMap operations during pod mutation to prevent
30
42
// goroutine leaks if the API server is slow. The MutatePod interface does not
0 commit comments