From 40deadb69f24ba818b38a8476852c86800d21034 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 5 Jun 2026 14:54:58 -0400 Subject: [PATCH 1/3] feat: Wire proxy-sidecar egress enforcement (UID fix + enforce-drop) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit proxy-sidecar mode routes egress through AuthBridge via HTTP_PROXY, which is cooperative and bypassable. kagenti-extensions#484 added MODE=enforce-drop to the shared proxy-init image (a fail-closed mangle-OUTPUT guard); this wires it into the operator, plus the prerequisite UID fix. - Fix: the proxy-sidecar authbridge-proxy container hardcoded RunAsUser 1001 (same as the app), so the enforce-drop `--uid-owner` exemption could not distinguish proxy from app egress. Derive it from cfg.Proxy.UID (1337) — the single source of truth also used for the proxy-init PROXY_UID — matching the envoy builder. Add RunAsGroup parity and validate Proxy.UID >= 1. - Add cluster config (PlatformConfig.Proxy): EgressEnforcement ("off"|"enforce", default "off") and ClusterCIDRs (default 10.0.0.0/8, Kind-shaped). Surfaced via the operator chart's defaults.proxy. Off by default keeps main shippable; a future release flips the compiled default to "enforce" — the knob is a permanent override, nothing to remove. - BuildProxyInitContainer gains a mode param: "redirect" (envoy-sidecar, unchanged) or "enforce-drop" (emits MODE + CLUSTER_CIDRS; omits the redirect-only POD_IP/exclude vars). The proxy-sidecar branch injects it when EgressEnforcement=="enforce"; envoy-sidecar is unaffected. enforce-drop only applies to proxy-sidecar — envoy-sidecar already enforces egress structurally via the transparent redirect, so the knob is consulted only on the proxy-sidecar/lite path (no mode-compatibility matrix needed). Tests: proxy container RunAsUser==Proxy.UID; proxy-init enforce-drop env; injection present on enforce / absent on default-off / not duplicated. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- charts/kagenti-operator/values.yaml | 15 ++- .../internal/webhook/config/defaults.go | 6 + .../internal/webhook/config/types.go | 41 +++++++ .../internal/webhook/injector/constants.go | 11 ++ .../webhook/injector/container_builder.go | 115 +++++++++++------- .../injector/container_builder_test.go | 61 +++++++++- .../internal/webhook/injector/pod_mutator.go | 17 ++- .../webhook/injector/pod_mutator_test.go | 80 ++++++++++++ 8 files changed, 296 insertions(+), 50 deletions(-) diff --git a/charts/kagenti-operator/values.yaml b/charts/kagenti-operator/values.yaml index f9fcc90c..99909703 100644 --- a/charts/kagenti-operator/values.yaml +++ b/charts/kagenti-operator/values.yaml @@ -174,7 +174,8 @@ featureGates: # * authbridgeLite (lite mode): authbridge-lite (jwt-validation + token-exchange only, # parsers dropped) + spiffe-helper. Same listener layout # as authbridge; for size-constrained deployments. -# proxy-init applies to envoy-sidecar mode only. +# proxy-init applies to envoy-sidecar mode (transparent redirect) and to +# proxy-sidecar mode when proxy.egressEnforcement is "enforce" (enforce-drop). defaults: images: envoyProxy: ghcr.io/kagenti/kagenti-extensions/authbridge-envoy:latest @@ -189,6 +190,18 @@ defaults: uid: 1337 adminPort: 9901 inboundProxyPort: 15124 + # Proxy-sidecar fail-closed egress enforcement. "off" (default) keeps egress + # cooperative (HTTP_PROXY only); "enforce" injects a proxy-init enforce-drop + # guard so direct egress is dropped. envoy-sidecar is unaffected (it already + # enforces via transparent redirect). + egressEnforcement: "off" + # In-cluster CIDRs (pods/services/DNS) allowed direct by the enforce-drop + # guard; everything else egressing the pod is dropped. The default is + # Kind-shaped (pods 10.244/16 + services 10.96/16). OpenShift/EKS MUST + # override — e.g. OCP services 172.30.0.0/16 (outside 10/8) + pods + # 10.128.0.0/14 — or in-cluster service traffic will be dropped. + clusterCIDRs: + - "10.0.0.0/8" # Resource defaults (conservative for dev) # Note: requests must be <= limits diff --git a/kagenti-operator/internal/webhook/config/defaults.go b/kagenti-operator/internal/webhook/config/defaults.go index 1b824a77..b4c7101c 100644 --- a/kagenti-operator/internal/webhook/config/defaults.go +++ b/kagenti-operator/internal/webhook/config/defaults.go @@ -32,6 +32,12 @@ func CompiledDefaults() *PlatformConfig { UID: 1337, InboundProxyPort: 15124, AdminPort: 9901, + // Off by default — proxy-sidecar stays cooperative (HTTP_PROXY only) + // until an operator opts in. Migrate to "enforce" in a future release. + EgressEnforcement: EgressEnforcementOff, + // Kind-shaped default (pods 10.244/16 + services 10.96/16). OCP/EKS + // MUST override (see ProxyConfig.ClusterCIDRs doc). + ClusterCIDRs: []string{"10.0.0.0/8"}, }, Resources: ResourcesConfig{ EnvoyProxy: corev1.ResourceRequirements{ diff --git a/kagenti-operator/internal/webhook/config/types.go b/kagenti-operator/internal/webhook/config/types.go index 96291ee9..2a63fc72 100644 --- a/kagenti-operator/internal/webhook/config/types.go +++ b/kagenti-operator/internal/webhook/config/types.go @@ -40,11 +40,37 @@ type ImageConfig struct { PullPolicy corev1.PullPolicy `json:"pullPolicy" yaml:"pullPolicy"` } +// EgressEnforcement values for ProxyConfig.EgressEnforcement. +const ( + EgressEnforcementOff = "off" + EgressEnforcementEnforce = "enforce" +) + type ProxyConfig struct { Port int32 `json:"port" yaml:"port"` UID int64 `json:"uid" yaml:"uid"` InboundProxyPort int32 `json:"inboundProxyPort" yaml:"inboundProxyPort"` AdminPort int32 `json:"adminPort" yaml:"adminPort"` + + // EgressEnforcement controls the proxy-sidecar fail-closed egress guard. + // "off" (default): the workload routes egress through the forward proxy + // via HTTP_PROXY only — cooperative and bypassable. + // "enforce": a proxy-init container installs the enforce-drop iptables + // guard, forcing all external egress through the forward proxy regardless + // of whether the workload honors HTTP_PROXY. + // envoy-sidecar mode is unaffected — it already enforces egress structurally + // via the transparent redirect, so this knob is consulted only on the + // proxy-sidecar / lite path. Migrate the default off->enforce in a future + // release once ClusterCIDRs sourcing is validated across distros. + EgressEnforcement string `json:"egressEnforcement" yaml:"egressEnforcement"` + + // ClusterCIDRs are the in-cluster ranges (pods / services / DNS) that the + // enforce-drop guard allows direct; everything else egressing the pod is + // dropped. The default 10.0.0.0/8 is Kind-shaped (pods 10.244/16 + services + // 10.96/16). OCP/EKS MUST override this (e.g. OCP services 172.30.0.0/16, + // pods 10.128.0.0/14 — 172.30/16 is outside 10/8) or in-cluster service + // traffic will be dropped. Only used when EgressEnforcement == "enforce". + ClusterCIDRs []string `json:"clusterCIDRs" yaml:"clusterCIDRs"` } type ResourcesConfig struct { @@ -83,6 +109,11 @@ func (c *PlatformConfig) DeepCopy() *PlatformConfig { copy(result.TokenExchange.DefaultScopes, c.TokenExchange.DefaultScopes) } + if c.Proxy.ClusterCIDRs != nil { + result.Proxy.ClusterCIDRs = make([]string, len(c.Proxy.ClusterCIDRs)) + copy(result.Proxy.ClusterCIDRs, c.Proxy.ClusterCIDRs) + } + // Deep copy ResourceRequirements — ResourceList is a map that would be shared result.Resources.EnvoyProxy = deepCopyResourceRequirements(c.Resources.EnvoyProxy) result.Resources.ProxyInit = deepCopyResourceRequirements(c.Resources.ProxyInit) @@ -119,6 +150,16 @@ func (c *PlatformConfig) Validate() error { if c.Proxy.AdminPort < 1024 || c.Proxy.AdminPort > 65535 { return fmt.Errorf("proxy.adminPort must be between 1024 and 65535") } + // The enforce-drop guard exempts this UID (--uid-owner) and the proxy + // container runs as it; it must be a real non-root user. + if c.Proxy.UID < 1 { + return fmt.Errorf("proxy.uid must be >= 1 (got %d): the proxy must not run as root and the enforce-drop exemption keys on this UID", c.Proxy.UID) + } + switch c.Proxy.EgressEnforcement { + case "", EgressEnforcementOff, EgressEnforcementEnforce: + default: + return fmt.Errorf("proxy.egressEnforcement must be \"off\" or \"enforce\" (got %q)", c.Proxy.EgressEnforcement) + } if c.Images.EnvoyProxy == "" { return fmt.Errorf("images.envoyProxy is required") } diff --git a/kagenti-operator/internal/webhook/injector/constants.go b/kagenti-operator/internal/webhook/injector/constants.go index f3eaf888..8b943f41 100644 --- a/kagenti-operator/internal/webhook/injector/constants.go +++ b/kagenti-operator/internal/webhook/injector/constants.go @@ -38,6 +38,17 @@ const ( ClientAuthTypeFederatedJWT = "federated-jwt" ) +// proxy-init MODE values — select the iptables strategy that +// BuildProxyInitContainer passes to init-iptables.sh. +const ( + // ProxyInitModeRedirect transparently REDIRECTs pod traffic to the Envoy + // listeners (envoy-sidecar mode). + ProxyInitModeRedirect = "redirect" + // ProxyInitModeEnforceDrop installs the fail-closed egress guard that DROPs + // any egress bypassing the forward proxy (proxy-sidecar egress enforcement). + ProxyInitModeEnforceDrop = "enforce-drop" +) + // mTLS modes for the proxy-sidecar / lite paths. Selected per workload // via AgentRuntime CR `Spec.MTLSMode`, falling back to the namespace // `authbridge-runtime-config` ConfigMap's `mtls.mode` field, then diff --git a/kagenti-operator/internal/webhook/injector/container_builder.go b/kagenti-operator/internal/webhook/injector/container_builder.go index 40bddc28..bd66b2d9 100644 --- a/kagenti-operator/internal/webhook/injector/container_builder.go +++ b/kagenti-operator/internal/webhook/injector/container_builder.go @@ -299,7 +299,13 @@ func (b *ContainerBuilder) BuildProxySidecarContainerWithPorts(spireEnabled bool }, Resources: b.cfg.Resources.AuthBridge, SecurityContext: &corev1.SecurityContext{ - RunAsUser: ptr.To(int64(1001)), + // Run as the dedicated proxy UID (default 1337), the SAME value the + // proxy-init enforce-drop guard exempts via `--uid-owner $PROXY_UID`. + // Deriving both from b.cfg.Proxy.UID keeps the exempted UID in lockstep + // with the process it exempts (a hardcoded literal could drift and + // silently break the proxy's own egress). Matches the envoy builder. + RunAsUser: ptr.To(b.cfg.Proxy.UID), + RunAsGroup: ptr.To(b.cfg.Proxy.UID), RunAsNonRoot: ptr.To(true), AllowPrivilegeEscalation: ptr.To(false), }, @@ -448,51 +454,72 @@ func (b *ContainerBuilder) buildEnvoyProxyEnvLegacy() []corev1.EnvVar { const mandatoryOutboundExclude = "8080" // BuildProxyInitContainer creates the proxy-init container. -// outboundPortsExclude is a comma-separated list of additional ports to -// exclude from outbound interception (mandatory 8080 is always included). -// inboundPortsExclude is a comma-separated list of ports to exclude from -// inbound interception (only set when non-empty). Both come from the -// kagenti.io/outbound-ports-exclude and kagenti.io/inbound-ports-exclude -// pod annotations. -func (b *ContainerBuilder) BuildProxyInitContainer(outboundPortsExclude, inboundPortsExclude string) corev1.Container { - outboundValue := buildOutboundExcludeValue(outboundPortsExclude) - inboundValue := buildPortExcludeValue(inboundPortsExclude, "inbound-ports-exclude") - - builderLog.Info("building ProxyInit Container", - "resolvedOutboundPortsExclude", outboundValue, - "resolvedInboundPortsExclude", inboundValue) - - env := []corev1.EnvVar{ - { - Name: "PROXY_PORT", - Value: fmt.Sprintf("%d", b.cfg.Proxy.Port), - }, - { - Name: "INBOUND_PROXY_PORT", - Value: fmt.Sprintf("%d", b.cfg.Proxy.InboundProxyPort), - }, - { - Name: "PROXY_UID", - Value: fmt.Sprintf("%d", b.cfg.Proxy.UID), - }, - { - Name: "OUTBOUND_PORTS_EXCLUDE", - Value: outboundValue, - }, - { - Name: "POD_IP", - ValueFrom: &corev1.EnvVarSource{ - FieldRef: &corev1.ObjectFieldSelector{ - FieldPath: "status.podIP", +// mode selects the interception strategy the init script runs: +// - "redirect" (envoy-sidecar): transparently REDIRECT pod traffic to the +// Envoy listeners. outboundPortsExclude / inboundPortsExclude (from the +// kagenti.io/outbound-ports-exclude and kagenti.io/inbound-ports-exclude +// pod annotations; mandatory 8080 always included outbound) tune it. +// - "enforce-drop" (proxy-sidecar): a fail-closed egress guard that DROPs any +// egress bypassing the forward proxy. Driven by PROXY_UID + CLUSTER_CIDRS; +// the exclude args do not apply (the script ignores them in this mode). +func (b *ContainerBuilder) BuildProxyInitContainer(mode, outboundPortsExclude, inboundPortsExclude string) corev1.Container { + var env []corev1.EnvVar + if mode == ProxyInitModeEnforceDrop { + // PROXY_UID is exempted from the DROP and MUST match the proxy + // container's RunAsUser (both derive from b.cfg.Proxy.UID). CLUSTER_CIDRS + // are allowed direct; everything else egressing the pod is dropped. + // POD_IP and the redirect-only exclude vars are unused in this mode. + clusterCIDRs := strings.Join(b.cfg.Proxy.ClusterCIDRs, ",") + env = []corev1.EnvVar{ + {Name: "MODE", Value: ProxyInitModeEnforceDrop}, + {Name: "PROXY_UID", Value: fmt.Sprintf("%d", b.cfg.Proxy.UID)}, + {Name: "CLUSTER_CIDRS", Value: clusterCIDRs}, + } + builderLog.Info("building ProxyInit Container", + "mode", "enforce-drop", + "proxyUID", b.cfg.Proxy.UID, + "clusterCIDRs", clusterCIDRs) + } else { + outboundValue := buildOutboundExcludeValue(outboundPortsExclude) + inboundValue := buildPortExcludeValue(inboundPortsExclude, "inbound-ports-exclude") + + builderLog.Info("building ProxyInit Container", + "mode", "redirect", + "resolvedOutboundPortsExclude", outboundValue, + "resolvedInboundPortsExclude", inboundValue) + + env = []corev1.EnvVar{ + { + Name: "PROXY_PORT", + Value: fmt.Sprintf("%d", b.cfg.Proxy.Port), + }, + { + Name: "INBOUND_PROXY_PORT", + Value: fmt.Sprintf("%d", b.cfg.Proxy.InboundProxyPort), + }, + { + Name: "PROXY_UID", + Value: fmt.Sprintf("%d", b.cfg.Proxy.UID), + }, + { + Name: "OUTBOUND_PORTS_EXCLUDE", + Value: outboundValue, + }, + { + Name: "POD_IP", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "status.podIP", + }, }, }, - }, - } - if inboundValue != "" { - env = append(env, corev1.EnvVar{ - Name: "INBOUND_PORTS_EXCLUDE", - Value: inboundValue, - }) + } + if inboundValue != "" { + env = append(env, corev1.EnvVar{ + Name: "INBOUND_PORTS_EXCLUDE", + Value: inboundValue, + }) + } } return corev1.Container{ diff --git a/kagenti-operator/internal/webhook/injector/container_builder_test.go b/kagenti-operator/internal/webhook/injector/container_builder_test.go index 6300d673..232ebff7 100644 --- a/kagenti-operator/internal/webhook/injector/container_builder_test.go +++ b/kagenti-operator/internal/webhook/injector/container_builder_test.go @@ -17,6 +17,8 @@ limitations under the License. package injector import ( + "strconv" + "strings" "testing" "github.com/kagenti/operator/internal/webhook/config" @@ -194,7 +196,7 @@ func TestBuildOutboundExcludeValue_BoundaryPorts(t *testing.T) { func TestBuildProxyInitContainer_DefaultExclude(t *testing.T) { builder := NewContainerBuilder(config.CompiledDefaults()) - container := builder.BuildProxyInitContainer("", "") + container := builder.BuildProxyInitContainer("redirect", "", "") var foundOutbound bool for _, env := range container.Env { @@ -215,7 +217,7 @@ func TestBuildProxyInitContainer_DefaultExclude(t *testing.T) { func TestBuildProxyInitContainer_WithAnnotationPorts(t *testing.T) { builder := NewContainerBuilder(config.CompiledDefaults()) - container := builder.BuildProxyInitContainer("11434,4317", "") + container := builder.BuildProxyInitContainer("redirect", "11434,4317", "") var foundOutbound bool for _, env := range container.Env { @@ -236,7 +238,7 @@ func TestBuildProxyInitContainer_WithAnnotationPorts(t *testing.T) { func TestBuildProxyInitContainer_WithInboundExclude(t *testing.T) { builder := NewContainerBuilder(config.CompiledDefaults()) - container := builder.BuildProxyInitContainer("", "8443,18789") + container := builder.BuildProxyInitContainer("redirect", "", "8443,18789") var foundInbound bool for _, env := range container.Env { @@ -257,7 +259,7 @@ func TestBuildProxyInitContainer_WithInboundExclude(t *testing.T) { func TestBuildProxyInitContainer_WithBothExcludes(t *testing.T) { builder := NewContainerBuilder(config.CompiledDefaults()) - container := builder.BuildProxyInitContainer("11434", "8443") + container := builder.BuildProxyInitContainer("redirect", "11434", "8443") var foundOutbound, foundInbound bool for _, env := range container.Env { @@ -282,6 +284,57 @@ func TestBuildProxyInitContainer_WithBothExcludes(t *testing.T) { } } +// The proxy-sidecar container must run as the configured Proxy.UID — the same +// value the enforce-drop guard exempts via --uid-owner. If these drift, the +// proxy's own egress would be dropped (or the agent would share the exempt UID). +func TestBuildProxySidecarContainer_RunAsProxyUID(t *testing.T) { + cfg := config.CompiledDefaults() + builder := NewContainerBuilder(cfg) + container := builder.BuildProxySidecarContainer(true) + + sc := container.SecurityContext + if sc == nil || sc.RunAsUser == nil { + t.Fatal("proxy-sidecar container missing SecurityContext.RunAsUser") + } + if got := *sc.RunAsUser; got != cfg.Proxy.UID { + t.Errorf("proxy-sidecar RunAsUser = %d, want Proxy.UID %d", got, cfg.Proxy.UID) + } + if sc.RunAsGroup == nil || *sc.RunAsGroup != cfg.Proxy.UID { + t.Errorf("proxy-sidecar RunAsGroup = %v, want Proxy.UID %d", sc.RunAsGroup, cfg.Proxy.UID) + } +} + +// enforce-drop mode emits MODE / PROXY_UID / CLUSTER_CIDRS and none of the +// redirect-only vars (POD_IP, OUTBOUND_PORTS_EXCLUDE). +func TestBuildProxyInitContainer_EnforceDrop(t *testing.T) { + cfg := config.CompiledDefaults() + builder := NewContainerBuilder(cfg) + container := builder.BuildProxyInitContainer("enforce-drop", "", "") + + got := map[string]string{} + for _, e := range container.Env { + if e.ValueFrom != nil { + t.Errorf("enforce-drop env %q must be a literal, not ValueFrom", e.Name) + } + got[e.Name] = e.Value + } + if got["MODE"] != "enforce-drop" { + t.Errorf("MODE = %q, want enforce-drop", got["MODE"]) + } + if want := strconv.FormatInt(cfg.Proxy.UID, 10); got["PROXY_UID"] != want { + t.Errorf("PROXY_UID = %q, want %q", got["PROXY_UID"], want) + } + if want := strings.Join(cfg.Proxy.ClusterCIDRs, ","); got["CLUSTER_CIDRS"] != want { + t.Errorf("CLUSTER_CIDRS = %q, want %q", got["CLUSTER_CIDRS"], want) + } + if _, ok := got["POD_IP"]; ok { + t.Error("enforce-drop must not set POD_IP") + } + if _, ok := got["OUTBOUND_PORTS_EXCLUDE"]; ok { + t.Error("enforce-drop must not set OUTBOUND_PORTS_EXCLUDE") + } +} + func TestBuildPortExcludeValue(t *testing.T) { tests := []struct { name string diff --git a/kagenti-operator/internal/webhook/injector/pod_mutator.go b/kagenti-operator/internal/webhook/injector/pod_mutator.go index 655ee711..a4725c03 100644 --- a/kagenti-operator/internal/webhook/injector/pod_mutator.go +++ b/kagenti-operator/internal/webhook/injector/pod_mutator.go @@ -525,6 +525,21 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp injectHTTPProxyEnv(c, forwardProxyPort) } + // Fail-closed egress enforcement (opt-in via proxy.egressEnforcement, + // default "off"). HTTP_PROXY above is cooperative — an app that ignores + // it egresses directly. When enforcement is on, inject the proxy-init + // enforce-drop guard so any direct egress is dropped. envoy-sidecar + // already enforces structurally via transparent redirect, so this is the + // proxy-sidecar / lite path only. The exempted PROXY_UID equals the proxy + // container's RunAsUser (both b.cfg.Proxy.UID). + if currentConfig.Proxy.EgressEnforcement == config.EgressEnforcementEnforce && + !containerExists(podSpec.InitContainers, ProxyInitContainerName) { + podSpec.InitContainers = append(podSpec.InitContainers, + builder.BuildProxyInitContainer(ProxyInitModeEnforceDrop, "", "")) + mutatorLog.Info("proxy-sidecar egress enforcement enabled (enforce-drop)", + "namespace", namespace, "crName", crName) + } + // spiffe-helper is bundled in the authbridge combined image and // gated by SPIRE_ENABLED; client-registration is operator-managed. @@ -608,7 +623,7 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp if decision.ProxyInit.Inject && !containerExists(podSpec.InitContainers, ProxyInitContainerName) { outboundExclude := annotations[OutboundPortsExcludeAnnotation] inboundExclude := annotations[InboundPortsExcludeAnnotation] - podSpec.InitContainers = append(podSpec.InitContainers, builder.BuildProxyInitContainer(outboundExclude, inboundExclude)) + podSpec.InitContainers = append(podSpec.InitContainers, builder.BuildProxyInitContainer(ProxyInitModeRedirect, outboundExclude, inboundExclude)) } // Inject volumes diff --git a/kagenti-operator/internal/webhook/injector/pod_mutator_test.go b/kagenti-operator/internal/webhook/injector/pod_mutator_test.go index 0e795d19..2204e7bf 100644 --- a/kagenti-operator/internal/webhook/injector/pod_mutator_test.go +++ b/kagenti-operator/internal/webhook/injector/pod_mutator_test.go @@ -781,6 +781,86 @@ func TestInjectAuthBridge_WaypointMode_SkipsInjection(t *testing.T) { } } +// Egress enforcement for proxy-sidecar is gated by proxy.egressEnforcement +// (default "off"). When "enforce", a proxy-init container is injected in +// enforce-drop mode; envoy-sidecar is unaffected (tested elsewhere). +func TestInjectAuthBridge_ProxySidecar_EgressEnforcement(t *testing.T) { + ctx := context.Background() + labels := map[string]string{KagentiTypeLabel: KagentiTypeAgent} + makePod := func() *corev1.PodSpec { + return &corev1.PodSpec{ + ServiceAccountName: "my-agent", + Containers: []corev1.Container{{Name: "agent", Image: "my-agent:latest"}}, + } + } + findProxyInit := func(spec *corev1.PodSpec) *corev1.Container { + for i := range spec.InitContainers { + if spec.InitContainers[i].Name == ProxyInitContainerName { + return &spec.InitContainers[i] + } + } + return nil + } + + t.Run("enforce injects proxy-init in enforce-drop mode", func(t *testing.T) { + m := newTestMutator(newAgentRuntimeWithMode("team1", "my-agent", ModeProxySidecar)) + cfg := config.CompiledDefaults() + cfg.Proxy.EgressEnforcement = "enforce" + m.GetPlatformConfig = func() *config.PlatformConfig { return cfg } + + spec := makePod() + if _, err := m.InjectAuthBridge(ctx, spec, "team1", "my-agent", labels, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + ic := findProxyInit(spec) + if ic == nil { + t.Fatal("proxy-init should be injected when egressEnforcement=enforce") + } + var mode string + for _, e := range ic.Env { + if e.Name == "MODE" { + mode = e.Value + } + } + if mode != "enforce-drop" { + t.Errorf("proxy-init MODE = %q, want enforce-drop", mode) + } + }) + + t.Run("default off does not inject proxy-init", func(t *testing.T) { + m := newTestMutator(newAgentRuntimeWithMode("team1", "my-agent", ModeProxySidecar)) + spec := makePod() + if _, err := m.InjectAuthBridge(ctx, spec, "team1", "my-agent", labels, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if findProxyInit(spec) != nil { + t.Error("proxy-init must not be injected when egressEnforcement is off (default)") + } + }) + + t.Run("does not duplicate an existing proxy-init", func(t *testing.T) { + m := newTestMutator(newAgentRuntimeWithMode("team1", "my-agent", ModeProxySidecar)) + cfg := config.CompiledDefaults() + cfg.Proxy.EgressEnforcement = "enforce" + m.GetPlatformConfig = func() *config.PlatformConfig { return cfg } + + spec := makePod() + spec.InitContainers = []corev1.Container{{Name: ProxyInitContainerName, Image: "preexisting"}} + if _, err := m.InjectAuthBridge(ctx, spec, "team1", "my-agent", labels, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + count := 0 + for _, c := range spec.InitContainers { + if c.Name == ProxyInitContainerName { + count++ + } + } + if count != 1 { + t.Errorf("expected proxy-init not duplicated, got %d", count) + } + }) +} + func TestInjectAuthBridge_ProxySidecarMode_InjectsCorrectly(t *testing.T) { m := newTestMutator(newAgentRuntimeWithMode("team1", "my-agent", ModeProxySidecar)) ctx := context.Background() From a45a878ee1aed7974dfbf5a4419140dcdefbd3c3 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 5 Jun 2026 15:48:54 -0400 Subject: [PATCH 2/3] fix(webhook): validate clusterCIDRs when egress enforcement is enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review on #406: Validate() checked the egressEnforcement enum but not ClusterCIDRs. When enforce is set, an empty list silently fell back to the Kind-shaped 10.0.0.0/8 default in init-iptables.sh, and a malformed entry crashed the proxy-init container under set -e. Now, when EgressEnforcement == enforce, require ClusterCIDRs non-empty and net.ParseCIDR each entry — misconfig fails fast at config load with a clear message instead of as a pod CrashLoop or a silent wrong default. Off mode leaves ClusterCIDRs unvalidated. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../internal/webhook/config/types.go | 16 ++++++ .../internal/webhook/config/types_test.go | 51 +++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 kagenti-operator/internal/webhook/config/types_test.go diff --git a/kagenti-operator/internal/webhook/config/types.go b/kagenti-operator/internal/webhook/config/types.go index 2a63fc72..c4ad2977 100644 --- a/kagenti-operator/internal/webhook/config/types.go +++ b/kagenti-operator/internal/webhook/config/types.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "net" corev1 "k8s.io/api/core/v1" ) @@ -160,6 +161,21 @@ func (c *PlatformConfig) Validate() error { default: return fmt.Errorf("proxy.egressEnforcement must be \"off\" or \"enforce\" (got %q)", c.Proxy.EgressEnforcement) } + // When enforce is on, ClusterCIDRs drive the only in-cluster allowance in the + // enforce-drop guard. Validate them at load time so a misconfig fails fast with + // a clear message rather than: (a) an empty list silently falling back to the + // Kind-shaped 10.0.0.0/8 default in init-iptables.sh, or (b) a malformed entry + // crashing the proxy-init container under `set -e` with a cryptic iptables error. + if c.Proxy.EgressEnforcement == EgressEnforcementEnforce { + if len(c.Proxy.ClusterCIDRs) == 0 { + return fmt.Errorf("proxy.clusterCIDRs must be non-empty when proxy.egressEnforcement is %q (set the cluster's pod+service CIDRs)", EgressEnforcementEnforce) + } + for _, cidr := range c.Proxy.ClusterCIDRs { + if _, _, err := net.ParseCIDR(cidr); err != nil { + return fmt.Errorf("proxy.clusterCIDRs entry %q is not a valid CIDR: %w", cidr, err) + } + } + } if c.Images.EnvoyProxy == "" { return fmt.Errorf("images.envoyProxy is required") } diff --git a/kagenti-operator/internal/webhook/config/types_test.go b/kagenti-operator/internal/webhook/config/types_test.go new file mode 100644 index 00000000..1649fd55 --- /dev/null +++ b/kagenti-operator/internal/webhook/config/types_test.go @@ -0,0 +1,51 @@ +package config + +import "testing" + +// When egressEnforcement is "enforce", ClusterCIDRs must be present and valid — +// an empty list (silent 10/8 fallback in the init script) or a malformed entry +// (init-container CrashLoop under set -e) must be rejected at config load. +func TestValidate_ClusterCIDRs_EnforceMode(t *testing.T) { + base := func() *PlatformConfig { + c := CompiledDefaults() + c.Proxy.EgressEnforcement = EgressEnforcementEnforce + return c + } + tests := []struct { + name string + mutate func(*PlatformConfig) + wantErr bool + }{ + {"default CIDRs ok", func(c *PlatformConfig) {}, false}, + {"empty CIDRs rejected", func(c *PlatformConfig) { c.Proxy.ClusterCIDRs = nil }, true}, + {"malformed CIDR rejected", func(c *PlatformConfig) { + c.Proxy.ClusterCIDRs = []string{"10.0.0.0/8", "garbage"} + }, true}, + {"valid OCP-shaped CIDRs ok", func(c *PlatformConfig) { + c.Proxy.ClusterCIDRs = []string{"10.128.0.0/14", "172.30.0.0/16"} + }, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := base() + tt.mutate(c) + err := c.Validate() + if tt.wantErr && err == nil { + t.Errorf("expected validation error, got nil") + } + if !tt.wantErr && err != nil { + t.Errorf("unexpected validation error: %v", err) + } + }) + } +} + +// When enforcement is off, ClusterCIDRs is unused and must not be validated. +func TestValidate_ClusterCIDRs_OffMode_NotValidated(t *testing.T) { + c := CompiledDefaults() + c.Proxy.EgressEnforcement = EgressEnforcementOff + c.Proxy.ClusterCIDRs = []string{"garbage"} + if err := c.Validate(); err != nil { + t.Errorf("off mode must not validate ClusterCIDRs, got: %v", err) + } +} From d2800935f3214b63ee54c626d3e1c627038b047c Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 5 Jun 2026 16:37:03 -0400 Subject: [PATCH 3/3] refactor: type proxy-init mode and fail closed on unknown values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on the exported BuildProxyInitContainer API: a mistyped mode literal at a future callsite would silently fall through to redirect, shipping a proxy-init with no egress guard — fail-open, the worst outcome for a fail-closed control. - Make ProxyInitMode a named type and the mode dispatch an explicit switch with a fail-closed default that logs an error and returns a zero-value container (injection breaks visibly instead of degrading to redirect). - Add TestBuildProxyInitContainer_InvalidMode covering the default branch. - Document that clusterCIDRs validation is syntactic only (overlap / v4-v6 mixing accepted; iptables and the init script handle both). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../internal/webhook/config/types.go | 3 +++ .../internal/webhook/injector/constants.go | 14 ++++++++++---- .../webhook/injector/container_builder.go | 17 +++++++++++++---- .../webhook/injector/container_builder_test.go | 15 +++++++++++++++ 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/kagenti-operator/internal/webhook/config/types.go b/kagenti-operator/internal/webhook/config/types.go index c4ad2977..d99ed36b 100644 --- a/kagenti-operator/internal/webhook/config/types.go +++ b/kagenti-operator/internal/webhook/config/types.go @@ -170,6 +170,9 @@ func (c *PlatformConfig) Validate() error { if len(c.Proxy.ClusterCIDRs) == 0 { return fmt.Errorf("proxy.clusterCIDRs must be non-empty when proxy.egressEnforcement is %q (set the cluster's pod+service CIDRs)", EgressEnforcementEnforce) } + // Syntactic validation only — overlapping ranges and IPv4/IPv6 mixing are + // accepted (iptables handles both, and the init script splits v4/v6 itself); + // we only reject malformed strings here. for _, cidr := range c.Proxy.ClusterCIDRs { if _, _, err := net.ParseCIDR(cidr); err != nil { return fmt.Errorf("proxy.clusterCIDRs entry %q is not a valid CIDR: %w", cidr, err) diff --git a/kagenti-operator/internal/webhook/injector/constants.go b/kagenti-operator/internal/webhook/injector/constants.go index 8b943f41..96332d80 100644 --- a/kagenti-operator/internal/webhook/injector/constants.go +++ b/kagenti-operator/internal/webhook/injector/constants.go @@ -38,15 +38,21 @@ const ( ClientAuthTypeFederatedJWT = "federated-jwt" ) -// proxy-init MODE values — select the iptables strategy that -// BuildProxyInitContainer passes to init-iptables.sh. +// ProxyInitMode is the iptables strategy BuildProxyInitContainer passes to +// init-iptables.sh. It is a named type so callsites read intent-fully and the +// compiler rejects passing an unrelated typed value; BuildProxyInitContainer +// additionally fails closed on any unknown value (see its default branch), +// since an untyped string literal can still convert to this type at a callsite. +type ProxyInitMode string + +// proxy-init MODE values. const ( // ProxyInitModeRedirect transparently REDIRECTs pod traffic to the Envoy // listeners (envoy-sidecar mode). - ProxyInitModeRedirect = "redirect" + ProxyInitModeRedirect ProxyInitMode = "redirect" // ProxyInitModeEnforceDrop installs the fail-closed egress guard that DROPs // any egress bypassing the forward proxy (proxy-sidecar egress enforcement). - ProxyInitModeEnforceDrop = "enforce-drop" + ProxyInitModeEnforceDrop ProxyInitMode = "enforce-drop" ) // mTLS modes for the proxy-sidecar / lite paths. Selected per workload diff --git a/kagenti-operator/internal/webhook/injector/container_builder.go b/kagenti-operator/internal/webhook/injector/container_builder.go index bd66b2d9..7d8b2c17 100644 --- a/kagenti-operator/internal/webhook/injector/container_builder.go +++ b/kagenti-operator/internal/webhook/injector/container_builder.go @@ -462,16 +462,17 @@ const mandatoryOutboundExclude = "8080" // - "enforce-drop" (proxy-sidecar): a fail-closed egress guard that DROPs any // egress bypassing the forward proxy. Driven by PROXY_UID + CLUSTER_CIDRS; // the exclude args do not apply (the script ignores them in this mode). -func (b *ContainerBuilder) BuildProxyInitContainer(mode, outboundPortsExclude, inboundPortsExclude string) corev1.Container { +func (b *ContainerBuilder) BuildProxyInitContainer(mode ProxyInitMode, outboundPortsExclude, inboundPortsExclude string) corev1.Container { var env []corev1.EnvVar - if mode == ProxyInitModeEnforceDrop { + switch mode { + case ProxyInitModeEnforceDrop: // PROXY_UID is exempted from the DROP and MUST match the proxy // container's RunAsUser (both derive from b.cfg.Proxy.UID). CLUSTER_CIDRS // are allowed direct; everything else egressing the pod is dropped. // POD_IP and the redirect-only exclude vars are unused in this mode. clusterCIDRs := strings.Join(b.cfg.Proxy.ClusterCIDRs, ",") env = []corev1.EnvVar{ - {Name: "MODE", Value: ProxyInitModeEnforceDrop}, + {Name: "MODE", Value: string(ProxyInitModeEnforceDrop)}, {Name: "PROXY_UID", Value: fmt.Sprintf("%d", b.cfg.Proxy.UID)}, {Name: "CLUSTER_CIDRS", Value: clusterCIDRs}, } @@ -479,7 +480,7 @@ func (b *ContainerBuilder) BuildProxyInitContainer(mode, outboundPortsExclude, i "mode", "enforce-drop", "proxyUID", b.cfg.Proxy.UID, "clusterCIDRs", clusterCIDRs) - } else { + case ProxyInitModeRedirect: outboundValue := buildOutboundExcludeValue(outboundPortsExclude) inboundValue := buildPortExcludeValue(inboundPortsExclude, "inbound-ports-exclude") @@ -520,6 +521,14 @@ func (b *ContainerBuilder) BuildProxyInitContainer(mode, outboundPortsExclude, i Value: inboundValue, }) } + default: + // Fail closed. An unknown mode must NOT silently degrade to redirect: + // that would ship a proxy-init with no egress guard — fail-open, the + // worst outcome for a fail-closed control. Return a zero-value container + // so injection breaks visibly (empty name/image → admission/scheduler + // rejects it) rather than passing a security-defeating container through. + builderLog.Error(nil, "unknown proxy-init mode; refusing to build container (fail closed)", "mode", mode) + return corev1.Container{} } return corev1.Container{ diff --git a/kagenti-operator/internal/webhook/injector/container_builder_test.go b/kagenti-operator/internal/webhook/injector/container_builder_test.go index 232ebff7..0ff46d7d 100644 --- a/kagenti-operator/internal/webhook/injector/container_builder_test.go +++ b/kagenti-operator/internal/webhook/injector/container_builder_test.go @@ -335,6 +335,21 @@ func TestBuildProxyInitContainer_EnforceDrop(t *testing.T) { } } +// An unknown mode must fail closed: BuildProxyInitContainer returns a +// zero-value container (no name/image/env) rather than silently degrading to +// redirect, which would ship a proxy-init with no egress guard (fail-open). +// This guards the exported API against a typo'd mode literal at a future +// callsite — a typo in a fail-closed control is the worst failure mode. +func TestBuildProxyInitContainer_InvalidMode(t *testing.T) { + builder := NewContainerBuilder(config.CompiledDefaults()) + container := builder.BuildProxyInitContainer(ProxyInitMode("enfore-drop"), "", "") + + if container.Name != "" || container.Image != "" || container.Env != nil { + t.Errorf("invalid mode must return a zero-value container, got name=%q image=%q env=%v", + container.Name, container.Image, container.Env) + } +} + func TestBuildPortExcludeValue(t *testing.T) { tests := []struct { name string