From 69fe28527a12b8045b8ac9e6d8ef5d54f477d6ab Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 18 Aug 2026 10:45:59 -0400 Subject: [PATCH 1/6] feat(authbridge): Add transparent inbound listener for proxy-sidecar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an opt-in inbound interception path that captures iptables-REDIRECTed connections, recovers the port the client actually addressed via SO_ORIGINAL_DST, and forwards there over loopback — replacing the single-fixed-backend assumption of the reverse proxy. This is the inbound half of rossoctl/cortex#330. The outbound half shipped in bb15d20 as the enforce-redirect egress guard; inbound had no equivalent, so JWT validation could be sidestepped pod-to-pod by dialing the agent's port directly (the operator relocates the agent to originalPort+1 and declares it in the pod spec). Selected by listener.inbound_interception: reverse-proxy (default) or transparent. Interception is two independent axes, so the inbound mechanism is a field on the reverse role rather than a role of its own — matching the outbound transparent listener, which likewise rides inside the forward role. Default is reverse-proxy, so existing deployments are byte-identical. Inbound cannot mirror the outbound shape. The egress path gates on destination host and blind-tunnels; inbound jwt-validation reads Authorization and Path and rewrites Authorization to a placeholder before forwarding, so a real HTTP server over the connection is required. Hence a net.Listener (transparentproxy.InboundListener) feeding the existing reverse-proxy handler, rather than a ConnHandler dispatcher. Notable details: - The recovered destination reaches the handler via http.Server.ConnContext. OrigDstFromConn walks the wrapper chain, because under mTLS the transparent conn sits two layers down (tlssniff peeks the first byte, then tls.Server). tlssniff's peeked conn gains NetConn() to make that walk possible, mirroring (*tls.Conn).NetConn. - Forwarding targets loopback, not the recovered IP: the egress guard RETURNs loopback (-o lo, -d 127.0.0.0/8) so the hop cannot be re-captured by our own outbound rules. The client's real IP survives via X-Forwarded-For. - Fails closed. A request with no recovered destination is rejected with 502 rather than forwarded to a guessed target, and the parked fixed backend is deliberately undialable. - The self-loop guard is extracted to CheckDst and shared with the outbound dispatcher. Its old comment assumed an external destination; for captured ingress the destination is the pod's own IP, which is now the documented normal case. Requires proxy-init to install the PREROUTING chain (follow-up) and the operator to stop stealing the agent's port when transparent is selected (follow-up). Until both land, selecting transparent binds a port nothing redirects to. Refs: rossoctl/cortex#330 Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/authlib/config/config.go | 45 ++++ .../config/inbound_interception_test.go | 140 +++++++++++ authbridge/authlib/config/presets.go | 11 +- authbridge/authlib/config/validate.go | 21 +- .../listener/internal/tlssniff/listener.go | 12 + .../authlib/listener/reverseproxy/server.go | 87 ++++++- .../reverseproxy/transparent_inbound_test.go | 219 ++++++++++++++++++ .../listener/transparentproxy/inbound.go | 146 ++++++++++++ .../listener/transparentproxy/inbound_test.go | 185 +++++++++++++++ .../listener/transparentproxy/server.go | 86 ++++--- authbridge/authlib/runtimeutil/runtimeutil.go | 46 ++++ authbridge/cmd/authbridge-proxy/main.go | 42 +++- 12 files changed, 998 insertions(+), 42 deletions(-) create mode 100644 authbridge/authlib/config/inbound_interception_test.go create mode 100644 authbridge/authlib/listener/reverseproxy/transparent_inbound_test.go create mode 100644 authbridge/authlib/listener/transparentproxy/inbound.go create mode 100644 authbridge/authlib/listener/transparentproxy/inbound_test.go diff --git a/authbridge/authlib/config/config.go b/authbridge/authlib/config/config.go index 10816a42d..aacbecdf5 100644 --- a/authbridge/authlib/config/config.go +++ b/authbridge/authlib/config/config.go @@ -383,6 +383,34 @@ type ListenerConfig struct { // the reverse role is active. Ignored outside proxy-sidecar mode. Roles []string `yaml:"roles" json:"roles"` + // InboundInterception selects HOW inbound traffic reaches the reverse + // proxy's pipeline, in proxy-sidecar mode with the reverse role active: + // + // reverse-proxy (default) — the operator binds AuthBridge on the agent's + // original port and relocates the agent to a free one + // ("port stealing"), so the Service needs no patching. + // Forwards to the single reverse_proxy_backend URL. + // transparent — proxy-init PREROUTING-REDIRECTs inbound TCP to + // transparent_inbound_addr; the listener recovers the port + // the client actually addressed via SO_ORIGINAL_DST and + // forwards there over loopback. The agent keeps its own + // port, so no PORT env var is imposed on it and every port + // it listens on is covered — not just the first declared + // one. Requires proxy-init (NET_ADMIN) and is Linux-only. + // + // Defaults to reverse-proxy: transparent adds a privileged init container, + // so it is opt-in, and leaving it unset keeps existing deployments + // byte-identical. + InboundInterception string `yaml:"inbound_interception" json:"inbound_interception"` + + // TransparentInboundAddr is the bind address for the inbound transparent + // listener (inbound_interception: transparent). The proxy-sidecar preset + // defaults it to ":8083" when that mode is selected — 8080 (reverse), 8081 + // (forward) and 8082 (transparent egress) are already taken. It MUST match + // proxy-init's INBOUND_TRANSPARENT_PORT, or PREROUTING will REDIRECT to a + // dead port and inbound traffic will break outright. + TransparentInboundAddr string `yaml:"transparent_inbound_addr" json:"transparent_inbound_addr"` + // TransparentProxyAddr is the bind address for the outbound transparent // listener used by proxy-sidecar enforce-redirect mode: iptables REDIRECTs // the agent's bypass egress here, and the listener recovers the original @@ -469,6 +497,23 @@ const ( RoleForward = "forward" // outbound forward proxy ) +// Valid ListenerConfig.InboundInterception values. Interception is two +// independent axes (inbound, outbound), so the inbound mechanism is a field on +// the reverse role rather than a role of its own — matching the outbound +// transparent listener, which likewise rides inside the forward role instead of +// being separately selectable. +const ( + InboundInterceptionReverseProxy = "reverse-proxy" // fixed backend (default) + InboundInterceptionTransparent = "transparent" // SO_ORIGINAL_DST per connection +) + +// InboundTransparent reports whether the inbound transparent listener should +// run instead of the fixed-backend reverse proxy. Empty means the default +// (reverse-proxy), so callers need no separate zero-value check. +func (l ListenerConfig) InboundTransparent() bool { + return l.InboundInterception == InboundInterceptionTransparent +} + // ActiveRoles returns the set of proxy roles to run in proxy-sidecar mode. An // empty Roles list defaults to BOTH roles (the full pod deployment), so // existing configs and the operator path are unchanged; a non-empty list runs diff --git a/authbridge/authlib/config/inbound_interception_test.go b/authbridge/authlib/config/inbound_interception_test.go new file mode 100644 index 000000000..0ccc4c110 --- /dev/null +++ b/authbridge/authlib/config/inbound_interception_test.go @@ -0,0 +1,140 @@ +package config + +import "testing" + +// TestApplyPreset_TransparentInbound covers the mutual exclusion between the +// two inbound mechanisms. Filling reverse_proxy_addr alongside transparent +// interception would bind a port nothing routes to — and if it collided with +// the agent's own port (which transparent mode deliberately leaves in place), +// the pod would fail to start. +func TestApplyPreset_TransparentInbound(t *testing.T) { + cfg := &Config{Mode: ModeProxySidecar, Listener: ListenerConfig{ + InboundInterception: InboundInterceptionTransparent, + }} + ApplyPreset(cfg) + + if cfg.Listener.TransparentInboundAddr != ":8083" { + t.Errorf("transparent_inbound_addr = %q, want :8083", cfg.Listener.TransparentInboundAddr) + } + if cfg.Listener.ReverseProxyAddr != "" { + t.Errorf("transparent inbound must not fill reverse_proxy_addr, got %q", cfg.Listener.ReverseProxyAddr) + } + // The forward role is still active by default, so egress is untouched. + if cfg.Listener.ForwardProxyAddr != ":8081" || cfg.Listener.TransparentProxyAddr != ":8082" { + t.Errorf("egress defaults changed: forward=%q transparent-out=%q", + cfg.Listener.ForwardProxyAddr, cfg.Listener.TransparentProxyAddr) + } +} + +// TestApplyPreset_DefaultIsReverseProxy locks the opt-in contract: an unset +// inbound_interception must leave the preset byte-identical to today. +func TestApplyPreset_DefaultIsReverseProxy(t *testing.T) { + cfg := &Config{Mode: ModeProxySidecar} + ApplyPreset(cfg) + + if cfg.Listener.ReverseProxyAddr != ":8080" { + t.Errorf("reverse_proxy_addr = %q, want :8080 (default mechanism unchanged)", cfg.Listener.ReverseProxyAddr) + } + if cfg.Listener.TransparentInboundAddr != "" { + t.Errorf("default must not fill transparent_inbound_addr, got %q", cfg.Listener.TransparentInboundAddr) + } +} + +// TestApplyPreset_TransparentInboundUserOverride ensures an operator-chosen port +// survives the preset — it has to match proxy-init's INBOUND_TRANSPARENT_PORT, +// so silently overwriting it would break ingress. +func TestApplyPreset_TransparentInboundUserOverride(t *testing.T) { + cfg := &Config{Mode: ModeProxySidecar, Listener: ListenerConfig{ + InboundInterception: InboundInterceptionTransparent, + TransparentInboundAddr: ":19083", + }} + ApplyPreset(cfg) + if cfg.Listener.TransparentInboundAddr != ":19083" { + t.Errorf("transparent_inbound_addr = %q, want the operator's :19083", cfg.Listener.TransparentInboundAddr) + } +} + +func TestInboundTransparent(t *testing.T) { + for _, tc := range []struct { + value string + want bool + }{ + {"", false}, + {InboundInterceptionReverseProxy, false}, + {InboundInterceptionTransparent, true}, + } { + if got := (ListenerConfig{InboundInterception: tc.value}).InboundTransparent(); got != tc.want { + t.Errorf("InboundTransparent(%q) = %v, want %v", tc.value, got, tc.want) + } + } +} + +func TestValidate_InboundInterception(t *testing.T) { + tests := []struct { + name string + cfg *Config + wantErr bool + }{ + { + name: "transparent needs no reverse_proxy_backend", + cfg: &Config{Mode: ModeProxySidecar, Listener: ListenerConfig{ + InboundInterception: InboundInterceptionTransparent, + }}, + }, + { + name: "reverse-proxy still requires a backend", + cfg: &Config{Mode: ModeProxySidecar, Listener: ListenerConfig{ + InboundInterception: InboundInterceptionReverseProxy, + }}, + wantErr: true, + }, + { + name: "default still requires a backend", + cfg: &Config{Mode: ModeProxySidecar}, + // unchanged behavior for existing configs + wantErr: true, + }, + { + name: "unknown value is rejected at startup", + cfg: &Config{Mode: ModeProxySidecar, Listener: ListenerConfig{ + InboundInterception: "transparant", // typo an operator would make + ReverseProxyBackend: "http://127.0.0.1:8000", + }}, + wantErr: true, + }, + { + name: "transparent without the reverse role is a no-op, so rejected", + cfg: &Config{Mode: ModeProxySidecar, Listener: ListenerConfig{ + Roles: []string{RoleForward}, + InboundInterception: InboundInterceptionTransparent, + }}, + wantErr: true, + }, + { + name: "envoy-sidecar rejects the field (Envoy already intercepts inbound)", + cfg: &Config{Mode: ModeEnvoySidecar, Listener: ListenerConfig{ + InboundInterception: InboundInterceptionTransparent, + }}, + wantErr: true, + }, + { + name: "waypoint rejects the field", + cfg: &Config{Mode: ModeWaypoint, Listener: ListenerConfig{ + InboundInterception: InboundInterceptionTransparent, + }}, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := Validate(tc.cfg) + 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) + } + }) + } +} diff --git a/authbridge/authlib/config/presets.go b/authbridge/authlib/config/presets.go index 30e015355..b9f117b3c 100644 --- a/authbridge/authlib/config/presets.go +++ b/authbridge/authlib/config/presets.go @@ -19,7 +19,16 @@ func ApplyPreset(cfg *Config) { // it didn't ask for. main.go starts a proxy iff its role is active. roles := cfg.Listener.ActiveRoles() if roles[RoleReverse] { - setDefault(&cfg.Listener.ReverseProxyAddr, ":8080") + // The two inbound mechanisms are mutually exclusive: transparent + // interception REDIRECTs to its own port and leaves the agent on the + // port it already binds, so filling reverse_proxy_addr there would + // bind a port nothing routes to (and, if it collided with the agent's + // own port, would break the pod). + if cfg.Listener.InboundTransparent() { + setDefault(&cfg.Listener.TransparentInboundAddr, ":8083") + } else { + setDefault(&cfg.Listener.ReverseProxyAddr, ":8080") + } } if roles[RoleForward] { setDefault(&cfg.Listener.ForwardProxyAddr, ":8081") diff --git a/authbridge/authlib/config/validate.go b/authbridge/authlib/config/validate.go index 0521e9471..9448be25f 100644 --- a/authbridge/authlib/config/validate.go +++ b/authbridge/authlib/config/validate.go @@ -32,6 +32,9 @@ func validateListeners(cfg *Config) error { if cfg.Listener.ReverseProxyAddr != "" { return fmt.Errorf("envoy-sidecar mode does not support reverse_proxy_addr (use proxy-sidecar mode)") } + if cfg.Listener.InboundInterception != "" { + return fmt.Errorf("envoy-sidecar mode does not support inbound_interception (Envoy already intercepts inbound transparently)") + } if cfg.Listener.ExtAuthzAddr != "" { return fmt.Errorf("envoy-sidecar mode does not support ext_authz_addr (use waypoint mode)") } @@ -39,6 +42,9 @@ func validateListeners(cfg *Config) error { if cfg.Listener.ExtProcAddr != "" { return fmt.Errorf("waypoint mode does not support ext_proc_addr (use envoy-sidecar mode)") } + if cfg.Listener.InboundInterception != "" { + return fmt.Errorf("waypoint mode does not support inbound_interception (the waypoint owns inbound)") + } if cfg.Listener.ReverseProxyAddr != "" { return fmt.Errorf("waypoint mode does not support reverse_proxy_addr") } @@ -54,11 +60,22 @@ func validateListeners(cfg *Config) error { return fmt.Errorf("listener.roles: %q is not a valid role (use %q and/or %q)", r, RoleReverse, RoleForward) } } + switch cfg.Listener.InboundInterception { + case "", InboundInterceptionReverseProxy, InboundInterceptionTransparent: + // valid + default: + return fmt.Errorf("listener.inbound_interception: %q is not valid (use %q or %q)", + cfg.Listener.InboundInterception, InboundInterceptionReverseProxy, InboundInterceptionTransparent) + } roles := cfg.Listener.ActiveRoles() + if cfg.Listener.InboundTransparent() && !roles[RoleReverse] { + return fmt.Errorf("listener.inbound_interception: transparent requires the %q role (it selects how inbound reaches the inbound pipeline)", RoleReverse) + } // The reverse proxy forwards inbound traffic to reverse_proxy_backend, // so it's required only when the reverse role is active. A forward-only - // deployment needs no backend. - if roles[RoleReverse] && cfg.Listener.ReverseProxyBackend == "" { + // deployment needs no backend, and transparent interception derives the + // backend per connection from SO_ORIGINAL_DST rather than from config. + if roles[RoleReverse] && !cfg.Listener.InboundTransparent() && cfg.Listener.ReverseProxyBackend == "" { return fmt.Errorf("proxy-sidecar mode with the reverse role requires listener.reverse_proxy_backend") } // The TLS bridge only rewrites outbound (forward-proxy) traffic; enabling diff --git a/authbridge/authlib/listener/internal/tlssniff/listener.go b/authbridge/authlib/listener/internal/tlssniff/listener.go index 700fb09bc..c03b717fc 100644 --- a/authbridge/authlib/listener/internal/tlssniff/listener.go +++ b/authbridge/authlib/listener/internal/tlssniff/listener.go @@ -165,6 +165,18 @@ type peekedConn struct { // socket reads via net.Conn embedding. func (c *peekedConn) Read(p []byte) (int, error) { return c.br.Read(p) } +// NetConn returns the connection this wrapper peeked, so a caller that +// needs something only a lower layer knows can unwrap through the +// sniffer. The transparent inbound listener uses it to reach the +// original destination it recovered via SO_ORIGINAL_DST, which would +// otherwise be hidden behind this wrapper (and behind *tls.Conn on the +// TLS path). Deliberately mirrors (*tls.Conn).NetConn so both layers +// satisfy one structural interface. +// +// The returned conn must not be read from directly — buffered bytes +// live in this wrapper's bufio.Reader. +func (c *peekedConn) NetConn() net.Conn { return c.Conn } + // ErrUnexpected is returned when the listener encounters a state it // can't recover from (e.g. SetReadDeadline failed). Callers don't // match against it — it surfaces only via Accept's error return. diff --git a/authbridge/authlib/listener/reverseproxy/server.go b/authbridge/authlib/listener/reverseproxy/server.go index a3097f2f2..1e56d6bfc 100644 --- a/authbridge/authlib/listener/reverseproxy/server.go +++ b/authbridge/authlib/listener/reverseproxy/server.go @@ -20,6 +20,7 @@ import ( "github.com/rossoctl/cortex/authbridge/authlib/listener/httpx" "github.com/rossoctl/cortex/authbridge/authlib/listener/internal/sseframe" "github.com/rossoctl/cortex/authbridge/authlib/listener/internal/tlssniff" + "github.com/rossoctl/cortex/authbridge/authlib/listener/transparentproxy" "github.com/rossoctl/cortex/authbridge/authlib/pipeline" "github.com/rossoctl/cortex/authbridge/authlib/session" "github.com/rossoctl/cortex/authbridge/authlib/spiffe" @@ -57,6 +58,14 @@ type Server struct { proxy *httputil.ReverseProxy backend string + // perConnBackend makes the forwarding target per-connection rather than + // fixed: the Director resolves it from the original destination the + // transparent inbound listener recovered via SO_ORIGINAL_DST. Set by + // NewTransparentServer. When true, backend may be empty and a request + // arriving without a recovered destination fails closed (502) rather than + // being forwarded to a guessed target. + perConnBackend bool + // mtlsCfg is the *tls.Config wrapping the local SVID for inbound // mTLS, or nil when mTLS is disabled. mtlsMode is consulted by // the byte-peek listener (Listen) to decide whether non-TLS @@ -103,6 +112,24 @@ func NewServer(inbound *pipeline.Holder, sessions *session.Store, backendURL str proxy.Director = func(req *http.Request) { orig(req) req.Host = target.Host + // Transparent inbound: the client addressed the app's real port, and + // iptables REDIRECTed it here. Forward to that port on loopback instead + // of to the single configured backend. + // + // Loopback specifically, and not the recovered IP: the enforce-redirect + // egress guard RETURNs loopback (-o lo, -d 127.0.0.0/8) so this hop + // cannot be re-captured by our own outbound rules. It also keeps the + // second hop out of Istio ambient's OUTPUT-chain handling entirely. + // + // The client's real IP is preserved by REDIRECT and reaches the app via + // X-Forwarded-For, which ReverseProxy appends from req.RemoteAddr. + if dst, ok := transparentproxy.OrigDstFromContext(req.Context()); ok { + if _, port, err := net.SplitHostPort(dst); err == nil { + req.URL.Scheme = "http" + req.URL.Host = net.JoinHostPort("127.0.0.1", port) + req.Host = req.URL.Host + } + } // Strip the client's Accept-Encoding, but only when a plugin will // actually inspect the response body: a StreamingResponder (SSE // re-framing) or any ReadsBody/WritesBody plugin (buffered read into @@ -158,6 +185,38 @@ func NewServer(inbound *pipeline.Holder, sessions *session.Store, backendURL str return s, nil } +// NewTransparentServer creates a reverse proxy whose forwarding target is +// resolved per connection from the original destination recovered by +// transparentproxy.InboundListener, rather than from a fixed backend URL. +// +// fallbackBackend may be empty. When it is, a request that arrives with no +// recovered destination is rejected with 502 instead of being forwarded +// somewhere unintended — the transparent listener exists to be a hard inbound +// boundary, so an unattributable request must not be forwarded at all. +func NewTransparentServer(inbound *pipeline.Holder, sessions *session.Store, fallbackBackend string, mtls *MTLSOptions) (*Server, error) { + backend := fallbackBackend + if backend == "" { + // url.Parse("") yields a URL with an empty Host, which would make the + // default Director emit a request with no target. Park the fixed target + // on a sentinel that can never be dialed by accident; the Director + // overrides it whenever a destination was recovered, and handleRequest + // fails closed when one wasn't. + backend = "http://" + unresolvableBackend + } + s, err := NewServer(inbound, sessions, backend, mtls) + if err != nil { + return nil, err + } + s.perConnBackend = fallbackBackend == "" + return s, nil +} + +// unresolvableBackend is the parked fixed target for a transparent server with +// no configured fallback. 127.0.0.1:0 is not dialable (port 0 is "pick one" for +// bind, not connect), so a bug that let a request through without a recovered +// destination fails loudly instead of reaching a real service. +const unresolvableBackend = "127.0.0.1:0" + // Listen returns a net.Listener bound to addr. When mTLS is configured // the listener is a tlssniff.Listener that dispatches TLS handshakes // through the local SVID and pass-throughs plain HTTP per the @@ -170,8 +229,17 @@ func (s *Server) Listen(addr string) (net.Listener, error) { if err != nil { return nil, err } + return s.WrapListener(inner), nil +} + +// WrapListener applies this server's inbound mTLS posture to an already-bound +// listener, returning it unchanged when mTLS is disabled. Split out of Listen so +// the transparent inbound path — which must bind a *net.TCPListener itself, to +// recover SO_ORIGINAL_DST before any bytes are read — gets the identical +// permissive/strict behavior instead of a second implementation of it. +func (s *Server) WrapListener(inner net.Listener) net.Listener { if s.mtlsCfg == nil { - return inner, nil + return inner } sniff := tlssniff.New(inner, s.mtlsCfg, s.mtlsMode) if s.mtlsMetrics != nil { @@ -179,7 +247,7 @@ func (s *Server) Listen(addr string) (net.Listener, error) { s.mtlsMetrics.InboundPlainRejected.Add(1) }) } - return sniff, nil + return sniff } // MTLSEnabled reports whether the listener is wrapping connections @@ -204,6 +272,21 @@ func (s *Server) Handler() http.Handler { } func (s *Server) handleRequest(w http.ResponseWriter, r *http.Request) { + // Fail closed on an unattributable request. In per-connection-backend mode + // the forwarding target comes from SO_ORIGINAL_DST; if the listener could + // not recover one there is no correct target, and the parked fixed backend + // is deliberately undialable. Reject here rather than let it reach the + // pipeline — a request we can't attribute to a captured destination is one + // this listener has no business forwarding. + if s.perConnBackend { + if _, ok := transparentproxy.OrigDstFromContext(r.Context()); !ok { + slog.Warn("reverse-proxy: rejecting request with no recovered destination", + "remote", r.RemoteAddr, "host", r.Host, "path", r.URL.Path) + http.Error(w, "no original destination recovered", http.StatusBadGateway) + return + } + } + pctx := &pipeline.Context{ Direction: pipeline.Inbound, Method: r.Method, diff --git a/authbridge/authlib/listener/reverseproxy/transparent_inbound_test.go b/authbridge/authlib/listener/reverseproxy/transparent_inbound_test.go new file mode 100644 index 000000000..95ae2ce11 --- /dev/null +++ b/authbridge/authlib/listener/reverseproxy/transparent_inbound_test.go @@ -0,0 +1,219 @@ +package reverseproxy + +import ( + "fmt" + "net" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/rossoctl/cortex/authbridge/authlib/auth" + "github.com/rossoctl/cortex/authbridge/authlib/listener/transparentproxy" + "github.com/rossoctl/cortex/authbridge/authlib/plugins/jwtvalidation/validation" +) + +// withOrigDst stands in for http.Server.ConnContext + the transparent inbound +// listener: it injects a recovered original destination into the request +// context, which is the only channel the Director reads it from. +func withOrigDst(dst string, h http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + h.ServeHTTP(w, r.WithContext(transparentproxy.ContextWithOrigDst(r.Context(), dst))) + }) +} + +func allowAllAuth() *auth.Auth { + return auth.New(auth.Config{ + Verifier: &mockVerifier{claims: &validation.Claims{Subject: "user"}}, + Identity: auth.IdentityConfig{Audiences: []string{"my-app"}}, + }) +} + +// portOfURL extracts the port an httptest server bound, so a test can build a +// realistic "podIP:appPort" destination whose PORT resolves to that server. +func portOfURL(t *testing.T, raw string) string { + t.Helper() + u, err := url.Parse(raw) + if err != nil { + t.Fatalf("parsing %q: %v", raw, err) + } + _, port, err := net.SplitHostPort(u.Host) + if err != nil { + t.Fatalf("splitting %q: %v", u.Host, err) + } + return port +} + +// TestTransparentInbound_ForwardsToRecoveredPort is the core behavior: the +// forwarding target comes from the destination the client addressed, not from +// config. The recovered destination names a pod IP that does not exist in the +// test environment — proving the Director rewrote the host to loopback while +// keeping the PORT, which is exactly the on-cluster behavior (the egress guard +// RETURNs loopback, so the hop can't be re-captured). +func TestTransparentInbound_ForwardsToRecoveredPort(t *testing.T) { + var gotHost, gotXFF string + app := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotHost = r.Host + gotXFF = r.Header.Get("X-Forwarded-For") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("app-ok")) + })) + defer app.Close() + + srv, err := NewTransparentServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, "", nil) + if err != nil { + t.Fatal(err) + } + + // 10.244.0.5 is a plausible pod IP and is NOT where the app listens; only + // the port is carried over. + dst := net.JoinHostPort("10.244.0.5", portOfURL(t, app.URL)) + proxy := httptest.NewServer(withOrigDst(dst, srv.Handler())) + defer proxy.Close() + + req, _ := http.NewRequest("GET", proxy.URL+"/api/data", nil) + req.Header.Set("Authorization", "Bearer valid-token") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want 200 (Director should have rewritten to loopback:%s)", + resp.StatusCode, portOfURL(t, app.URL)) + } + wantHost := net.JoinHostPort("127.0.0.1", portOfURL(t, app.URL)) + if gotHost != wantHost { + t.Errorf("app saw Host = %q, want %q (loopback, recovered port)", gotHost, wantHost) + } + // REDIRECT preserves the client's source IP, so the app must still be able + // to see it after the loopback hop. + if gotXFF == "" { + t.Error("X-Forwarded-For must reach the app so the real client IP survives the loopback hop") + } +} + +// TestTransparentInbound_NoDestinationFailsClosed locks the fail-closed +// contract: a request the listener cannot attribute to a captured destination +// must be rejected, not forwarded to a guessed target. Without this, the parked +// sentinel backend (or worse, a real one) would receive unvalidated traffic. +func TestTransparentInbound_NoDestinationFailsClosed(t *testing.T) { + app := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + t.Error("app must not be reached when no destination was recovered") + w.WriteHeader(http.StatusOK) + })) + defer app.Close() + + srv, err := NewTransparentServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, "", nil) + if err != nil { + t.Fatal(err) + } + + // No withOrigDst wrapper: the context carries nothing. + proxy := httptest.NewServer(srv.Handler()) + defer proxy.Close() + + req, _ := http.NewRequest("GET", proxy.URL+"/api/data", nil) + req.Header.Set("Authorization", "Bearer valid-token") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusBadGateway { + t.Fatalf("status = %d, want 502 (fail closed with no recovered destination)", resp.StatusCode) + } +} + +// TestTransparentInbound_StillValidatesJWT guards against the per-connection +// backend path accidentally bypassing the inbound pipeline — the entire reason +// this listener exists is that validation cannot be sidestepped. +func TestTransparentInbound_StillValidatesJWT(t *testing.T) { + app := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + t.Error("app must not be reached by an unauthenticated request") + w.WriteHeader(http.StatusOK) + })) + defer app.Close() + + a := auth.New(auth.Config{ + Verifier: &mockVerifier{err: fmt.Errorf("invalid token")}, + Identity: auth.IdentityConfig{Audiences: []string{"my-app"}}, + }) + srv, err := NewTransparentServer(inboundPipelineFromAuth(t, a), nil, "", nil) + if err != nil { + t.Fatal(err) + } + + dst := net.JoinHostPort("10.244.0.5", portOfURL(t, app.URL)) + proxy := httptest.NewServer(withOrigDst(dst, srv.Handler())) + defer proxy.Close() + + req, _ := http.NewRequest("GET", proxy.URL+"/api/data", nil) + req.Header.Set("Authorization", "Bearer bogus-token") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusOK { + t.Fatalf("status = 200, want a denial — transparent inbound must still validate") + } +} + +// TestTransparentInbound_FallbackBackendKeepsFixedTarget covers the non-empty +// fallback: a deployment that wants transparent capture but also a defined +// target for anything arriving without a recovered destination should forward +// rather than 502. +func TestTransparentInbound_FallbackBackendKeepsFixedTarget(t *testing.T) { + app := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("fallback-ok")) + })) + defer app.Close() + + srv, err := NewTransparentServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, app.URL, nil) + if err != nil { + t.Fatal(err) + } + if srv.perConnBackend { + t.Fatal("a configured fallback must not enable the fail-closed path") + } + + proxy := httptest.NewServer(srv.Handler()) + defer proxy.Close() + + req, _ := http.NewRequest("GET", proxy.URL+"/api/data", nil) + req.Header.Set("Authorization", "Bearer valid-token") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want 200 via the configured fallback", resp.StatusCode) + } +} + +// TestWrapListener_NoMTLSIsPassthrough documents the split-out wrap: with mTLS +// off it must return the listener untouched, so the transparent path's +// *net.TCPListener (needed for SO_ORIGINAL_DST) is not replaced by a wrapper +// that would hide it from the unwrap walk. +func TestWrapListener_NoMTLSIsPassthrough(t *testing.T) { + srv, err := NewTransparentServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, "", nil) + if err != nil { + t.Fatal(err) + } + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer func() { _ = ln.Close() }() + + if got := srv.WrapListener(ln); got != ln { + t.Errorf("WrapListener with mTLS off = %T, want the same listener back", got) + } +} diff --git a/authbridge/authlib/listener/transparentproxy/inbound.go b/authbridge/authlib/listener/transparentproxy/inbound.go new file mode 100644 index 000000000..e89832acd --- /dev/null +++ b/authbridge/authlib/listener/transparentproxy/inbound.go @@ -0,0 +1,146 @@ +package transparentproxy + +import ( + "context" + "log/slog" + "net" +) + +// maxUnwrapDepth bounds the wrapper walk in OrigDstFromConn. The real chain is +// at most two deep (tls.Conn -> peekedConn -> Conn), so this only exists so a +// pathological or cyclic wrapper can't spin the loop forever. +const maxUnwrapDepth = 8 + +// Conn is a connection accepted by an InboundListener, carrying the original +// destination recovered from the kernel before any protocol bytes were read. +// It embeds *net.TCPConn so keepalive and syscall access still work upstack. +type Conn struct { + *net.TCPConn + dst string +} + +// OrigDst returns the pre-REDIRECT destination as "host:port" — for captured +// ingress, this pod's own IP on the port the client actually addressed. +func (c *Conn) OrigDst() string { return c.dst } + +// InboundListener accepts iptables-PREROUTING-REDIRECTed connections and +// recovers each one's original destination via SO_ORIGINAL_DST, so an HTTP +// server upstack can forward to the port the client actually addressed rather +// than to a single configured backend. +// +// It is a net.Listener rather than a ConnHandler dispatcher (the shape the +// outbound path uses) because inbound cannot blind-tunnel: JWT validation reads +// the Authorization header and Path, and rewrites Authorization to a +// placeholder before forwarding. That requires a real HTTP server over the +// connection, so the natural seam is http.Server.Serve — which wants a +// net.Listener. +type InboundListener struct { + inner *net.TCPListener +} + +// NewInboundListener wraps ln so each accepted connection carries its recovered +// original destination. Pair it with ConnContextHook so the destination reaches +// the HTTP handler. +func NewInboundListener(ln *net.TCPListener) *InboundListener { + return &InboundListener{inner: ln} +} + +// Accept returns the next capture whose original destination was recovered and +// passed CheckDst. +// +// Connections that fail either step are logged, closed, and skipped rather than +// surfaced as an Accept error: an error return would terminate +// http.Server.Serve and take the whole inbound listener down, turning one bad +// connection into an outage. This mirrors tlssniff.Listener.Accept's handling of +// strict-mode rejections, and Server.dispatch's per-connection drop. +func (l *InboundListener) Accept() (net.Conn, error) { + for { + conn, err := l.inner.AcceptTCP() + if err != nil { + return nil, err + } + dst, err := originalDst(conn) + if err != nil { + // No recoverable original destination means this connection did not + // arrive via the REDIRECT (e.g. a direct dial to the listener port). + // Drop it rather than guess — we will not forward to a destination we + // cannot attribute to the kernel's conntrack record. + slog.Warn("transparent-inbound: dropping connection with no original destination", + "remote", conn.RemoteAddr().String(), "error", err) + _ = conn.Close() + continue + } + if err := CheckDst(conn.LocalAddr(), dst); err != nil { + slog.Warn("transparent-inbound: dropping self-referential connection", + "remote", conn.RemoteAddr().String(), "dst", dst, + "local", conn.LocalAddr().String(), "error", err) + _ = conn.Close() + continue + } + slog.Debug("transparent-inbound: captured connection", + "remote", conn.RemoteAddr().String(), "dst", dst) + return &Conn{TCPConn: conn, dst: dst}, nil + } +} + +// Close shuts down the underlying listener. +func (l *InboundListener) Close() error { return l.inner.Close() } + +// Addr returns the listener's bind address. +func (l *InboundListener) Addr() net.Addr { return l.inner.Addr() } + +type origDstKey struct{} + +// ContextWithOrigDst returns ctx carrying dst. +func ContextWithOrigDst(ctx context.Context, dst string) context.Context { + return context.WithValue(ctx, origDstKey{}, dst) +} + +// OrigDstFromContext returns the original destination stashed by +// ConnContextHook, if any. Handlers use it to pick a per-request backend. +func OrigDstFromContext(ctx context.Context) (string, bool) { + dst, ok := ctx.Value(origDstKey{}).(string) + return dst, ok && dst != "" +} + +// netConner is satisfied by connection wrappers that expose their underlying +// connection: *tls.Conn (stdlib) and tlssniff's peeked conn. Declared +// structurally so this package needs no import of either. +type netConner interface{ NetConn() net.Conn } + +// OrigDstFromConn walks a connection's wrapper chain looking for a *Conn and +// returns its recovered original destination. +// +// The walk is necessary because the mTLS path wraps our conn twice — tlssniff +// peeks the first byte (returning a buffered wrapper) and then hands TLS +// handshakes to tls.Server — so by the time http.Server sees the connection, +// the *Conn is two layers down. +func OrigDstFromConn(c net.Conn) (string, bool) { + for i := 0; i < maxUnwrapDepth && c != nil; i++ { + if tc, ok := c.(*Conn); ok { + return tc.dst, true + } + u, ok := c.(netConner) + if !ok { + return "", false + } + c = u.NetConn() + } + return "", false +} + +// ConnContextHook is an http.Server.ConnContext function that stashes each +// connection's recovered original destination into the request context. Wire it +// into the http.Server that serves an InboundListener: +// +// srv := &http.Server{Handler: h, ConnContext: transparentproxy.ConnContextHook} +// +// Connections with no recoverable destination pass through unchanged; the +// handler decides how to treat a missing destination (the reverse proxy fails +// closed with 502 when it has no configured fallback backend). +func ConnContextHook(ctx context.Context, c net.Conn) context.Context { + if dst, ok := OrigDstFromConn(c); ok { + return ContextWithOrigDst(ctx, dst) + } + return ctx +} diff --git a/authbridge/authlib/listener/transparentproxy/inbound_test.go b/authbridge/authlib/listener/transparentproxy/inbound_test.go new file mode 100644 index 000000000..967b3ceee --- /dev/null +++ b/authbridge/authlib/listener/transparentproxy/inbound_test.go @@ -0,0 +1,185 @@ +package transparentproxy + +import ( + "context" + "crypto/tls" + "errors" + "net" + "testing" +) + +// fakeAddr is a net.Addr with a caller-chosen String(), so CheckDst's +// "dst equals the listener's own address" arm can be exercised without +// binding a real socket. +type fakeAddr string + +func (a fakeAddr) Network() string { return "tcp" } +func (a fakeAddr) String() string { return string(a) } + +// TestCheckDst covers the guard shared by the outbound dispatcher and the +// inbound listener. The inbound cases are the ones the original outbound-only +// guard did not consider: a pod's own IP on the app's port is the NORMAL +// recovered destination for captured ingress and must be allowed through. +func TestCheckDst(t *testing.T) { + tests := []struct { + name string + local net.Addr + dst string + wantErr error + }{ + { + name: "outbound: external host is fine", + local: fakeAddr("10.244.0.5:8082"), + dst: "93.184.216.34:443", + }, + { + name: "inbound: pod's own IP on the app port is the normal case", + local: fakeAddr("10.244.0.5:8083"), + dst: "10.244.0.5:8000", + }, + { + name: "self-dial to the listener's own address", + local: fakeAddr("10.244.0.5:8083"), + dst: "10.244.0.5:8083", + wantErr: ErrSelfReferential, + }, + { + name: "loopback dst would spiral", + local: fakeAddr("10.244.0.5:8083"), + dst: "127.0.0.1:8000", + wantErr: ErrSelfReferential, + }, + { + name: "IPv6 loopback dst", + local: fakeAddr("[fd00::5]:8083"), + dst: "[::1]:8000", + wantErr: ErrSelfReferential, + }, + { + name: "nil local skips the self-address arm", + local: nil, + dst: "10.244.0.5:8000", + }, + { + name: "malformed dst is rejected, but not as self-referential", + local: fakeAddr("10.244.0.5:8083"), + dst: "not-a-host-port", + // non-nil, but deliberately NOT ErrSelfReferential + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := CheckDst(tc.local, tc.dst) + switch { + case tc.wantErr != nil: + if !errors.Is(err, tc.wantErr) { + t.Fatalf("CheckDst(%v, %q) = %v, want %v", tc.local, tc.dst, err, tc.wantErr) + } + case tc.name == "malformed dst is rejected, but not as self-referential": + if err == nil { + t.Fatal("malformed dst must be rejected") + } + if errors.Is(err, ErrSelfReferential) { + t.Error("malformed dst must not be reported as self-referential") + } + default: + if err != nil { + t.Fatalf("CheckDst(%v, %q) = %v, want nil", tc.local, tc.dst, err) + } + } + }) + } +} + +// TestOrigDstFromConn_UnwrapsThroughWrappers locks the reason OrigDstFromConn +// walks a chain instead of doing a single type assertion: under mTLS the +// transparent conn sits TWO layers down (tlssniff's peeked wrapper, then +// tls.Server), so a direct assertion in ConnContext would silently find nothing +// and every request would fail closed with 502. +func TestOrigDstFromConn_UnwrapsThroughWrappers(t *testing.T) { + base := &Conn{dst: "10.244.0.5:8000"} + + if dst, ok := OrigDstFromConn(base); !ok || dst != "10.244.0.5:8000" { + t.Fatalf("bare conn: got (%q, %v), want (10.244.0.5:8000, true)", dst, ok) + } + + // One layer: a peek-style wrapper exposing NetConn, as tlssniff's does. + one := &wrapConn{inner: base} + if dst, ok := OrigDstFromConn(one); !ok || dst != "10.244.0.5:8000" { + t.Fatalf("one wrapper: got (%q, %v), want (10.244.0.5:8000, true)", dst, ok) + } + + // Two layers: tls.Conn over the peeked wrapper. *tls.Conn.NetConn() is the + // stdlib method this walk relies on. + two := tls.Server(one, &tls.Config{}) + if dst, ok := OrigDstFromConn(two); !ok || dst != "10.244.0.5:8000" { + t.Fatalf("tls over wrapper: got (%q, %v), want (10.244.0.5:8000, true)", dst, ok) + } +} + +// TestOrigDstFromConn_NoTransparentConn ensures a plain connection reports +// absence rather than an empty-string false positive — the reverse proxy keys +// its fail-closed 502 on this. +func TestOrigDstFromConn_NoTransparentConn(t *testing.T) { + if dst, ok := OrigDstFromConn(&wrapConn{inner: &plainConn{}}); ok { + t.Fatalf("plain conn chain: got (%q, true), want absent", dst) + } +} + +// TestOrigDstFromConn_CycleTerminates guards the maxUnwrapDepth bound: a +// wrapper that returns itself must not spin forever. +func TestOrigDstFromConn_CycleTerminates(t *testing.T) { + c := &selfWrap{} + if _, ok := OrigDstFromConn(c); ok { + t.Fatal("cyclic wrapper must not report a destination") + } +} + +// TestConnContextHook covers both directions of the context round-trip, since +// http.Server calls the hook and the reverse proxy's Director reads it back. +func TestConnContextHook(t *testing.T) { + ctx := ConnContextHook(context.Background(), &Conn{dst: "10.244.0.5:9000"}) + dst, ok := OrigDstFromContext(ctx) + if !ok || dst != "10.244.0.5:9000" { + t.Fatalf("round-trip: got (%q, %v), want (10.244.0.5:9000, true)", dst, ok) + } + + // A connection with nothing to contribute must leave ctx untouched, so the + // handler sees a clean absence. + plain := ConnContextHook(context.Background(), &plainConn{}) + if _, ok := OrigDstFromContext(plain); ok { + t.Error("plain conn must not populate the context") + } +} + +// TestOrigDstFromContext_EmptyStringIsAbsent stops an empty destination from +// reading as present, which would send the Director to "127.0.0.1:" . +func TestOrigDstFromContext_EmptyStringIsAbsent(t *testing.T) { + if _, ok := OrigDstFromContext(ContextWithOrigDst(context.Background(), "")); ok { + t.Error("empty destination must report absent") + } +} + +// --- test doubles ------------------------------------------------------------ + +// plainConn is a net.Conn that knows nothing about transparent capture. +type plainConn struct{ net.Conn } + +func (c *plainConn) Close() error { return nil } +func (c *plainConn) LocalAddr() net.Addr { return fakeAddr("10.244.0.5:8083") } + +// wrapConn mimics tlssniff's peekedConn: it wraps another conn and exposes it +// via NetConn. +type wrapConn struct { + net.Conn + inner net.Conn +} + +func (c *wrapConn) NetConn() net.Conn { return c.inner } + +// selfWrap returns itself from NetConn, the pathological case maxUnwrapDepth +// exists for. +type selfWrap struct{ net.Conn } + +func (c *selfWrap) NetConn() net.Conn { return c } diff --git a/authbridge/authlib/listener/transparentproxy/server.go b/authbridge/authlib/listener/transparentproxy/server.go index 2d75dc44b..e330a2d8c 100644 --- a/authbridge/authlib/listener/transparentproxy/server.go +++ b/authbridge/authlib/listener/transparentproxy/server.go @@ -1,19 +1,27 @@ -// Package transparentproxy implements an outbound transparent proxy listener -// for proxy-sidecar enforce-redirect mode. Unlike the forward proxy (which -// requires the agent to honor HTTP_PROXY and speak explicit CONNECT), this -// listener receives connections that iptables transparently REDIRECTed to it. -// The agent believes it is connecting directly to the destination, so the -// listener recovers the original destination from the kernel via -// SO_ORIGINAL_DST and hands the connection to a ConnHandler that gates and -// blind-tunnels it — emitting no proxy-protocol bytes back to the agent. +// Package transparentproxy implements transparent (iptables-REDIRECTed) proxy +// listeners for proxy-sidecar mode. In both directions the peer believes it is +// talking directly to its chosen destination, so the listener recovers that +// destination from the kernel via SO_ORIGINAL_DST rather than from any protocol +// header. This is the Go equivalent of Envoy's original_dst listener filter + +// ORIGINAL_DST cluster used by envoy-sidecar mode. // -// This is the Go equivalent of Envoy's original_dst listener filter + -// ORIGINAL_DST cluster used by envoy-sidecar mode; the auth pipeline behind -// the ConnHandler is identical to the forward proxy's CONNECT path. +// Two shapes, because the two directions have different requirements: +// +// - Outbound (Server, enforce-redirect egress guard): dispatches each capture +// to a ConnHandler that gates on destination and then blind-tunnels, +// emitting no proxy-protocol bytes back to the agent. Policy is host-based, +// so the bytes can stay opaque and the agent's end-to-end TLS is preserved. +// - Inbound (InboundListener): a net.Listener, because inbound cannot +// blind-tunnel. JWT validation reads the Authorization header and Path and +// rewrites Authorization to a placeholder before forwarding, which requires +// a real HTTP server over the connection. +// +// Both share CheckDst and the platform-specific SO_ORIGINAL_DST recovery. package transparentproxy import ( "errors" + "fmt" "log/slog" "net" ) @@ -76,26 +84,48 @@ func (s *Server) dispatch(conn *net.TCPConn) { _ = conn.Close() return } - // Defense-in-depth against a self-redirect loop: a genuinely REDIRECTed - // connection's original destination is always some external host — never - // this listener itself. Two ways a connection could point back at us: - // - a loopback dst (a direct dial to 127.0.0.1:); the enforce-redirect - // rules RETURN loopback before the REDIRECT, so a real capture never has one; - // - the listener's own address (e.g. a podIP: self-dial that slipped - // past the iptables CLUSTER_CIDRS RETURN under a misconfigured CIDR set). - // Tunnelling either would spiral into ever more connections/goroutines. The - // iptables layer is the primary control; this is belt-and-suspenders. Drop it. - selfLoop := dst == conn.LocalAddr().String() - if host, _, splitErr := net.SplitHostPort(dst); !selfLoop && splitErr == nil { - if ip := net.ParseIP(host); ip != nil && ip.IsLoopback() { - selfLoop = true - } - } - if selfLoop { + if err := CheckDst(conn.LocalAddr(), dst); err != nil { slog.Warn("transparent-proxy: dropping self-referential connection (would self-loop)", - "remote", conn.RemoteAddr().String(), "dst", dst, "local", conn.LocalAddr().String()) + "remote", conn.RemoteAddr().String(), "dst", dst, + "local", conn.LocalAddr().String(), "error", err) _ = conn.Close() return } s.handle(conn, dst) } + +// ErrSelfReferential reports a recovered original destination that points back +// at this proxy. Returned by CheckDst. +var ErrSelfReferential = errors.New("transparentproxy: self-referential destination") + +// CheckDst is defense-in-depth against a self-redirect loop. A genuinely +// REDIRECTed connection's original destination is the address the peer chose — +// for captured egress some external host, for captured ingress this pod's own +// IP on the app's port. In neither case is it this listener's own address, and +// in neither case is it loopback: +// +// - a loopback dst means a direct dial to 127.0.0.1:. The +// enforce-redirect rules RETURN loopback before the REDIRECT, and loopback +// traffic never traverses PREROUTING, so a real capture never has one. +// - the listener's own address means a self-dial that slipped past the +// iptables RETURN/exclusion rules (e.g. a misconfigured port-exclusion +// list that fails to exempt the transparent port itself). +// +// Handing either to a handler would spiral into ever more connections and +// goroutines — tunnelled straight back into the listener that produced them. +// The iptables layer is the primary control; this is belt-and-suspenders. +// +// local may be nil, in which case only the loopback arm is checked. +func CheckDst(local net.Addr, dst string) error { + if local != nil && dst == local.String() { + return fmt.Errorf("%w: dst equals listener address %s", ErrSelfReferential, dst) + } + host, _, err := net.SplitHostPort(dst) + if err != nil { + return fmt.Errorf("transparentproxy: malformed destination %q: %w", dst, err) + } + if ip := net.ParseIP(host); ip != nil && ip.IsLoopback() { + return fmt.Errorf("%w: loopback dst %s", ErrSelfReferential, dst) + } + return nil +} diff --git a/authbridge/authlib/runtimeutil/runtimeutil.go b/authbridge/authlib/runtimeutil/runtimeutil.go index fe2d827f9..f3c53e082 100644 --- a/authbridge/authlib/runtimeutil/runtimeutil.go +++ b/authbridge/authlib/runtimeutil/runtimeutil.go @@ -7,6 +7,7 @@ package runtimeutil import ( + "fmt" "log/slog" "net" "net/http" @@ -18,6 +19,7 @@ import ( "github.com/rossoctl/cortex/authbridge/authlib/config" "github.com/rossoctl/cortex/authbridge/authlib/listener/reverseproxy" + "github.com/rossoctl/cortex/authbridge/authlib/listener/transparentproxy" "github.com/rossoctl/cortex/authbridge/authlib/observe" "github.com/rossoctl/cortex/authbridge/authlib/pipeline" ) @@ -158,6 +160,50 @@ func StartHTTPServer(name string, handler http.Handler, addr string) (*http.Serv // Logged "mtls" attribute makes the listener mode visible at startup; // operators expecting a separate :8443 port for TLS get a clear hint // that this is the same :8080 with byte-peek detection. +// StartTransparentInboundServer binds the inbound transparent listener and +// serves the reverse proxy's handler over it, resolving each request's +// forwarding target from the destination the client actually addressed +// (recovered via SO_ORIGINAL_DST) rather than from a fixed backend URL. +// +// Pair with proxy-init's INBOUND_TRANSPARENT_PORT: the port here MUST match the +// PREROUTING REDIRECT target, or inbound traffic is redirected to a dead port. +// Callers should treat a returned error as fatal for that reason. +func StartTransparentInboundServer(name string, rp *reverseproxy.Server, addr string) (*http.Server, error) { + if addr == "" { + return nil, fmt.Errorf("%s: transparent_inbound_addr is empty but inbound_interception is transparent", name) + } + // Bind a *net.TCPListener directly rather than via rp.Listen: SO_ORIGINAL_DST + // must be read off the raw TCP connection before any bytes are consumed. The + // mTLS posture is then applied with the same WrapListener the fixed-backend + // reverse proxy uses, so permissive/strict behavior cannot drift between the + // two inbound shapes. + la, err := net.ResolveTCPAddr("tcp", addr) + if err != nil { + return nil, fmt.Errorf("%s: resolve addr %q: %w", name, addr, err) + } + tcpLn, err := net.ListenTCP("tcp", la) + if err != nil { + return nil, fmt.Errorf("%s: listen on %q: %w", name, addr, err) + } + listener := rp.WrapListener(transparentproxy.NewInboundListener(tcpLn)) + srv := &http.Server{ + Addr: addr, + Handler: rp.Handler(), + ReadHeaderTimeout: 10 * time.Second, + // Carries each connection's recovered original destination into the + // request context, unwrapping through tlssniff / tls.Conn as needed. + ConnContext: transparentproxy.ConnContextHook, + } + go func() { + slog.Info("Transparent inbound server listening", + "name", name, "addr", listener.Addr().String(), "mtls", rp.MTLSEnabled()) + if err := srv.Serve(listener); err != nil && err != http.ErrServerClosed { + slog.Error("Transparent inbound server failed", "name", name, "error", err) + } + }() + return srv, nil +} + func StartReverseProxyServer(name string, rp *reverseproxy.Server, addr string) (*http.Server, error) { srv := &http.Server{ Addr: addr, diff --git a/authbridge/cmd/authbridge-proxy/main.go b/authbridge/cmd/authbridge-proxy/main.go index 131ca85b4..e6434f971 100644 --- a/authbridge/cmd/authbridge-proxy/main.go +++ b/authbridge/cmd/authbridge-proxy/main.go @@ -384,16 +384,40 @@ func main() { defer sharedStore.Close() // stop the TTL janitor on normal main return if roles[config.RoleReverse] { - rpSrv, rerr := reverseproxy.NewServer(inboundH, sessions, cfg.Listener.ReverseProxyBackend, rpMTLS) - if rerr != nil { - log.Fatalf("creating reverse proxy: %v", rerr) - } - rpSrv.Shared = sharedStore - rpHTTP, rerr := runtimeutil.StartReverseProxyServer("reverse-proxy", rpSrv, cfg.Listener.ReverseProxyAddr) - if rerr != nil { - log.Fatalf("reverse-proxy listen: %v", rerr) + // Two inbound shapes, selected by listener.inbound_interception: + // transparent — iptables PREROUTING REDIRECTs here; the forwarding + // target is per-connection, from SO_ORIGINAL_DST. + // reverse-proxy — the default; one fixed reverse_proxy_backend, reached + // because the operator stole the agent's port. + if cfg.Listener.InboundTransparent() { + rpSrv, rerr := reverseproxy.NewTransparentServer(inboundH, sessions, "", rpMTLS) + if rerr != nil { + log.Fatalf("creating transparent inbound proxy: %v", rerr) + } + rpSrv.Shared = sharedStore + // Skipped in --demo: there is no iptables there, so nothing would ever + // be REDIRECTed to the listener and every request would fail closed. + if demoMode { + slog.Warn("demo mode: transparent inbound listener not started (no iptables to REDIRECT to it)") + } else { + rpHTTP, rerr := runtimeutil.StartTransparentInboundServer("transparent-inbound", rpSrv, cfg.Listener.TransparentInboundAddr) + if rerr != nil { + log.Fatalf("transparent-inbound listen: %v", rerr) + } + httpServers = append(httpServers, rpHTTP) + } + } else { + rpSrv, rerr := reverseproxy.NewServer(inboundH, sessions, cfg.Listener.ReverseProxyBackend, rpMTLS) + if rerr != nil { + log.Fatalf("creating reverse proxy: %v", rerr) + } + rpSrv.Shared = sharedStore + rpHTTP, rerr := runtimeutil.StartReverseProxyServer("reverse-proxy", rpSrv, cfg.Listener.ReverseProxyAddr) + if rerr != nil { + log.Fatalf("reverse-proxy listen: %v", rerr) + } + httpServers = append(httpServers, rpHTTP) } - httpServers = append(httpServers, rpHTTP) } // The transparent (enforce-redirect) listener rides with the forward proxy; From b3a9f81e89bf19b72217687e91c214d2973f215f Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 18 Aug 2026 11:05:18 -0400 Subject: [PATCH 2/6] feat(proxy-init): Add transparent inbound capture to enforce-redirect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds INBOUND_TRANSPARENT_PORT (empty = off) to init-iptables.sh. When set under MODE=enforce-redirect, inbound TCP is captured so AuthBridge's inbound transparent listener can validate it, closing the pod-to-pod bypass that let any pod reach the agent's real port without JWT validation. Inbound arrives by two capturable paths and BOTH are covered. Handling only the obvious one would have shipped a guard that silently waves through all mesh traffic: A. Plain network (ClusterIP/NodePort/non-mesh pod) -> nat PREROUTING. New AB_INBOUND chain, inserted at position 1 so it precedes Istio's appended ISTIO_PRERT, exactly as redirect mode's PROXY_INBOUND does. B. Istio ambient HBONE -> the remote ztunnel sends to this pod's ztunnel on :15008, which terminates mTLS and re-originates a LOCAL connection. That appears in nat OUTPUT, never PREROUTING. Captured by a mark-based DNAT installed at the HEAD of AB_REDIRECT — it must precede that chain's existing ztunnel-mark RETURN, which would otherwise let every mesh-delivered request through unvalidated. The test asserts this ordering. Intra-pod loopback is deliberately not captured. Containers share a network namespace and are a single entity to every network enforcement layer, so that traffic is inside the trust boundary by construction. It is also not capturable without breaking AuthBridge's own forward hop, which is loopback by design. Details worth noting: - DNAT to POD_IP, not REDIRECT: REDIRECT in OUTPUT hardcodes dst to 127.0.0.1, and ztunnel preserves the client IP via IP_TRANSPARENT, so the resulting src=external/dst=loopback packet is dropped as martian without route_localnet=1. Same reasoning redirect mode already documents. SO_ORIGINAL_DST is unaffected — conntrack records the pre-NAT tuple for both. - POD_IP is now required when inbound capture is requested, and its absence is fail-closed at init. PREROUTING-only rules would validate direct traffic while waving through all mesh traffic; a failed init container is far easier to triage than that. - The sidecar's own ports are exempted (SIDECAR_PORTS_EXCLUDE, default 8081,9091,9093,9094). Gating :9091 would put kubelet probes behind JWT validation and crash-loop the pod. The operator overrides the list when it assigns a non-default forward-proxy port. - The forward-hop mangle MARK rule is -C-guarded, so an init container re-run on pod restart cannot stack duplicates. - An env var rather than a fourth MODE value: interception is two independent axes, so folding it in would multiply MODE's strict case combinatorially. Also wires test-enforce-redirect.sh into CI, which required fixing two pre-existing bugs that made it unable to pass anywhere: - The capture/preemption assertion matched /REDIRECT/ line-wise, which also matches the "Chain AB_REDIRECT" header (yielding the literal "Chain") and, in OUTPUT, the "-j AB_REDIRECT" jump rule's own counter. Now matches the target column, and demonstrates real capture (AB=1, ISTIO=0). - The backend-detection unit tests could never exercise detection: the harness's own netns re-exec exports IPTABLES_CMD, which detect_iptables_cmd honors first. Cleared for the two auto-detection cases. Harness is 45/45 green under `unshare --net` on iptables-nft. Refs: rossoctl/cortex#330 Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .github/workflows/ci.yaml | 25 ++ authbridge/proxy-init/init-iptables.sh | 232 ++++++++++++++++++ .../proxy-init/test-enforce-redirect.sh | 128 +++++++++- 3 files changed, 381 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2fbe79804..4075b02ea 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -115,6 +115,31 @@ jobs: - name: Test run: go test -v -race -cover ./... + proxy-init-iptables: + name: proxy-init iptables rules + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + # The harness builds its rules inside `unshare --net`, so it never touches + # the runner's own networking. It needs root for unshare + iptables, the + # dummy module to generate a routable external packet, and both iptables + # backends so the legacy-detection case is exercised rather than skipped. + - name: Install iptables backends + run: | + sudo apt-get update -qq + sudo apt-get install -y -qq iptables iproute2 kmod + sudo modprobe dummy || echo "dummy module unavailable; capture packet may not be generated" + + # Gates the interception rules themselves: chain placement and ordering, + # the DNS carve-out, the non-TCP drop, the fail-closed guards, and — for + # transparent inbound — that the ambient DNAT precedes AB_REDIRECT's + # ztunnel-mark RETURN. That ordering decides whether mesh-delivered traffic + # is validated or waved through, and nothing else in CI covers it. + - name: Test enforce-redirect + transparent inbound rules + run: sudo -E authbridge/proxy-init/test-enforce-redirect.sh + python-test: name: Python Tests runs-on: ubuntu-latest diff --git a/authbridge/proxy-init/init-iptables.sh b/authbridge/proxy-init/init-iptables.sh index f6cc6b0c3..3fbe596a2 100644 --- a/authbridge/proxy-init/init-iptables.sh +++ b/authbridge/proxy-init/init-iptables.sh @@ -103,6 +103,23 @@ # redirect Envoy's local delivery back to ztunnel (15001). Mangle OUTPUT runs # before nat OUTPUT in the netfilter hook ordering. # +# ─── Inbound flow, enforce-redirect + INBOUND_TRANSPARENT_PORT ─────────────── +# +# Opt-in; off unless INBOUND_TRANSPARENT_PORT is set. Gives proxy-sidecar the +# inbound counterpart of the egress guard, so JWT validation cannot be sidestepped +# by another pod dialing the agent's real port. Two capturable paths: +# +# Plain network: PREROUTING -> AB_INBOUND -> REDIRECT to the inbound port -> +# AuthBridge recovers the app's port via SO_ORIGINAL_DST -> +# forwards to 127.0.0.1:. +# Ambient HBONE: ztunnel terminates mTLS and re-originates LOCALLY, so it +# appears in OUTPUT, not PREROUTING. Captured by a mark-based +# DNAT at the head of AB_REDIRECT (see setup_enforce_redirect). +# +# Intra-pod loopback is intentionally NOT captured: containers share a netns and +# are a single entity to every network enforcement layer, so that traffic is +# inside the trust boundary. See setup_transparent_inbound() for the full notes. +# # ─── Debugging tips ────────────────────────────────────────────────────────── # # conntrack -L — shows actual DNAT/REDIRECT targets and connection states @@ -203,6 +220,25 @@ INBOUND_PROXY_PORT="${INBOUND_PROXY_PORT:-15124}" # REDIRECT target for captured external TCP egress. Must match the authbridge # proxy-sidecar listener.transparent_proxy_addr (default :8082). TRANSPARENT_PORT="${TRANSPARENT_PORT:-8082}" +# enforce-redirect mode: the INBOUND transparent listener port. Empty (the +# default) leaves inbound interception OFF, so enforce-redirect stays a pure +# egress guard and nothing about existing deployments changes. When set, inbound +# TCP is REDIRECTed here and AuthBridge recovers the port the client actually +# addressed via SO_ORIGINAL_DST. Must match the authbridge proxy-sidecar +# listener.transparent_inbound_addr (preset default :8083). +# +# An env var rather than a fourth MODE value: interception is two independent +# axes (inbound, outbound), so folding it into MODE would multiply the strict +# `case` above combinatorially (redirect, enforce-redirect, +# enforce-redirect+inbound, inbound-only, ...). +INBOUND_TRANSPARENT_PORT="${INBOUND_TRANSPARENT_PORT:-}" +# AuthBridge's own listeners, excluded from the inbound REDIRECT: traffic to +# these is for the sidecar itself, not the app. Redirecting them would put the +# health endpoint behind JWT validation (breaking probes) and gate the stats and +# session-events APIs. The default covers the proxy-sidecar preset layout; the +# operator overrides it when it assigns a non-default forward-proxy port. +# The transparent ports themselves are excluded unconditionally below. +SIDECAR_PORTS_EXCLUDE="${SIDECAR_PORTS_EXCLUDE:-8081,9091,9093,9094}" PROXY_UID="${PROXY_UID:-1337}" SSH_PORT="${SSH_PORT:-22}" OUTBOUND_PORTS_EXCLUDE="${OUTBOUND_PORTS_EXCLUDE:-}" @@ -231,6 +267,16 @@ get_nameservers() { return 0 } +# is_ipv6 reports whether an address literal is IPv6, by the presence of a +# colon. Sufficient here because the only inputs are resolv.conf nameservers and +# the downward-API POD_IP, both of which are bare literals (never host:port). +is_ipv6() { + case "$1" in + *:*) return 0 ;; + *) return 1 ;; + esac +} + # IPv6 counterpart of the detected iptables backend (iptables-legacy -> # ip6tables-legacy, iptables -> ip6tables). Override with IP6TABLES_CMD. IP6T="${IP6TABLES_CMD:-$(echo "${IPT}" | sed 's/iptables/ip6tables/')}" @@ -252,6 +298,20 @@ if [ "${MODE}" = "redirect" ] && [ -z "${POD_IP}" ]; then exit 1 fi +# Transparent inbound needs POD_IP for the ambient DNAT target, for the same +# route_localnet reason redirect mode does (see the inbound-flow notes above). +# Fail loud rather than install PREROUTING-only rules: those silently miss every +# mesh-delivered request, since ztunnel re-originates inbound locally through +# OUTPUT and never traverses PREROUTING. A pod that validates direct traffic but +# waves through all mesh traffic is far worse than a failed init container. +if [ -n "${INBOUND_TRANSPARENT_PORT}" ] && [ -z "${POD_IP}" ]; then + echo "ERROR: POD_IP is not set but INBOUND_TRANSPARENT_PORT=${INBOUND_TRANSPARENT_PORT} requests inbound interception." >&2 + echo "ERROR: without it the Istio ambient inbound path (ztunnel -> OUTPUT, not PREROUTING) cannot be captured," >&2 + echo "ERROR: so mesh traffic would bypass inbound validation entirely. Refusing to start half-enforced." >&2 + echo "Set POD_IP via the Kubernetes Downward API (status.podIP)." >&2 + exit 1 +fi + # ============================================================================= # enforce-redirect mode (proxy-sidecar fail-closed egress guard, capture variant) # ============================================================================= @@ -314,6 +374,32 @@ setup_enforce_redirect() { # --- IPv4: nat REDIRECT for TCP --- ${IPT} -t nat -N "${REDIR_CHAIN}" 2>/dev/null || true ${IPT} -t nat -F "${REDIR_CHAIN}" + # Transparent inbound, ambient path — MUST precede the ztunnel-mark RETURN + # below, which would otherwise let every mesh-delivered request through + # unvalidated. Ambient inbound does NOT traverse PREROUTING: the remote + # ztunnel sends HBONE to this pod's ztunnel on 15008, which terminates mTLS + # and re-originates a LOCAL connection to the app — so it appears in OUTPUT. + # + # All three matches are load-bearing: + # mark 0x539 — identifies ztunnel's sockets. + # ! --uid-owner — excludes AuthBridge's own delivery to the app, which the + # mangle rule below also marks 0x539; without this the + # forward hop would be DNATed back into the listener. + # --dst-type LOCAL — ztunnel reuses 0x539 for OUTBOUND HBONE to remote pods + # on :15008 too. Capturing those would hand an HTTP + # listener an mTLS stream (InvalidContentType). + # + # DNAT to POD_IP rather than REDIRECT: REDIRECT in OUTPUT hardcodes dst to + # 127.0.0.1, and ztunnel preserves the original client IP via IP_TRANSPARENT, + # so the resulting src=external/dst=loopback packet is dropped as martian + # unless route_localnet=1 (which needs a privileged write to /proc/sys). + # SO_ORIGINAL_DST is unaffected — conntrack records the pre-NAT tuple for DNAT + # and REDIRECT identically. + if [ -n "${INBOUND_TRANSPARENT_PORT}" ] && ! is_ipv6 "${POD_IP}"; then + ${IPT} -t nat -A "${REDIR_CHAIN}" -m mark --mark "${ZTUNNEL_MARK}" \ + -m owner ! --uid-owner "${PROXY_UID}" -m addrtype --dst-type LOCAL \ + -p tcp -j DNAT --to-destination "${POD_IP}:${INBOUND_TRANSPARENT_PORT}" + fi # ztunnel's own sockets (ambient) carry fwmark 0x539 — let them through. ${IPT} -t nat -A "${REDIR_CHAIN}" -m mark --mark "${ZTUNNEL_MARK}" -j RETURN # the AuthBridge proxy's own re-originated egress (runs as PROXY_UID) — avoids @@ -373,6 +459,14 @@ setup_enforce_redirect() { if command -v "${IP6T%% *}" >/dev/null 2>&1 && ${IP6T} -t nat -L >/dev/null 2>&1; then ${IP6T} -t nat -N "${REDIR_CHAIN}" 2>/dev/null || true ${IP6T} -t nat -F "${REDIR_CHAIN}" + # Ambient inbound DNAT, v6 mirror. Installed only when POD_IP is itself v6: + # the downward API yields the pod's primary address, and a v4 target in + # ip6tables is rejected outright. + if [ -n "${INBOUND_TRANSPARENT_PORT}" ] && is_ipv6 "${POD_IP}"; then + ${IP6T} -t nat -A "${REDIR_CHAIN}" -m mark --mark "${ZTUNNEL_MARK}" \ + -m owner ! --uid-owner "${PROXY_UID}" -m addrtype --dst-type LOCAL \ + -p tcp -j DNAT --to-destination "[${POD_IP}]:${INBOUND_TRANSPARENT_PORT}" + fi ${IP6T} -t nat -A "${REDIR_CHAIN}" -m mark --mark "${ZTUNNEL_MARK}" -j RETURN ${IP6T} -t nat -A "${REDIR_CHAIN}" -m owner --uid-owner "${PROXY_UID}" -j RETURN ${IP6T} -t nat -A "${REDIR_CHAIN}" -o lo -j RETURN @@ -419,10 +513,148 @@ setup_enforce_redirect() { echo "enforce-redirect: fail-closed egress capture active" } +# ============================================================================= +# transparent inbound (proxy-sidecar hard inbound boundary) +# ============================================================================= +# +# Opt-in companion to the egress guard, enabled by INBOUND_TRANSPARENT_PORT. +# Closes the pod-to-pod inbound bypass: without it, inbound validation only +# covers traffic that happens to reach AuthBridge's listener, so any pod dialing +# the agent's real port talks to it directly. The pod is the granularity both +# Kubernetes NetworkPolicy and ztunnel enforce at, so that is the boundary that +# matters — and it is the one this closes. +# +# Inbound arrives by two capturable paths, and BOTH must be covered or the guard +# is half a guard: +# +# A. Plain network (ClusterIP/NodePort/non-mesh pod) -> nat PREROUTING. +# Handled by the AB_INBOUND chain here. +# B. Istio ambient HBONE -> remote ztunnel to local ztunnel :15008, which +# terminates mTLS and re-originates a LOCAL connection. That appears in +# nat OUTPUT, never PREROUTING, and is handled by the DNAT rule installed +# at the head of AB_REDIRECT in setup_enforce_redirect(). +# +# A third path is deliberately NOT captured: a container in this pod dialing +# 127.0.0.1:. Containers share a network namespace and are one entity to +# every network enforcement layer, so intra-pod traffic is inside the trust +# boundary by construction — not a gap. Capturing it is also impossible without +# breaking AuthBridge's own forward hop, which is loopback by design. +# +# The forward hop (AuthBridge -> app) targets 127.0.0.1:, which +# AB_REDIRECT exempts three ways (-o lo, -d 127.0.0.0/8, --uid-owner PROXY_UID), +# so it can never be re-captured by our own egress rules. Consequence: the app +# must bind 0.0.0.0, not just its pod IP. +setup_transparent_inbound() { + IN_CHAIN="AB_INBOUND" + + echo "transparent-inbound: installing inbound capture -> :${INBOUND_TRANSPARENT_PORT}" + echo "transparent-inbound: exempt sidecar ports=${SIDECAR_PORTS_EXCLUDE} transparent ports=${TRANSPARENT_PORT},${INBOUND_TRANSPARENT_PORT} operator excludes=${INBOUND_PORTS_EXCLUDE:-}" + + # inbound_chain_rules emits the shared rule set for one address family. Both + # families get identical policy; only the command differs. + inbound_chain_rules() { + _ic="$1" + ${_ic} -t nat -N "${IN_CHAIN}" 2>/dev/null || true + ${_ic} -t nat -F "${IN_CHAIN}" + + # Istio rewrites kubelet health checks to a synthetic source. Gating them + # would fail probes and crash-loop the pod. + ${_ic} -t nat -A "${IN_CHAIN}" -s "${ISTIO_HEALTH_PROBE_SRC}/32" -p tcp -j RETURN 2>/dev/null || true + + # The transparent ports themselves. Redirecting INBOUND_TRANSPARENT_PORT to + # itself would self-loop (AuthBridge's CheckDst drops it, but not before + # burning a connection); TRANSPARENT_PORT is the egress listener and receives + # nothing from the network. + ${_ic} -t nat -A "${IN_CHAIN}" -p tcp --dport "${INBOUND_TRANSPARENT_PORT}" -j RETURN + ${_ic} -t nat -A "${IN_CHAIN}" -p tcp --dport "${TRANSPARENT_PORT}" -j RETURN + + # AuthBridge's own listeners: health, stats, session-events API, forward + # proxy. Gating health breaks probes; gating the others breaks abctl and + # operator introspection. + for _p in $(echo "${SIDECAR_PORTS_EXCLUDE}" | tr ',' ' '); do + if [ -n "${_p}" ]; then + ${_ic} -t nat -A "${IN_CHAIN}" -p tcp --dport "${_p}" -j RETURN + fi + done + + ${_ic} -t nat -A "${IN_CHAIN}" -p tcp --dport "${SSH_PORT}" -j RETURN + + # ztunnel's HBONE port takes mTLS tunnels straight from the network and must + # reach ztunnel's in-pod socket, not an HTTP listener. + ${_ic} -t nat -A "${IN_CHAIN}" -p tcp --dport "${ZTUNNEL_HBONE_PORT}" -j RETURN + + # Operator/user escape hatch (kagenti.io/inbound-ports-exclude), for app + # ports that must not be validated — e.g. an oauth-proxy doing its own auth. + for _p in $(echo "${INBOUND_PORTS_EXCLUDE}" | tr ',' ' '); do + if [ -n "${_p}" ]; then + ${_ic} -t nat -A "${IN_CHAIN}" -p tcp --dport "${_p}" -j RETURN + fi + done + + # Everything else — the app's ports, whichever and however many they are. + # SO_ORIGINAL_DST recovers which one each connection targeted, so no per-port + # configuration is needed. + ${_ic} -t nat -A "${IN_CHAIN}" -p tcp -j REDIRECT --to-port "${INBOUND_TRANSPARENT_PORT}" + + # -I 1 so we precede Istio's appended ISTIO_PRERT, exactly as redirect mode + # does. No conntrack ESTABLISHED rule: nat evaluates only a flow's first + # packet, so replies are un-NATed by conntrack automatically. + if ! ${_ic} -t nat -C PREROUTING -p tcp -j "${IN_CHAIN}" 2>/dev/null; then + ${_ic} -t nat -I PREROUTING 1 -p tcp -j "${IN_CHAIN}" + fi + require_jump "${_ic}" nat PREROUTING "${IN_CHAIN}" -p tcp + } + + inbound_chain_rules "${IPT}" + echo "transparent-inbound: IPv4 inbound capture configured" + + # Keep AuthBridge's delivery to the app off ISTIO_OUTPUT's redirect. Without + # the mark, ambient's ISTIO_OUTPUT sees mark != 0x539 and can redirect our + # forward hop to ztunnel :15001, looping the request back into the mesh + # instead of handing it to the app. mangle OUTPUT runs before nat OUTPUT, so + # the mark is set before ISTIO_OUTPUT evaluates it. Scoped to PROXY_UID + + # dst-type LOCAL so only the local forward hop is marked, never AuthBridge's + # external egress (which must stay unmarked so ztunnel wraps it in mTLS). + # + # Mirrors the equivalent rule redirect mode installs for Envoy. That one + # targets a podIP delivery while ours is loopback, so whether ambient's + # loopback branch makes this strictly necessary should be confirmed against a + # live ambient pod; it is scoped narrowly enough to be harmless if not. + # -C-guarded: an init container can re-run on pod restart, and an unguarded + # -I would stack a duplicate mark rule on every pass. + mark_local_delivery() { + _mc="$1" + if ! ${_mc} -t mangle -C OUTPUT -m owner --uid-owner "${PROXY_UID}" \ + -m addrtype --dst-type LOCAL -p tcp -j MARK --set-mark "${ZTUNNEL_MARK}" 2>/dev/null; then + ${_mc} -t mangle -I OUTPUT 1 -m owner --uid-owner "${PROXY_UID}" \ + -m addrtype --dst-type LOCAL -p tcp -j MARK --set-mark "${ZTUNNEL_MARK}" + fi + } + mark_local_delivery "${IPT}" + + if command -v "${IP6T%% *}" >/dev/null 2>&1 && ${IP6T} -t nat -L >/dev/null 2>&1; then + inbound_chain_rules "${IP6T}" + mark_local_delivery "${IP6T}" + echo "transparent-inbound: IPv6 inbound capture configured" + else + echo "transparent-inbound: ip6tables unavailable — skipping IPv6 inbound capture" + fi + + echo "transparent-inbound: hard inbound boundary active" +} + # Dispatch enforce-redirect here and exit; redirect mode falls through to the # transparent-interception logic below. if [ "${MODE}" = "enforce-redirect" ]; then setup_enforce_redirect + # Inbound interception is opt-in and layers on top of the egress guard: the + # ambient DNAT rule it depends on is installed at the head of AB_REDIRECT + # above, so setup_enforce_redirect must run first. + if [ -n "${INBOUND_TRANSPARENT_PORT}" ]; then + setup_transparent_inbound + else + echo "enforce-redirect: inbound interception disabled (INBOUND_TRANSPARENT_PORT unset)" + fi exit 0 fi diff --git a/authbridge/proxy-init/test-enforce-redirect.sh b/authbridge/proxy-init/test-enforce-redirect.sh index 79d4cae06..564789402 100755 --- a/authbridge/proxy-init/test-enforce-redirect.sh +++ b/authbridge/proxy-init/test-enforce-redirect.sh @@ -19,6 +19,13 @@ # counters: our REDIRECT increments, the simulated ISTIO REDIRECT does not. # 3. NON-TCP DROP — an external UDP datagram (QUIC/HTTP-3 bypass attempt) hits # the mangle AB_NOTCP DROP, proving non-TCP external egress cannot bypass. +# 4. TRANSPARENT INBOUND (INBOUND_TRANSPARENT_PORT) — off by default; when on, +# AB_INBOUND is hooked at nat PREROUTING position 1 with the sidecar's own +# ports exempted (health 9091 in particular, or kubelet probes would be +# JWT-gated and crash-loop the pod), and the ambient DNAT is installed +# BEFORE AB_REDIRECT's ztunnel-mark RETURN — the ordering that decides +# whether mesh-delivered traffic is validated or waved through. Also covers +# the POD_IP fail-closed guard and re-run idempotency of the mark rule. # # Requirements: root (for unshare --net + iptables), iproute2, iptables-nft, # bash, the dummy kernel module. Runs on Linux / CI (e.g. ubuntu-latest); not on @@ -31,6 +38,8 @@ INIT="${INIT_SCRIPT:-${SCRIPT_DIR}/init-iptables.sh}" IPT="${IPTABLES_CMD:-iptables-nft}" EXTERNAL="198.51.100.7" # RFC5737 TEST-NET-2, guaranteed unused TPORT="8082" +IN_TPORT="8083" # inbound transparent listener port +POD_IP_MOCK="10.244.1.7" # stands in for the downward-API status.podIP RESOLVER_V4="172.31.0.10" # OCP-style resolver, deliberately OUTSIDE 10/8 RESOLVER_V6="fd00:10:96::10" # IPv6 resolver, exercises the ip6tables path @@ -128,8 +137,11 @@ echo "### Capture + preemption test: append a simulated ISTIO_OUTPUT nat REDIREC # listener on TPORT the redirected SYN gets an RST; the rule counter still ticks. timeout 2 bash -c "exec 3<>/dev/tcp/${EXTERNAL}/80" 2>/dev/null || true -capc=$("${IPT}" -t nat -L AB_REDIRECT -n -v | awk '/REDIRECT/{print $1; exit}') -istioc=$("${IPT}" -t nat -L OUTPUT -n -v | awk '/REDIRECT/{print $1; exit}') +# $3 is the target column under -v. Matching the column rather than the line is +# required: /REDIRECT/ also matches the "Chain AB_REDIRECT" header (yielding the +# literal "Chain") and, in OUTPUT, the "-j AB_REDIRECT" jump rule's own counter. +capc=$("${IPT}" -t nat -L AB_REDIRECT -n -v | awk '$3=="REDIRECT"{print $1; exit}') +istioc=$("${IPT}" -t nat -L OUTPUT -n -v | awk '$3=="REDIRECT"{print $1; exit}') echo "AB_REDIRECT REDIRECT pkts=${capc:-?} | simulated ISTIO REDIRECT pkts=${istioc:-?}" if [ "${capc:-0}" -gt 0 ] && [ "${istioc:-0}" -eq 0 ]; then echo "PASS: external TCP captured to transparent port, preempting nat REDIRECT (ambient-robust)" @@ -160,6 +172,114 @@ else echo "PASS: init aborts fail-closed when resolv.conf has no nameservers" fi +# ============================================================================= +# Transparent inbound (INBOUND_TRANSPARENT_PORT) — opt-in inbound capture +# ============================================================================= + +echo "### Inbound OFF by default: no AB_INBOUND without INBOUND_TRANSPARENT_PORT" +if echo "${natdump}" | grep -q 'AB_INBOUND'; then + echo "FAIL: AB_INBOUND present without INBOUND_TRANSPARENT_PORT (inbound must be opt-in)"; fail=1 +else + echo "PASS: inbound capture off by default — egress guard unchanged" +fi + +echo "### Fail-closed test: inbound port without POD_IP must abort init" +# PREROUTING-only rules would silently miss every ambient (HBONE) request, since +# ztunnel re-originates inbound locally through OUTPUT. Half-enforced is worse +# than not started, so this must exit non-zero. +if env MODE=enforce-redirect PROXY_UID=1337 RESOLV_CONF="${RESOLV_MOCK}" \ + TRANSPARENT_PORT="${TPORT}" INBOUND_TRANSPARENT_PORT="${IN_TPORT}" \ + IPTABLES_CMD="${IPT}" IP6TABLES_CMD=ip6tables-nft \ + sh "${INIT}" >/dev/null 2>&1; then + echo "FAIL: init succeeded with INBOUND_TRANSPARENT_PORT but no POD_IP"; fail=1 +else + echo "PASS: init aborts fail-closed when inbound capture is requested without POD_IP" +fi + +echo "### Installing enforce-redirect + transparent inbound (POD_IP=${POD_IP_MOCK})" +env MODE=enforce-redirect PROXY_UID=1337 RESOLV_CONF="${RESOLV_MOCK}" \ + TRANSPARENT_PORT="${TPORT}" INBOUND_TRANSPARENT_PORT="${IN_TPORT}" \ + POD_IP="${POD_IP_MOCK}" INBOUND_PORTS_EXCLUDE=8443 \ + IPTABLES_CMD="${IPT}" IP6TABLES_CMD=ip6tables-nft \ + sh "${INIT}" || { echo "FAIL: init script exited non-zero with inbound enabled"; exit 1; } + +innat=$("${IPT}" -t nat -S) +inmangle=$("${IPT}" -t mangle -S) +echo "--- nat ruleset (inbound enabled) ---"; echo "${innat}" + +assert "AB_INBOUND hooked from nat PREROUTING" '^-A PREROUTING -p tcp -j AB_INBOUND' "${innat}" +assert "inbound catch-all REDIRECTs to the inbound port" \ + "AB_INBOUND -p tcp -j REDIRECT --to-ports ${IN_TPORT}" "${innat}" +# Self-loop and sidecar-port exemptions. Redirecting the health port would put +# kubelet probes behind JWT validation and crash-loop the pod. +assert "inbound port exempted (no self-loop)" "AB_INBOUND -p tcp -m tcp --dport ${IN_TPORT} -j RETURN" "${innat}" +assert "egress transparent port exempted" "AB_INBOUND -p tcp -m tcp --dport ${TPORT} -j RETURN" "${innat}" +assert "health port 9091 exempted (probes)" 'AB_INBOUND -p tcp -m tcp --dport 9091 -j RETURN' "${innat}" +assert "stats port 9093 exempted" 'AB_INBOUND -p tcp -m tcp --dport 9093 -j RETURN' "${innat}" +assert "session-events port 9094 exempted" 'AB_INBOUND -p tcp -m tcp --dport 9094 -j RETURN' "${innat}" +assert "forward-proxy port 8081 exempted" 'AB_INBOUND -p tcp -m tcp --dport 8081 -j RETURN' "${innat}" +assert "ztunnel HBONE 15008 exempted" 'AB_INBOUND -p tcp -m tcp --dport 15008 -j RETURN' "${innat}" +assert "operator exclude 8443 honored" 'AB_INBOUND -p tcp -m tcp --dport 8443 -j RETURN' "${innat}" + +# The ambient path is the one a PREROUTING-only implementation silently misses. +assert "ambient inbound DNAT installed in AB_REDIRECT" \ + "AB_REDIRECT .*0x539.*--uid-owner 1337.*LOCAL.*-j DNAT --to-destination ${POD_IP_MOCK}:${IN_TPORT}" "${innat}" +assert "forward-hop mark rule in mangle OUTPUT" \ + 'OUTPUT -p tcp -m owner --uid-owner 1337 -m addrtype --dst-type LOCAL -j MARK --set-x?mark 0x539' "${inmangle}" + +echo "### Ambient DNAT must precede the ztunnel-mark RETURN (else mesh bypasses)" +# Rule ORDER is the whole correctness argument here: AB_REDIRECT's ztunnel-mark +# RETURN would let every HBONE-delivered request through unvalidated if it were +# evaluated first. +dnat_line=$(echo "${innat}" | grep -n 'AB_REDIRECT.*-j DNAT' | head -1 | cut -d: -f1) +ret_line=$(echo "${innat}" | grep -n 'AB_REDIRECT -m mark --mark 0x539/0xfff -j RETURN' | head -1 | cut -d: -f1) +if [ -n "${dnat_line}" ] && [ -n "${ret_line}" ] && [ "${dnat_line}" -lt "${ret_line}" ]; then + echo "PASS: ambient DNAT precedes the ztunnel-mark RETURN" +else + echo "FAIL: ambient DNAT not before ztunnel RETURN (dnat=${dnat_line:-?} return=${ret_line:-?})"; fail=1 +fi + +echo "### AB_INBOUND must be at nat PREROUTING position 1 (precede ISTIO_PRERT)" +inpos1=$("${IPT}" -t nat -L PREROUTING --line-numbers 2>/dev/null | awk '$1=="1"{print $2}') +if [ "${inpos1}" = "AB_INBOUND" ]; then echo "PASS: AB_INBOUND at nat PREROUTING position 1" +else echo "FAIL: AB_INBOUND not at nat PREROUTING position 1 (got '${inpos1}')"; fail=1; fi + +echo "### Idempotency: re-running init must not stack duplicate mark rules" +env MODE=enforce-redirect PROXY_UID=1337 RESOLV_CONF="${RESOLV_MOCK}" \ + TRANSPARENT_PORT="${TPORT}" INBOUND_TRANSPARENT_PORT="${IN_TPORT}" \ + POD_IP="${POD_IP_MOCK}" IPTABLES_CMD="${IPT}" IP6TABLES_CMD=ip6tables-nft \ + sh "${INIT}" >/dev/null 2>&1 || true +markcount=$("${IPT}" -t mangle -S OUTPUT | grep -c 'MARK --set-x\?mark 0x539' || true) +if [ "${markcount}" -eq 1 ]; then + echo "PASS: forward-hop mark rule is idempotent across init re-runs" +else + echo "FAIL: mark rule stacked ${markcount} times across re-runs (expected 1)"; fail=1 +fi + +echo "### Inbound capture test: packet to the app port must hit AB_INBOUND" +# PREROUTING only sees packets arriving on an interface, which a netns cannot +# easily synthesise without a peer. Assert the REDIRECT rule exists and is +# reachable instead; live capture is covered by the Kind e2e. +if "${IPT}" -t nat -L AB_INBOUND -n >/dev/null 2>&1; then + echo "PASS: AB_INBOUND chain exists and is listable in the live backend" +else + echo "FAIL: AB_INBOUND chain missing from the live backend"; fail=1 +fi + +echo "### Malformed exclude list must not abort init (set -e trap)" +# A trailing comma yields an empty field. If the port loop used `[ -n ] && cmd` +# as its last statement, the loop would exit non-zero and `set -e` would abort +# init — turning a cosmetic annotation typo into a pod that never starts. +if env MODE=enforce-redirect PROXY_UID=1337 RESOLV_CONF="${RESOLV_MOCK}" \ + TRANSPARENT_PORT="${TPORT}" INBOUND_TRANSPARENT_PORT="${IN_TPORT}" \ + POD_IP="${POD_IP_MOCK}" INBOUND_PORTS_EXCLUDE="8443," SIDECAR_PORTS_EXCLUDE="9091," \ + IPTABLES_CMD="${IPT}" IP6TABLES_CMD=ip6tables-nft \ + sh "${INIT}" >/dev/null 2>&1; then + echo "PASS: trailing comma in an exclude list is tolerated" +else + echo "FAIL: init aborted on a trailing comma in an exclude list"; fail=1 +fi + echo "### Backend detection unit test (/proc/modules seam)" # Pull detect_iptables_cmd (and its PROC_MODULES default) out of the script and # exercise it against fixture module tables — no real kernel needed. The legacy @@ -169,13 +289,13 @@ eval "$(sed -n '/^PROC_MODULES=/,/^}/p' "${INIT}")" mods_legacy=$(mktemp); printf 'ip_tables 28672 4 - Live 0x0\niptable_nat 12288 19 - Live 0x0\n' > "${mods_legacy}" mods_nft=$(mktemp); printf 'nf_tables 315392 344 nft_compat - Live 0x0\nnft_compat 20480 0 - Live 0x0\n' > "${mods_nft}" if command -v iptables-legacy >/dev/null 2>&1; then - got=$(PROC_MODULES="${mods_legacy}" detect_iptables_cmd) + got=$(IPTABLES_CMD= PROC_MODULES="${mods_legacy}" detect_iptables_cmd) [ "${got}" = "iptables-legacy" ] && echo "PASS: iptable_nat loaded => iptables-legacy" \ || { echo "FAIL: expected iptables-legacy, got '${got}'"; fail=1; } else echo "SKIP: iptables-legacy not installed on host — legacy-positive case skipped" fi -got=$(PROC_MODULES="${mods_nft}" detect_iptables_cmd) +got=$(IPTABLES_CMD= PROC_MODULES="${mods_nft}" detect_iptables_cmd) [ "${got}" = "iptables" ] && echo "PASS: iptable_nat absent => iptables (nft)" \ || { echo "FAIL: expected iptables, got '${got}'"; fail=1; } got=$(IPTABLES_CMD=iptables-legacy PROC_MODULES="${mods_nft}" detect_iptables_cmd) From 552ef02f5d2c40bf51ffe82a18e33af7999bbad3 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 18 Aug 2026 12:25:54 -0400 Subject: [PATCH 3/6] docs(authbridge): Document transparent inbound ports and rules Adds the transparent inbound listener to the proxy-sidecar ports table and documents the proxy-init side: the two capturable inbound paths and why both need rules, why the ambient path uses DNAT-to-POD_IP rather than REDIRECT, and why POD_IP is fail-closed rather than optional. Also records two constraints that are easy to hit and hard to diagnose: - The app must bind 0.0.0.0. AuthBridge forwards to 127.0.0.1:, so a pod-IP-only bind is unreachable. - 8082/8083 must match proxy-init's TRANSPARENT_PORT / INBOUND_TRANSPARENT_PORT, and 8080/8083 are mutually exclusive. Fixes two stale claims while here: the ports table omitted 8082 entirely (it has existed since bb15d20), and "Default deployment: no iptables" predates the always-on enforce-redirect guard. Refs: rossoctl/cortex#330 Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/README.md | 17 +++++++++--- authbridge/proxy-init/README.md | 46 ++++++++++++++++++++++++++++++--- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/authbridge/cmd/README.md b/authbridge/cmd/README.md index 256abc8b8..7e366c81f 100644 --- a/authbridge/cmd/README.md +++ b/authbridge/cmd/README.md @@ -40,12 +40,21 @@ ConfigMap contracts are documented in | Port | Purpose | |---|---| -| 8080 | Reverse proxy (inbound) | +| 8080 | Reverse proxy (inbound, `inbound_interception: reverse-proxy` — the default) | | 8081 | Forward proxy (outbound; HTTP_PROXY target) | +| 8082 | Transparent egress listener (enforce-redirect capture target) | +| 8083 | Transparent inbound listener (`inbound_interception: transparent`) | | 9091 | Health | | 9093 | Stats / config inspection | | 9094 | Session Events API (consumed by `abctl`) | +`8080` and `8083` are mutually exclusive: `inbound_interception` picks one +inbound mechanism, and the preset fills only that one's address. + +`8082` and `8083` are the iptables REDIRECT targets installed by +[`proxy-init`](../proxy-init/) and must match its `TRANSPARENT_PORT` / +`INBOUND_TRANSPARENT_PORT`. A mismatch redirects traffic to a dead port. + **Envoy-sidecar (`authbridge-envoy`):** | Port | Purpose | @@ -57,8 +66,10 @@ ConfigMap contracts are documented in ## Choosing a binary -- **Default deployment**: use `authbridge-proxy`. No iptables, no - Envoy, observable via abctl. +- **Default deployment**: use `authbridge-proxy`. No Envoy, observable via + abctl. Cooperative egress (HTTP_PROXY) needs no iptables; the always-on + `enforce-redirect` egress guard and the opt-in transparent inbound listener + both use [`proxy-init`](../proxy-init/). - **Need ambient/transparent interception via Envoy**: use `authbridge-envoy`. Requires the [`proxy-init`](../proxy-init/) iptables init container. diff --git a/authbridge/proxy-init/README.md b/authbridge/proxy-init/README.md index f05ea1867..8ce991745 100644 --- a/authbridge/proxy-init/README.md +++ b/authbridge/proxy-init/README.md @@ -110,13 +110,45 @@ whichever the host kernel exposes. Override with `IPTABLES_CMD` (and | `PROXY_PORT` | `15123` | redirect | AuthBridge outbound listener port | | `INBOUND_PROXY_PORT` | `15124` | redirect | AuthBridge inbound listener port | | `TRANSPARENT_PORT` | `8082` | enforce-redirect | AuthBridge transparent listener port; REDIRECT target for captured external TCP egress | +| `INBOUND_TRANSPARENT_PORT` | (empty = off) | enforce-redirect | AuthBridge **inbound** transparent listener port; PREROUTING REDIRECT target. Opt-in — see below. Requires `POD_IP`. | +| `SIDECAR_PORTS_EXCLUDE` | `8081,9091,9093,9094` | enforce-redirect + inbound | AuthBridge's own listeners, exempted from the inbound REDIRECT. Override when the forward proxy is not on 8081. | | `OUTBOUND_PORTS_EXCLUDE` | (empty) | redirect | Comma-separated outbound port list to skip (e.g. `8080`) | -| `INBOUND_PORTS_EXCLUDE` | (empty) | redirect | Comma-separated inbound port list to skip | -| `POD_IP` | (required in `redirect`) | redirect | Set via Downward API; DNAT target for ambient-mesh inbound. Not used by `enforce-redirect`. | +| `INBOUND_PORTS_EXCLUDE` | (empty) | redirect + enforce-redirect w/ inbound | Comma-separated inbound app-port list to skip validation for (e.g. an oauth-proxy doing its own auth) | +| `POD_IP` | required in `redirect`, and in `enforce-redirect` when `INBOUND_TRANSPARENT_PORT` is set | both | Set via Downward API; DNAT target for ambient-mesh inbound | | `RESOLV_CONF` | `/etc/resolv.conf` | enforce-redirect | Path read at init for `nameserver` IPs; DNS (`tcp/53` + `udp/53`) to those IPs is left direct (IPv4→`iptables`, IPv6→`ip6tables`). Override mainly for tests. | | `IPTABLES_CMD` | auto-detected | all | Override iptables binary (`iptables-legacy` / `iptables-nft`) | | `IP6TABLES_CMD` | derived from `IPTABLES_CMD` | enforce-redirect | Override ip6tables binary | +## Transparent inbound (opt-in) + +Setting `INBOUND_TRANSPARENT_PORT` under `MODE=enforce-redirect` adds the +**inbound** counterpart of the egress guard, so JWT validation cannot be +sidestepped by another pod dialing the agent's real port. Without it, +`enforce-redirect` is egress-only. + +Inbound arrives by two capturable paths and both are covered — handling only the +first would silently wave all mesh traffic through: + +| Path | Netfilter hook | Rule | +|---|---|---| +| Plain network (ClusterIP / NodePort / non-mesh pod) | `nat PREROUTING` | `AB_INBOUND` chain, inserted at position 1 to precede Istio's `ISTIO_PRERT` | +| Istio ambient HBONE | `nat OUTPUT` | ztunnel terminates mTLS and re-originates a LOCAL connection, so PREROUTING never runs. A mark-based `DNAT` at the **head** of `AB_REDIRECT` — it must precede that chain's ztunnel-mark `RETURN`, which would otherwise let every mesh-delivered request through unvalidated. | + +`POD_IP` is required (and its absence is fail-closed at init) because the ambient +rule DNATs to it. `REDIRECT` cannot be used there: it hardcodes the destination +to `127.0.0.1`, and ztunnel preserves the client IP via `IP_TRANSPARENT`, so the +resulting packet is dropped as martian without `route_localnet=1`. + +Intra-pod loopback is deliberately **not** captured. Containers share a network +namespace and are a single entity to every network enforcement layer, so that +traffic is inside the trust boundary — and capturing it would break AuthBridge's +own forward hop, which targets `127.0.0.1:` by design. That also +means **the app must bind `0.0.0.0`**, not only its pod IP. + +Pair with authbridge's `listener.inbound_interception: transparent` and +`listener.transparent_inbound_addr` (preset default `:8083`); the ports must +match or inbound traffic is redirected to a dead port. + ## Required Kubernetes capabilities The container needs `NET_ADMIN` and `NET_RAW` capabilities and runs as @@ -142,8 +174,14 @@ in [`.github/workflows/build.yaml`](../../.github/workflows/build.yaml)). it asserts the `AB_REDIRECT` / `AB_NOTCP` rule structure, proves external TCP is captured to `TRANSPARENT_PORT` while preempting a simulated Istio ambient `nat OUTPUT` REDIRECT, and proves external UDP is dropped — all via -packet counters. Requires root + iptables-nft on Linux (runs on CI; not -macOS): +packet counters. + +With transparent inbound it additionally asserts that `AB_INBOUND` lands at +`nat PREROUTING` position 1 with AuthBridge's own ports exempted (gating `9091` +would put kubelet probes behind JWT validation and crash-loop the pod), that the +ambient DNAT precedes `AB_REDIRECT`'s ztunnel-mark `RETURN`, that a missing +`POD_IP` is fail-closed, and that re-running init does not stack duplicate mark +rules. Requires root + iptables-nft on Linux (runs on CI; not macOS): ```sh sudo ./test-enforce-redirect.sh From 5c9a2bb936d1ca0c05d276829337382f3b8b4f70 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 18 Aug 2026 15:37:11 -0400 Subject: [PATCH 4/6] fix(proxy-init): Apply inbound exemptions on the ambient path too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review caught that the ambient DNAT carried none of AB_INBOUND's exemptions, so every one of them was a silent no-op for mesh-delivered traffic. Consequences were worse than "the operator's exclude list is ignored": - :9091 (health) was captured, so under ambient a JWT-gated readiness probe crash-loops the pod. - The Istio synthetic health-probe source was exempted on PREROUTING only. - ztunnel's own HBONE port and the transparent inbound port were unexempted. - INBOUND_PORTS_EXCLUDE — documented for exactly the OpenShift oauth-proxy 8443 case — did nothing on the mesh path. This is the bug redirect mode already solved in this file ("Rule 1", whose comment names the pitfall verbatim) and it drifted back in because the two hooks had two hand-maintained rule lists. Both are now emitted from one emit_inbound_exemptions function, so they cannot diverge again. RETURN and not ACCEPT, and therefore not a shared sub-chain: an exempt port must fall through to Istio's appended chain to keep ambient mTLS, and a sub-chain RETURN resumes in the caller — landing on the very REDIRECT/DNAT it was meant to skip. Also fixes a dual-stack gap in the same rule. The DNAT target was keyed off POD_IP, the pod's PRIMARY address, so on a dual-stack pod the other family's HBONE delivery hit AB_REDIRECT's ztunnel-mark RETURN and passed unvalidated while that family's PREROUTING rules WERE installed — the half-enforcement this mode refuses to ship elsewhere. Now keyed off POD_IPS (status.podIPs) per family, falling back to POD_IP, and warning explicitly when a family is uncovered rather than leaving it implicit. Two test-quality fixes, both masking real regressions: - The ambient DNAT assertion matched `--uid-owner 1337` under grep -E, so the `.*` absorbed the `!` and dropping the negation still passed — while dropping it would DNAT AuthBridge's own forward hop back into its listener. Now anchored on `! --uid-owner`, plus a separate check that no DNAT rule lacks it. - "Inbound capture test" only asserted the chain was listable. Retitled to what it checks, so the tally does not overstate. Health-probe source exemption now branches on address family instead of suppressing the error, so a genuine IPv4 failure (gated probes) stays loud. New coverage: ambient exemptions present per port and ordered before the DNAT, dual-stack DNAT for both families, and no ambient DNAT without the UID negation. Harness 57/57 (was 45/45); verified the new assertions fail (6 FAILs) when the ambient emit is removed. Refs: rossoctl/cortex#330 Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/proxy-init/README.md | 27 +++- authbridge/proxy-init/init-iptables.sh | 149 +++++++++++++----- .../proxy-init/test-enforce-redirect.sh | 73 ++++++++- 3 files changed, 203 insertions(+), 46 deletions(-) diff --git a/authbridge/proxy-init/README.md b/authbridge/proxy-init/README.md index 8ce991745..02b60fa9e 100644 --- a/authbridge/proxy-init/README.md +++ b/authbridge/proxy-init/README.md @@ -114,7 +114,8 @@ whichever the host kernel exposes. Override with `IPTABLES_CMD` (and | `SIDECAR_PORTS_EXCLUDE` | `8081,9091,9093,9094` | enforce-redirect + inbound | AuthBridge's own listeners, exempted from the inbound REDIRECT. Override when the forward proxy is not on 8081. | | `OUTBOUND_PORTS_EXCLUDE` | (empty) | redirect | Comma-separated outbound port list to skip (e.g. `8080`) | | `INBOUND_PORTS_EXCLUDE` | (empty) | redirect + enforce-redirect w/ inbound | Comma-separated inbound app-port list to skip validation for (e.g. an oauth-proxy doing its own auth) | -| `POD_IP` | required in `redirect`, and in `enforce-redirect` when `INBOUND_TRANSPARENT_PORT` is set | both | Set via Downward API; DNAT target for ambient-mesh inbound | +| `POD_IP` | required in `redirect`, and in `enforce-redirect` when `INBOUND_TRANSPARENT_PORT` is set | both | Set via Downward API (`status.podIP`); DNAT target for ambient-mesh inbound | +| `POD_IPS` | falls back to `POD_IP` | enforce-redirect w/ inbound | Set via Downward API (`status.podIPs`). Supplies a per-family DNAT target so a dual-stack pod covers ambient on BOTH families — `POD_IP` alone is the primary address, leaving the other family's HBONE delivery unvalidated | | `RESOLV_CONF` | `/etc/resolv.conf` | enforce-redirect | Path read at init for `nameserver` IPs; DNS (`tcp/53` + `udp/53`) to those IPs is left direct (IPv4→`iptables`, IPv6→`ip6tables`). Override mainly for tests. | | `IPTABLES_CMD` | auto-detected | all | Override iptables binary (`iptables-legacy` / `iptables-nft`) | | `IP6TABLES_CMD` | derived from `IPTABLES_CMD` | enforce-redirect | Override ip6tables binary | @@ -139,6 +140,23 @@ rule DNATs to it. `REDIRECT` cannot be used there: it hardcodes the destination to `127.0.0.1`, and ztunnel preserves the client IP via `IP_TRANSPARENT`, so the resulting packet is dropped as martian without `route_localnet=1`. +**The exemptions apply to both hooks.** Every port and source exempted from the +PREROUTING chain is also exempted on the ambient path, emitted from a single +`emit_inbound_exemptions` function. This is not incidental: ztunnel delivers via +`OUTPUT`, so an exemption living only in `AB_INBOUND` is a silent no-op for mesh +traffic — a JWT-gated `:9091` crash-loops the pod, and a captured `:8443` breaks +an oauth-proxy doing its own auth. The redirect-mode rules drifted in exactly +this way and needed a second hand-maintained copy in `PROXY_OUTPUT`. + +`RETURN` rather than `ACCEPT`, so an exempt port still falls through to Istio's +appended chain and keeps ambient mTLS. That is also why the exemptions cannot be +a shared sub-chain: a sub-chain `RETURN` resumes in the caller, landing on the +terminal `REDIRECT`/`DNAT` it was meant to skip. + +On a dual-stack pod set `POD_IPS`; with only `POD_IP` the non-primary family's +ambient inbound is not captured, and init warns about it explicitly rather than +leaving it implicit. + Intra-pod loopback is deliberately **not** captured. Containers share a network namespace and are a single entity to every network enforcement layer, so that traffic is inside the trust boundary — and capturing it would break AuthBridge's @@ -180,8 +198,11 @@ With transparent inbound it additionally asserts that `AB_INBOUND` lands at `nat PREROUTING` position 1 with AuthBridge's own ports exempted (gating `9091` would put kubelet probes behind JWT validation and crash-loop the pod), that the ambient DNAT precedes `AB_REDIRECT`'s ztunnel-mark `RETURN`, that a missing -`POD_IP` is fail-closed, and that re-running init does not stack duplicate mark -rules. Requires root + iptables-nft on Linux (runs on CI; not macOS): +`POD_IP` is fail-closed, that the ambient path carries the SAME exemptions as the +PREROUTING chain and that they precede the DNAT, that every ambient DNAT rule +negates the proxy UID, that `POD_IPS` yields a DNAT for both families, and that +re-running init does not stack duplicate mark rules. Requires root + iptables-nft +on Linux (runs on CI; not macOS): ```sh sudo ./test-enforce-redirect.sh diff --git a/authbridge/proxy-init/init-iptables.sh b/authbridge/proxy-init/init-iptables.sh index 3fbe596a2..9206824f8 100644 --- a/authbridge/proxy-init/init-iptables.sh +++ b/authbridge/proxy-init/init-iptables.sh @@ -114,7 +114,9 @@ # forwards to 127.0.0.1:. # Ambient HBONE: ztunnel terminates mTLS and re-originates LOCALLY, so it # appears in OUTPUT, not PREROUTING. Captured by a mark-based -# DNAT at the head of AB_REDIRECT (see setup_enforce_redirect). +# DNAT at the head of AB_REDIRECT (see setup_enforce_redirect), +# preceded by the SAME exemptions AB_INBOUND applies — both are +# emitted from emit_inbound_exemptions so they cannot drift. # # Intra-pod loopback is intentionally NOT captured: containers share a netns and # are a single entity to every network enforcement layer, so that traffic is @@ -267,6 +269,29 @@ get_nameservers() { return 0 } +# POD_IPS is the pod's full address list (Downward API status.podIPs, +# comma-separated). Needed because the ambient DNAT target must match the address +# family of the traffic: keying only off POD_IP — the PRIMARY address — leaves the +# other family's HBONE delivery hitting AB_REDIRECT's ztunnel-mark RETURN and +# passing unvalidated, while that family's PREROUTING rules ARE installed. That is +# the half-enforcement this mode otherwise refuses to ship. Falls back to POD_IP +# so an older operator that injects only the singular field still works for its +# family. +POD_IPS="${POD_IPS:-${POD_IP}}" + +# pod_ip_for_family echoes the first POD_IPS entry of that family, or +# nothing when the pod has no address in it. +pod_ip_for_family() { + for _pif in $(echo "${POD_IPS}" | tr ',' ' '); do + if [ "$1" = "v6" ]; then + if is_ipv6 "${_pif}"; then echo "${_pif}"; return 0; fi + else + if is_ipv6 "${_pif}"; then :; else echo "${_pif}"; return 0; fi + fi + done + return 0 +} + # is_ipv6 reports whether an address literal is IPv6, by the presence of a # colon. Sufficient here because the only inputs are resolv.conf nameservers and # the downward-API POD_IP, both of which are bare literals (never host:port). @@ -312,6 +337,61 @@ if [ -n "${INBOUND_TRANSPARENT_PORT}" ] && [ -z "${POD_IP}" ]; then exit 1 fi +# emit_inbound_exemptions [match-prefix...] +# +# Emits the RETURN rules for every port/source that must NOT be inbound- +# intercepted. ONE definition, because inbound arrives over TWO hooks: +# nat PREROUTING (plain network) and nat OUTPUT (ambient — ztunnel terminates +# HBONE and re-originates a LOCAL connection, so it never reaches PREROUTING). +# +# The redirect-mode rules drifted in exactly this way: PROXY_INBOUND's +# exclusions had no effect on ambient traffic until a second, hand-maintained +# copy was added to PROXY_OUTPUT ("Rule 1" below, whose comment names the +# pitfall). Emitting both from one function makes that drift impossible. +# +# RETURN and not ACCEPT: an exempt port must fall through to Istio's appended +# chain (ISTIO_PRERT / ISTIO_OUTPUT) so ambient still terminates mTLS for it. +# ACCEPT would end nat traversal and silently drop the port out of the mesh. +# That also rules out a shared sub-chain of RETURNs — a sub-chain RETURN resumes +# in the caller, landing straight on the terminal REDIRECT/DNAT it was meant to +# skip. +# +# $3.. is an optional match prefix. On the OUTPUT hook it scopes every rule to +# ztunnel's inbound delivery, so egress capture is untouched. +emit_inbound_exemptions() { + _ec="$1"; _ech="$2"; shift 2 + + # Istio rewrites kubelet health checks to a synthetic source. Gating them + # fails probes and crash-loops the pod. The literal is IPv4-only, so emit it + # only for the v4 command — branching on family rather than suppressing the + # error keeps a genuine IPv4 failure (gated probes) loud. + case "${_ec}" in + *ip6tables*) ;; + *) ${_ec} -t nat -A "${_ech}" "$@" -s "${ISTIO_HEALTH_PROBE_SRC}/32" -p tcp -j RETURN ;; + esac + + # The transparent ports themselves (redirecting INBOUND_TRANSPARENT_PORT to + # itself would self-loop), SSH, and ztunnel's HBONE port — which takes mTLS + # tunnels straight from the network and must reach ztunnel's in-pod socket, + # not an HTTP listener. + for _p in "${INBOUND_TRANSPARENT_PORT}" "${TRANSPARENT_PORT}" "${SSH_PORT}" "${ZTUNNEL_HBONE_PORT}"; do + if [ -n "${_p}" ]; then + ${_ec} -t nat -A "${_ech}" "$@" -p tcp --dport "${_p}" -j RETURN + fi + done + + # SIDECAR_PORTS_EXCLUDE: AuthBridge's own listeners (health, stats, + # session-events, forward proxy). Gating health breaks probes; gating the rest + # breaks abctl and operator introspection. + # INBOUND_PORTS_EXCLUDE: the operator/user escape hatch for app ports that must + # not be validated — e.g. an OpenShift oauth-proxy doing its own auth on 8443. + for _p in $(echo "${SIDECAR_PORTS_EXCLUDE},${INBOUND_PORTS_EXCLUDE}" | tr ',' ' '); do + if [ -n "${_p}" ]; then + ${_ec} -t nat -A "${_ech}" "$@" -p tcp --dport "${_p}" -j RETURN + fi + done +} + # ============================================================================= # enforce-redirect mode (proxy-sidecar fail-closed egress guard, capture variant) # ============================================================================= @@ -395,10 +475,19 @@ setup_enforce_redirect() { # unless route_localnet=1 (which needs a privileged write to /proc/sys). # SO_ORIGINAL_DST is unaffected — conntrack records the pre-NAT tuple for DNAT # and REDIRECT identically. - if [ -n "${INBOUND_TRANSPARENT_PORT}" ] && ! is_ipv6 "${POD_IP}"; then + _dnat4=$(pod_ip_for_family v4) + if [ -n "${INBOUND_TRANSPARENT_PORT}" ] && [ -n "${_dnat4}" ]; then + # The SAME exemptions AB_INBOUND applies, scoped to ztunnel's inbound + # delivery and emitted BEFORE the DNAT. Without them every exemption + # (health 9091, the operator's INBOUND_PORTS_EXCLUDE, ztunnel's own HBONE + # port) is a silent no-op on the ambient path — and a JWT-gated 9091 + # crash-loops the pod. + emit_inbound_exemptions "${IPT}" "${REDIR_CHAIN}" \ + -m mark --mark "${ZTUNNEL_MARK}" -m owner ! --uid-owner "${PROXY_UID}" \ + -m addrtype --dst-type LOCAL ${IPT} -t nat -A "${REDIR_CHAIN}" -m mark --mark "${ZTUNNEL_MARK}" \ -m owner ! --uid-owner "${PROXY_UID}" -m addrtype --dst-type LOCAL \ - -p tcp -j DNAT --to-destination "${POD_IP}:${INBOUND_TRANSPARENT_PORT}" + -p tcp -j DNAT --to-destination "${_dnat4}:${INBOUND_TRANSPARENT_PORT}" fi # ztunnel's own sockets (ambient) carry fwmark 0x539 — let them through. ${IPT} -t nat -A "${REDIR_CHAIN}" -m mark --mark "${ZTUNNEL_MARK}" -j RETURN @@ -459,13 +548,23 @@ setup_enforce_redirect() { if command -v "${IP6T%% *}" >/dev/null 2>&1 && ${IP6T} -t nat -L >/dev/null 2>&1; then ${IP6T} -t nat -N "${REDIR_CHAIN}" 2>/dev/null || true ${IP6T} -t nat -F "${REDIR_CHAIN}" - # Ambient inbound DNAT, v6 mirror. Installed only when POD_IP is itself v6: - # the downward API yields the pod's primary address, and a v4 target in - # ip6tables is rejected outright. - if [ -n "${INBOUND_TRANSPARENT_PORT}" ] && is_ipv6 "${POD_IP}"; then + # Ambient inbound DNAT, v6 mirror. Keyed on the pod's v6 address rather than + # on POD_IP's family, so a dual-stack pod (whose primary is usually v4) still + # gets v6 ambient covered. A v4 target in ip6tables is rejected outright. + _dnat6=$(pod_ip_for_family v6) + if [ -n "${INBOUND_TRANSPARENT_PORT}" ] && [ -n "${_dnat6}" ]; then + emit_inbound_exemptions "${IP6T}" "${REDIR_CHAIN}" \ + -m mark --mark "${ZTUNNEL_MARK}" -m owner ! --uid-owner "${PROXY_UID}" \ + -m addrtype --dst-type LOCAL ${IP6T} -t nat -A "${REDIR_CHAIN}" -m mark --mark "${ZTUNNEL_MARK}" \ -m owner ! --uid-owner "${PROXY_UID}" -m addrtype --dst-type LOCAL \ - -p tcp -j DNAT --to-destination "[${POD_IP}]:${INBOUND_TRANSPARENT_PORT}" + -p tcp -j DNAT --to-destination "[${_dnat6}]:${INBOUND_TRANSPARENT_PORT}" + elif [ -n "${INBOUND_TRANSPARENT_PORT}" ]; then + # v6 PREROUTING rules are still installed below, so non-mesh v6 inbound is + # covered; only the v6 ambient path is not. Say so rather than leave it + # implicit — on a v4-only cluster this is expected and harmless. + echo "transparent-inbound: WARNING: no IPv6 pod address in POD_IPS — IPv6 ambient (HBONE) inbound is NOT captured" >&2 + echo "transparent-inbound: set POD_IPS from the Downward API (status.podIPs) if this pod is dual-stack" >&2 fi ${IP6T} -t nat -A "${REDIR_CHAIN}" -m mark --mark "${ZTUNNEL_MARK}" -j RETURN ${IP6T} -t nat -A "${REDIR_CHAIN}" -m owner --uid-owner "${PROXY_UID}" -j RETURN @@ -557,39 +656,7 @@ setup_transparent_inbound() { ${_ic} -t nat -N "${IN_CHAIN}" 2>/dev/null || true ${_ic} -t nat -F "${IN_CHAIN}" - # Istio rewrites kubelet health checks to a synthetic source. Gating them - # would fail probes and crash-loop the pod. - ${_ic} -t nat -A "${IN_CHAIN}" -s "${ISTIO_HEALTH_PROBE_SRC}/32" -p tcp -j RETURN 2>/dev/null || true - - # The transparent ports themselves. Redirecting INBOUND_TRANSPARENT_PORT to - # itself would self-loop (AuthBridge's CheckDst drops it, but not before - # burning a connection); TRANSPARENT_PORT is the egress listener and receives - # nothing from the network. - ${_ic} -t nat -A "${IN_CHAIN}" -p tcp --dport "${INBOUND_TRANSPARENT_PORT}" -j RETURN - ${_ic} -t nat -A "${IN_CHAIN}" -p tcp --dport "${TRANSPARENT_PORT}" -j RETURN - - # AuthBridge's own listeners: health, stats, session-events API, forward - # proxy. Gating health breaks probes; gating the others breaks abctl and - # operator introspection. - for _p in $(echo "${SIDECAR_PORTS_EXCLUDE}" | tr ',' ' '); do - if [ -n "${_p}" ]; then - ${_ic} -t nat -A "${IN_CHAIN}" -p tcp --dport "${_p}" -j RETURN - fi - done - - ${_ic} -t nat -A "${IN_CHAIN}" -p tcp --dport "${SSH_PORT}" -j RETURN - - # ztunnel's HBONE port takes mTLS tunnels straight from the network and must - # reach ztunnel's in-pod socket, not an HTTP listener. - ${_ic} -t nat -A "${IN_CHAIN}" -p tcp --dport "${ZTUNNEL_HBONE_PORT}" -j RETURN - - # Operator/user escape hatch (kagenti.io/inbound-ports-exclude), for app - # ports that must not be validated — e.g. an oauth-proxy doing its own auth. - for _p in $(echo "${INBOUND_PORTS_EXCLUDE}" | tr ',' ' '); do - if [ -n "${_p}" ]; then - ${_ic} -t nat -A "${IN_CHAIN}" -p tcp --dport "${_p}" -j RETURN - fi - done + emit_inbound_exemptions "${_ic}" "${IN_CHAIN}" # Everything else — the app's ports, whichever and however many they are. # SO_ORIGINAL_DST recovers which one each connection targeted, so no per-port diff --git a/authbridge/proxy-init/test-enforce-redirect.sh b/authbridge/proxy-init/test-enforce-redirect.sh index 564789402..d710b93e7 100755 --- a/authbridge/proxy-init/test-enforce-redirect.sh +++ b/authbridge/proxy-init/test-enforce-redirect.sh @@ -223,7 +223,51 @@ assert "operator exclude 8443 honored" 'AB_INBOUND -p tcp -m tcp --dport # The ambient path is the one a PREROUTING-only implementation silently misses. assert "ambient inbound DNAT installed in AB_REDIRECT" \ - "AB_REDIRECT .*0x539.*--uid-owner 1337.*LOCAL.*-j DNAT --to-destination ${POD_IP_MOCK}:${IN_TPORT}" "${innat}" + "AB_REDIRECT .*0x539.*! --uid-owner 1337.*--dst-type LOCAL.*-j DNAT --to-destination ${POD_IP_MOCK}:${IN_TPORT}" "${innat}" +# The negation is load-bearing and easy to lose: without it the rule matches +# AuthBridge's OWN delivery to the app and DNATs it back into the inbound +# listener, looping. Asserted separately so a regex that merely tolerates its +# absence cannot pass. +if echo "${innat}" | grep -E 'AB_REDIRECT.*-j DNAT' | grep -qv '! --uid-owner'; then + echo "FAIL: an ambient DNAT rule lacks '! --uid-owner' (would loop the proxy's own forward hop)"; fail=1 +else + echo "PASS: every ambient DNAT rule negates the proxy UID" +fi + +echo "### Ambient path must honor the SAME exemptions as AB_INBOUND" +# The bug this guards: ztunnel delivers inbound via OUTPUT, so exemptions living +# only in AB_INBOUND are silently a no-op for mesh traffic. A JWT-gated 9091 +# crash-loops the pod; a captured 8443 breaks an oauth-proxy doing its own auth. +for port_desc in "9091 health" "9093 stats" "9094 session-api" "8443 operator-exclude" "15008 ztunnel-hbone" "${IN_TPORT} inbound-transparent"; do + _p=${port_desc%% *}; _d=${port_desc#* } + if echo "${innat}" | grep -qE "AB_REDIRECT.*0x539.*! --uid-owner 1337.*--dst-type LOCAL.*--dport ${_p} -j RETURN"; then + echo "PASS: ambient path exempts ${_p} (${_d})" + else + echo "FAIL: ambient path does NOT exempt ${_p} (${_d}) — exemption is a no-op for mesh traffic"; fail=1 + fi +done +# Ordering: the exemptions are useless if the DNAT is evaluated first. +ex_line=$(echo "${innat}" | grep -nE "AB_REDIRECT.*0x539.*! --uid-owner 1337.*--dst-type LOCAL.*--dport 9091 -j RETURN" | head -1 | cut -d: -f1) +dn_line=$(echo "${innat}" | grep -nE 'AB_REDIRECT.*-j DNAT' | head -1 | cut -d: -f1) +if [ -n "${ex_line}" ] && [ -n "${dn_line}" ] && [ "${ex_line}" -lt "${dn_line}" ]; then + echo "PASS: ambient exemptions precede the DNAT" +else + echo "FAIL: ambient exemptions do not precede the DNAT (exempt=${ex_line:-?} dnat=${dn_line:-?})"; fail=1 +fi + +echo "### Health-probe source exemption present on IPv4, absent from IPv6" +# Branching on family rather than suppressing the error: the v4 rule must exist +# (or probes are gated), and the v4-only literal cannot be emitted into ip6tables. +assert "IPv4 health-probe source exempted in AB_INBOUND" \ + "AB_INBOUND -s 169.254.7.127/32 -p tcp -j RETURN" "${innat}" +in6nat=$(ip6tables-nft -t nat -S 2>/dev/null || true) +if [ -n "${in6nat}" ]; then + if echo "${in6nat}" | grep -q "169.254.7.127"; then + echo "FAIL: IPv4 health-probe literal leaked into the IPv6 ruleset"; fail=1 + else + echo "PASS: IPv4-only health-probe literal not emitted into ip6tables" + fi +fi assert "forward-hop mark rule in mangle OUTPUT" \ 'OUTPUT -p tcp -m owner --uid-owner 1337 -m addrtype --dst-type LOCAL -j MARK --set-x?mark 0x539' "${inmangle}" @@ -256,7 +300,7 @@ else echo "FAIL: mark rule stacked ${markcount} times across re-runs (expected 1)"; fail=1 fi -echo "### Inbound capture test: packet to the app port must hit AB_INBOUND" +echo "### AB_INBOUND chain is present in the live backend (not a capture test)" # PREROUTING only sees packets arriving on an interface, which a netns cannot # easily synthesise without a peer. Assert the REDIRECT rule exists and is # reachable instead; live capture is covered by the Kind e2e. @@ -266,6 +310,31 @@ else echo "FAIL: AB_INBOUND chain missing from the live backend"; fail=1 fi +echo "### Dual-stack: POD_IPS must drive a DNAT for BOTH families" +# With only POD_IP (primary, usually v4) the other family's HBONE delivery hit +# AB_REDIRECT's ztunnel-mark RETURN and passed unvalidated, while that family's +# PREROUTING rules WERE installed — exactly the half-enforcement this mode +# refuses to ship elsewhere. +env MODE=enforce-redirect PROXY_UID=1337 RESOLV_CONF="${RESOLV_MOCK}" \ + TRANSPARENT_PORT="${TPORT}" INBOUND_TRANSPARENT_PORT="${IN_TPORT}" \ + POD_IP="${POD_IP_MOCK}" POD_IPS="${POD_IP_MOCK},fd00:10:244::7" \ + IPTABLES_CMD="${IPT}" IP6TABLES_CMD=ip6tables-nft \ + sh "${INIT}" >/dev/null 2>&1 || { echo "FAIL: init failed with dual-stack POD_IPS"; fail=1; } +ds4=$("${IPT}" -t nat -S 2>/dev/null || true) +ds6=$(ip6tables-nft -t nat -S 2>/dev/null || true) +if echo "${ds4}" | grep -qE "AB_REDIRECT.*-j DNAT --to-destination ${POD_IP_MOCK}:${IN_TPORT}"; then + echo "PASS: dual-stack v4 ambient DNAT installed" +else + echo "FAIL: dual-stack v4 ambient DNAT missing"; fail=1 +fi +if [ -n "${ds6}" ]; then + if echo "${ds6}" | grep -qE "AB_REDIRECT.*-j DNAT --to-destination \[?fd00:10:244::7\]?:${IN_TPORT}"; then + echo "PASS: dual-stack v6 ambient DNAT installed (v6 HBONE cannot bypass)" + else + echo "FAIL: dual-stack v6 ambient DNAT missing — v6 HBONE delivery bypasses validation"; fail=1 + fi +fi + echo "### Malformed exclude list must not abort init (set -e trap)" # A trailing comma yields an empty field. If the port loop used `[ -n ] && cmd` # as its last statement, the loop would exit non-zero and `set -e` would abort From aee1a052d693ec6779f200c82538f2fb60e33d0f Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 18 Aug 2026 15:37:11 -0400 Subject: [PATCH 5/6] fix(authbridge): Gate the per-connection rewrite; fix a misplaced doc comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings in the Go half. The Director's per-connection target rewrite was live on EVERY server NewServer builds, including fixed-backend ones. It was safe only because nothing populates the context key without an InboundListener — an invariant enforced nowhere. Now gated on a transparentInbound field that only NewTransparentServer sets. That field is deliberately separate from perConnBackend rather than reusing it: perConnBackend is false when a fallback backend is configured, yet such a server still wants the rewrite whenever a destination was recovered. perConnBackend now means only "the target is REQUIRED" (fail closed with 502 when absent). The Server is constructed before the Director so the closure can capture it. Also moves StartTransparentInboundServer's doc comment out of StartReverseProxyServer's. The insertion had landed inside the preceding comment block, so godoc attributed "uses the reverseproxy.Server's Listen() method so the byte-peek TLS-sniffing listener is wired in" to the function that deliberately does the opposite (net.ListenTCP + WrapListener, because SO_ORIGINAL_DST must be read off the raw conn), and left StartReverseProxyServer undocumented. Two tests lock the gate: a fixed-backend server must ignore a recovered destination (injected pointing at a dead port, so an ungated rewrite fails), and a transparent server WITH a fallback must still honor one. Refs: rossoctl/cortex#330 Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../authlib/listener/reverseproxy/server.go | 49 +++++++---- .../reverseproxy/transparent_inbound_test.go | 88 +++++++++++++++++++ authbridge/authlib/runtimeutil/runtimeutil.go | 38 ++++---- 3 files changed, 139 insertions(+), 36 deletions(-) diff --git a/authbridge/authlib/listener/reverseproxy/server.go b/authbridge/authlib/listener/reverseproxy/server.go index 1e56d6bfc..579986f12 100644 --- a/authbridge/authlib/listener/reverseproxy/server.go +++ b/authbridge/authlib/listener/reverseproxy/server.go @@ -58,12 +58,19 @@ type Server struct { proxy *httputil.ReverseProxy backend string - // perConnBackend makes the forwarding target per-connection rather than - // fixed: the Director resolves it from the original destination the - // transparent inbound listener recovered via SO_ORIGINAL_DST. Set by - // NewTransparentServer. When true, backend may be empty and a request - // arriving without a recovered destination fails closed (502) rather than - // being forwarded to a guessed target. + // transparentInbound enables the Director's per-connection target rewrite. + // Set by NewTransparentServer only. Without it the rewrite would be live on + // EVERY server NewServer builds, including fixed-backend ones — safe today + // only because nothing populates the context key without an InboundListener, + // an invariant nothing enforces. Deliberately separate from perConnBackend, + // which is false when a fallback backend is configured even though such a + // server still wants the rewrite. + transparentInbound bool + + // perConnBackend additionally makes the target REQUIRED rather than merely + // preferred: with no configured fallback, a request arriving without a + // recovered destination fails closed (502) instead of being forwarded to a + // guessed target. perConnBackend bool // mtlsCfg is the *tls.Config wrapping the local SVID for inbound @@ -101,6 +108,10 @@ func NewServer(inbound *pipeline.Holder, sessions *session.Store, backendURL str if err != nil { return nil, err } + // Declared before the Director so the closure can capture it: the + // transparent-inbound rewrite is gated on s.transparentInbound, which + // NewTransparentServer sets after this constructor returns. + s := &Server{} proxy := httputil.NewSingleHostReverseProxy(target) // The default Director rewrites the outbound scheme/host/path but // deliberately leaves req.Host as the inbound caller's Host (e.g. @@ -123,11 +134,13 @@ func NewServer(inbound *pipeline.Holder, sessions *session.Store, backendURL str // // The client's real IP is preserved by REDIRECT and reaches the app via // X-Forwarded-For, which ReverseProxy appends from req.RemoteAddr. - if dst, ok := transparentproxy.OrigDstFromContext(req.Context()); ok { - if _, port, err := net.SplitHostPort(dst); err == nil { - req.URL.Scheme = "http" - req.URL.Host = net.JoinHostPort("127.0.0.1", port) - req.Host = req.URL.Host + if s.transparentInbound { + if dst, ok := transparentproxy.OrigDstFromContext(req.Context()); ok { + if _, port, err := net.SplitHostPort(dst); err == nil { + req.URL.Scheme = "http" + req.URL.Host = net.JoinHostPort("127.0.0.1", port) + req.Host = req.URL.Host + } } } // Strip the client's Accept-Encoding, but only when a plugin will @@ -159,12 +172,10 @@ func NewServer(inbound *pipeline.Holder, sessions *session.Store, backendURL str // uniform across content types we install via // installStreamingResponseBody. proxy.FlushInterval = -1 - s := &Server{ - InboundPipeline: inbound, - Sessions: sessions, - proxy: proxy, - backend: backendURL, - } + s.InboundPipeline = inbound + s.Sessions = sessions + s.proxy = proxy + s.backend = backendURL if mtls != nil { if mtls.Source == nil { return nil, fmt.Errorf("reverseproxy: MTLSOptions.Source is required when mtls is non-nil") @@ -207,6 +218,10 @@ func NewTransparentServer(inbound *pipeline.Holder, sessions *session.Store, fal if err != nil { return nil, err } + // Enables the Director's rewrite. Set unconditionally, unlike + // perConnBackend: a transparent server WITH a fallback backend still resolves + // the target per connection whenever one was recovered. + s.transparentInbound = true s.perConnBackend = fallbackBackend == "" return s, nil } diff --git a/authbridge/authlib/listener/reverseproxy/transparent_inbound_test.go b/authbridge/authlib/listener/reverseproxy/transparent_inbound_test.go index 95ae2ce11..69ad9c1dd 100644 --- a/authbridge/authlib/listener/reverseproxy/transparent_inbound_test.go +++ b/authbridge/authlib/listener/reverseproxy/transparent_inbound_test.go @@ -217,3 +217,91 @@ func TestWrapListener_NoMTLSIsPassthrough(t *testing.T) { t.Errorf("WrapListener with mTLS off = %T, want the same listener back", got) } } + +// TestFixedBackendIgnoresRecoveredDestination locks the gate on the Director's +// rewrite. The closure is installed by NewServer, so without an explicit flag it +// is live on every fixed-backend server too — safe only while nothing populates +// the context key without an InboundListener, which nothing enforces. A stray +// key must not silently redirect a port-stealing deployment's traffic. +func TestFixedBackendIgnoresRecoveredDestination(t *testing.T) { + var reached string + app := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reached = r.Host + w.WriteHeader(http.StatusOK) + })) + defer app.Close() + + // A fixed-backend server, exactly as the reverse-proxy mechanism builds it. + srv, err := NewServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, app.URL, nil) + if err != nil { + t.Fatal(err) + } + if srv.transparentInbound { + t.Fatal("NewServer must not enable the transparent rewrite") + } + + // Inject a destination naming a port the app is NOT on. If the rewrite were + // ungated, the request would be sent to loopback:9 and never arrive. + proxy := httptest.NewServer(withOrigDst("10.244.0.5:9", srv.Handler())) + defer proxy.Close() + + req, _ := http.NewRequest("GET", proxy.URL+"/api/data", nil) + req.Header.Set("Authorization", "Bearer valid-token") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want 200: a fixed-backend server must ignore the recovered destination", resp.StatusCode) + } + if want := portOfURL(t, app.URL); reached == "" || reached == net.JoinHostPort("127.0.0.1", "9") { + t.Errorf("request did not reach the configured backend (app Host=%q, want the :%s backend)", reached, want) + } +} + +// TestTransparentServerWithFallbackStillRewrites guards the reason +// transparentInbound is separate from perConnBackend: a transparent server that +// also has a fallback backend must still honor a recovered destination. +func TestTransparentServerWithFallbackStillRewrites(t *testing.T) { + var reached string + app := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reached = r.Host + w.WriteHeader(http.StatusOK) + })) + defer app.Close() + + fallback := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + t.Error("fallback must not be used when a destination was recovered") + w.WriteHeader(http.StatusOK) + })) + defer fallback.Close() + + srv, err := NewTransparentServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, fallback.URL, nil) + if err != nil { + t.Fatal(err) + } + if srv.perConnBackend { + t.Fatal("a configured fallback must leave perConnBackend false") + } + if !srv.transparentInbound { + t.Fatal("a transparent server must enable the rewrite even with a fallback") + } + + dst := net.JoinHostPort("10.244.0.5", portOfURL(t, app.URL)) + proxy := httptest.NewServer(withOrigDst(dst, srv.Handler())) + defer proxy.Close() + + req, _ := http.NewRequest("GET", proxy.URL+"/api/data", nil) + req.Header.Set("Authorization", "Bearer valid-token") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + if want := net.JoinHostPort("127.0.0.1", portOfURL(t, app.URL)); reached != want { + t.Errorf("app saw Host = %q, want %q (recovered destination should win over the fallback)", reached, want) + } +} diff --git a/authbridge/authlib/runtimeutil/runtimeutil.go b/authbridge/authlib/runtimeutil/runtimeutil.go index f3c53e082..17f053d03 100644 --- a/authbridge/authlib/runtimeutil/runtimeutil.go +++ b/authbridge/authlib/runtimeutil/runtimeutil.go @@ -160,6 +160,25 @@ func StartHTTPServer(name string, handler http.Handler, addr string) (*http.Serv // Logged "mtls" attribute makes the listener mode visible at startup; // operators expecting a separate :8443 port for TLS get a clear hint // that this is the same :8080 with byte-peek detection. +func StartReverseProxyServer(name string, rp *reverseproxy.Server, addr string) (*http.Server, error) { + srv := &http.Server{ + Addr: addr, + Handler: rp.Handler(), + ReadHeaderTimeout: 10 * time.Second, + } + listener, err := rp.Listen(addr) + if err != nil { + return nil, err + } + go func() { + slog.Info("Reverse server listening", "name", name, "addr", listener.Addr().String(), "mtls", rp.MTLSEnabled()) + if err := srv.Serve(listener); err != nil && err != http.ErrServerClosed { + slog.Error("Reverse server failed", "name", name, "error", err) + } + }() + return srv, nil +} + // StartTransparentInboundServer binds the inbound transparent listener and // serves the reverse proxy's handler over it, resolving each request's // forwarding target from the destination the client actually addressed @@ -203,22 +222,3 @@ func StartTransparentInboundServer(name string, rp *reverseproxy.Server, addr st }() return srv, nil } - -func StartReverseProxyServer(name string, rp *reverseproxy.Server, addr string) (*http.Server, error) { - srv := &http.Server{ - Addr: addr, - Handler: rp.Handler(), - ReadHeaderTimeout: 10 * time.Second, - } - listener, err := rp.Listen(addr) - if err != nil { - return nil, err - } - go func() { - slog.Info("Reverse server listening", "name", name, "addr", listener.Addr().String(), "mtls", rp.MTLSEnabled()) - if err := srv.Serve(listener); err != nil && err != http.ErrServerClosed { - slog.Error("Reverse server failed", "name", name, "error", err) - } - }() - return srv, nil -} From 2003008a1ee9d095588dc9c905b1111645d56a6d Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Wed, 19 Aug 2026 08:20:43 -0400 Subject: [PATCH 6/6] =?UTF-8?q?fix:=20Address=20PR=20#776=20review=20?= =?UTF-8?q?=E2=80=94=20remove=20the=20fail-closed=20opt-out,=20accept=20PO?= =?UTF-8?q?D=5FIPS?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four review items. One design change, one correctness fix, two hygiene. Removes NewTransparentServer's fallbackBackend parameter (esnible). It disarmed the fail-closed 502 whenever it was non-empty, so "unattributable inbound request is rejected" flipped to "forwarded to the fallback" as a side effect of an argument that reads like it only adds a backend — a security posture change hidden behind unrelated-looking config, on the one listener whose entire purpose is to be a hard inbound boundary. Removed rather than made explicit, because nothing needed it: the only caller passed "", and the case it would serve is close to unreachable anyway since InboundListener rejects unrecoverable and self-referential destinations at Accept time. That also lets transparentInbound and perConnBackend collapse into one honest flag. If a fallback is ever genuinely wanted it should return as an options struct with a separate fail-closed field, so a caller has to weaken the boundary on purpose. The test that documented the old behavior is replaced by one pinning that the boundary can no longer be weakened from the call site. The POD_IP guard now tests the RESOLVED address list (CodeRabbit). A deployment injecting only status.podIPs has everything the ambient DNAT needs for both families, but aborted at init because the guard checked the singular field — rejecting a strictly better-specified pod. Supplying neither is still fail-closed; both cases are now covered by the harness (59/59). Synchronizes handler-observed state in the reverse-proxy tests (CodeRabbit). gotHost/gotXFF/reached were written in server goroutines and read after Do() returned; the ordering holds in practice but rests on net/http internals rather than an edge the race detector is guaranteed to see, making it a latent -race flake. A small mutex-guarded recorder makes the edge explicit. Disables credential persistence on the proxy-init CI job's checkout (CodeRabbit): it runs a repo script as root via `sudo -E` and needs no git access afterwards, so the job token should not sit in .git/config. Not changed: the transparent_inbound_addr / INBOUND_TRANSPARENT_PORT drift risk (esnible). The suggested mitigations are already in place — the bound address is logged at startup ("Transparent inbound server listening ... addr=:8083"), so it is greppable when debugging connection-refused, and the operator derives BOTH the listener address and the proxy-init env from one field (cfg.Proxy.TransparentInboundPort), so the shipped path cannot drift. Only hand-edited config can, which the docs call out. Verified: authlib 46 packages green under -race, go vet clean, builds for linux and darwin; proxy-init harness 59/59. Refs: rossoctl/cortex#330 Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .github/workflows/ci.yaml | 4 + .../authlib/listener/reverseproxy/server.go | 73 +++++----- .../reverseproxy/transparent_inbound_test.go | 131 +++++++----------- authbridge/cmd/authbridge-proxy/main.go | 2 +- authbridge/proxy-init/README.md | 5 +- authbridge/proxy-init/init-iptables.sh | 10 +- .../proxy-init/test-enforce-redirect.sh | 29 ++++ 7 files changed, 129 insertions(+), 125 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 4075b02ea..ede476b26 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -121,6 +121,10 @@ jobs: timeout-minutes: 10 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + # This job runs a repo script as root via `sudo -E` and needs no git + # access afterwards, so don't leave the job token in .git/config. + persist-credentials: false # The harness builds its rules inside `unshare --net`, so it never touches # the runner's own networking. It needs root for unshare + iptables, the diff --git a/authbridge/authlib/listener/reverseproxy/server.go b/authbridge/authlib/listener/reverseproxy/server.go index 579986f12..63bf71198 100644 --- a/authbridge/authlib/listener/reverseproxy/server.go +++ b/authbridge/authlib/listener/reverseproxy/server.go @@ -58,21 +58,18 @@ type Server struct { proxy *httputil.ReverseProxy backend string - // transparentInbound enables the Director's per-connection target rewrite. - // Set by NewTransparentServer only. Without it the rewrite would be live on - // EVERY server NewServer builds, including fixed-backend ones — safe today - // only because nothing populates the context key without an InboundListener, - // an invariant nothing enforces. Deliberately separate from perConnBackend, - // which is false when a fallback backend is configured even though such a - // server still wants the rewrite. + // transparentInbound marks this as the transparent inbound shape. It does two + // things, and they are deliberately one flag: the Director resolves the + // forwarding target per connection from SO_ORIGINAL_DST, and a request with no + // recovered destination fails closed (502) rather than being forwarded + // anywhere. Set by NewTransparentServer only. + // + // The Director gate is not optional hygiene: that closure is installed by + // NewServer, so without it the rewrite would be live on every fixed-backend + // server too — safe only while nothing populates the context key without an + // InboundListener, which nothing enforces. transparentInbound bool - // perConnBackend additionally makes the target REQUIRED rather than merely - // preferred: with no configured fallback, a request arriving without a - // recovered destination fails closed (502) instead of being forwarded to a - // guessed target. - perConnBackend bool - // mtlsCfg is the *tls.Config wrapping the local SVID for inbound // mTLS, or nil when mTLS is disabled. mtlsMode is consulted by // the byte-peek listener (Listen) to decide whether non-TLS @@ -198,38 +195,38 @@ func NewServer(inbound *pipeline.Holder, sessions *session.Store, backendURL str // NewTransparentServer creates a reverse proxy whose forwarding target is // resolved per connection from the original destination recovered by -// transparentproxy.InboundListener, rather than from a fixed backend URL. +// transparentproxy.InboundListener, rather than from a fixed backend URL. A +// request arriving without a recovered destination is rejected with 502. +// +// There is deliberately no fallback-backend parameter. An earlier draft took one +// and disarmed the 502 whenever it was non-empty, which made "unattributable +// inbound request is rejected" flip to "forwarded to the fallback" as a side +// effect of an argument that reads like it only adds a backend — a security +// posture change hidden behind unrelated-looking config, on the one listener +// whose entire purpose is to be a hard inbound boundary. // -// fallbackBackend may be empty. When it is, a request that arrives with no -// recovered destination is rejected with 502 instead of being forwarded -// somewhere unintended — the transparent listener exists to be a hard inbound -// boundary, so an unattributable request must not be forwarded at all. -func NewTransparentServer(inbound *pipeline.Holder, sessions *session.Store, fallbackBackend string, mtls *MTLSOptions) (*Server, error) { - backend := fallbackBackend - if backend == "" { - // url.Parse("") yields a URL with an empty Host, which would make the - // default Director emit a request with no target. Park the fixed target - // on a sentinel that can never be dialed by accident; the Director - // overrides it whenever a destination was recovered, and handleRequest - // fails closed when one wasn't. - backend = "http://" + unresolvableBackend - } - s, err := NewServer(inbound, sessions, backend, mtls) +// Nothing needed it: the only caller passed "", and the case it would serve is +// close to unreachable anyway, since InboundListener rejects unrecoverable and +// self-referential destinations at Accept time. If a fallback is ever genuinely +// wanted, it should return as an options struct with a separate, explicit +// fail-closed field, so a caller has to weaken the boundary on purpose. +func NewTransparentServer(inbound *pipeline.Holder, sessions *session.Store, mtls *MTLSOptions) (*Server, error) { + // url.Parse("") yields a URL with an empty Host, which would make the default + // Director emit a request with no target. Park the fixed target on a sentinel + // that can never be dialed by accident; the Director overrides it whenever a + // destination was recovered, and handleRequest fails closed when one wasn't. + s, err := NewServer(inbound, sessions, "http://"+unresolvableBackend, mtls) if err != nil { return nil, err } - // Enables the Director's rewrite. Set unconditionally, unlike - // perConnBackend: a transparent server WITH a fallback backend still resolves - // the target per connection whenever one was recovered. s.transparentInbound = true - s.perConnBackend = fallbackBackend == "" return s, nil } -// unresolvableBackend is the parked fixed target for a transparent server with -// no configured fallback. 127.0.0.1:0 is not dialable (port 0 is "pick one" for -// bind, not connect), so a bug that let a request through without a recovered -// destination fails loudly instead of reaching a real service. +// unresolvableBackend is the parked fixed target for a transparent server. +// 127.0.0.1:0 is not dialable (port 0 is "pick one" for bind, not connect), so a +// bug that let a request through without a recovered destination fails loudly +// instead of reaching a real service. const unresolvableBackend = "127.0.0.1:0" // Listen returns a net.Listener bound to addr. When mTLS is configured @@ -293,7 +290,7 @@ func (s *Server) handleRequest(w http.ResponseWriter, r *http.Request) { // is deliberately undialable. Reject here rather than let it reach the // pipeline — a request we can't attribute to a captured destination is one // this listener has no business forwarding. - if s.perConnBackend { + if s.transparentInbound { if _, ok := transparentproxy.OrigDstFromContext(r.Context()); !ok { slog.Warn("reverse-proxy: rejecting request with no recovered destination", "remote", r.RemoteAddr, "host", r.Host, "path", r.URL.Path) diff --git a/authbridge/authlib/listener/reverseproxy/transparent_inbound_test.go b/authbridge/authlib/listener/reverseproxy/transparent_inbound_test.go index 69ad9c1dd..074ef10a6 100644 --- a/authbridge/authlib/listener/reverseproxy/transparent_inbound_test.go +++ b/authbridge/authlib/listener/reverseproxy/transparent_inbound_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "sync" "testing" "github.com/rossoctl/cortex/authbridge/authlib/auth" @@ -13,6 +14,32 @@ import ( "github.com/rossoctl/cortex/authbridge/authlib/plugins/jwtvalidation/validation" ) +// observed records what a test backend saw, under a mutex. +// +// The values are written in the server goroutine and read after +// http.DefaultClient.Do returns. That ordering holds in practice, but it rests on +// net/http internals rather than a synchronization edge the race detector is +// guaranteed to observe — so without this the tests are a latent -race flake +// rather than a genuine data race today. +type observed struct { + mu sync.Mutex + host string + xff string +} + +func (o *observed) record(r *http.Request) { + o.mu.Lock() + defer o.mu.Unlock() + o.host = r.Host + o.xff = r.Header.Get("X-Forwarded-For") +} + +func (o *observed) get() (host, xff string) { + o.mu.Lock() + defer o.mu.Unlock() + return o.host, o.xff +} + // withOrigDst stands in for http.Server.ConnContext + the transparent inbound // listener: it injects a recovered original destination into the request // context, which is the only channel the Director reads it from. @@ -51,16 +78,15 @@ func portOfURL(t *testing.T, raw string) string { // keeping the PORT, which is exactly the on-cluster behavior (the egress guard // RETURNs loopback, so the hop can't be re-captured). func TestTransparentInbound_ForwardsToRecoveredPort(t *testing.T) { - var gotHost, gotXFF string + var seen observed app := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - gotHost = r.Host - gotXFF = r.Header.Get("X-Forwarded-For") + seen.record(r) w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte("app-ok")) })) defer app.Close() - srv, err := NewTransparentServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, "", nil) + srv, err := NewTransparentServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, nil) if err != nil { t.Fatal(err) } @@ -83,6 +109,7 @@ func TestTransparentInbound_ForwardsToRecoveredPort(t *testing.T) { t.Fatalf("status = %d, want 200 (Director should have rewritten to loopback:%s)", resp.StatusCode, portOfURL(t, app.URL)) } + gotHost, gotXFF := seen.get() wantHost := net.JoinHostPort("127.0.0.1", portOfURL(t, app.URL)) if gotHost != wantHost { t.Errorf("app saw Host = %q, want %q (loopback, recovered port)", gotHost, wantHost) @@ -105,7 +132,7 @@ func TestTransparentInbound_NoDestinationFailsClosed(t *testing.T) { })) defer app.Close() - srv, err := NewTransparentServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, "", nil) + srv, err := NewTransparentServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, nil) if err != nil { t.Fatal(err) } @@ -141,7 +168,7 @@ func TestTransparentInbound_StillValidatesJWT(t *testing.T) { Verifier: &mockVerifier{err: fmt.Errorf("invalid token")}, Identity: auth.IdentityConfig{Audiences: []string{"my-app"}}, }) - srv, err := NewTransparentServer(inboundPipelineFromAuth(t, a), nil, "", nil) + srv, err := NewTransparentServer(inboundPipelineFromAuth(t, a), nil, nil) if err != nil { t.Fatal(err) } @@ -163,38 +190,22 @@ func TestTransparentInbound_StillValidatesJWT(t *testing.T) { } } -// TestTransparentInbound_FallbackBackendKeepsFixedTarget covers the non-empty -// fallback: a deployment that wants transparent capture but also a defined -// target for anything arriving without a recovered destination should forward -// rather than 502. -func TestTransparentInbound_FallbackBackendKeepsFixedTarget(t *testing.T) { - app := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("fallback-ok")) - })) - defer app.Close() - - srv, err := NewTransparentServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, app.URL, nil) +// TestTransparentInbound_NoFallbackCanDisarmFailClosed replaces an earlier test +// that asserted a configured fallback backend kept serving requests with no +// recovered destination. That was the footgun: the fail-closed 502 could be +// disarmed by an argument that read like it only added a backend. The constructor +// no longer accepts one, so the boundary cannot be weakened from the call site — +// this test pins that by construction. +func TestTransparentInbound_NoFallbackCanDisarmFailClosed(t *testing.T) { + srv, err := NewTransparentServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, nil) if err != nil { t.Fatal(err) } - if srv.perConnBackend { - t.Fatal("a configured fallback must not enable the fail-closed path") - } - - proxy := httptest.NewServer(srv.Handler()) - defer proxy.Close() - - req, _ := http.NewRequest("GET", proxy.URL+"/api/data", nil) - req.Header.Set("Authorization", "Bearer valid-token") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatal(err) + if !srv.transparentInbound { + t.Fatal("a transparent server must always fail closed on an unattributable request") } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - t.Fatalf("status = %d, want 200 via the configured fallback", resp.StatusCode) + if srv.backend != "http://"+unresolvableBackend { + t.Errorf("backend = %q, want the undialable sentinel so a leak fails loudly", srv.backend) } } @@ -203,7 +214,7 @@ func TestTransparentInbound_FallbackBackendKeepsFixedTarget(t *testing.T) { // *net.TCPListener (needed for SO_ORIGINAL_DST) is not replaced by a wrapper // that would hide it from the unwrap walk. func TestWrapListener_NoMTLSIsPassthrough(t *testing.T) { - srv, err := NewTransparentServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, "", nil) + srv, err := NewTransparentServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, nil) if err != nil { t.Fatal(err) } @@ -224,9 +235,9 @@ func TestWrapListener_NoMTLSIsPassthrough(t *testing.T) { // the context key without an InboundListener, which nothing enforces. A stray // key must not silently redirect a port-stealing deployment's traffic. func TestFixedBackendIgnoresRecoveredDestination(t *testing.T) { - var reached string + var seen observed app := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - reached = r.Host + seen.record(r) w.WriteHeader(http.StatusOK) })) defer app.Close() @@ -256,52 +267,8 @@ func TestFixedBackendIgnoresRecoveredDestination(t *testing.T) { if resp.StatusCode != http.StatusOK { t.Fatalf("status = %d, want 200: a fixed-backend server must ignore the recovered destination", resp.StatusCode) } + reached, _ := seen.get() if want := portOfURL(t, app.URL); reached == "" || reached == net.JoinHostPort("127.0.0.1", "9") { t.Errorf("request did not reach the configured backend (app Host=%q, want the :%s backend)", reached, want) } } - -// TestTransparentServerWithFallbackStillRewrites guards the reason -// transparentInbound is separate from perConnBackend: a transparent server that -// also has a fallback backend must still honor a recovered destination. -func TestTransparentServerWithFallbackStillRewrites(t *testing.T) { - var reached string - app := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - reached = r.Host - w.WriteHeader(http.StatusOK) - })) - defer app.Close() - - fallback := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - t.Error("fallback must not be used when a destination was recovered") - w.WriteHeader(http.StatusOK) - })) - defer fallback.Close() - - srv, err := NewTransparentServer(inboundPipelineFromAuth(t, allowAllAuth()), nil, fallback.URL, nil) - if err != nil { - t.Fatal(err) - } - if srv.perConnBackend { - t.Fatal("a configured fallback must leave perConnBackend false") - } - if !srv.transparentInbound { - t.Fatal("a transparent server must enable the rewrite even with a fallback") - } - - dst := net.JoinHostPort("10.244.0.5", portOfURL(t, app.URL)) - proxy := httptest.NewServer(withOrigDst(dst, srv.Handler())) - defer proxy.Close() - - req, _ := http.NewRequest("GET", proxy.URL+"/api/data", nil) - req.Header.Set("Authorization", "Bearer valid-token") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatal(err) - } - defer resp.Body.Close() - - if want := net.JoinHostPort("127.0.0.1", portOfURL(t, app.URL)); reached != want { - t.Errorf("app saw Host = %q, want %q (recovered destination should win over the fallback)", reached, want) - } -} diff --git a/authbridge/cmd/authbridge-proxy/main.go b/authbridge/cmd/authbridge-proxy/main.go index e6434f971..4fcec77d6 100644 --- a/authbridge/cmd/authbridge-proxy/main.go +++ b/authbridge/cmd/authbridge-proxy/main.go @@ -390,7 +390,7 @@ func main() { // reverse-proxy — the default; one fixed reverse_proxy_backend, reached // because the operator stole the agent's port. if cfg.Listener.InboundTransparent() { - rpSrv, rerr := reverseproxy.NewTransparentServer(inboundH, sessions, "", rpMTLS) + rpSrv, rerr := reverseproxy.NewTransparentServer(inboundH, sessions, rpMTLS) if rerr != nil { log.Fatalf("creating transparent inbound proxy: %v", rerr) } diff --git a/authbridge/proxy-init/README.md b/authbridge/proxy-init/README.md index 02b60fa9e..9ff9797bd 100644 --- a/authbridge/proxy-init/README.md +++ b/authbridge/proxy-init/README.md @@ -114,8 +114,11 @@ whichever the host kernel exposes. Override with `IPTABLES_CMD` (and | `SIDECAR_PORTS_EXCLUDE` | `8081,9091,9093,9094` | enforce-redirect + inbound | AuthBridge's own listeners, exempted from the inbound REDIRECT. Override when the forward proxy is not on 8081. | | `OUTBOUND_PORTS_EXCLUDE` | (empty) | redirect | Comma-separated outbound port list to skip (e.g. `8080`) | | `INBOUND_PORTS_EXCLUDE` | (empty) | redirect + enforce-redirect w/ inbound | Comma-separated inbound app-port list to skip validation for (e.g. an oauth-proxy doing its own auth) | -| `POD_IP` | required in `redirect`, and in `enforce-redirect` when `INBOUND_TRANSPARENT_PORT` is set | both | Set via Downward API (`status.podIP`); DNAT target for ambient-mesh inbound | +| `POD_IP` | required in `redirect` | both | Set via Downward API (`status.podIP`); DNAT target for ambient-mesh inbound | | `POD_IPS` | falls back to `POD_IP` | enforce-redirect w/ inbound | Set via Downward API (`status.podIPs`). Supplies a per-family DNAT target so a dual-stack pod covers ambient on BOTH families — `POD_IP` alone is the primary address, leaving the other family's HBONE delivery unvalidated | + +With `INBOUND_TRANSPARENT_PORT` set, **either** `POD_IP` or `POD_IPS` satisfies the +requirement (the guard tests the resolved list). Supplying neither is fail-closed. | `RESOLV_CONF` | `/etc/resolv.conf` | enforce-redirect | Path read at init for `nameserver` IPs; DNS (`tcp/53` + `udp/53`) to those IPs is left direct (IPv4→`iptables`, IPv6→`ip6tables`). Override mainly for tests. | | `IPTABLES_CMD` | auto-detected | all | Override iptables binary (`iptables-legacy` / `iptables-nft`) | | `IP6TABLES_CMD` | derived from `IPTABLES_CMD` | enforce-redirect | Override ip6tables binary | diff --git a/authbridge/proxy-init/init-iptables.sh b/authbridge/proxy-init/init-iptables.sh index 9206824f8..6fc1d5927 100644 --- a/authbridge/proxy-init/init-iptables.sh +++ b/authbridge/proxy-init/init-iptables.sh @@ -329,11 +329,15 @@ fi # mesh-delivered request, since ztunnel re-originates inbound locally through # OUTPUT and never traverses PREROUTING. A pod that validates direct traffic but # waves through all mesh traffic is far worse than a failed init container. -if [ -n "${INBOUND_TRANSPARENT_PORT}" ] && [ -z "${POD_IP}" ]; then - echo "ERROR: POD_IP is not set but INBOUND_TRANSPARENT_PORT=${INBOUND_TRANSPARENT_PORT} requests inbound interception." >&2 +# Tests the RESOLVED address list, not POD_IP alone: a deployment that injects +# only status.podIPs has everything the ambient DNAT needs (for both families), +# and aborting on the absence of the singular field would reject a strictly +# better-specified pod. +if [ -n "${INBOUND_TRANSPARENT_PORT}" ] && [ -z "${POD_IPS}" ]; then + echo "ERROR: neither POD_IP nor POD_IPS is set, but INBOUND_TRANSPARENT_PORT=${INBOUND_TRANSPARENT_PORT} requests inbound interception." >&2 echo "ERROR: without it the Istio ambient inbound path (ztunnel -> OUTPUT, not PREROUTING) cannot be captured," >&2 echo "ERROR: so mesh traffic would bypass inbound validation entirely. Refusing to start half-enforced." >&2 - echo "Set POD_IP via the Kubernetes Downward API (status.podIP)." >&2 + echo "Set POD_IP (status.podIP) or POD_IPS (status.podIPs) via the Kubernetes Downward API." >&2 exit 1 fi diff --git a/authbridge/proxy-init/test-enforce-redirect.sh b/authbridge/proxy-init/test-enforce-redirect.sh index d710b93e7..1d40321c9 100755 --- a/authbridge/proxy-init/test-enforce-redirect.sh +++ b/authbridge/proxy-init/test-enforce-redirect.sh @@ -335,6 +335,35 @@ if [ -n "${ds6}" ]; then fi fi +echo "### POD_IPS alone must satisfy the inbound guard (POD_IP not required)" +# A deployment that injects only status.podIPs has everything the ambient DNAT +# needs for both families; aborting on the absence of the singular field would +# reject a strictly better-specified pod. +if env MODE=enforce-redirect PROXY_UID=1337 RESOLV_CONF="${RESOLV_MOCK}" \ + TRANSPARENT_PORT="${TPORT}" INBOUND_TRANSPARENT_PORT="${IN_TPORT}" \ + POD_IPS="${POD_IP_MOCK}" \ + IPTABLES_CMD="${IPT}" IP6TABLES_CMD=ip6tables-nft \ + sh "${INIT}" >/dev/null 2>&1; then + only6=$("${IPT}" -t nat -S 2>/dev/null || true) + if echo "${only6}" | grep -qE "AB_REDIRECT.*-j DNAT --to-destination ${POD_IP_MOCK}:${IN_TPORT}"; then + echo "PASS: POD_IPS alone satisfies the guard and yields the ambient DNAT" + else + echo "FAIL: init accepted POD_IPS but installed no ambient DNAT"; fail=1 + fi +else + echo "FAIL: init rejected a pod that supplied POD_IPS but not POD_IP"; fail=1 +fi + +echo "### Neither POD_IP nor POD_IPS must still be fail-closed" +if env MODE=enforce-redirect PROXY_UID=1337 RESOLV_CONF="${RESOLV_MOCK}" \ + TRANSPARENT_PORT="${TPORT}" INBOUND_TRANSPARENT_PORT="${IN_TPORT}" \ + IPTABLES_CMD="${IPT}" IP6TABLES_CMD=ip6tables-nft \ + sh "${INIT}" >/dev/null 2>&1; then + echo "FAIL: init succeeded with neither POD_IP nor POD_IPS"; fail=1 +else + echo "PASS: init still aborts when no pod address is available at all" +fi + echo "### Malformed exclude list must not abort init (set -e trap)" # A trailing comma yields an empty field. If the port loop used `[ -n ] && cmd` # as its last statement, the loop would exit non-zero and `set -e` would abort