fix(proxy): gate tunnel-peer fast-path on inbound listener marker

forwardWithTunnelPeer previously accepted any RFC1918 / ULA / CGNAT source IP, so a public client whose address happened to fall in those ranges could bypass the configured operator auth scheme by colliding with a known tunnel IP. The fast-path is now gated on TunnelLookupFromContext(r.Context()) being present — that context value is attached only by the per-account inbound (overlay) listener, so the host-facing listener never enters this branch. Tests updated to reflect the new requirement: requests that don't carry the inbound marker now fall through to the regular auth flow.
2026-09-01 12:31:42 +02:00 · 2026-05-26 14:38:52 +02:00
parent e89b1e0596
commit e09d51c1a8
3 changed files with 151 additions and 23 deletions
--- a/proxy/internal/auth/middleware.go
+++ b/proxy/internal/auth/middleware.go
@@ -346,13 +346,15 @@ func (mw *Middleware) forwardWithSessionCookie(w http.ResponseWriter, r *http.Re
 // management unreachable, peer unknown, user not in group) returns false so
 // the caller falls back to the existing OIDC scheme dispatch.
 //
-// Phase 3 adds a local-first short-circuit: when the request arrived on a
-// per-account inbound listener the context carries a peerstore lookup
-// (TunnelLookupFromContext). If the lookup says the IP isn't in the account's
-// roster the proxy denies fast without calling management. If the lookup
-// confirms a known peer the RPC still runs for the user-identity tail
-// (UserID + group access), but its result is cached for tunnelCacheTTL so
-// repeat requests skip management entirely.
+// The fast-path is gated on TunnelLookupFromContext(r.Context()) being
+// present — that context value is attached only by the per-account
+// inbound (overlay) listener. The host listener never sets it, so a
+// public client whose source IP happens to fall inside an RFC1918 / ULA
+// / CGNAT range can't impersonate a mesh peer by colliding with a
+// tunnel-IP. Once we know the request arrived over WireGuard the
+// per-account peerstore lookup is consulted: a miss denies fast (no
+// management round-trip), a hit gates the cached ValidateTunnelPeer RPC
+// that mints the session JWT.
 func (mw *Middleware) forwardWithTunnelPeer(w http.ResponseWriter, r *http.Request, host string, config DomainConfig, next http.Handler) bool {
 	if mw.sessionValidator == nil {
 		return false
@@ -361,18 +363,24 @@ func (mw *Middleware) forwardWithTunnelPeer(w http.ResponseWriter, r *http.Reque
 	if !clientIP.IsValid() {
 		return false
 	}
+
+	// Anti-spoof: only honour the tunnel-peer fast-path on requests that
+	// were stamped by an overlay listener. Without that marker an
+	// attacker could send a request from a colliding RFC1918 / CGNAT
+	// source on the public listener and bypass operator auth.
+	lookup := TunnelLookupFromContext(r.Context())
+	if lookup == nil {
+		return false
+	}
 	if !isTunnelSourceIP(clientIP) {
 		return false
 	}
-
-	if lookup := TunnelLookupFromContext(r.Context()); lookup != nil {
-		if _, ok := lookup(clientIP); !ok {
-			mw.logger.WithFields(log.Fields{
-				"host":   host,
-				"remote": clientIP,
-			}).Debug("local peerstore: tunnel IP not in account roster; denying without RPC")
-			return false
-		}
+	if _, ok := lookup(clientIP); !ok {
+		mw.logger.WithFields(log.Fields{
+			"host":   host,
+			"remote": clientIP,
+		}).Debug("local peerstore: tunnel IP not in account roster; denying without RPC")
+		return false
 	}

 	resp, _, err := mw.tunnelCache.fetch(r.Context(), tunnelCacheKey{
--- a/proxy/internal/auth/middleware_test.go
+++ b/proxy/internal/auth/middleware_test.go
@@ -1227,3 +1227,93 @@ func TestProtect_NonOIDCSchemes_PlainHTTP_NotBlocked(t *testing.T) {

 	assert.Equal(t, http.StatusUnauthorized, rec.Code, "PIN-only domain should serve the login page on plain HTTP")
 }
+
+// stubTunnelValidator records ValidateTunnelPeer calls so a test can
+// assert whether the fast-path reached management.
+type stubTunnelValidator struct {
+	called bool
+	resp   *proto.ValidateTunnelPeerResponse
+}
+
+func (s *stubTunnelValidator) ValidateSession(context.Context, *proto.ValidateSessionRequest, ...grpc.CallOption) (*proto.ValidateSessionResponse, error) {
+	return nil, errors.New("not used in this test")
+}
+
+func (s *stubTunnelValidator) ValidateTunnelPeer(context.Context, *proto.ValidateTunnelPeerRequest, ...grpc.CallOption) (*proto.ValidateTunnelPeerResponse, error) {
+	s.called = true
+	return s.resp, nil
+}
+
+// TestProtect_TunnelPeerFastPath_RequiresInboundMarker guards the
+// anti-spoof gate: a request with an RFC1918 source IP arriving on the
+// public listener (no TunnelLookupFromContext attached) must not be
+// allowed to take the tunnel-peer fast-path. Without this gate a public
+// client whose source IP happens to fall inside an RFC1918 range could
+// bypass the configured auth scheme by colliding with a known tunnel
+// IP.
+func TestProtect_TunnelPeerFastPath_RequiresInboundMarker(t *testing.T) {
+	validator := &stubTunnelValidator{
+		resp: &proto.ValidateTunnelPeerResponse{
+			Valid:        true,
+			SessionToken: "should-not-be-used",
+			UserId:       "user-1",
+		},
+	}
+	mw := NewMiddleware(log.StandardLogger(), validator, nil)
+	kp := generateTestKeyPair(t)
+
+	scheme := &stubScheme{method: auth.MethodPIN, promptID: "pin"}
+	require.NoError(t, mw.AddDomain("example.com", []Scheme{scheme}, kp.PublicKey, time.Hour, "", "", nil, false))
+
+	handler := mw.Protect(newPassthroughHandler())
+
+	// Request from an RFC1918 source IP on the public listener — no
+	// TunnelLookupFromContext attached. The fast-path must reject this
+	// and fall through to the PIN scheme (which renders 401 on plain
+	// HTTP for a non-authenticated request).
+	req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
+	req.RemoteAddr = "100.64.0.5:5000"
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, req)
+
+	assert.False(t, validator.called,
+		"ValidateTunnelPeer must not be invoked when the request lacks the inbound TunnelLookup marker")
+	assert.Equal(t, http.StatusUnauthorized, rec.Code,
+		"without the inbound marker the request must fall through to the operator auth scheme")
+}
+
+// TestProtect_TunnelPeerFastPath_TakesPathWithInboundMarker verifies
+// the positive side: a request marked as overlay-origin (carrying the
+// TunnelLookup context value) and matching a tunnel-IP range does take
+// the fast-path and reach management.
+func TestProtect_TunnelPeerFastPath_TakesPathWithInboundMarker(t *testing.T) {
+	validator := &stubTunnelValidator{
+		resp: &proto.ValidateTunnelPeerResponse{
+			Valid:        true,
+			SessionToken: "tunnel-session-token",
+			UserId:       "user-1",
+		},
+	}
+	mw := NewMiddleware(log.StandardLogger(), validator, nil)
+	kp := generateTestKeyPair(t)
+
+	scheme := &stubScheme{method: auth.MethodPIN, promptID: "pin"}
+	require.NoError(t, mw.AddDomain("example.com", []Scheme{scheme}, kp.PublicKey, time.Hour, "", "", nil, false))
+
+	handler := mw.Protect(newPassthroughHandler())
+
+	lookup := TunnelLookupFunc(func(_ netip.Addr) (PeerIdentity, bool) {
+		return PeerIdentity{}, true
+	})
+
+	req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil)
+	req.RemoteAddr = "100.64.0.5:5000"
+	req = req.WithContext(WithTunnelLookup(req.Context(), lookup))
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, req)
+
+	assert.True(t, validator.called,
+		"ValidateTunnelPeer must run when the request carries the inbound TunnelLookup marker")
+	assert.Equal(t, http.StatusOK, rec.Code,
+		"a successful tunnel-peer validation must forward to the next handler")
+}
--- a/proxy/internal/auth/tunnel_lookup_test.go
+++ b/proxy/internal/auth/tunnel_lookup_test.go
@@ -101,7 +101,10 @@ func TestForwardWithTunnelPeer_GroupsPropagateToCapturedData(t *testing.T) {

 	w, r := newTunnelRequest("100.64.0.10:55555")
 	cd := proxy.NewCapturedData("")
-	r = r.WithContext(proxy.WithCapturedData(r.Context(), cd))
+	lookup := TunnelLookupFunc(func(_ netip.Addr) (PeerIdentity, bool) {
+		return PeerIdentity{}, true
+	})
+	r = r.WithContext(proxy.WithCapturedData(WithTunnelLookup(r.Context(), lookup), cd))

 	called := false
 	next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { called = true })
@@ -148,9 +151,13 @@ func TestForwardWithTunnelPeer_LocalLookupKnownPeerStillRPCs(t *testing.T) {
 	assert.Equal(t, int32(1), validator.tunnelCalls.Load(), "RPC must run for the user-identity tail when local lookup confirms the peer")
 }

-// TestForwardWithTunnelPeer_NoLookupKeepsLegacyPath ensures the existing
-// behaviour stays intact on the host-level listener (no lookup attached).
-func TestForwardWithTunnelPeer_NoLookupKeepsLegacyPath(t *testing.T) {
+// TestForwardWithTunnelPeer_NoLookupRefusesFastPath guards the
+// anti-spoof gate: requests that didn't arrive on the per-account
+// inbound listener (no TunnelLookup attached) must never reach
+// management's ValidateTunnelPeer, even when the source IP looks like
+// a tunnel address. A colliding RFC1918 / CGNAT source on the public
+// listener would otherwise impersonate a mesh peer.
+func TestForwardWithTunnelPeer_NoLookupRefusesFastPath(t *testing.T) {
 	validator := &stubSessionValidator{
 		respFn: func(_ *proto.ValidateTunnelPeerRequest) *proto.ValidateTunnelPeerResponse {
 			return &proto.ValidateTunnelPeerResponse{Valid: true, SessionToken: "tok", UserId: "user-1"}
@@ -165,9 +172,9 @@ func TestForwardWithTunnelPeer_NoLookupKeepsLegacyPath(t *testing.T) {
 	config, _ := mw.getDomainConfig("svc.example")
 	handled := mw.forwardWithTunnelPeer(w, r, "svc.example", config, next)

-	assert.True(t, handled, "host-level path forwards on positive RPC result")
-	assert.True(t, called, "next handler runs on host-level success")
-	assert.Equal(t, int32(1), validator.tunnelCalls.Load(), "host-level path always RPCs (Phase 3 unchanged)")
+	assert.False(t, handled, "fast-path must refuse without the inbound marker")
+	assert.False(t, called, "next handler must not run")
+	assert.Equal(t, int32(0), validator.tunnelCalls.Load(), "ValidateTunnelPeer must not be invoked without the inbound marker")
 }

 // TestForwardWithTunnelPeer_RPCErrorFallsThrough validates that an RPC
@@ -201,8 +208,13 @@ func TestForwardWithTunnelPeer_CacheReusesPositiveResponse(t *testing.T) {
 	}
 	mw := newTunnelMiddleware(t, validator)

+	lookup := TunnelLookupFunc(func(_ netip.Addr) (PeerIdentity, bool) {
+		return PeerIdentity{}, true
+	})
+
 	for i := 0; i < 4; i++ {
 		w, r := newTunnelRequest("100.64.0.10:55555")
+		r = r.WithContext(WithTunnelLookup(r.Context(), lookup))
 		next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})
 		config, _ := mw.getDomainConfig("svc.example")
 		handled := mw.forwardWithTunnelPeer(w, r, "svc.example", config, next)
@@ -226,11 +238,21 @@ func TestForwardWithTunnelPeer_RoutesAccountIDIntoCacheKey(t *testing.T) {
 	require.NoError(t, mw.AddDomain("svc-a.example", nil, "", 0, "acct-a", "svc-a", nil, false))
 	require.NoError(t, mw.AddDomain("svc-b.example", nil, "", 0, "acct-b", "svc-b", nil, false))

+	// The fast-path requires the inbound-listener marker on the context.
+	// The peerstore lookup itself is account-agnostic at this level
+	// (one TunnelLookupFunc per account is attached by inbound.go); a
+	// trivial "always hit" lookup is enough to exercise the cache-key
+	// branch this test covers.
+	lookup := TunnelLookupFunc(func(_ netip.Addr) (PeerIdentity, bool) {
+		return PeerIdentity{}, true
+	})
+
 	for _, host := range []string{"svc-a.example", "svc-b.example"} {
 		w := httptest.NewRecorder()
 		r := httptest.NewRequest(http.MethodGet, "https://"+host+"/", nil)
 		r.Host = host
 		r.RemoteAddr = "100.64.0.10:55555"
+		r = r.WithContext(WithTunnelLookup(r.Context(), lookup))
 		config, _ := mw.getDomainConfig(host)
 		handled := mw.forwardWithTunnelPeer(w, r, host, config, http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}))
 		require.True(t, handled, "host %s should forward", host)
@@ -314,9 +336,17 @@ func TestPrivateService_ForwardsOnTunnelPeerSuccess(t *testing.T) {
 		w.WriteHeader(http.StatusOK)
 	}))

+	// Per-account inbound listener attaches WithTunnelLookup; without it
+	// forwardWithTunnelPeer refuses to take the fast-path. Mirror the
+	// real flow so this test exercises the post-gating success branch.
+	lookup := TunnelLookupFunc(func(_ netip.Addr) (PeerIdentity, bool) {
+		return PeerIdentity{}, true
+	})
+
 	req := httptest.NewRequest(http.MethodGet, "https://private.svc/", nil)
 	req.Host = "private.svc"
 	req.RemoteAddr = "100.64.0.10:55555"
+	req = req.WithContext(WithTunnelLookup(req.Context(), lookup))
 	w := httptest.NewRecorder()
 	handler.ServeHTTP(w, req)