From 37fb13639b8ef66630de0772d263babd63276eef Mon Sep 17 00:00:00 2001 From: "Juan Carlos Moreno [ackstorm]" Date: Mon, 7 Sep 2026 12:47:30 +0200 Subject: [PATCH] Allow opt-in credential header passthrough in vMCP vMCP rejects Authorization and Cookie in passthroughHeaders unconditionally, which blocks legitimate zero-trust chains where a trusted upstream mints a per-backend, audience-scoped token that the backend verifies itself. Operators with that topology have no way to relay the caller's credential. Add spec.allowCredentialHeaderPassthrough, default false. It unblocks those two names only; Host, hop-by-hop, Transfer-Encoding, Content-Length and X-Forwarded-* stay rejected, since those are smuggling and spoofing vectors rather than a credential policy question. Validation stays at config-parse time, so a bad config still fails at startup. When enabled, vMCP warns at startup naming the credential headers it will forward, and marks every audit event whose request carried one with metadata.extra.credential_header_passthrough. Header names only; values are never recorded. Per-backend behaviour comes from the existing transport chain: the header-forward stage is outermost and skips headers already present, the auth stage is innermost and sets unconditionally, so a backend on a real strategy receives its own token and never the caller's. That ordering is now load-bearing, so document it at both ends and on the field itself. Closes #6394 Co-Authored-By: Claude Opus 5 --- .../api/v1beta1/virtualmcpserver_types.go | 24 +++++- cmd/thv-operator/pkg/vmcpconfig/converter.go | 9 +++ .../pkg/vmcpconfig/converter_test.go | 37 +++++++++ ...olhive.stacklok.dev_virtualmcpservers.yaml | 78 ++++++++++++++++++- ...olhive.stacklok.dev_virtualmcpservers.yaml | 78 ++++++++++++++++++- docs/operator/crd-api.md | 6 +- docs/operator/virtualmcpserver-api.md | 45 ++++++++++- pkg/audit/auditor.go | 47 ++++++++++- pkg/audit/auditor_test.go | 59 ++++++++++++++ pkg/audit/mcp_events.go | 5 ++ pkg/vmcp/config/config.go | 15 ++++ pkg/vmcp/config/validator.go | 33 ++++++-- pkg/vmcp/config/validator_test.go | 57 ++++++++++---- pkg/vmcp/headerforward/transport.go | 4 + pkg/vmcp/server/server.go | 8 ++ .../session/internal/backend/mcp_session.go | 4 + 16 files changed, 471 insertions(+), 38 deletions(-) diff --git a/cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go b/cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go index 80ddb2f6dc..aa77295117 100644 --- a/cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go +++ b/cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go @@ -41,12 +41,32 @@ type VirtualMCPServerSpec struct { // PassthroughHeaders is an allowlist of incoming client request header names // forwarded verbatim to all backends (e.g. an API key the backend resolves to // a user). Takes precedence over config.PassthroughHeaders. Names must not be - // restricted headers (Host, hop-by-hop, X-Forwarded-*). Forwarded headers are - // attacker-influenceable unless a trusted upstream sets them. + // restricted headers (Host, hop-by-hop, X-Forwarded-*). Authorization and + // Cookie additionally require allowCredentialHeaderPassthrough. Forwarded + // headers are attacker-influenceable unless a trusted upstream sets them. // +optional // +listType=atomic PassthroughHeaders []string `json:"passthroughHeaders,omitempty"` + // AllowCredentialHeaderPassthrough opts in to listing Authorization and Cookie + // in passthroughHeaders. Defaults to false, which rejects them at startup. + // Host, hop-by-hop, Transfer-Encoding, Content-Length and X-Forwarded-* stay + // rejected regardless. Enabling it here also enables + // config.allowCredentialHeaderPassthrough, and never disables it. + // + // Only enable it when a trusted upstream mints per-backend, audience-scoped + // credentials: Virtual MCP cannot check that the caller's token was ever + // intended for the backends it reaches. + // + // SECURITY: this is safe only because of the backend transport chain's nesting + // order — header-forward is outermost and skips headers already present, auth + // is innermost and sets unconditionally, so backends on a real auth strategy + // get their own token, not the caller's. Reordering those stages, or making + // header-forward overwrite instead of skip, leaks the caller's credential to + // every backend. See pkg/vmcp/session/internal/backend/mcp_session.go. + // +optional + AllowCredentialHeaderPassthrough bool `json:"allowCredentialHeaderPassthrough,omitempty"` + // ServiceType specifies the Kubernetes service type for the Virtual MCP server // +kubebuilder:validation:Enum=ClusterIP;NodePort;LoadBalancer // +kubebuilder:default=ClusterIP diff --git a/cmd/thv-operator/pkg/vmcpconfig/converter.go b/cmd/thv-operator/pkg/vmcpconfig/converter.go index 018a98693e..5634f9d3a8 100644 --- a/cmd/thv-operator/pkg/vmcpconfig/converter.go +++ b/cmd/thv-operator/pkg/vmcpconfig/converter.go @@ -110,6 +110,8 @@ func (c *Converter) Convert( config.PassthroughHeaders = vmcp.Spec.PassthroughHeaders } + config.AllowCredentialHeaderPassthrough = allowCredentialHeaderPassthrough(vmcp) + // Override name with the CR name (authoritative source) config.Name = vmcp.Name @@ -185,6 +187,13 @@ func (c *Converter) Convert( return config, authServerRC, nil } +// allowCredentialHeaderPassthrough resolves the credential-passthrough opt-in. +// The promoted top-level field only ever enables; neither level can switch the +// other off. +func allowCredentialHeaderPassthrough(vmcp *mcpv1beta1.VirtualMCPServer) bool { + return vmcp.Spec.AllowCredentialHeaderPassthrough || vmcp.Spec.Config.AllowCredentialHeaderPassthrough +} + // convertIncomingAuth converts IncomingAuthConfig from CRD to vmcp config. func (c *Converter) convertIncomingAuth( ctx context.Context, diff --git a/cmd/thv-operator/pkg/vmcpconfig/converter_test.go b/cmd/thv-operator/pkg/vmcpconfig/converter_test.go index 617689f928..2333a9fdb7 100644 --- a/cmd/thv-operator/pkg/vmcpconfig/converter_test.go +++ b/cmd/thv-operator/pkg/vmcpconfig/converter_test.go @@ -2300,3 +2300,40 @@ func TestConverter_PassthroughHeaders(t *testing.T) { }) } } + +func TestConverter_AllowCredentialHeaderPassthrough(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + topLevel bool // spec.allowCredentialHeaderPassthrough + config bool // spec.config.allowCredentialHeaderPassthrough + want bool + }{ + {name: "neither set defaults to false"}, + {name: "top-level enables", topLevel: true, want: true}, + {name: "auto-passthrough: config-level enables, top-level false does not disable", config: true, want: true}, + {name: "both set stays enabled", topLevel: true, config: true, want: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + vmcp := v1beta1test.NewVirtualMCPServer("test-vmcp", "default", + v1beta1test.WithVMCPGroupRef("test-group"), + v1beta1test.WithVMCPIncomingAuth(&mcpv1beta1.IncomingAuthConfig{Type: "anonymous"}), + v1beta1test.WithVMCPConfig(vmcpconfig.Config{AllowCredentialHeaderPassthrough: tt.config}), + v1beta1test.MutateVMCP(func(v *mcpv1beta1.VirtualMCPServer) { + v.Spec.AllowCredentialHeaderPassthrough = tt.topLevel + }), + ) + + converter := newTestConverter(t, newNoOpMockResolver(t)) + ctx := log.IntoContext(context.Background(), logr.Discard()) + config, _, err := converter.Convert(ctx, vmcp, nil) + require.NoError(t, err) + require.NotNil(t, config) + assert.Equal(t, tt.want, config.AllowCredentialHeaderPassthrough) + }) + } +} diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml index 623c5cef27..4e91e69859 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -69,6 +69,25 @@ spec: spec: description: VirtualMCPServerSpec defines the desired state of VirtualMCPServer properties: + allowCredentialHeaderPassthrough: + description: |- + AllowCredentialHeaderPassthrough opts in to listing Authorization and Cookie + in passthroughHeaders. Defaults to false, which rejects them at startup. + Host, hop-by-hop, Transfer-Encoding, Content-Length and X-Forwarded-* stay + rejected regardless. Enabling it here also enables + config.allowCredentialHeaderPassthrough, and never disables it. + + Only enable it when a trusted upstream mints per-backend, audience-scoped + credentials: Virtual MCP cannot check that the caller's token was ever + intended for the backends it reaches. + + SECURITY: this is safe only because of the backend transport chain's nesting + order — header-forward is outermost and skips headers already present, auth + is innermost and sets unconditionally, so backends on a real auth strategy + get their own token, not the caller's. Reordering those stages, or making + header-forward overwrite instead of skip, leaks the caller's credential to + every backend. See pkg/vmcp/session/internal/backend/mcp_session.go. + type: boolean authServerConfig: description: |- AuthServerConfig configures an embedded OAuth authorization server. @@ -2403,6 +2422,21 @@ spec: type: object type: array type: object + allowCredentialHeaderPassthrough: + description: |- + AllowCredentialHeaderPassthrough opts in to listing Authorization and Cookie + in PassthroughHeaders. Defaults to false, which rejects them at startup. + Only enable it when a trusted upstream mints per-backend, audience-scoped + credentials: vMCP cannot check that the caller's token was ever intended for + the backends it reaches. + + SECURITY: this is safe only because of the backend transport chain's nesting + order — header-forward is outermost and skips headers already present, auth + is innermost and Sets unconditionally, so backends on a real auth strategy + get their own token, not the caller's. Reordering those stages, or making + header-forward overwrite instead of skip, leaks the caller's credential to + every backend. See pkg/vmcp/session/internal/backend/mcp_session.go. + type: boolean audit: description: |- Audit configures audit logging for the Virtual MCP server. @@ -4518,8 +4552,9 @@ spec: PassthroughHeaders is an allowlist of incoming client request header names forwarded verbatim to all backends (e.g. an API key the backend resolves to a user). Takes precedence over config.PassthroughHeaders. Names must not be - restricted headers (Host, hop-by-hop, X-Forwarded-*). Forwarded headers are - attacker-influenceable unless a trusted upstream sets them. + restricted headers (Host, hop-by-hop, X-Forwarded-*). Authorization and + Cookie additionally require allowCredentialHeaderPassthrough. Forwarded + headers are attacker-influenceable unless a trusted upstream sets them. items: type: string type: array @@ -5032,6 +5067,25 @@ spec: spec: description: VirtualMCPServerSpec defines the desired state of VirtualMCPServer properties: + allowCredentialHeaderPassthrough: + description: |- + AllowCredentialHeaderPassthrough opts in to listing Authorization and Cookie + in passthroughHeaders. Defaults to false, which rejects them at startup. + Host, hop-by-hop, Transfer-Encoding, Content-Length and X-Forwarded-* stay + rejected regardless. Enabling it here also enables + config.allowCredentialHeaderPassthrough, and never disables it. + + Only enable it when a trusted upstream mints per-backend, audience-scoped + credentials: Virtual MCP cannot check that the caller's token was ever + intended for the backends it reaches. + + SECURITY: this is safe only because of the backend transport chain's nesting + order — header-forward is outermost and skips headers already present, auth + is innermost and sets unconditionally, so backends on a real auth strategy + get their own token, not the caller's. Reordering those stages, or making + header-forward overwrite instead of skip, leaks the caller's credential to + every backend. See pkg/vmcp/session/internal/backend/mcp_session.go. + type: boolean authServerConfig: description: |- AuthServerConfig configures an embedded OAuth authorization server. @@ -7366,6 +7420,21 @@ spec: type: object type: array type: object + allowCredentialHeaderPassthrough: + description: |- + AllowCredentialHeaderPassthrough opts in to listing Authorization and Cookie + in PassthroughHeaders. Defaults to false, which rejects them at startup. + Only enable it when a trusted upstream mints per-backend, audience-scoped + credentials: vMCP cannot check that the caller's token was ever intended for + the backends it reaches. + + SECURITY: this is safe only because of the backend transport chain's nesting + order — header-forward is outermost and skips headers already present, auth + is innermost and Sets unconditionally, so backends on a real auth strategy + get their own token, not the caller's. Reordering those stages, or making + header-forward overwrite instead of skip, leaks the caller's credential to + every backend. See pkg/vmcp/session/internal/backend/mcp_session.go. + type: boolean audit: description: |- Audit configures audit logging for the Virtual MCP server. @@ -9481,8 +9550,9 @@ spec: PassthroughHeaders is an allowlist of incoming client request header names forwarded verbatim to all backends (e.g. an API key the backend resolves to a user). Takes precedence over config.PassthroughHeaders. Names must not be - restricted headers (Host, hop-by-hop, X-Forwarded-*). Forwarded headers are - attacker-influenceable unless a trusted upstream sets them. + restricted headers (Host, hop-by-hop, X-Forwarded-*). Authorization and + Cookie additionally require allowCredentialHeaderPassthrough. Forwarded + headers are attacker-influenceable unless a trusted upstream sets them. items: type: string type: array diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml index 920ac89612..ca6f2f7ce8 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -72,6 +72,25 @@ spec: spec: description: VirtualMCPServerSpec defines the desired state of VirtualMCPServer properties: + allowCredentialHeaderPassthrough: + description: |- + AllowCredentialHeaderPassthrough opts in to listing Authorization and Cookie + in passthroughHeaders. Defaults to false, which rejects them at startup. + Host, hop-by-hop, Transfer-Encoding, Content-Length and X-Forwarded-* stay + rejected regardless. Enabling it here also enables + config.allowCredentialHeaderPassthrough, and never disables it. + + Only enable it when a trusted upstream mints per-backend, audience-scoped + credentials: Virtual MCP cannot check that the caller's token was ever + intended for the backends it reaches. + + SECURITY: this is safe only because of the backend transport chain's nesting + order — header-forward is outermost and skips headers already present, auth + is innermost and sets unconditionally, so backends on a real auth strategy + get their own token, not the caller's. Reordering those stages, or making + header-forward overwrite instead of skip, leaks the caller's credential to + every backend. See pkg/vmcp/session/internal/backend/mcp_session.go. + type: boolean authServerConfig: description: |- AuthServerConfig configures an embedded OAuth authorization server. @@ -2406,6 +2425,21 @@ spec: type: object type: array type: object + allowCredentialHeaderPassthrough: + description: |- + AllowCredentialHeaderPassthrough opts in to listing Authorization and Cookie + in PassthroughHeaders. Defaults to false, which rejects them at startup. + Only enable it when a trusted upstream mints per-backend, audience-scoped + credentials: vMCP cannot check that the caller's token was ever intended for + the backends it reaches. + + SECURITY: this is safe only because of the backend transport chain's nesting + order — header-forward is outermost and skips headers already present, auth + is innermost and Sets unconditionally, so backends on a real auth strategy + get their own token, not the caller's. Reordering those stages, or making + header-forward overwrite instead of skip, leaks the caller's credential to + every backend. See pkg/vmcp/session/internal/backend/mcp_session.go. + type: boolean audit: description: |- Audit configures audit logging for the Virtual MCP server. @@ -4521,8 +4555,9 @@ spec: PassthroughHeaders is an allowlist of incoming client request header names forwarded verbatim to all backends (e.g. an API key the backend resolves to a user). Takes precedence over config.PassthroughHeaders. Names must not be - restricted headers (Host, hop-by-hop, X-Forwarded-*). Forwarded headers are - attacker-influenceable unless a trusted upstream sets them. + restricted headers (Host, hop-by-hop, X-Forwarded-*). Authorization and + Cookie additionally require allowCredentialHeaderPassthrough. Forwarded + headers are attacker-influenceable unless a trusted upstream sets them. items: type: string type: array @@ -5035,6 +5070,25 @@ spec: spec: description: VirtualMCPServerSpec defines the desired state of VirtualMCPServer properties: + allowCredentialHeaderPassthrough: + description: |- + AllowCredentialHeaderPassthrough opts in to listing Authorization and Cookie + in passthroughHeaders. Defaults to false, which rejects them at startup. + Host, hop-by-hop, Transfer-Encoding, Content-Length and X-Forwarded-* stay + rejected regardless. Enabling it here also enables + config.allowCredentialHeaderPassthrough, and never disables it. + + Only enable it when a trusted upstream mints per-backend, audience-scoped + credentials: Virtual MCP cannot check that the caller's token was ever + intended for the backends it reaches. + + SECURITY: this is safe only because of the backend transport chain's nesting + order — header-forward is outermost and skips headers already present, auth + is innermost and sets unconditionally, so backends on a real auth strategy + get their own token, not the caller's. Reordering those stages, or making + header-forward overwrite instead of skip, leaks the caller's credential to + every backend. See pkg/vmcp/session/internal/backend/mcp_session.go. + type: boolean authServerConfig: description: |- AuthServerConfig configures an embedded OAuth authorization server. @@ -7369,6 +7423,21 @@ spec: type: object type: array type: object + allowCredentialHeaderPassthrough: + description: |- + AllowCredentialHeaderPassthrough opts in to listing Authorization and Cookie + in PassthroughHeaders. Defaults to false, which rejects them at startup. + Only enable it when a trusted upstream mints per-backend, audience-scoped + credentials: vMCP cannot check that the caller's token was ever intended for + the backends it reaches. + + SECURITY: this is safe only because of the backend transport chain's nesting + order — header-forward is outermost and skips headers already present, auth + is innermost and Sets unconditionally, so backends on a real auth strategy + get their own token, not the caller's. Reordering those stages, or making + header-forward overwrite instead of skip, leaks the caller's credential to + every backend. See pkg/vmcp/session/internal/backend/mcp_session.go. + type: boolean audit: description: |- Audit configures audit logging for the Virtual MCP server. @@ -9484,8 +9553,9 @@ spec: PassthroughHeaders is an allowlist of incoming client request header names forwarded verbatim to all backends (e.g. an API key the backend resolves to a user). Takes precedence over config.PassthroughHeaders. Names must not be - restricted headers (Host, hop-by-hop, X-Forwarded-*). Forwarded headers are - attacker-influenceable unless a trusted upstream sets them. + restricted headers (Host, hop-by-hop, X-Forwarded-*). Authorization and + Cookie additionally require allowCredentialHeaderPassthrough. Forwarded + headers are attacker-influenceable unless a trusted upstream sets them. items: type: string type: array diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index 12b0c6e172..d0ce3c0dca 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -21,6 +21,8 @@ + + #### pkg.audit.Config @@ -421,6 +423,7 @@ _Appears in:_ | `sessionStorage` _[vmcp.config.SessionStorageConfig](#vmcpconfigsessionstorageconfig)_ | SessionStorage configures session storage for stateful horizontal scaling.
When provider is "redis", the operator injects Redis connection parameters
(address, db, keyPrefix) here. The Redis password is provided separately via
the THV_SESSION_REDIS_PASSWORD environment variable. | | Optional: \{\}
| | `rateLimiting` _[ratelimit.types.RateLimitConfig](#ratelimittypesratelimitconfig)_ | RateLimiting defines rate limiting configuration for the Virtual MCP server.
Requires Redis session storage to be configured for distributed rate limiting. | | Optional: \{\}
| | `passthroughHeaders` _string array_ | PassthroughHeaders is an allowlist of incoming client request header names
forwarded verbatim to all backends. Captured at the vMCP incoming edge by
headerforward.CaptureMiddleware and consumed once at session creation
when the per-session backend client's HeaderForwardConfig is built. Names
must not be in the restricted set (Host, hop-by-hop, X-Forwarded-*, etc.). | | Optional: \{\}
| +| `allowCredentialHeaderPassthrough` _boolean_ | AllowCredentialHeaderPassthrough opts in to listing Authorization and Cookie
in PassthroughHeaders. Defaults to false, which rejects them at startup.
Only enable it when a trusted upstream mints per-backend, audience-scoped
credentials: vMCP cannot check that the caller's token was ever intended for
the backends it reaches.
SECURITY: this is safe only because of the backend transport chain's nesting
order — header-forward is outermost and skips headers already present, auth
is innermost and Sets unconditionally, so backends on a real auth strategy
get their own token, not the caller's. Reordering those stages, or making
header-forward overwrite instead of skip, leaks the caller's credential to
every backend. See pkg/vmcp/session/internal/backend/mcp_session.go. | | Optional: \{\}
| #### vmcp.config.ConflictResolutionConfig @@ -4961,7 +4964,8 @@ _Appears in:_ | --- | --- | --- | --- | | `incomingAuth` _[api.v1beta1.IncomingAuthConfig](#apiv1beta1incomingauthconfig)_ | IncomingAuth configures authentication for clients connecting to the Virtual MCP server.
Must be explicitly set - use "anonymous" type when no authentication is required.
This field takes precedence over config.IncomingAuth and should be preferred because it
supports Kubernetes-native secret references (SecretKeyRef, ConfigMapRef) for secure
dynamic discovery of credentials, rather than requiring secrets to be embedded in config. | | Required: \{\}
| | `outgoingAuth` _[api.v1beta1.OutgoingAuthConfig](#apiv1beta1outgoingauthconfig)_ | OutgoingAuth configures authentication from Virtual MCP to backend MCPServers.
This field takes precedence over config.OutgoingAuth and should be preferred because it
supports Kubernetes-native secret references (SecretKeyRef, ConfigMapRef) for secure
dynamic discovery of credentials, rather than requiring secrets to be embedded in config. | | Optional: \{\}
| -| `passthroughHeaders` _string array_ | PassthroughHeaders is an allowlist of incoming client request header names
forwarded verbatim to all backends (e.g. an API key the backend resolves to
a user). Takes precedence over config.PassthroughHeaders. Names must not be
restricted headers (Host, hop-by-hop, X-Forwarded-*). Forwarded headers are
attacker-influenceable unless a trusted upstream sets them. | | Optional: \{\}
| +| `passthroughHeaders` _string array_ | PassthroughHeaders is an allowlist of incoming client request header names
forwarded verbatim to all backends (e.g. an API key the backend resolves to
a user). Takes precedence over config.PassthroughHeaders. Names must not be
restricted headers (Host, hop-by-hop, X-Forwarded-*). Authorization and
Cookie additionally require allowCredentialHeaderPassthrough. Forwarded
headers are attacker-influenceable unless a trusted upstream sets them. | | Optional: \{\}
| +| `allowCredentialHeaderPassthrough` _boolean_ | AllowCredentialHeaderPassthrough opts in to listing Authorization and Cookie
in passthroughHeaders. Defaults to false, which rejects them at startup.
Host, hop-by-hop, Transfer-Encoding, Content-Length and X-Forwarded-* stay
rejected regardless. Enabling it here also enables
config.allowCredentialHeaderPassthrough, and never disables it.
Only enable it when a trusted upstream mints per-backend, audience-scoped
credentials: Virtual MCP cannot check that the caller's token was ever
intended for the backends it reaches.
SECURITY: this is safe only because of the backend transport chain's nesting
order — header-forward is outermost and skips headers already present, auth
is innermost and sets unconditionally, so backends on a real auth strategy
get their own token, not the caller's. Reordering those stages, or making
header-forward overwrite instead of skip, leaks the caller's credential to
every backend. See pkg/vmcp/session/internal/backend/mcp_session.go. | | Optional: \{\}
| | `serviceType` _string_ | ServiceType specifies the Kubernetes service type for the Virtual MCP server | ClusterIP | Enum: [ClusterIP NodePort LoadBalancer]
Optional: \{\}
| | `sessionAffinity` _string_ | SessionAffinity controls whether the Service routes repeated client connections to the same pod.
MCP protocols (SSE, streamable-http) are stateful, so ClientIP is the default.
Set to "None" for stateless servers or when using an external load balancer with its own affinity. | ClientIP | Enum: [ClientIP None]
Optional: \{\}
| | `serviceAccount` _string_ | ServiceAccount is the name of an already existing service account to use by the Virtual MCP server.
If not specified, a ServiceAccount will be created automatically and used by the Virtual MCP server. | | Optional: \{\}
| diff --git a/docs/operator/virtualmcpserver-api.md b/docs/operator/virtualmcpserver-api.md index 61f13cdd49..ca82907fb3 100644 --- a/docs/operator/virtualmcpserver-api.md +++ b/docs/operator/virtualmcpserver-api.md @@ -198,9 +198,11 @@ spec: Allowlist of incoming client request headers forwarded verbatim to every backend. The value is captured at the auth boundary and injected into the session's outgoing backend requests. **Type**: `[]string`. Names are matched -case-insensitively; restricted headers (`Host`, `Authorization`, -`X-Forwarded-*`, hop-by-hop) are rejected at startup. Takes precedence over -`spec.config.passthroughHeaders`. +case-insensitively; restricted headers (`Host`, `X-Forwarded-*`, +`Transfer-Encoding`, `Content-Length`, hop-by-hop) are rejected at startup. +`Authorization` and `Cookie` are also rejected unless +[`spec.allowCredentialHeaderPassthrough`](#specallowcredentialheaderpassthrough-optional) +is `true`. Takes precedence over `spec.config.passthroughHeaders`. ```yaml spec: @@ -217,6 +219,43 @@ spec: > another replica re-captures them from the next request — keep clients pinned > with `sessionAffinity: ClientIP`. +### `.spec.allowCredentialHeaderPassthrough` (optional) + +Opts in to listing `Authorization` and `Cookie` in `passthroughHeaders`. +**Type**: `bool`. **Default**: `false`, which rejects both at startup. + +The opt-in is scoped to those two names. `Host`, `X-Forwarded-*`, +`Transfer-Encoding`, `Content-Length` and hop-by-hop headers stay rejected +regardless — those are request-smuggling and identity-spoofing vectors, not a +credential policy question. Setting this field also enables +`spec.config.allowCredentialHeaderPassthrough`; it never disables it. + +```yaml +spec: + allowCredentialHeaderPassthrough: true + passthroughHeaders: + - Authorization +``` + +When enabled, vMCP logs a warning at startup naming the credential headers it +will forward, and marks every audit event whose request carried one with +`metadata.extra.credential_header_passthrough` (header names only, never +values). + +Per-backend behaviour is decided by each backend's outgoing auth strategy. A +backend on `token_exchange`, `upstream_inject`, `aws_sts`, `obo` or `xaa` +receives the token that strategy mints — never the caller's. Only backends +resolving to `unauthenticated` receive the forwarded value. + +> **Security:** enabling this forwards the caller's credential verbatim to every +> backend that sets no competing header of its own. vMCP cannot verify the token +> was ever intended for those backends. Enable it only when a trusted upstream +> mints per-backend, audience-scoped credentials (for example a gateway +> implementing [LiteLLM's MCP zero-trust +> pattern](https://docs.litellm.ai/docs/mcp_zero_trust)). A vMCP running its own +> OIDC incoming auth would forward a token minted for *vMCP* to backends that +> are not its audience. + ### `.spec.config.aggregation` (optional) Defines tool aggregation and conflict resolution strategies. diff --git a/pkg/audit/auditor.go b/pkg/audit/auditor.go index 9a6d288668..bf07b890b2 100644 --- a/pkg/audit/auditor.go +++ b/pkg/audit/auditor.go @@ -60,10 +60,29 @@ type Auditor struct { auditLogger *slog.Logger transportType string // e.g., "sse", "streamable-http" logWriter io.Writer + + credentialPassthroughHeaders []string // canonicalized +} + +// AuditorOption configures an Auditor at construction time. +type AuditorOption func(*Auditor) + +// WithCredentialPassthroughHeaders tells the Auditor which credential headers the +// surrounding proxy forwards verbatim to backends, so requests carrying one are +// attributable after the fact. Records header names only, under +// MetadataExtraKeyCredentialPassthrough — never their values. Unset by default. +func WithCredentialPassthroughHeaders(names []string) AuditorOption { + return func(a *Auditor) { + canonical := make([]string, 0, len(names)) + for _, name := range names { + canonical = append(canonical, http.CanonicalHeaderKey(name)) + } + a.credentialPassthroughHeaders = canonical + } } // NewAuditorWithTransport creates a new Auditor with the given configuration and transport information. -func NewAuditorWithTransport(config *Config, transportType string) (*Auditor, error) { +func NewAuditorWithTransport(config *Config, transportType string, opts ...AuditorOption) (*Auditor, error) { var logWriter io.Writer = os.Stdout // default to stdout if config != nil { @@ -77,12 +96,16 @@ func NewAuditorWithTransport(config *Config, transportType string) (*Auditor, er logWriter = w } - return &Auditor{ + auditor := &Auditor{ config: config, auditLogger: NewAuditLogger(logWriter), transportType: transportType, logWriter: logWriter, - }, nil + } + for _, opt := range opts { + opt(auditor) + } + return auditor, nil } // Close closes the underlying log writer if it implements io.Closer. @@ -700,6 +723,24 @@ func (a *Auditor) addMetadata(event *AuditEvent, r *http.Request, duration time. if backendInfo, ok := BackendInfoFromContext(r.Context()); ok && backendInfo != nil && backendInfo.BackendName != "" { event.Metadata.Extra["backend_name"] = backendInfo.BackendName } + + // Read off the inbound request, so this reflects what the caller actually sent + // rather than what the allowlist permits. + if forwarded := a.forwardedCredentialHeaders(r); len(forwarded) > 0 { + event.Metadata.Extra[MetadataExtraKeyCredentialPassthrough] = forwarded + } +} + +// forwardedCredentialHeaders returns the configured credential passthrough header +// names present on r. Values are never returned. +func (a *Auditor) forwardedCredentialHeaders(r *http.Request) []string { + var present []string + for _, name := range a.credentialPassthroughHeaders { + if r.Header.Get(name) != "" { + present = append(present, name) + } + } + return present } // addEventData adds request/response data to the audit event if configured. diff --git a/pkg/audit/auditor_test.go b/pkg/audit/auditor_test.go index 31ca2166c1..136bad4ce1 100644 --- a/pkg/audit/auditor_test.go +++ b/pkg/audit/auditor_test.go @@ -34,6 +34,65 @@ func TestNewAuditor(t *testing.T) { assert.Equal(t, config, auditor.config) } +func TestAuditorCredentialPassthroughMetadata(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + configured []string + reqHeaders map[string]string + want []string // nil means the key must be absent + }{ + { + name: "not configured records nothing", + reqHeaders: map[string]string{"Authorization": "Bearer t"}, + }, + { + name: "configured but header absent records nothing", + configured: []string{"Authorization"}, + }, + { + name: "records only the names present on the request", + configured: []string{"Authorization", "Cookie"}, + reqHeaders: map[string]string{"Authorization": "Bearer t"}, + want: []string{"Authorization"}, + }, + { + name: "non-canonical configuration still matches", + configured: []string{"authorization"}, + reqHeaders: map[string]string{"Authorization": "Bearer t"}, + want: []string{"Authorization"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + auditor, err := NewAuditorWithTransport(&Config{}, "sse", + WithCredentialPassthroughHeaders(tt.configured)) + require.NoError(t, err) + + req := httptest.NewRequest("POST", "/test", nil) + for k, v := range tt.reqHeaders { + req.Header.Set(k, v) + } + + event := &AuditEvent{} + auditor.addMetadata(event, req, time.Millisecond, &responseWriter{}) + + got, ok := event.Metadata.Extra[MetadataExtraKeyCredentialPassthrough] + if tt.want == nil { + assert.False(t, ok, "credential passthrough key must be absent") + return + } + require.True(t, ok) + assert.Equal(t, tt.want, got) + // The credential value itself must never reach the event. + assert.NotContains(t, fmt.Sprint(event.Metadata.Extra), "Bearer t") + }) + } +} + func TestAuditorMiddlewareDisabled(t *testing.T) { t.Parallel() config := &Config{} diff --git a/pkg/audit/mcp_events.go b/pkg/audit/mcp_events.go index 4ca1923f15..b47c28dd98 100644 --- a/pkg/audit/mcp_events.go +++ b/pkg/audit/mcp_events.go @@ -142,4 +142,9 @@ const ( MetadataExtraKeyStepCount = coreaudit.MetadataExtraKeyStepCount // MetadataExtraKeyTimeout is the key for the workflow timeout in milliseconds MetadataExtraKeyTimeout = coreaudit.MetadataExtraKeyTimeout + // MetadataExtraKeyCredentialPassthrough is the key for the credential header + // names the request carried and the proxy forwards verbatim to backends. + // Names only, never values. + //nolint:gosec // G101: this is an audit metadata key name, not a credential value + MetadataExtraKeyCredentialPassthrough = "credential_header_passthrough" ) diff --git a/pkg/vmcp/config/config.go b/pkg/vmcp/config/config.go index 86008d4f17..db0d7b627a 100644 --- a/pkg/vmcp/config/config.go +++ b/pkg/vmcp/config/config.go @@ -202,6 +202,21 @@ type Config struct { // +optional // +listType=atomic PassthroughHeaders []string `json:"passthroughHeaders,omitempty" yaml:"passthroughHeaders,omitempty"` + + // AllowCredentialHeaderPassthrough opts in to listing Authorization and Cookie + // in PassthroughHeaders. Defaults to false, which rejects them at startup. + // Only enable it when a trusted upstream mints per-backend, audience-scoped + // credentials: vMCP cannot check that the caller's token was ever intended for + // the backends it reaches. + // + // SECURITY: this is safe only because of the backend transport chain's nesting + // order — header-forward is outermost and skips headers already present, auth + // is innermost and Sets unconditionally, so backends on a real auth strategy + // get their own token, not the caller's. Reordering those stages, or making + // header-forward overwrite instead of skip, leaks the caller's credential to + // every backend. See pkg/vmcp/session/internal/backend/mcp_session.go. + // +optional + AllowCredentialHeaderPassthrough bool `json:"allowCredentialHeaderPassthrough,omitempty" yaml:"allowCredentialHeaderPassthrough,omitempty"` //nolint:lll } // IncomingAuthConfig configures client authentication to the virtual MCP server. diff --git a/pkg/vmcp/config/validator.go b/pkg/vmcp/config/validator.go index 92ea73e1d8..67546ac148 100644 --- a/pkg/vmcp/config/validator.go +++ b/pkg/vmcp/config/validator.go @@ -591,16 +591,27 @@ func (*DefaultValidator) validateCompositeToolRefs(refs []CompositeToolRef) erro return nil } -// The standalone header-forward middleware deliberately allows Authorization -// (an operator may legitimately forward it), but for vMCP passthrough the -// documented contract is stricter: Authorization and Cookie are rejected at -// startup because forwarding caller-supplied credentials verbatim to every -// backend is a credential-leak footgun, not a pass-through use case. -var vmcpRestrictedHeaders = map[string]bool{ +// vmcpCredentialHeaders are rejected in passthroughHeaders unless +// Config.AllowCredentialHeaderPassthrough is set. That opt-in applies to this set +// only, never to middleware.RestrictedHeaders. +var vmcpCredentialHeaders = map[string]bool{ "Authorization": true, "Cookie": true, } +// CredentialPassthroughHeaders returns the canonicalized credential header names +// in a passthrough allowlist. Empty unless AllowCredentialHeaderPassthrough is +// set, since validatePassthroughHeaders rejects those names otherwise. +func CredentialPassthroughHeaders(passthroughHeaders []string) []string { + var names []string + for _, name := range passthroughHeaders { + if canonical := http.CanonicalHeaderKey(name); vmcpCredentialHeaders[canonical] { + names = append(names, canonical) + } + } + return names +} + func (*DefaultValidator) validatePassthroughHeaders(cfg *Config) error { for i, name := range cfg.PassthroughHeaders { if name == "" { @@ -609,10 +620,18 @@ func (*DefaultValidator) validatePassthroughHeaders(cfg *Config) error { canonical := http.CanonicalHeaderKey(name) - if middleware.RestrictedHeaders[canonical] || vmcpRestrictedHeaders[canonical] { + if middleware.RestrictedHeaders[canonical] { return fmt.Errorf("passthroughHeaders[%d]: %q is a restricted header and cannot be forwarded", i, canonical) } + if vmcpCredentialHeaders[canonical] && !cfg.AllowCredentialHeaderPassthrough { + return fmt.Errorf( + "passthroughHeaders[%d]: %q is a credential header and cannot be forwarded "+ + "unless allowCredentialHeaderPassthrough is true", + i, canonical, + ) + } + if err := httpval.ValidateHeaderName(name); err != nil { return fmt.Errorf("passthroughHeaders[%d]: invalid header name %q: %w", i, name, err) } diff --git a/pkg/vmcp/config/validator_test.go b/pkg/vmcp/config/validator_test.go index 4dbc2a83dc..467c102d99 100644 --- a/pkg/vmcp/config/validator_test.go +++ b/pkg/vmcp/config/validator_test.go @@ -1483,7 +1483,7 @@ func TestValidator_ValidatePassthroughHeaders(t *testing.T) { // validBaseConfig returns a minimally-valid Config so that only // passthroughHeaders validation is under test. - validBaseConfig := func(headers []string) *Config { + validBaseConfig := func(headers []string, allowCredentials bool) *Config { return &Config{ Name: "test-vmcp", Group: "test-group", @@ -1499,15 +1499,17 @@ func TestValidator_ValidatePassthroughHeaders(t *testing.T) { PrefixFormat: "{workload}_", }, }, - PassthroughHeaders: headers, + PassthroughHeaders: headers, + AllowCredentialHeaderPassthrough: allowCredentials, } } tests := []struct { - name string - headers []string - wantErr bool - errMsg string + name string + headers []string + allowCredentials bool + wantErr bool + errMsg string }{ { // nil and []string{} both produce zero iterations in range — same code path. @@ -1530,19 +1532,46 @@ func TestValidator_ValidatePassthroughHeaders(t *testing.T) { errMsg: "X-Forwarded-For", }, { - // Documented contract (virtualmcpserver-api.md): Authorization is - // rejected at startup. Forwarding caller-supplied credentials - // verbatim to every backend is a credential-leak footgun. - name: "Authorization is restricted", + // Credential headers are rejected by default; allowCredentialHeaderPassthrough + // is the only way to forward them. + name: "Authorization is rejected without the opt-in", headers: []string{"authorization"}, wantErr: true, - errMsg: "Authorization", + errMsg: "allowCredentialHeaderPassthrough", }, { - name: "Cookie is restricted", + name: "Cookie is rejected without the opt-in", headers: []string{"Cookie"}, wantErr: true, - errMsg: "Cookie", + errMsg: "allowCredentialHeaderPassthrough", + }, + { + name: "Authorization is allowed with the opt-in", + headers: []string{"authorization"}, + allowCredentials: true, + wantErr: false, + }, + { + name: "Cookie is allowed with the opt-in", + headers: []string{"Cookie"}, + allowCredentials: true, + wantErr: false, + }, + { + // The opt-in is scoped to credential headers; smuggling and spoofing + // vectors stay rejected. + name: "opt-in does not unblock X-Forwarded-For", + headers: []string{"X-Forwarded-For"}, + allowCredentials: true, + wantErr: true, + errMsg: "X-Forwarded-For", + }, + { + name: "opt-in does not unblock Transfer-Encoding", + headers: []string{"Transfer-Encoding"}, + allowCredentials: true, + wantErr: true, + errMsg: "Transfer-Encoding", }, { name: "empty string header name is rejected", @@ -1562,7 +1591,7 @@ func TestValidator_ValidatePassthroughHeaders(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() v := NewValidator() - err := v.Validate(validBaseConfig(tt.headers)) + err := v.Validate(validBaseConfig(tt.headers, tt.allowCredentials)) if tt.wantErr { require.Error(t, err) diff --git a/pkg/vmcp/headerforward/transport.go b/pkg/vmcp/headerforward/transport.go index d939853fa2..604c73e3c1 100644 --- a/pkg/vmcp/headerforward/transport.go +++ b/pkg/vmcp/headerforward/transport.go @@ -43,6 +43,10 @@ type headerForwardRoundTripper struct { // overlapping name those stages still win on the wire. Restricted names are // blocked at resolve time, so user-supplied config cannot reach this point // for Host, hop-by-hop, or X-Forwarded-* anyway. +// +// SECURITY: skipping rather than overwriting is load-bearing for +// Config.AllowCredentialHeaderPassthrough — it is what keeps a caller's forwarded +// Authorization off backends that mint their own token. func (h *headerForwardRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { if len(h.headers) == 0 { return h.base.RoundTrip(req) diff --git a/pkg/vmcp/server/server.go b/pkg/vmcp/server/server.go index c6deb3c6f9..b70bd4756f 100644 --- a/pkg/vmcp/server/server.go +++ b/pkg/vmcp/server/server.go @@ -747,6 +747,9 @@ func (s *Server) Handler(_ context.Context) (http.Handler, error) { auditor, err := audit.NewAuditorWithTransport( s.config.AuditConfig, "streamable-http", // vMCP uses streamable HTTP transport + audit.WithCredentialPassthroughHeaders( + vmcpconfig.CredentialPassthroughHeaders(s.config.PassthroughHeaders), + ), ) if err != nil { return nil, fmt.Errorf("failed to create auditor: %w", err) @@ -793,6 +796,11 @@ func (s *Server) applyForwardedHeaderCapture(next http.Handler) http.Handler { return next } slog.Info("forwarded-header capture enabled for MCP endpoints", "headers", s.config.PassthroughHeaders) + if creds := vmcpconfig.CredentialPassthroughHeaders(s.config.PassthroughHeaders); len(creds) > 0 { + slog.Warn("credential header passthrough is enabled: caller-supplied credentials are "+ + "forwarded verbatim to every backend whose outgoing auth strategy sets no competing header", + "headers", creds) + } return headerforward.CaptureMiddleware(s.config.PassthroughHeaders)(next) } diff --git a/pkg/vmcp/session/internal/backend/mcp_session.go b/pkg/vmcp/session/internal/backend/mcp_session.go index 89ba749020..02ef6c7004 100644 --- a/pkg/vmcp/session/internal/backend/mcp_session.go +++ b/pkg/vmcp/session/internal/backend/mcp_session.go @@ -634,6 +634,10 @@ func createMCPClient( // rejected at resolve time by resolveHeaderForward, so user-supplied // HeaderForward cannot inject them in the first place. // The per-transport sections below may add a size-limiting wrapper on top. + // + // SECURITY: this ordering is load-bearing for Config.AllowCredentialHeaderPassthrough. + // It is what keeps a caller's forwarded Authorization off backends that mint + // their own token. Reordering these stages leaks that credential to every backend. base := backendBaseTransport(dialControl) base = &authRoundTripper{ base: base,