[Backport 7.79.x] fix(appsec/nginx): reject cross-namespace --configmap refs in pod mutation

pkg/clusteragent/appsec/nginx/configmap.go

@@ -18,6 +18,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/client-go/dynamic"
 )
 
@@ -83,9 +84,26 @@ func stripDDSnippet(snippet string) string {
 	return snippet[:startIdx] + snippet[endIdx:]
 }
 
+// validateConfigMapTarget enforces Kubernetes DNS-1123 naming on the namespace
+// and name reaching the API client. This is a defense-in-depth guard against
+// any future call path that bypasses findControllerConfigMapArg; it is a no-op
+// for the reconciler path where values come from valid Kubernetes objects.
+func validateConfigMapTarget(namespace, name string) error {
+	if errs := validation.IsDNS1123Label(namespace); len(errs) > 0 {
+		return fmt.Errorf("invalid ConfigMap namespace %q: %v", namespace, errs)
+	}
+	if errs := validation.IsDNS1123Subdomain(name); len(errs) > 0 {
+		return fmt.Errorf("invalid ConfigMap name %q: %v", name, errs)
+	}
+	return nil
+}
+
 // createOrUpdateDDConfigMap creates or updates the DD-owned ConfigMap by mirroring the original
 // and prepending Datadog AppSec directives to main-snippet and http-snippet.
 func createOrUpdateDDConfigMap(ctx context.Context, client dynamic.Interface, namespace, originalCMName, moduleMountPath string, labels, annotations map[string]string) error {
+	if err := validateConfigMapTarget(namespace, originalCMName); err != nil {
+		return err
+	}
 	ddName := ddConfigMapName(originalCMName)
 
 	// Fetch original ConfigMap (may not exist if user hasn't customized anything)

pkg/clusteragent/appsec/nginx/events.go

@@ -14,11 +14,12 @@ import (
 
 // Event reasons for ingress-nginx ConfigMap operations
 const (
-	EventReasonConfigMapCreated      = "DatadogConfigMapCreated"
-	EventReasonConfigMapCreateFailed = "DatadogConfigMapCreateFailed"
-	EventReasonConfigMapDeleted      = "DatadogConfigMapDeleted"
-	EventReasonConfigMapDeleteFailed = "DatadogConfigMapDeleteFailed"
-	EventReasonVersionParseFailed    = "VersionParseFailed"
+	EventReasonConfigMapCreated               = "DatadogConfigMapCreated"
+	EventReasonConfigMapCreateFailed          = "DatadogConfigMapCreateFailed"
+	EventReasonConfigMapDeleted               = "DatadogConfigMapDeleted"
+	EventReasonConfigMapDeleteFailed          = "DatadogConfigMapDeleteFailed"
+	EventReasonVersionParseFailed             = "VersionParseFailed"
+	EventReasonCrossNamespaceConfigMapRefused = "CrossNamespaceConfigMapRefused"
 )
 
 // eventRecorder provides methods to record Kubernetes events for appsec nginx resources
@@ -86,3 +87,25 @@ func (e *eventRecorder) recordVersionParseFailed(podName, image string) {
 		image,
 	)
 }
+
+// recordCrossNamespaceConfigMapRefused emits a Warning event on the pod itself
+// (not the target ConfigMap or IngressClass) so the diagnostic appears in the
+// namespace owned by the pod creator who triggered the rejection. Pod UID may
+// be empty at admission time since the API server assigns it after the
+// mutating webhook chain; the recorder accepts an empty UID and the event will
+// still post, just without UID-based correlation.
+func (e *eventRecorder) recordCrossNamespaceConfigMapRefused(pod *corev1.Pod, err error) {
+	e.recorder.Eventf(
+		&corev1.ObjectReference{
+			Kind:       "Pod",
+			APIVersion: "v1",
+			Name:       pod.Name,
+			Namespace:  pod.Namespace,
+			UID:        pod.UID,
+		},
+		corev1.EventTypeWarning,
+		EventReasonCrossNamespaceConfigMapRefused,
+		"AppSec nginx mutation skipped: %v",
+		err,
+	)
+}

pkg/clusteragent/appsec/nginx/sidecar.go

@@ -9,6 +9,7 @@ package nginx
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"maps"
 	"slices"
@@ -25,6 +26,17 @@ import (
 	"k8s.io/utils/ptr"
 )
 
+// errCrossNamespaceConfigMap signals that the pod's --configmap arg references
+// a namespace different from the pod's own. We must not act on this because
+// the DCA service account holds cluster-wide ConfigMap permissions and the pod
+// creator may be a low-privileged tenant.
+var errCrossNamespaceConfigMap = errors.New("--configmap references a namespace different from the pod's namespace; refusing to mutate to avoid confused-deputy ConfigMap writes")
+
+// errEmptyConfigMapName signals that the pod's --configmap arg has an empty
+// name after the slash (e.g. "--configmap=foo/"). This is a malformed arg
+// and we refuse to act on it.
+var errEmptyConfigMapName = errors.New("--configmap has empty name after namespace separator")
+
 const (
 	// mutateTimeout bounds ConfigMap operations during pod mutation to prevent
 	// goroutine leaks if the API server is slow. The MutatePod interface does not
@@ -84,8 +96,17 @@ func (n *nginxSidecarPattern) MutatePod(pod *corev1.Pod, ns string, client dynam
 		return false, fmt.Errorf("pod %s has no containers", mutatecommon.PodString(pod))
 	}
 
-	// Find the controller container with --configmap arg (or note it's absent)
-	containerIdx, argIdx, cmNamespace, cmName, found := findControllerConfigMapArg(pod, ns)
+	containerIdx, argIdx, cmNamespace, cmName, found, err := findControllerConfigMapArg(pod, ns)
+	if err != nil {
+		// Refusing to mutate on cross-namespace or malformed --configmap args is
+		// a security policy decision, not a failure. Return (false, nil) so the
+		// pod is admitted unmodified (fail-open) without polluting the error
+		// path used by genuine mutation failures. The warning event lands on
+		// the pod itself so the owning namespace operator can see the diagnostic.
+		n.eventRecorder.recordCrossNamespaceConfigMapRefused(pod, err)
+		n.logger.Warnf("nginx AppSec mutation skipped for pod %s: %v", mutatecommon.PodString(pod), err)
+		return false, nil
+	}
 	if !found {
 		cmName = "ingress-nginx-controller"
 		cmNamespace = ns
@@ -171,11 +192,21 @@ func (n *nginxSidecarPattern) MatchCondition() admissionregistrationv1.MatchCond
 	}
 }
 
-// findControllerConfigMapArg finds the controller container and its --configmap arg,
-// resolving $(POD_NAMESPACE) to the actual pod namespace.
-// If the arg is not found, found is false and containerIdx 0 / argIdx -1 are returned
-// so the caller can append the arg to the first container instead.
-func findControllerConfigMapArg(pod *corev1.Pod, podNamespace string) (containerIdx, argIdx int, cmNamespace, cmName string, found bool) {
+// findControllerConfigMapArg finds the controller container and its --configmap arg.
+// It resolves $(POD_NAMESPACE) to the pod's namespace and rejects any other
+// namespace value, because the pod arg is attacker-controlled and the DCA holds
+// cluster-wide ConfigMap permissions (confused-deputy primitive). It also
+// rejects empty names (after the slash separator).
+//
+// Return contract:
+//   - arg absent: found=false, err=nil — caller defaults to (podNamespace, "ingress-nginx-controller").
+//   - arg present and valid: found=true, err=nil.
+//   - arg present but malformed/cross-namespace: err!=nil — caller must skip mutation.
+//
+// The webhook runs before kubelet substitution, so "$(POD_NAMESPACE)" arrives
+// as a literal string and we resolve it ourselves. Upstream ingress-nginx only
+// supports this single syntax, so variants like ${POD_NAMESPACE} are not recognized.
+func findControllerConfigMapArg(pod *corev1.Pod, podNamespace string) (containerIdx, argIdx int, cmNamespace, cmName string, found bool, err error) {
 	for ci, c := range pod.Spec.Containers {
 		for ai, arg := range c.Args {
 			value, ok := strings.CutPrefix(arg, configmapArgPrefix)
@@ -186,14 +217,21 @@ func findControllerConfigMapArg(pod *corev1.Pod, podNamespace string) (container
 			if !ok {
 				continue
 			}
-			// Resolve $(POD_NAMESPACE) to the actual namespace
 			if ns == "$(POD_NAMESPACE)" {
 				ns = podNamespace
 			}
-			return ci, ai, ns, name, true
+			if ns != podNamespace {
+				return ci, ai, "", "", false, fmt.Errorf("%w: pod %s, arg %q",
+					errCrossNamespaceConfigMap, mutatecommon.PodString(pod), arg)
+			}
+			if name == "" {
+				return ci, ai, "", "", false, fmt.Errorf("%w: pod %s, arg %q",
+					errEmptyConfigMapName, mutatecommon.PodString(pod), arg)
+			}
+			return ci, ai, ns, name, true, nil
 		}
 	}
-	return 0, -1, podNamespace, "", false
+	return 0, -1, podNamespace, "", false, nil
 }
 
 // parseControllerVersion extracts the version tag from an ingress-nginx controller image reference.

pkg/clusteragent/appsec/nginx/sidecar_test.go

@@ -223,31 +223,92 @@ func TestFindControllerConfigMapArg(t *testing.T) {
 		wantNS       string
 		wantName     string
 		wantFound    bool
+		wantErr      error
 	}{
 		{
-			name:         "standard $(POD_NAMESPACE) form",
+			name:         "standard $(POD_NAMESPACE) form is accepted",
 			pod:          newControllerPod("test", "ingress-nginx", "img:v1"),
 			podNamespace: "ingress-nginx",
 			wantNS:       "ingress-nginx",
 			wantName:     "ingress-nginx-controller",
 			wantFound:    true,
 		},
 		{
-			name: "hardcoded namespace form",
+			name: "hardcoded same namespace is accepted",
 			pod: &corev1.Pod{
 				Spec: corev1.PodSpec{
 					Containers: []corev1.Container{{
-						Args: []string{"--configmap=custom-ns/my-config"},
+						Args: []string{"--configmap=ingress-nginx/my-config"},
 					}},
 				},
 			},
 			podNamespace: "ingress-nginx",
-			wantNS:       "custom-ns",
+			wantNS:       "ingress-nginx",
 			wantName:     "my-config",
 			wantFound:    true,
 		},
 		{
-			name: "no configmap arg",
+			name: "hardcoded foreign namespace is rejected (confused-deputy guard)",
+			pod: &corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Args: []string{"--configmap=custom-ns/my-config"},
+					}},
+				},
+			},
+			podNamespace: "ingress-nginx",
+			wantErr:      errCrossNamespaceConfigMap,
+		},
+		{
+			name: "kube-system reference is rejected",
+			pod: &corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Args: []string{"--configmap=kube-system/coredns"},
+					}},
+				},
+			},
+			podNamespace: "attacker-ns",
+			wantErr:      errCrossNamespaceConfigMap,
+		},
+		{
+			name: "leading slash with empty namespace is rejected",
+			pod: &corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Args: []string{"--configmap=/foo"},
+					}},
+				},
+			},
+			podNamespace: "ingress-nginx",
+			wantErr:      errCrossNamespaceConfigMap,
+		},
+		{
+			name: "trailing slash with empty name is rejected",
+			pod: &corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Args: []string{"--configmap=ingress-nginx/"},
+					}},
+				},
+			},
+			podNamespace: "ingress-nginx",
+			wantErr:      errEmptyConfigMapName,
+		},
+		{
+			name: "no slash skips the arg and falls through to not-found",
+			pod: &corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Args: []string{"--configmap=foo"},
+					}},
+				},
+			},
+			podNamespace: "ingress-nginx",
+			wantFound:    false,
+		},
+		{
+			name: "no configmap arg falls back to defaults",
 			pod: &corev1.Pod{
 				Spec: corev1.PodSpec{
 					Containers: []corev1.Container{{
@@ -258,11 +319,45 @@ func TestFindControllerConfigMapArg(t *testing.T) {
 			podNamespace: "ingress-nginx",
 			wantFound:    false,
 		},
+		{
+			name: "multi-container first match wins - malicious arg rejected even if later container is benign",
+			pod: &corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{Args: []string{"--configmap=kube-system/coredns"}},
+						{Args: []string{"--configmap=ingress-nginx/legit"}},
+					},
+				},
+			},
+			podNamespace: "ingress-nginx",
+			wantErr:      errCrossNamespaceConfigMap,
+		},
+		{
+			name: "multi-container second container holds the arg",
+			pod: &corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{Args: []string{"--election-id=x"}},
+						{Args: []string{"--configmap=ingress-nginx/legit"}},
+					},
+				},
+			},
+			podNamespace: "ingress-nginx",
+			wantNS:       "ingress-nginx",
+			wantName:     "legit",
+			wantFound:    true,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			_, _, ns, name, found := findControllerConfigMapArg(tt.pod, tt.podNamespace)
+			_, _, ns, name, found, err := findControllerConfigMapArg(tt.pod, tt.podNamespace)
+			if tt.wantErr != nil {
+				require.ErrorIs(t, err, tt.wantErr)
+				assert.False(t, found)
+				return
+			}
+			require.NoError(t, err)
 			assert.Equal(t, tt.wantFound, found)
 			if tt.wantFound {
 				assert.Equal(t, tt.wantNS, ns)
@@ -431,3 +526,46 @@ func TestMutatePodVersionParseFailed(t *testing.T) {
 	assert.False(t, mutated)
 	assert.ErrorContains(t, err, "manual extraModules")
 }
+
+// TestMutatePod_CrossNamespaceConfigMapRefused is the bisect anchor for the
+// confused-deputy ConfigMap mitigation. It MUST fail against the unpatched
+// code (which trusted the pod's --configmap arg verbatim) and pass against the
+// patched code.
+func TestMutatePod_CrossNamespaceConfigMapRefused(t *testing.T) {
+	pattern, client := newTestNginxSidecarPattern(t)
+
+	pod := newControllerPod("attacker", "attacker-ns", "registry.k8s.io/ingress-nginx/controller:v1.15.1")
+	pod.Spec.Containers[0].Args = []string{
+		"/nginx-ingress-controller",
+		"--configmap=kube-system/coredns",
+		"--election-id=ingress-nginx-leader",
+	}
+
+	mutated, err := pattern.MutatePod(pod, "attacker-ns", client)
+	require.NoError(t, err, "MutatePod must not fail admission on cross-ns refs (fail-open)")
+	assert.False(t, mutated, "MutatePod must skip mutation on cross-ns refs")
+
+	assert.Empty(t, client.Actions(), "no API operations may occur on rejection")
+
+	assert.Equal(t, "--configmap=kube-system/coredns", pod.Spec.Containers[0].Args[1],
+		"pod arg must be unmodified")
+	assert.Empty(t, pod.Spec.InitContainers, "no init container must be injected")
+	assert.Empty(t, pod.Spec.Volumes, "no volume must be added")
+	assert.Empty(t, pod.Spec.Containers[0].VolumeMounts, "no volume mount must be added")
+}
+
+func TestMutatePod_EmptyConfigMapNameRefused(t *testing.T) {
+	pattern, client := newTestNginxSidecarPattern(t)
+
+	pod := newControllerPod("test", "ingress-nginx", "registry.k8s.io/ingress-nginx/controller:v1.15.1")
+	pod.Spec.Containers[0].Args = []string{
+		"/nginx-ingress-controller",
+		"--configmap=ingress-nginx/",
+	}
+
+	mutated, err := pattern.MutatePod(pod, "ingress-nginx", client)
+	require.NoError(t, err)
+	assert.False(t, mutated)
+	assert.Empty(t, pod.Spec.InitContainers)
+	assert.Empty(t, pod.Spec.Volumes)
+}

releasenotes/notes/fix-appsec-nginx-configmap-confused-deputy-f3c92abf7a3097dd.yaml

@@ -0,0 +1,13 @@
+---
+security:
+  - |
+    Fixed a confused-deputy vulnerability in the Cluster Agent's AppSec
+    ingress-nginx admission mutator where the pod's
+    ``--configmap=<namespace>/<name>`` argument was trusted verbatim,
+    allowing a user with pod-create permission in one namespace to make
+    the Cluster Agent service account create or update ConfigMaps and add
+    labels and annotations in arbitrary namespaces. The mutator now
+    requires the ``<namespace>`` portion to match the pod's own namespace
+    (or use the ``$(POD_NAMESPACE)`` downward-API substitution) and skips
+    mutation otherwise, emitting a warning event on the pod. The
+    vulnerability affected Cluster Agent releases starting from 7.78.0.