diff --git a/charts/operator/values.yaml b/charts/operator/values.yaml index e266c949..b0ec3a17 100644 --- a/charts/operator/values.yaml +++ b/charts/operator/values.yaml @@ -262,6 +262,11 @@ defaults: # always-on enforce-redirect egress guard (proxy-sidecar / lite). MUST match # the authbridge listener.transparent_proxy_addr (default :8082). transparentPort: 8082 + # INBOUND transparent listener port — the PREROUTING REDIRECT target when a + # workload selects inboundInterception: transparent. MUST match the authbridge + # listener.transparent_inbound_addr (preset default :8083), and must differ + # from transparentPort (they are separate listeners in the same container). + transparentInboundPort: 8083 # Cluster DNS is kept direct by proxy-init itself (it reads the pod's # /etc/resolv.conf nameservers), so there is no in-cluster CIDR knob to set — # works on Kind / OpenShift / EKS / NodeLocal-DNSCache with no per-cluster config. @@ -280,6 +285,16 @@ defaults: allowedEgressEnforcement: - enforce-redirect - none + # Which inbound interception mechanisms workloads in this cluster may select. + # A resolved value outside this list falls back to the FIRST entry, so order + # matters. Transparent inbound needs a privileged proxy-init container + # (NET_ADMIN), so an admin may want to forbid or mandate it: + # ["reverse-proxy"] — port stealing only, no NET_ADMIN + # ["transparent"] — hard inbound boundary required + # ["reverse-proxy", "transparent"] — workloads choose (default) + allowedInboundInterception: + - reverse-proxy + - transparent # Resource defaults (conservative for dev) # Note: requests must be <= limits diff --git a/operator/api/v1alpha1/zz_generated.deepcopy.go b/operator/api/v1alpha1/zz_generated.deepcopy.go index 9d1b5559..7c3a495e 100644 --- a/operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/operator/api/v1alpha1/zz_generated.deepcopy.go @@ -324,7 +324,7 @@ func (in *AgentRuntime) DeepCopyInto(out *AgentRuntime) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) } @@ -382,6 +382,11 @@ func (in *AgentRuntimeList) DeepCopyObject() runtime.Object { func (in *AgentRuntimeSpec) DeepCopyInto(out *AgentRuntimeSpec) { *out = *in out.TargetRef = in.TargetRef + if in.Auth != nil { + in, out := &in.Auth, &out.Auth + *out = new(AuthConfig) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AgentRuntimeSpec. @@ -468,6 +473,28 @@ func (in *AgentSkill) DeepCopy() *AgentSkill { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AuthConfig) DeepCopyInto(out *AuthConfig) { + *out = *in + if in.Outbound != nil { + in, out := &in.Outbound, &out.Outbound + *out = make([]OutboundRoute, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AuthConfig. +func (in *AuthConfig) DeepCopy() *AuthConfig { + if in == nil { + return nil + } + out := new(AuthConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AuthorizationPolicy) DeepCopyInto(out *AuthorizationPolicy) { *out = *in @@ -632,6 +659,27 @@ func (in *IdentityBinding) DeepCopy() *IdentityBinding { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OutboundRoute) DeepCopyInto(out *OutboundRoute) { + *out = *in + out.Destination = in.Destination + if in.Audiences != nil { + in, out := &in.Audiences, &out.Audiences + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OutboundRoute. +func (in *OutboundRoute) DeepCopy() *OutboundRoute { + if in == nil { + return nil + } + out := new(OutboundRoute) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PolicyEntry) DeepCopyInto(out *PolicyEntry) { *out = *in @@ -647,6 +695,21 @@ func (in *PolicyEntry) DeepCopy() *PolicyEntry { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RouteMatch) DeepCopyInto(out *RouteMatch) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RouteMatch. +func (in *RouteMatch) DeepCopy() *RouteMatch { + if in == nil { + return nil + } + out := new(RouteMatch) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SignatureHeader) DeepCopyInto(out *SignatureHeader) { *out = *in diff --git a/operator/internal/webhook/config/defaults.go b/operator/internal/webhook/config/defaults.go index 87cb0d9e..c54eb3bf 100644 --- a/operator/internal/webhook/config/defaults.go +++ b/operator/internal/webhook/config/defaults.go @@ -53,13 +53,20 @@ func CompiledDefaults() *PlatformConfig { // Transparent listener port — must match the authbridge proxy-sidecar // preset (listener.transparent_proxy_addr default :8082). TransparentPort: 8082, + // Inbound transparent listener. Matches the authbridge proxy-sidecar + // preset (listener.transparent_inbound_addr default :8083); 8080/8081/8082 + // are the reverse, forward and transparent-egress listeners. + TransparentInboundPort: 8083, // Empty by default: proxy-init auto-detects the iptables backend from // /proc/modules. Set (e.g. "iptables") to force a backend per-platform. IptablesCmd: "", // Both modes allowed by default. Set to ["none"] on platforms // where iptables is unavailable (ROSA HCP, managed OpenShift), // or ["enforce-redirect"] to prevent opt-out. - AllowedEgressEnforcement: []string{"enforce-redirect", "none"}, + // Both allowed by default: transparent inbound is opt-in per workload, + // and a platform admin can narrow this to forbid or mandate it. + AllowedInboundInterception: []string{"reverse-proxy", "transparent"}, + AllowedEgressEnforcement: []string{"enforce-redirect", "none"}, }, Resources: ResourcesConfig{ EnvoyProxy: corev1.ResourceRequirements{ diff --git a/operator/internal/webhook/config/transparent_inbound_test.go b/operator/internal/webhook/config/transparent_inbound_test.go new file mode 100644 index 00000000..b4ed2b28 --- /dev/null +++ b/operator/internal/webhook/config/transparent_inbound_test.go @@ -0,0 +1,107 @@ +package config + +import "testing" + +// TestValidate_TransparentInboundPort covers the misconfigurations that would +// otherwise surface as a pod that passes admission and then fails to start. +func TestValidate_TransparentInboundPort(t *testing.T) { + tests := []struct { + name string + mutate func(*PlatformConfig) + wantErr bool + }{ + { + name: "defaults are valid", + mutate: func(*PlatformConfig) {}, + }, + { + name: "below the privileged range", + mutate: func(c *PlatformConfig) { c.Proxy.TransparentInboundPort = 80 }, + wantErr: true, + }, + { + name: "above the port range", + mutate: func(c *PlatformConfig) { c.Proxy.TransparentInboundPort = 70000 }, + wantErr: true, + }, + { + name: "unset (zero) is rejected rather than silently defaulted", + mutate: func(c *PlatformConfig) { c.Proxy.TransparentInboundPort = 0 }, + wantErr: true, + }, + { + // Both listeners live in one container, so a shared value makes the + // second bind fail at pod start — long after admission succeeded. + name: "colliding with the egress transparent port", + mutate: func(c *PlatformConfig) { + c.Proxy.TransparentInboundPort = c.Proxy.TransparentPort + }, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cfg := CompiledDefaults() + tc.mutate(cfg) + err := cfg.Validate() + if tc.wantErr && err == nil { + t.Fatal("expected a validation error, got nil") + } + if !tc.wantErr && err != nil { + t.Fatalf("unexpected validation error: %v", err) + } + }) + } +} + +func TestValidate_AllowedInboundInterception(t *testing.T) { + tests := []struct { + name string + allowed []string + wantErr bool + }{ + {name: "both", allowed: []string{"reverse-proxy", "transparent"}}, + {name: "forbid transparent", allowed: []string{"reverse-proxy"}}, + {name: "mandate transparent", allowed: []string{"transparent"}}, + { + // An empty list would make the fallback index panic, and "allow + // nothing" has no sensible meaning. + name: "empty is rejected", allowed: []string{}, wantErr: true, + }, + {name: "unknown value is rejected", allowed: []string{"reverse-proxy", "tranparent"}, wantErr: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cfg := CompiledDefaults() + cfg.Proxy.AllowedInboundInterception = tc.allowed + err := cfg.Validate() + if tc.wantErr && err == nil { + t.Fatal("expected a validation error, got nil") + } + if !tc.wantErr && err != nil { + t.Fatalf("unexpected validation error: %v", err) + } + }) + } +} + +// TestCompiledDefaults_TransparentInboundDefaults pins the values that must stay +// in lockstep with authbridge's proxy-sidecar preset; a drift here silently +// redirects inbound traffic to a dead port. +func TestCompiledDefaults_TransparentInboundDefaults(t *testing.T) { + cfg := CompiledDefaults() + if cfg.Proxy.TransparentInboundPort != 8083 { + t.Errorf("TransparentInboundPort = %d, want 8083 (authbridge preset transparent_inbound_addr)", cfg.Proxy.TransparentInboundPort) + } + if len(cfg.Proxy.AllowedInboundInterception) != 2 { + t.Errorf("AllowedInboundInterception = %v, want both mechanisms allowed by default", cfg.Proxy.AllowedInboundInterception) + } + // Order matters: the first entry is the fallback when a workload requests a + // value outside the list, and the no-privilege shape must win. + if cfg.Proxy.AllowedInboundInterception[0] != "reverse-proxy" { + t.Errorf("AllowedInboundInterception[0] = %q, want reverse-proxy (the fallback must not grant NET_ADMIN)", + cfg.Proxy.AllowedInboundInterception[0]) + } +} diff --git a/operator/internal/webhook/config/types.go b/operator/internal/webhook/config/types.go index 9735b06b..7190644c 100644 --- a/operator/internal/webhook/config/types.go +++ b/operator/internal/webhook/config/types.go @@ -64,6 +64,20 @@ type ProxyConfig struct { // where auto-detection is wrong or undesired. IptablesCmd string `json:"iptablesCmd" yaml:"iptablesCmd"` + // TransparentInboundPort is the INBOUND transparent listener port — the + // PREROUTING REDIRECT target when inboundInterception is "transparent". + // MUST match the authbridge listener.transparent_inbound_addr (preset + // default :8083); a mismatch redirects inbound traffic to a dead port. + TransparentInboundPort int32 `json:"transparentInboundPort" yaml:"transparentInboundPort"` + + // AllowedInboundInterception restricts which inboundInterception values + // workloads in this cluster may select, mirroring + // AllowedEgressEnforcement. Transparent inbound requires a privileged + // proxy-init container, so a platform admin may want to forbid it + // (["reverse-proxy"]) or mandate it (["transparent"]). A resolved value + // outside the list falls back to the list's first entry. + AllowedInboundInterception []string `json:"allowedInboundInterception,omitempty" yaml:"allowedInboundInterception,omitempty"` + // AllowedEgressEnforcement restricts which egressEnforcement values // workloads (AgentRuntime CR / namespace ConfigMap) may select. // The webhook rejects resolved values not in this list, falling back @@ -108,6 +122,10 @@ func (c *PlatformConfig) DeepCopy() *PlatformConfig { copy(result.TokenExchange.DefaultScopes, c.TokenExchange.DefaultScopes) } + if c.Proxy.AllowedInboundInterception != nil { + result.Proxy.AllowedInboundInterception = make([]string, len(c.Proxy.AllowedInboundInterception)) + copy(result.Proxy.AllowedInboundInterception, c.Proxy.AllowedInboundInterception) + } if c.Proxy.AllowedEgressEnforcement != nil { result.Proxy.AllowedEgressEnforcement = make([]string, len(c.Proxy.AllowedEgressEnforcement)) copy(result.Proxy.AllowedEgressEnforcement, c.Proxy.AllowedEgressEnforcement) @@ -152,6 +170,15 @@ func (c *PlatformConfig) Validate() error { if c.Proxy.TransparentPort < 1024 || c.Proxy.TransparentPort > 65535 { return fmt.Errorf("proxy.transparentPort must be between 1024 and 65535") } + if c.Proxy.TransparentInboundPort < 1024 || c.Proxy.TransparentInboundPort > 65535 { + return fmt.Errorf("proxy.transparentInboundPort must be between 1024 and 65535") + } + // The two transparent listeners are separate sockets in one container; a + // shared value would make the second bind fail at pod start, after admission + // has already succeeded. Catch it at operator startup instead. + if c.Proxy.TransparentInboundPort == c.Proxy.TransparentPort { + return fmt.Errorf("proxy.transparentInboundPort (%d) must differ from proxy.transparentPort — they are distinct listeners in the same container", c.Proxy.TransparentInboundPort) + } // The enforce-redirect 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 { @@ -166,6 +193,14 @@ func (c *PlatformConfig) Validate() error { default: return fmt.Errorf("proxy.iptablesCmd %q is not a recognized backend (want one of: \"\" (auto-detect), iptables, iptables-nft, iptables-legacy)", c.Proxy.IptablesCmd) } + if len(c.Proxy.AllowedInboundInterception) == 0 { + return fmt.Errorf("proxy.allowedInboundInterception must not be empty (set [\"reverse-proxy\"] to forbid transparent inbound, [\"transparent\"] to require it, or both to allow workload choice)") + } + for _, mode := range c.Proxy.AllowedInboundInterception { + if mode != "reverse-proxy" && mode != "transparent" { + return fmt.Errorf("proxy.allowedInboundInterception contains invalid value %q (allowed: reverse-proxy, transparent)", mode) + } + } if len(c.Proxy.AllowedEgressEnforcement) == 0 { return fmt.Errorf("proxy.allowedEgressEnforcement must not be empty (set [\"enforce-redirect\"] to require enforcement, [\"none\"] to disable it, or both to allow workload choice)") } diff --git a/operator/internal/webhook/injector/constants.go b/operator/internal/webhook/injector/constants.go index 66a0d605..c458b8f8 100644 --- a/operator/internal/webhook/injector/constants.go +++ b/operator/internal/webhook/injector/constants.go @@ -65,6 +65,38 @@ const ( EgressEnforcementNone = "none" ) +// AuthBridge's fixed (non-negotiable) listener ports in proxy-sidecar / lite. +// Unlike the reverse/forward/transparent ports these are not configurable, so +// the operator must exempt them from the transparent inbound REDIRECT by number. +// Gating AuthBridgeHealthPort in particular would put kubelet probes behind JWT +// validation and crash-loop the pod. +const ( + authBridgeHealthPort = 9091 // /healthz + authBridgeStatsPort = 9093 // stats + config inspection + /reload/status + authBridgeSessionAPIPort = 9094 // session events API (consumed by abctl) +) + +// Inbound interception mechanisms for the proxy-sidecar / lite paths. These +// select HOW inbound traffic reaches AuthBridge's inbound pipeline — they are +// not two levels of the same knob, but two different deployment shapes. +const ( + // InboundInterceptionReverseProxy is the default: port stealing. AuthBridge + // binds the agent's original port and the agent is relocated to a free one + // via the PORT env var, so the Service needs no patching. Requires no + // privileges, but leaves the relocated port reachable directly (a pod-to-pod + // bypass of JWT validation), only covers the first declared container port, + // and silently fails for agents that hardcode their listen port. + InboundInterceptionReverseProxy = "reverse-proxy" + + // InboundInterceptionTransparent installs a PREROUTING REDIRECT via + // proxy-init and lets AuthBridge recover each connection's real destination + // via SO_ORIGINAL_DST. The agent keeps its own port — no relocation, no PORT + // env var, no second port to discover — and every port it listens on is + // covered. Costs a privileged proxy-init container (NET_ADMIN) and is + // Linux-only, so it is opt-in. + InboundInterceptionTransparent = "transparent" +) + // mTLS modes for the proxy-sidecar / lite paths. Selected via the // namespace `authbridge-runtime-config` ConfigMap's `mtls.mode` field, // then MTLSModeDisabled. envoy-sidecar mode is incompatible with mTLS diff --git a/operator/internal/webhook/injector/container_builder.go b/operator/internal/webhook/injector/container_builder.go index 74bc607b..62d17b0a 100644 --- a/operator/internal/webhook/injector/container_builder.go +++ b/operator/internal/webhook/injector/container_builder.go @@ -221,7 +221,7 @@ func spireSocketDir(socketPath string) string { // The app uses HTTP_PROXY env vars to route outbound traffic through the forward proxy. // Inbound traffic goes through the reverse proxy. func (b *ContainerBuilder) BuildProxySidecarContainer(spireEnabled bool) corev1.Container { - return b.BuildProxySidecarContainerWithPorts(spireEnabled, b.cfg.Images.AuthBridge, 8080, 8000, 8081) + return b.BuildProxySidecarContainerWithPorts(spireEnabled, b.cfg.Images.AuthBridge, 8080, 8081) } // BuildProxySidecarContainerWithPorts creates a proxy-sidecar container with dynamic ports. @@ -231,9 +231,29 @@ func (b *ContainerBuilder) BuildProxySidecarContainer(spireEnabled bool) corev1. // on the same ports; only the plugin set compiled into the binary differs. // // reverseProxyPort: where the reverse proxy listens (takes over the agent's original port) -// agentBackendPort: where the agent actually listens (moved to a free port) // forwardProxyPort: where the forward proxy listens (HTTP_PROXY target) -func (b *ContainerBuilder) BuildProxySidecarContainerWithPorts(spireEnabled bool, image string, reverseProxyPort, agentBackendPort, forwardProxyPort int32) corev1.Container { +func (b *ContainerBuilder) BuildProxySidecarContainerWithPorts(spireEnabled bool, image string, reverseProxyPort, forwardProxyPort int32) corev1.Container { + // No agent-backend port parameter: where the agent was relocated to is + // something the sidecar learns from its ConfigMap (reverse_proxy_backend), not + // from a declared container port. + return b.buildProxySidecarContainer(spireEnabled, image, "reverse-proxy", reverseProxyPort, forwardProxyPort) +} + +// BuildProxySidecarContainerTransparent builds the sidecar for +// inboundInterception "transparent": AuthBridge binds its own inbound listener +// on transparentInboundPort and iptables REDIRECTs to it, so the agent keeps its +// own port and there is no relocated backend port to declare. +// +// The declared port is named "transparent-in" rather than "reverse-proxy" so +// `kubectl describe pod` reflects which inbound shape is actually running — +// otherwise the two are indistinguishable from the pod spec. +func (b *ContainerBuilder) BuildProxySidecarContainerTransparent(spireEnabled bool, image string, transparentInboundPort, forwardProxyPort int32) corev1.Container { + // No backend port: with transparent interception the forwarding target is + // resolved per connection from SO_ORIGINAL_DST, not configured. + return b.buildProxySidecarContainer(spireEnabled, image, "transparent-in", transparentInboundPort, forwardProxyPort) +} + +func (b *ContainerBuilder) buildProxySidecarContainer(spireEnabled bool, image, inboundPortName string, inboundPort, forwardProxyPort int32) corev1.Container { volumeMounts := []corev1.VolumeMount{ { Name: "shared-data", @@ -287,8 +307,8 @@ func (b *ContainerBuilder) BuildProxySidecarContainerWithPorts(spireEnabled bool }, Ports: []corev1.ContainerPort{ { - Name: "reverse-proxy", - ContainerPort: reverseProxyPort, + Name: inboundPortName, + ContainerPort: inboundPort, Protocol: corev1.ProtocolTCP, }, { @@ -465,6 +485,35 @@ const mandatoryOutboundExclude = "8080" // TRANSPARENT_PORT; the exclude args do not apply. Cluster DNS is kept // direct by the init script reading the pod's resolv.conf nameservers. func (b *ContainerBuilder) BuildProxyInitContainer(mode ProxyInitMode, outboundPortsExclude, inboundPortsExclude string) corev1.Container { + return b.BuildProxyInitContainerWithInbound(mode, outboundPortsExclude, inboundPortsExclude, ProxyInitInbound{}) +} + +// ProxyInitInbound carries the transparent-inbound additions to proxy-init's +// enforce-redirect mode. The zero value leaves inbound interception off, which +// is what BuildProxyInitContainer passes — so existing callers keep egress-only +// behavior with no change. +type ProxyInitInbound struct { + // Port is the PREROUTING REDIRECT target (INBOUND_TRANSPARENT_PORT). Zero + // means inbound interception is off. + Port int32 + + // SidecarPortsExclude is the comma-separated list of AuthBridge's own + // listeners to exempt from the inbound REDIRECT. The operator supplies the + // real forward-proxy port here, which may not be the script's default 8081 + // when findFreePort had to move it. + SidecarPortsExclude string + + // PortsExclude is the operator/user app-port exemption list + // (kagenti.io/inbound-ports-exclude), for app ports that must not be + // validated — e.g. an oauth-proxy doing its own authentication. + PortsExclude string +} + +// BuildProxyInitContainerWithInbound is BuildProxyInitContainer plus the +// transparent-inbound environment. Split out rather than folded into the +// existing signature so the redirect-mode and egress-only callers stay +// untouched. +func (b *ContainerBuilder) BuildProxyInitContainerWithInbound(mode ProxyInitMode, outboundPortsExclude, inboundPortsExclude string, inbound ProxyInitInbound) corev1.Container { var env []corev1.EnvVar switch mode { case ProxyInitModeEnforceRedirect: @@ -479,10 +528,46 @@ func (b *ContainerBuilder) BuildProxyInitContainer(mode ProxyInitMode, outboundP {Name: "PROXY_UID", Value: fmt.Sprintf("%d", b.cfg.Proxy.UID)}, {Name: "TRANSPARENT_PORT", Value: fmt.Sprintf("%d", b.cfg.Proxy.TransparentPort)}, } + // Transparent inbound (opt-in). POD_IP is REQUIRED alongside the port: + // the init script uses it as the DNAT target for the Istio ambient inbound + // path, which arrives through OUTPUT rather than PREROUTING. The script + // refuses to start without it rather than install PREROUTING-only rules + // that would wave all mesh traffic through unvalidated. + if inbound.Port > 0 { + env = append(env, + corev1.EnvVar{Name: "INBOUND_TRANSPARENT_PORT", Value: fmt.Sprintf("%d", inbound.Port)}, + corev1.EnvVar{ + Name: "POD_IP", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{FieldPath: "status.podIP"}, + }, + }, + // POD_IPS carries BOTH families on a dual-stack pod. POD_IP alone is + // the primary address (usually v4), and the ambient DNAT target must + // match the family of the traffic — so keying only off POD_IP leaves + // the other family's HBONE delivery passing unvalidated while its + // PREROUTING rules are installed. proxy-init falls back to POD_IP + // when this is absent. + corev1.EnvVar{ + Name: "POD_IPS", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{FieldPath: "status.podIPs"}, + }, + }, + ) + if inbound.SidecarPortsExclude != "" { + env = append(env, corev1.EnvVar{Name: "SIDECAR_PORTS_EXCLUDE", Value: inbound.SidecarPortsExclude}) + } + if v := buildPortExcludeValue(inbound.PortsExclude, "inbound-ports-exclude"); v != "" { + env = append(env, corev1.EnvVar{Name: "INBOUND_PORTS_EXCLUDE", Value: v}) + } + } builderLog.Info("building ProxyInit Container", "mode", "enforce-redirect", "proxyUID", b.cfg.Proxy.UID, - "transparentPort", b.cfg.Proxy.TransparentPort) + "transparentPort", b.cfg.Proxy.TransparentPort, + "inboundTransparentPort", inbound.Port, + "sidecarPortsExclude", inbound.SidecarPortsExclude) case ProxyInitModeRedirect: outboundValue := buildOutboundExcludeValue(outboundPortsExclude) inboundValue := buildPortExcludeValue(inboundPortsExclude, "inbound-ports-exclude") diff --git a/operator/internal/webhook/injector/namespace_config.go b/operator/internal/webhook/injector/namespace_config.go index 3d263044..00d81332 100644 --- a/operator/internal/webhook/injector/namespace_config.go +++ b/operator/internal/webhook/injector/namespace_config.go @@ -144,6 +144,25 @@ func ExtractMode(authbridgeYAML string) string { return top.Mode } +// ExtractInboundInterception parses an authbridge-runtime-config config.yaml +// string and returns the value of its top-level `inboundInterception:` key. +// Returns "" if the YAML is empty, malformed, or has no `inboundInterception` +// key, so the caller falls through to the next resolution layer. +func ExtractInboundInterception(authbridgeYAML string) string { + if authbridgeYAML == "" { + return "" + } + var top struct { + InboundInterception string `json:"inboundInterception"` + } + if err := yaml.Unmarshal([]byte(authbridgeYAML), &top); err != nil { + nsConfigLog.Info("WARN: failed to parse authbridge-runtime-config config.yaml for inboundInterception; falling back to next resolution layer", + "error", err) + return "" + } + return top.InboundInterception +} + // ExtractEgressEnforcement parses an authbridge-runtime-config config.yaml // string and returns the value of its top-level `egressEnforcement:` key. // Returns "" if the YAML is empty, malformed, or has no `egressEnforcement` diff --git a/operator/internal/webhook/injector/pod_mutator.go b/operator/internal/webhook/injector/pod_mutator.go index 8f07928f..7a41df5b 100644 --- a/operator/internal/webhook/injector/pod_mutator.go +++ b/operator/internal/webhook/injector/pod_mutator.go @@ -69,6 +69,10 @@ const ( InboundPortsExcludeAnnotation = "rossoctl.io/inbound-ports-exclude" sourceNamespaceConfigMap = "namespace-configmap" + // Resolution-source labels shared by the mode / egress / inbound resolvers, + // so the log strings stay identical across them. + sourceClusterDefault = "cluster-default" + sourceInvalidFallback = "default-invalid-fallback" // RossoctlTypeLabel is the label key that identifies the workload type RossoctlTypeLabel = "rossoctl.io/type" @@ -321,7 +325,7 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp } if egressEnforcement == "" { egressEnforcement = EgressEnforcementEnforceRedirect - egressEnforcementSource = "cluster-default" + egressEnforcementSource = sourceClusterDefault } switch egressEnforcement { case EgressEnforcementEnforceRedirect, EgressEnforcementNone: @@ -331,7 +335,7 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp "namespace", namespace, "crName", crName, "unrecognized", egressEnforcement, "source", egressEnforcementSource) egressEnforcement = EgressEnforcementEnforceRedirect - egressEnforcementSource = "default-invalid-fallback" + egressEnforcementSource = sourceInvalidFallback } // Validate against the platform's allowed list. If the resolved value // is not permitted, fall back to the first allowed value (fail closed @@ -452,6 +456,71 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp "namespace", namespace, "crName", crName, "mode", authBridgeMode, "source", modeSource) + // ======================================== + // Resolve inboundInterception (namespace > reverse-proxy) + // ======================================== + // + // Selects the inbound deployment shape in proxy-sidecar / lite modes: + // "reverse-proxy" (default) steals the agent's port; "transparent" installs a + // PREROUTING REDIRECT and lets AuthBridge recover the real destination. + // envoy-sidecar ignores this — Envoy already intercepts inbound. + inboundInterception := "" + inboundInterceptionSource := "" + if ii := ExtractInboundInterception(nsConfig.AuthBridgeRuntimeYAML); ii != "" { + inboundInterception = ii + inboundInterceptionSource = sourceNamespaceConfigMap + } + if inboundInterception == "" { + inboundInterception = InboundInterceptionReverseProxy + inboundInterceptionSource = sourceClusterDefault + } + switch inboundInterception { + case InboundInterceptionReverseProxy, InboundInterceptionTransparent: + // recognized, keep as-is + default: + // Fall back to the shape that needs no privileges: an unrecognized value + // must not silently grant a NET_ADMIN init container. + mutatorLog.Info("WARN: unrecognized inboundInterception; defaulting to reverse-proxy", + "namespace", namespace, "crName", crName, + "unrecognized", inboundInterception, "source", inboundInterceptionSource) + inboundInterception = InboundInterceptionReverseProxy + inboundInterceptionSource = sourceInvalidFallback + } + allowedInbound := currentConfig.Proxy.AllowedInboundInterception + if len(allowedInbound) > 0 && !slices.Contains(allowedInbound, inboundInterception) { + mutatorLog.Info("WARN: inboundInterception value not in platform allowedInboundInterception; overriding", + "namespace", namespace, "crName", crName, + "requested", inboundInterception, "allowed", allowedInbound, + "overrideTo", allowedInbound[0]) + inboundInterception = allowedInbound[0] + inboundInterceptionSource = "platform-policy-override" + } + // Transparent inbound and the egress guard share one proxy-init container. + // With egressEnforcement "none" there is no init container to carry the + // PREROUTING rules, so nothing would REDIRECT to the inbound listener and it + // would bind a port that never receives traffic — inbound silently unenforced. + // Fall back to port stealing, which at least validates Service-routed traffic. + if inboundInterception == InboundInterceptionTransparent && egressEnforcement != EgressEnforcementEnforceRedirect { + mutatorLog.Info("WARN: inboundInterception=transparent requires egressEnforcement=enforce-redirect (shared proxy-init); falling back to reverse-proxy", + "namespace", namespace, "crName", crName, + "egressEnforcement", egressEnforcement) + inboundInterception = InboundInterceptionReverseProxy + inboundInterceptionSource = "requires-enforce-redirect-fallback" + } + // Only proxy-sidecar / lite consult this. Saying so keeps the log honest: the + // resolver runs before the mode branch, so an envoy-sidecar or waypoint + // namespace carrying inboundInterception would otherwise read as though the + // value took effect. + if authBridgeMode == ModeProxySidecar || authBridgeMode == ModeLite { + mutatorLog.Info("resolved inbound interception", + "namespace", namespace, "crName", crName, + "mechanism", inboundInterception, "source", inboundInterceptionSource) + } else if inboundInterceptionSource == sourceNamespaceConfigMap { + mutatorLog.Info("ignoring inboundInterception: not applicable to this mode", + "namespace", namespace, "crName", crName, + "mode", authBridgeMode, "requested", inboundInterception) + } + if authBridgeMode == ModeWaypoint { mutatorLog.Info("waypoint mode — skipping sidecar injection (waypoint is a standalone deployment)", "namespace", namespace, "crName", crName) @@ -537,21 +606,148 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp // Reserve the original agent port for the reverse proxy usedPorts[originalAgentPort] = true - newAgentPort, err := findFreePort(originalAgentPort + 1) - if err != nil { - return false, fmt.Errorf("proxy-sidecar port assignment: %w", err) + transparentInbound := inboundInterception == InboundInterceptionTransparent + transparentInboundPort := builder.cfg.Proxy.TransparentInboundPort + + // Ports the sidecar owns. Reserved unconditionally so findFreePort can never + // hand one to the forward proxy: the transparent EGRESS port in particular + // was previously unreserved, so an agent declaring 8081 could push the + // forward proxy onto 8082 and collide with a listener that is always on in + // this mode. The inbound port is reserved even under reverse-proxy, so port + // assignment stays stable if a namespace later flips mechanism. + sidecarPorts := map[int32]string{ + builder.cfg.Proxy.TransparentPort: "transparent egress listener", + transparentInboundPort: "transparent inbound listener", + authBridgeHealthPort: "health endpoint", + authBridgeStatsPort: "stats endpoint", + authBridgeSessionAPIPort: "session-events API", + } + for p := range sidecarPorts { + usedPorts[p] = true + } + + // A proxy-init container already in the INCOMING spec is not one we built, + // so we cannot assume it carries the inbound env — and the injection below + // is skipped when the container exists. Left alone that yields the one + // outcome this mode exists to prevent: the sidecar binds the inbound + // listener and the ConfigMap says transparent, but nothing REDIRECTs to it, + // so inbound is silently unenforced with no error anywhere. + // + // Reachable only via a hand-authored proxy-init (the webhook is CREATE-only + // on pods, and a fresh pod comes from an unmutated template), but the + // failure is silent, so check rather than rely on that. + if transparentInbound { + for i := range podSpec.InitContainers { + if podSpec.InitContainers[i].Name != ProxyInitContainerName { + continue + } + armed := false + for _, e := range podSpec.InitContainers[i].Env { + if e.Name == "INBOUND_TRANSPARENT_PORT" && e.Value != "" { + armed = true + break + } + } + if !armed { + mutatorLog.Info("WARN: pod already carries a proxy-init container without inbound capture; overriding inbound interception to reverse-proxy rather than bind a listener nothing redirects to", + "namespace", namespace, "crName", crName, + "effectiveMechanism", InboundInterceptionReverseProxy, + "source", "preexisting-proxy-init-fallback", + "hint", "remove the hand-added proxy-init container to use transparent interception") + transparentInbound = false + inboundInterception = InboundInterceptionReverseProxy + } + break + } + } + + // Which of those are actually BOUND depends on the mechanism: under + // reverse-proxy nothing listens on the inbound port, so an agent declaring + // it is harmless and must not trigger the fallback below. + boundBySidecar := make(map[int32]string, len(sidecarPorts)) + for port, owner := range sidecarPorts { + if port == transparentInboundPort && !transparentInbound { + continue + } + boundBySidecar[port] = owner + } + + // Reserving is not enough for the agent's OWN ports: containerPort is + // informational, so the operator cannot move a port the agent actually + // binds. Under port stealing a collision was accidentally survivable — + // Ports[0] got relocated off the conflicting port. Transparent mode + // deliberately removes that, so an agent declaring the sidecar's inbound + // port would leave two processes binding one port in a shared netns: + // admission succeeds, then the sidecar dies on "address already in use" in + // a container the user never wrote. + // + // Fall back to port stealing rather than reject. Rejecting would block + // pods that declare a colliding port informationally without ever binding + // it — which works today — and for a first-port collision the relocation + // genuinely resolves it. The warning is the actionable part. + if transparentInbound { + for i := range podSpec.Containers { + c := &podSpec.Containers[i] + if c.Name == AuthBridgeProxyContainerName { + continue + } + for _, cp := range c.Ports { + if owner, clash := boundBySidecar[cp.ContainerPort]; clash { + // States the EFFECTIVE mechanism: the "resolved inbound + // interception" line above already logged transparent, and + // this is the point where that stops being true. + mutatorLog.Info("WARN: container declares a port the authbridge sidecar binds; overriding inbound interception to reverse-proxy to avoid a bind collision", + "namespace", namespace, "crName", crName, + "container", c.Name, "port", cp.ContainerPort, + "conflictsWith", owner, + "effectiveMechanism", InboundInterceptionReverseProxy, + "source", "sidecar-port-conflict-fallback") + transparentInbound = false + inboundInterception = InboundInterceptionReverseProxy + break + } + } + if !transparentInbound { + break + } + } + + } + + // newAgentPort exists only for the port-stealing shape. Transparent + // interception leaves the agent exactly where it is, so there is nothing + // to relocate and no relocated port for anyone to reach around us. + var newAgentPort int32 + if !transparentInbound { + p, perr := findFreePort(originalAgentPort + 1) + if perr != nil { + return false, fmt.Errorf("proxy-sidecar port assignment: %w", perr) + } + newAgentPort = p } forwardProxyPort, err := findFreePort(8081) if err != nil { return false, fmt.Errorf("proxy-sidecar port assignment: %w", err) } - // Move the agent to the free port. - // Most agent frameworks (Python/uvicorn, Node/express, FastAPI) read the - // PORT env var to determine the bind address. Go agents that hardcode their - // listen port won't be affected by this env var — they must use PORT or - // be configured via their own config mechanism. - if agentContainer != nil { + if transparentInbound { + // No port mutation and no PORT env var: iptables REDIRECTs inbound + // traffic to AuthBridge, which recovers the port the client actually + // addressed via SO_ORIGINAL_DST and forwards there over loopback. That + // removes the two failure modes of port stealing — an agent that + // hardcodes its listen port (which would collide with AuthBridge) and + // the unvalidated relocated port — and covers every port the agent + // listens on, not just the first declared one. + mutatorLog.Info("proxy-sidecar transparent inbound (no port stealing)", + "agentPort", originalAgentPort, + "transparentInboundPort", transparentInboundPort, + "forwardProxyPort", forwardProxyPort) + } else if agentContainer != nil { + // Move the agent to the free port. + // Most agent frameworks (Python/uvicorn, Node/express, FastAPI) read the + // PORT env var to determine the bind address. Go agents that hardcode their + // listen port won't be affected by this env var — they must use PORT or + // be configured via their own config mechanism. agentContainer.Ports[0].ContainerPort = newAgentPort setOrAddEnv(agentContainer, "PORT", fmt.Sprintf("%d", newAgentPort)) mutatorLog.Info("proxy-sidecar port stealing", @@ -562,13 +758,24 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp } // Create per-agent ConfigMap with proxy-sidecar listener addresses + // Transparent interception derives the backend per connection, so it must + // NOT get reverse_proxy_addr / reverse_proxy_backend: authbridge's config + // validation rejects a reverse_proxy_addr alongside inbound_interception + // transparent, and binding it would either be dead or collide with the + // agent's own port. + listenerOverrides := map[string]string{ + "forward_proxy_addr": fmt.Sprintf(":%d", forwardProxyPort), + } + if transparentInbound { + listenerOverrides["inbound_interception"] = InboundInterceptionTransparent + listenerOverrides["transparent_inbound_addr"] = fmt.Sprintf(":%d", transparentInboundPort) + } else { + listenerOverrides["reverse_proxy_addr"] = fmt.Sprintf(":%d", originalAgentPort) + listenerOverrides["reverse_proxy_backend"] = fmt.Sprintf("http://127.0.0.1:%d", newAgentPort) + } perAgentCMName, err := m.ensurePerAgentConfigMap(ctx, namespace, crName, ModeProxySidecar, nsConfig.AuthBridgeRuntimeYAML, nsConfig, - map[string]string{ - "reverse_proxy_addr": fmt.Sprintf(":%d", originalAgentPort), - "reverse_proxy_backend": fmt.Sprintf("http://127.0.0.1:%d", newAgentPort), - "forward_proxy_addr": fmt.Sprintf(":%d", forwardProxyPort), - }, + listenerOverrides, mtlsMode, tlsBridgeMode, spireEnabled, agentRuntime) if err != nil { return false, fmt.Errorf("proxy-sidecar per-agent ConfigMap: %w", err) @@ -576,14 +783,23 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp // Inject authbridge-proxy container listening on the original agent port if !containerExists(podSpec.Containers, AuthBridgeProxyContainerName) { - podSpec.Containers = append(podSpec.Containers, - builder.BuildProxySidecarContainerWithPorts( + var sidecar corev1.Container + if transparentInbound { + sidecar = builder.BuildProxySidecarContainerTransparent( + spireEnabled, + proxyImage, + transparentInboundPort, // iptables REDIRECTs inbound here + forwardProxyPort, // forward proxy listens here + ) + } else { + sidecar = builder.BuildProxySidecarContainerWithPorts( spireEnabled, proxyImage, originalAgentPort, // reverse proxy listens here - newAgentPort, // forwards to agent here forwardProxyPort, // forward proxy listens here - )) + ) + } + podSpec.Containers = append(podSpec.Containers, sidecar) } // Set MTLS_MODE env var on the authbridge container so it knows the @@ -615,10 +831,35 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp // this setting. if egressEnforcement == EgressEnforcementEnforceRedirect { if !containerExists(podSpec.InitContainers, ProxyInitContainerName) { + // Transparent inbound rides on the same proxy-init container as the + // egress guard; its PREROUTING rules are installed only when + // INBOUND_TRANSPARENT_PORT is set. SIDECAR_PORTS_EXCLUDE carries the + // RESOLVED forward-proxy port, which findFreePort may have moved off + // 8081 — the script's default would then leave it unexempted and the + // inbound REDIRECT would swallow the forward proxy's own port. + var inbound ProxyInitInbound + if transparentInbound { + inbound = ProxyInitInbound{ + Port: transparentInboundPort, + // Only the ports the SCRIPT cannot know. The forward-proxy port + // is here because findFreePort may have moved it off 8081; the + // health/stats/session ports because they are operator-side + // constants. The two transparent ports (8082 egress, 8083 + // inbound) are deliberately absent: init-iptables.sh exempts + // both unconditionally in emit_inbound_exemptions, on BOTH the + // PREROUTING and ambient hooks, since it already knows them from + // TRANSPARENT_PORT / INBOUND_TRANSPARENT_PORT. Listing them + // again would just duplicate RETURN rules. + SidecarPortsExclude: fmt.Sprintf("%d,%d,%d,%d", + forwardProxyPort, authBridgeHealthPort, authBridgeStatsPort, authBridgeSessionAPIPort), + PortsExclude: annotations[InboundPortsExcludeAnnotation], + } + } podSpec.InitContainers = append(podSpec.InitContainers, - builder.BuildProxyInitContainer(ProxyInitModeEnforceRedirect, "", "")) + builder.BuildProxyInitContainerWithInbound(ProxyInitModeEnforceRedirect, "", "", inbound)) mutatorLog.Info("proxy-sidecar egress enforcement enabled (enforce-redirect)", - "namespace", namespace, "crName", crName) + "namespace", namespace, "crName", crName, + "inboundInterception", inboundInterception) } } else { mutatorLog.Info("proxy-sidecar egress enforcement disabled (cooperative mode)", diff --git a/operator/internal/webhook/injector/pod_mutator_test.go b/operator/internal/webhook/injector/pod_mutator_test.go index 38559de2..eee6bd31 100644 --- a/operator/internal/webhook/injector/pod_mutator_test.go +++ b/operator/internal/webhook/injector/pod_mutator_test.go @@ -1106,8 +1106,18 @@ func TestInjectAuthBridge_ProxySidecarMode_ForwardProxyCollision(t *testing.T) { if p.ContainerPort == 8081 { t.Error("forward-proxy should not be 8081 (collision with agent metrics)") } - if p.ContainerPort != 8082 { - t.Errorf("forward-proxy port = %d, want 8082", p.ContainerPort) + // 8084, not 8082: the sidecar's own ports are now reserved, so + // findFreePort skips the transparent egress listener (8082) and + // the transparent inbound listener (8083). This expectation used + // to be 8082, which would have put the forward proxy on top of a + // listener that is always on in proxy-sidecar mode. + if p.ContainerPort != 8084 { + t.Errorf("forward-proxy port = %d, want 8084", p.ContainerPort) + } + for _, owned := range []int32{8082, 8083, 9091, 9093, 9094} { + if p.ContainerPort == owned { + t.Errorf("forward-proxy assigned %d, a port the sidecar binds", owned) + } } } } @@ -1122,8 +1132,8 @@ func TestInjectAuthBridge_ProxySidecarMode_ForwardProxyCollision(t *testing.T) { if env.Value == "http://127.0.0.1:8081" { t.Error("HTTP_PROXY should not use 8081 (collides with agent metrics)") } - if env.Value != "http://127.0.0.1:8082" { - t.Errorf("HTTP_PROXY = %q, want http://127.0.0.1:8082", env.Value) + if env.Value != "http://127.0.0.1:8084" { + t.Errorf("HTTP_PROXY = %q, want http://127.0.0.1:8084", env.Value) } } } diff --git a/operator/internal/webhook/injector/transparent_inbound_test.go b/operator/internal/webhook/injector/transparent_inbound_test.go new file mode 100644 index 00000000..0c873382 --- /dev/null +++ b/operator/internal/webhook/injector/transparent_inbound_test.go @@ -0,0 +1,570 @@ +package injector + +import ( + "context" + "fmt" + "strings" + "testing" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/rossoctl/operator/internal/webhook/config" +) + +// inboundCM builds a namespace authbridge-runtime-config pinning the inbound +// mechanism (and optionally the egress posture, which transparent depends on). +func inboundCM(inbound, egress string) *corev1.ConfigMap { + yaml := "mode: " + ModeProxySidecar + "\n" + if inbound != "" { + yaml += "inboundInterception: " + inbound + "\n" + } + if egress != "" { + yaml += "egressEnforcement: " + egress + "\n" + } + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: AuthBridgeRuntimeConfigMapName, Namespace: "test-ns"}, + Data: map[string]string{"config.yaml": yaml}, + } +} + +func newTestMutatorWithAllowedInbound(allowed []string, objs ...client.Object) *PodMutator { + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + _ = appsv1.AddToScheme(scheme) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() + return &PodMutator{ + Client: fakeClient, + APIReader: fakeClient, + GetPlatformConfig: func() *config.PlatformConfig { + cfg := config.CompiledDefaults() + cfg.Proxy.AllowedInboundInterception = allowed + return cfg + }, + GetFeatureGates: config.DefaultFeatureGates, + } +} + +func agentPodSpec() *corev1.PodSpec { + return &corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "agent", + Image: "my-agent:latest", + Ports: []corev1.ContainerPort{{ContainerPort: 8000}}, + }}, + } +} + +func containerByName(spec *corev1.PodSpec, name string) *corev1.Container { + for i := range spec.Containers { + if spec.Containers[i].Name == name { + return &spec.Containers[i] + } + } + for i := range spec.InitContainers { + if spec.InitContainers[i].Name == name { + return &spec.InitContainers[i] + } + } + return nil +} + +func envVarOf(c *corev1.Container, name string) (corev1.EnvVar, bool) { + for _, e := range c.Env { + if e.Name == name { + return e, true + } + } + return corev1.EnvVar{}, false +} + +// TestTransparentInbound_DoesNotStealAgentPort is the central behavioral claim: +// transparent interception leaves the agent exactly where it is. That removes +// both failure modes of port stealing — an agent that hardcodes its listen port +// (which collides with AuthBridge) and the relocated port that another pod can +// reach without JWT validation. +func TestTransparentInbound_DoesNotStealAgentPort(t *testing.T) { + m := newTestMutator(inboundCM(InboundInterceptionTransparent, "")) + ctx := context.Background() + podSpec := agentPodSpec() + + injected, err := m.InjectAuthBridge(ctx, podSpec, "test-ns", "my-agent", "Deployment", + map[string]string{RossoctlTypeLabel: RossoctlTypeAgent}, nil) + if err != nil { + t.Fatalf("InjectAuthBridge() error: %v", err) + } + if !injected { + t.Fatal("expected injection") + } + + agent := containerByName(podSpec, "agent") + if agent == nil { + t.Fatal("agent container disappeared") + } + if got := agent.Ports[0].ContainerPort; got != 8000 { + t.Errorf("agent port = %d, want 8000 unchanged (transparent must not relocate the agent)", got) + } + if _, ok := envVarOf(agent, "PORT"); ok { + t.Error("transparent inbound must not inject PORT — the agent keeps its own port") + } +} + +// TestReverseProxyDefault_StillStealsPort locks the opt-in contract: with the +// field unset, behavior must be byte-identical to before this feature existed. +func TestReverseProxyDefault_StillStealsPort(t *testing.T) { + m := newTestMutator() + ctx := context.Background() + podSpec := agentPodSpec() + + if _, err := m.InjectAuthBridge(ctx, podSpec, "test-ns", "my-agent", "Deployment", + map[string]string{RossoctlTypeLabel: RossoctlTypeAgent}, nil); err != nil { + t.Fatalf("InjectAuthBridge() error: %v", err) + } + + agent := containerByName(podSpec, "agent") + if got := agent.Ports[0].ContainerPort; got == 8000 { + t.Error("default (reverse-proxy) must still relocate the agent off its original port") + } + if _, ok := envVarOf(agent, "PORT"); !ok { + t.Error("default (reverse-proxy) must still inject PORT") + } +} + +// TestTransparentInbound_SidecarDeclaresTransparentPort checks the sidecar binds +// the REDIRECT target and no longer claims the agent's port. +func TestTransparentInbound_SidecarDeclaresTransparentPort(t *testing.T) { + m := newTestMutator(inboundCM(InboundInterceptionTransparent, "")) + ctx := context.Background() + podSpec := agentPodSpec() + + if _, err := m.InjectAuthBridge(ctx, podSpec, "test-ns", "my-agent", "Deployment", + map[string]string{RossoctlTypeLabel: RossoctlTypeAgent}, nil); err != nil { + t.Fatalf("InjectAuthBridge() error: %v", err) + } + + proxy := containerByName(podSpec, AuthBridgeProxyContainerName) + if proxy == nil { + t.Fatal("authbridge-proxy container not injected") + } + names := make([]string, 0, len(proxy.Ports)) + found := false + for _, p := range proxy.Ports { + names = append(names, fmt.Sprintf("%s=%d", p.Name, p.ContainerPort)) + if p.ContainerPort == 8083 { + found = true + if p.Name != "transparent-in" { + t.Errorf("inbound port name = %q, want transparent-in (so the shape is visible in the pod spec)", p.Name) + } + } + if p.ContainerPort == 8000 { + t.Error("sidecar must not claim the agent's port under transparent interception") + } + } + if !found { + t.Errorf("sidecar does not declare the transparent inbound port 8083; got %v", names) + } +} + +// TestTransparentInbound_ConfigMapOmitsReverseProxyKeys matters because +// authbridge's own config validation REJECTS a reverse_proxy_addr alongside +// inbound_interception: transparent — emitting both would crash-loop the pod. +func TestTransparentInbound_ConfigMapOmitsReverseProxyKeys(t *testing.T) { + m := newTestMutator(inboundCM(InboundInterceptionTransparent, "")) + ctx := context.Background() + + if _, err := m.InjectAuthBridge(ctx, agentPodSpec(), "test-ns", "my-agent", "Deployment", + map[string]string{RossoctlTypeLabel: RossoctlTypeAgent}, nil); err != nil { + t.Fatalf("InjectAuthBridge() error: %v", err) + } + + cm := fetchConfigMap(t, m, "test-ns", perAgentConfigMapName("my-agent")) + cfg := parseConfigYAML(t, cm) + listener, ok := cfg["listener"].(map[string]interface{}) + if !ok { + t.Fatalf("listener block missing or not a map: %T", cfg["listener"]) + } + + if listener["inbound_interception"] != InboundInterceptionTransparent { + t.Errorf("inbound_interception = %v, want %s", listener["inbound_interception"], InboundInterceptionTransparent) + } + if listener["transparent_inbound_addr"] != ":8083" { + t.Errorf("transparent_inbound_addr = %v, want :8083", listener["transparent_inbound_addr"]) + } + for _, key := range []string{"reverse_proxy_addr", "reverse_proxy_backend"} { + if v, present := listener[key]; present { + t.Errorf("%s must be absent under transparent interception (authbridge rejects it), got %v", key, v) + } + } + // Egress is unaffected. + if listener["forward_proxy_addr"] == nil { + t.Error("forward_proxy_addr must still be set — transparent inbound changes only ingress") + } +} + +// TestTransparentInbound_ProxyInitEnv covers the two env vars without which the +// feature is either inert or half-enforced: the REDIRECT target, and POD_IP for +// the Istio ambient path that never traverses PREROUTING. +func TestTransparentInbound_ProxyInitEnv(t *testing.T) { + m := newTestMutator(inboundCM(InboundInterceptionTransparent, "")) + ctx := context.Background() + podSpec := agentPodSpec() + + if _, err := m.InjectAuthBridge(ctx, podSpec, "test-ns", "my-agent", "Deployment", + map[string]string{RossoctlTypeLabel: RossoctlTypeAgent}, nil); err != nil { + t.Fatalf("InjectAuthBridge() error: %v", err) + } + + init := containerByName(podSpec, ProxyInitContainerName) + if init == nil { + t.Fatal("proxy-init not injected — transparent inbound has nothing to REDIRECT with") + } + + port, ok := envVarOf(init, "INBOUND_TRANSPARENT_PORT") + if !ok || port.Value != "8083" { + t.Errorf("INBOUND_TRANSPARENT_PORT = %q (present=%v), want 8083", port.Value, ok) + } + + podIP, ok := envVarOf(init, "POD_IP") + if !ok { + t.Fatal("POD_IP missing: without it proxy-init cannot capture the ambient inbound path and refuses to start") + } + if podIP.ValueFrom == nil || podIP.ValueFrom.FieldRef == nil || + podIP.ValueFrom.FieldRef.FieldPath != "status.podIP" { + t.Errorf("POD_IP must come from the downward API status.podIP, got %+v", podIP.ValueFrom) + } + + // POD_IPS covers the dual-stack case: with only POD_IP (the primary address) + // the other family's ambient delivery would pass unvalidated while its + // PREROUTING rules were installed. + podIPs, ok := envVarOf(init, "POD_IPS") + if !ok { + t.Fatal("POD_IPS missing — IPv6 ambient inbound is uncaptured on a dual-stack pod") + } + if podIPs.ValueFrom == nil || podIPs.ValueFrom.FieldRef == nil || + podIPs.ValueFrom.FieldRef.FieldPath != "status.podIPs" { + t.Errorf("POD_IPS must come from the downward API status.podIPs, got %+v", podIPs.ValueFrom) + } + + // SIDECAR_PORTS_EXCLUDE must carry the RESOLVED forward-proxy port. If the + // script's 8081 default were used and findFreePort had moved it, the inbound + // REDIRECT would swallow the forward proxy's own port. + excl, ok := envVarOf(init, "SIDECAR_PORTS_EXCLUDE") + if !ok { + t.Fatal("SIDECAR_PORTS_EXCLUDE missing — health/stats/session ports would be JWT-gated") + } + for _, want := range []string{"9091", "9093", "9094"} { + if !strings.Contains(excl.Value, want) { + t.Errorf("SIDECAR_PORTS_EXCLUDE=%q missing %s", excl.Value, want) + } + } + + proxy := containerByName(podSpec, AuthBridgeProxyContainerName) + var fwd int32 + for _, p := range proxy.Ports { + if p.Name == "forward-proxy" { + fwd = p.ContainerPort + } + } + if fwd == 0 { + t.Fatal("could not determine the resolved forward-proxy port") + } + if !strings.Contains(excl.Value, fmt.Sprintf("%d", fwd)) { + t.Errorf("SIDECAR_PORTS_EXCLUDE=%q must contain the resolved forward-proxy port %d", excl.Value, fwd) + } +} + +// TestTransparentInbound_HonorsInboundPortsExcludeAnnotation covers the escape +// hatch for app ports that must not be validated (e.g. an oauth-proxy doing its +// own auth). +func TestTransparentInbound_HonorsInboundPortsExcludeAnnotation(t *testing.T) { + m := newTestMutator(inboundCM(InboundInterceptionTransparent, "")) + ctx := context.Background() + podSpec := agentPodSpec() + + annotations := map[string]string{InboundPortsExcludeAnnotation: "8443"} + if _, err := m.InjectAuthBridge(ctx, podSpec, "test-ns", "my-agent", "Deployment", + map[string]string{RossoctlTypeLabel: RossoctlTypeAgent}, annotations); err != nil { + t.Fatalf("InjectAuthBridge() error: %v", err) + } + + init := containerByName(podSpec, ProxyInitContainerName) + excl, ok := envVarOf(init, "INBOUND_PORTS_EXCLUDE") + if !ok || !strings.Contains(excl.Value, "8443") { + t.Errorf("INBOUND_PORTS_EXCLUDE = %q (present=%v), want it to contain 8443", excl.Value, ok) + } +} + +// TestTransparentInbound_RequiresEnforceRedirect: the two features share one +// proxy-init container. With egressEnforcement "none" there is no init container +// to carry the PREROUTING rules, so the listener would bind a port nothing +// redirects to — inbound silently unenforced. Fall back to port stealing, which +// at least validates Service-routed traffic. +func TestTransparentInbound_RequiresEnforceRedirect(t *testing.T) { + m := newTestMutator(inboundCM(InboundInterceptionTransparent, EgressEnforcementNone)) + ctx := context.Background() + podSpec := agentPodSpec() + + if _, err := m.InjectAuthBridge(ctx, podSpec, "test-ns", "my-agent", "Deployment", + map[string]string{RossoctlTypeLabel: RossoctlTypeAgent}, nil); err != nil { + t.Fatalf("InjectAuthBridge() error: %v", err) + } + + agent := containerByName(podSpec, "agent") + if got := agent.Ports[0].ContainerPort; got == 8000 { + t.Error("expected fallback to port stealing when egressEnforcement=none (no proxy-init to REDIRECT with)") + } + cm := fetchConfigMap(t, m, "test-ns", perAgentConfigMapName("my-agent")) + listener := parseConfigYAML(t, cm)["listener"].(map[string]interface{}) + if _, present := listener["transparent_inbound_addr"]; present { + t.Error("must not configure a transparent inbound listener that nothing can REDIRECT to") + } +} + +// TestTransparentInbound_PlatformPolicyCanForbid: transparent inbound grants a +// NET_ADMIN init container, so an admin must be able to forbid it cluster-wide. +func TestTransparentInbound_PlatformPolicyCanForbid(t *testing.T) { + m := newTestMutatorWithAllowedInbound( + []string{InboundInterceptionReverseProxy}, + inboundCM(InboundInterceptionTransparent, ""), + ) + ctx := context.Background() + podSpec := agentPodSpec() + + if _, err := m.InjectAuthBridge(ctx, podSpec, "test-ns", "my-agent", "Deployment", + map[string]string{RossoctlTypeLabel: RossoctlTypeAgent}, nil); err != nil { + t.Fatalf("InjectAuthBridge() error: %v", err) + } + + agent := containerByName(podSpec, "agent") + if got := agent.Ports[0].ContainerPort; got == 8000 { + t.Error("platform policy forbidding transparent must force the port-stealing shape") + } +} + +// TestTransparentInbound_UnrecognizedValueFallsBackToNoPrivilege: an unknown +// value must not silently grant a privileged init container. +func TestTransparentInbound_UnrecognizedValueFallsBackToNoPrivilege(t *testing.T) { + m := newTestMutator(inboundCM("tranparent", "")) // plausible typo (dropped letter) + ctx := context.Background() + podSpec := agentPodSpec() + + if _, err := m.InjectAuthBridge(ctx, podSpec, "test-ns", "my-agent", "Deployment", + map[string]string{RossoctlTypeLabel: RossoctlTypeAgent}, nil); err != nil { + t.Fatalf("InjectAuthBridge() error: %v", err) + } + + init := containerByName(podSpec, ProxyInitContainerName) + if init != nil { + if _, ok := envVarOf(init, "INBOUND_TRANSPARENT_PORT"); ok { + t.Error("an unrecognized inboundInterception must not enable inbound capture") + } + } + agent := containerByName(podSpec, "agent") + if got := agent.Ports[0].ContainerPort; got == 8000 { + t.Error("unrecognized value must fall back to the port-stealing default") + } +} + +// TestExtractInboundInterception covers the namespace-config parse layer, +// including malformed YAML falling through rather than erroring. +func TestExtractInboundInterception(t *testing.T) { + for _, tc := range []struct{ name, yaml, want string }{ + {"present", "mode: proxy-sidecar\ninboundInterception: transparent\n", "transparent"}, + {"absent", "mode: proxy-sidecar\n", ""}, + {"empty", "", ""}, + {"malformed falls through", "mode: [unclosed\n", ""}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := ExtractInboundInterception(tc.yaml); got != tc.want { + t.Errorf("ExtractInboundInterception(%q) = %q, want %q", tc.yaml, got, tc.want) + } + }) + } +} + +// agentPodSpecOnPort is agentPodSpec with a caller-chosen container port, for the +// collision cases. The existing tests all use 8000, which is why the sidecar-port +// conflict went uncovered. +func agentPodSpecOnPort(port int32) *corev1.PodSpec { + return &corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "agent", + Image: "my-agent:latest", + Ports: []corev1.ContainerPort{{ContainerPort: port}}, + }}, + } +} + +// TestTransparentInbound_AgentPortCollidesWithSidecar covers the failure mode +// transparent interception newly creates. Under port stealing a collision was +// accidentally survivable — Ports[0] got relocated off the conflicting port. +// Transparent mode deliberately does not relocate, so an agent declaring a port +// the sidecar binds would leave two processes on one port in a shared netns: +// admission succeeds, then the sidecar dies on "address already in use" in a +// container the user never wrote. +// +// containerPort is informational, so the operator cannot move a port the agent +// actually binds. Falling back to port stealing is the survivable answer. +func TestTransparentInbound_AgentPortCollidesWithSidecar(t *testing.T) { + // Each of these is a port the sidecar itself binds. + for _, tc := range []struct { + name string + port int32 + }{ + {"transparent inbound listener", 8083}, + {"transparent egress listener", 8082}, + {"health endpoint", 9091}, + {"stats endpoint", 9093}, + {"session-events API", 9094}, + } { + t.Run(tc.name, func(t *testing.T) { + m := newTestMutator(inboundCM(InboundInterceptionTransparent, "")) + podSpec := agentPodSpecOnPort(tc.port) + + if _, err := m.InjectAuthBridge(context.Background(), podSpec, "test-ns", "my-agent", + "Deployment", map[string]string{RossoctlTypeLabel: RossoctlTypeAgent}, nil); err != nil { + t.Fatalf("InjectAuthBridge() error: %v", err) + } + + proxy := containerByName(podSpec, AuthBridgeProxyContainerName) + if proxy == nil { + t.Fatal("authbridge-proxy not injected") + } + // Fell back to port stealing: the agent is relocated, so nothing + // double-binds. + agent := containerByName(podSpec, "agent") + if got := agent.Ports[0].ContainerPort; got == tc.port { + t.Errorf("agent still on %d; expected relocation after falling back to reverse-proxy", tc.port) + } + // And the sidecar must not be running the transparent shape. + for _, p := range proxy.Ports { + if p.Name == "transparent-in" { + t.Errorf("sidecar still declares transparent-in after a collision on %d", tc.port) + } + } + init := containerByName(podSpec, ProxyInitContainerName) + if init != nil { + if _, ok := envVarOf(init, "INBOUND_TRANSPARENT_PORT"); ok { + t.Errorf("inbound capture still armed after a collision on %d", tc.port) + } + } + }) + } +} + +// TestTransparentInbound_NonCollidingPortStillTransparent guards the guard: it +// must not fire for an ordinary agent port. +func TestTransparentInbound_NonCollidingPortStillTransparent(t *testing.T) { + m := newTestMutator(inboundCM(InboundInterceptionTransparent, "")) + podSpec := agentPodSpecOnPort(8000) + + if _, err := m.InjectAuthBridge(context.Background(), podSpec, "test-ns", "my-agent", + "Deployment", map[string]string{RossoctlTypeLabel: RossoctlTypeAgent}, nil); err != nil { + t.Fatalf("InjectAuthBridge() error: %v", err) + } + agent := containerByName(podSpec, "agent") + if got := agent.Ports[0].ContainerPort; got != 8000 { + t.Errorf("agent relocated to %d; a non-colliding port must stay transparent", got) + } +} + +// TestForwardProxyNeverLandsOnSidecarPort covers the pre-existing hole the +// reservation also closes: the transparent EGRESS port was never reserved, so an +// agent declaring 8081 could push findFreePort's forward proxy onto 8082 and +// collide with a listener that is always on in proxy-sidecar mode. +func TestForwardProxyNeverLandsOnSidecarPort(t *testing.T) { + for _, mechanism := range []string{InboundInterceptionReverseProxy, InboundInterceptionTransparent} { + t.Run(mechanism, func(t *testing.T) { + m := newTestMutator(inboundCM(mechanism, "")) + // 8081 declared by the agent forces findFreePort to look further. + podSpec := agentPodSpecOnPort(8081) + + if _, err := m.InjectAuthBridge(context.Background(), podSpec, "test-ns", "my-agent", + "Deployment", map[string]string{RossoctlTypeLabel: RossoctlTypeAgent}, nil); err != nil { + t.Fatalf("InjectAuthBridge() error: %v", err) + } + proxy := containerByName(podSpec, AuthBridgeProxyContainerName) + for _, p := range proxy.Ports { + if p.Name != "forward-proxy" { + continue + } + for _, owned := range []int32{8082, 8083, 9091, 9093, 9094} { + if p.ContainerPort == owned { + t.Errorf("forward proxy assigned %d, which the sidecar already binds", owned) + } + } + } + }) + } +} + +// TestTransparentInbound_PreexistingProxyInitFallsBack covers the divergence a +// reviewer flagged: the proxy-init injection is skipped when a container of that +// name already exists, so a hand-authored one without the inbound env would leave +// the sidecar binding :8083 and the ConfigMap saying transparent while nothing +// REDIRECTs to it — inbound silently unenforced, with no error anywhere. +func TestTransparentInbound_PreexistingProxyInitFallsBack(t *testing.T) { + m := newTestMutator(inboundCM(InboundInterceptionTransparent, "")) + podSpec := agentPodSpec() + // A proxy-init the operator did not build: right name, no inbound capture. + podSpec.InitContainers = append(podSpec.InitContainers, corev1.Container{ + Name: ProxyInitContainerName, + Image: "someone-elses/proxy-init:hand-written", + Env: []corev1.EnvVar{{Name: "MODE", Value: "enforce-redirect"}}, + }) + + if _, err := m.InjectAuthBridge(context.Background(), podSpec, "test-ns", "my-agent", + "Deployment", map[string]string{RossoctlTypeLabel: RossoctlTypeAgent}, nil); err != nil { + t.Fatalf("InjectAuthBridge() error: %v", err) + } + + // Must NOT advertise a transparent listener that nothing feeds. + proxy := containerByName(podSpec, AuthBridgeProxyContainerName) + if proxy == nil { + t.Fatal("authbridge-proxy not injected") + } + for _, p := range proxy.Ports { + if p.Name == "transparent-in" { + t.Error("sidecar declares transparent-in despite no inbound capture being installed") + } + } + cm := fetchConfigMap(t, m, "test-ns", perAgentConfigMapName("my-agent")) + listener := parseConfigYAML(t, cm)["listener"].(map[string]interface{}) + if _, present := listener["transparent_inbound_addr"]; present { + t.Error("ConfigMap still configures a transparent inbound listener nothing redirects to") + } + if listener["reverse_proxy_backend"] == nil { + t.Error("expected the reverse-proxy fallback to be fully configured") + } +} + +// TestTransparentInbound_PreexistingArmedProxyInitStaysTransparent guards the +// guard: an existing proxy-init that DOES carry inbound capture is consistent, so +// the fallback must not fire. +func TestTransparentInbound_PreexistingArmedProxyInitStaysTransparent(t *testing.T) { + m := newTestMutator(inboundCM(InboundInterceptionTransparent, "")) + podSpec := agentPodSpec() + podSpec.InitContainers = append(podSpec.InitContainers, corev1.Container{ + Name: ProxyInitContainerName, + Image: "ghcr.io/rossoctl/cortex/proxy-init:latest", + Env: []corev1.EnvVar{ + {Name: "MODE", Value: "enforce-redirect"}, + {Name: "INBOUND_TRANSPARENT_PORT", Value: "8083"}, + }, + }) + + if _, err := m.InjectAuthBridge(context.Background(), podSpec, "test-ns", "my-agent", + "Deployment", map[string]string{RossoctlTypeLabel: RossoctlTypeAgent}, nil); err != nil { + t.Fatalf("InjectAuthBridge() error: %v", err) + } + + cm := fetchConfigMap(t, m, "test-ns", perAgentConfigMapName("my-agent")) + listener := parseConfigYAML(t, cm)["listener"].(map[string]interface{}) + if listener["transparent_inbound_addr"] != ":8083" { + t.Errorf("transparent_inbound_addr = %v; an already-armed proxy-init is consistent and must stay transparent", listener["transparent_inbound_addr"]) + } +}