From 968147b82690cd6e9801d154c45f10236fab96f7 Mon Sep 17 00:00:00 2001 From: Trey Date: Wed, 9 Sep 2026 07:58:46 -0700 Subject: [PATCH 1/3] Wire backend dial-control policy into vMCP serve.go The WithDialControl options on the vMCP backend client and session factory existed and were tested, but neither was connected to a real policy in production, so the shipped binary gained no private-address protection on backend dials. Implements changes for issue #6564: - Export networking.ProtectedDialerControl so serve.go can install the private-IP-blocking net.Dialer.Control hook - Add BackendAllowPrivateIP config toggle following the *AllowPrivateIP precedent; default refuses private/loopback/link-local dials - Build the policy once via backendDialControl and pass it to both NewSessionFactory and NewHTTPBackendClient - Regenerate CRD manifests and docs for the new spec.config field --- ...olhive.stacklok.dev_virtualmcpservers.yaml | 18 ++++++++ ...olhive.stacklok.dev_virtualmcpservers.yaml | 18 ++++++++ docs/operator/crd-api.md | 1 + pkg/networking/backend_transport_test.go | 31 +++++++++++++ pkg/networking/http_client.go | 16 +++++-- pkg/vmcp/cli/serve.go | 44 +++++++++++++++++-- pkg/vmcp/cli/serve_test.go | 25 +++++++++++ pkg/vmcp/config/config.go | 9 ++++ 8 files changed, 155 insertions(+), 7 deletions(-) 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..e933c46c8f 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 @@ -2469,6 +2469,15 @@ spec: minimum: 1 type: integer type: object + backendAllowPrivateIp: + description: |- + BackendAllowPrivateIP allows the virtual MCP server to dial backend + endpoints that resolve to private, loopback, or link-local addresses. + When false (the default), backend dials into those ranges are refused + after DNS resolution to blunt SSRF / DNS-rebinding, which is the safe + production behavior. Enable only for in-cluster or development + deployments where backends legitimately resolve to private addresses. + type: boolean backends: description: |- Backends defines pre-configured backend servers for static mode. @@ -7432,6 +7441,15 @@ spec: minimum: 1 type: integer type: object + backendAllowPrivateIp: + description: |- + BackendAllowPrivateIP allows the virtual MCP server to dial backend + endpoints that resolve to private, loopback, or link-local addresses. + When false (the default), backend dials into those ranges are refused + after DNS resolution to blunt SSRF / DNS-rebinding, which is the safe + production behavior. Enable only for in-cluster or development + deployments where backends legitimately resolve to private addresses. + type: boolean backends: description: |- Backends defines pre-configured backend servers for static mode. 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..ff7ff43feb 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -2472,6 +2472,15 @@ spec: minimum: 1 type: integer type: object + backendAllowPrivateIp: + description: |- + BackendAllowPrivateIP allows the virtual MCP server to dial backend + endpoints that resolve to private, loopback, or link-local addresses. + When false (the default), backend dials into those ranges are refused + after DNS resolution to blunt SSRF / DNS-rebinding, which is the safe + production behavior. Enable only for in-cluster or development + deployments where backends legitimately resolve to private addresses. + type: boolean backends: description: |- Backends defines pre-configured backend servers for static mode. @@ -7435,6 +7444,15 @@ spec: minimum: 1 type: integer type: object + backendAllowPrivateIp: + description: |- + BackendAllowPrivateIP allows the virtual MCP server to dial backend + endpoints that resolve to private, loopback, or link-local addresses. + When false (the default), backend dials into those ranges are refused + after DNS resolution to blunt SSRF / DNS-rebinding, which is the safe + production behavior. Enable only for in-cluster or development + deployments where backends legitimately resolve to private addresses. + type: boolean backends: description: |- Backends defines pre-configured backend servers for static mode. diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index 99a4ce5f0d..e646863b40 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -413,6 +413,7 @@ _Appears in:_ | `compositeTools` _[vmcp.config.CompositeToolConfig](#vmcpconfigcompositetoolconfig) array_ | CompositeTools defines inline composite tool workflows.
Full workflow definitions are embedded in the configuration.
For Kubernetes, complex workflows can also reference VirtualMCPCompositeToolDefinition CRDs. | | Optional: \{\}
| | `compositeToolRefs` _[vmcp.config.CompositeToolRef](#vmcpconfigcompositetoolref) array_ | CompositeToolRefs references VirtualMCPCompositeToolDefinition resources
for complex, reusable workflows. Only applicable when running in Kubernetes.
Referenced resources must be in the same namespace as the VirtualMCPServer. | | Optional: \{\}
| | `operational` _[vmcp.config.OperationalConfig](#vmcpconfigoperationalconfig)_ | Operational configures operational settings. | | | +| `backendAllowPrivateIp` _boolean_ | BackendAllowPrivateIP allows the virtual MCP server to dial backend
endpoints that resolve to private, loopback, or link-local addresses.
When false (the default), backend dials into those ranges are refused
after DNS resolution to blunt SSRF / DNS-rebinding, which is the safe
production behavior. Enable only for in-cluster or development
deployments where backends legitimately resolve to private addresses. | | Optional: \{\}
| | `metadata` _object (keys:string, values:string)_ | Refer to Kubernetes API documentation for fields of `metadata`. | | | | `telemetry` _[pkg.telemetry.Config](#pkgtelemetryconfig)_ | Telemetry configures OpenTelemetry-based observability for the Virtual MCP server
including distributed tracing, OTLP metrics export, and Prometheus metrics endpoint.
Deprecated (Kubernetes operator only): When deploying via the operator, use
VirtualMCPServer.spec.telemetryConfigRef to reference a shared MCPTelemetryConfig
resource instead. This field remains valid for standalone (non-operator) deployments. | | Optional: \{\}
| | `audit` _[pkg.audit.Config](#pkgauditconfig)_ | Audit configures audit logging for the Virtual MCP server.
When present, audit logs include MCP protocol operations.
See audit.Config for available configuration options. | | Optional: \{\}
| diff --git a/pkg/networking/backend_transport_test.go b/pkg/networking/backend_transport_test.go index 3c4e936d89..d4a8187b60 100644 --- a/pkg/networking/backend_transport_test.go +++ b/pkg/networking/backend_transport_test.go @@ -22,6 +22,37 @@ type roundTripperFunc func(*http.Request) (*http.Response, error) func (f roundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } +// TestProtectedDialerControl verifies the exported dial-control hook refuses +// private/loopback/link-local peers while allowing a public one. It is the +// policy wired into the vMCP backend dial paths by pkg/vmcp/cli. +func TestProtectedDialerControl(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + address string + wantErr bool + }{ + {name: "loopback IPv4 blocked", address: "127.0.0.1:8080", wantErr: true}, + {name: "loopback IPv6 blocked", address: "[::1]:8080", wantErr: true}, + {name: "RFC 1918 blocked", address: "10.0.0.5:443", wantErr: true}, + {name: "link-local blocked", address: "169.254.169.254:80", wantErr: true}, + {name: "public IPv4 allowed", address: "93.184.216.34:443", wantErr: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := ProtectedDialerControl("tcp", tt.address, nil) + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} + // TestCloneDefaultTransportWithDialControl verifies the shared backend // transport construction: a nil control clones DefaultTransport (a distinct // value that still reaches the server), and a non-nil control's hook fires on diff --git a/pkg/networking/http_client.go b/pkg/networking/http_client.go index 94e65ecff1..7e6a6671a1 100644 --- a/pkg/networking/http_client.go +++ b/pkg/networking/http_client.go @@ -101,8 +101,16 @@ func SameHostRedirectPolicy() func(req *http.Request, via []*http.Request) error } } -// Dialer control function for validating addresses prior to connection -func protectedDialerControl(_, address string, _ syscall.RawConn) error { +// ProtectedDialerControl is a net.Dialer.Control hook that refuses to connect to +// private, loopback, or link-local addresses. It runs on the resolved peer IP +// (in address) before the TCP handshake, so it also defends against DNS +// rebinding: a name that passed a host-based check can still resolve to a +// blocked IP, and this hook catches that at dial time. +// +// The signature matches net.Dialer.Control exactly, so it can be passed +// directly to the WithDialControl options in pkg/vmcp/client and +// pkg/vmcp/session, or installed on a net.Dialer. +func ProtectedDialerControl(_, address string, _ syscall.RawConn) error { err := AddressReferencesPrivateIp(address) if err != nil { return err @@ -122,7 +130,7 @@ func protectedDialerControl(_, address string, _ syscall.RawConn) error { // operator-configured target is public; SameHostRedirectPolicy is the // redirect-following counterpart. func NewPrivateIPBlockingDialContext() func(ctx context.Context, network, addr string) (net.Conn, error) { - return (&net.Dialer{Control: protectedDialerControl}).DialContext + return (&net.Dialer{Control: ProtectedDialerControl}).DialContext } // Dial timeouts applied to backend connections. Both match the Go standard @@ -431,7 +439,7 @@ func (b *HttpClientBuilder) Build() (*http.Client, error) { if !b.allowPrivate { transport.DialContext = (&net.Dialer{ - Control: protectedDialerControl, + Control: ProtectedDialerControl, }).DialContext } diff --git a/pkg/vmcp/cli/serve.go b/pkg/vmcp/cli/serve.go index 61620f8b20..254cc5f4ea 100644 --- a/pkg/vmcp/cli/serve.go +++ b/pkg/vmcp/cli/serve.go @@ -15,6 +15,7 @@ import ( "net" "os" "path/filepath" + "syscall" "time" "go.opentelemetry.io/otel/trace" @@ -31,6 +32,7 @@ import ( "github.com/stacklok/toolhive/pkg/container/runtime" "github.com/stacklok/toolhive/pkg/groups" "github.com/stacklok/toolhive/pkg/migration" + "github.com/stacklok/toolhive/pkg/networking" "github.com/stacklok/toolhive/pkg/telemetry" "github.com/stacklok/toolhive/pkg/versions" "github.com/stacklok/toolhive/pkg/vmcp" @@ -144,6 +146,15 @@ func Serve(ctx context.Context, cfg ServeConfig) error { slog.Info("audit logging enabled with default configuration") } + // Warn when the backend SSRF / DNS-rebinding guard is disabled. Both backend + // dial paths (per-call client and session factory) then dial private ranges + // unchecked; this is intended only for in-cluster / development use. + if vmcpCfg.BackendAllowPrivateIP { + slog.Warn("backendAllowPrivateIp is enabled; backend dials into private, loopback, and " + + "link-local ranges are NOT blocked (SSRF / DNS-rebinding guard disabled). " + + "Intended for in-cluster / development use only.") + } + // Load auth server config from sibling file if present. // Skip in quick mode (no config file) — there is no sibling directory to search. var authServerRC *authserverconfig.RunConfig @@ -358,6 +369,12 @@ func Serve(ctx context.Context, cfg ServeConfig) error { sessionFactoryOpts, vmcpsession.WithRequestTimeoutResolver(backendRequestTimeoutResolver(vmcpCfg)), ) + // Guard session-init dials against SSRF / DNS-rebinding into private ranges, + // unless the operator opted out for in-cluster / development use. The same + // policy guards the per-call backend client built in discoverBackends. + if dialControl := backendDialControl(vmcpCfg); dialControl != nil { + sessionFactoryOpts = append(sessionFactoryOpts, vmcpsession.WithDialControl(dialControl)) + } sessionFactory := vmcpsession.NewSessionFactory(outgoingRegistry, sessionFactoryOpts...) // When the optimizer is enabled, its meta-tools are pass-through tools. @@ -545,6 +562,22 @@ func backendRequestTimeoutResolver(cfg *config.Config) func(workloadID string) t } } +// backendDialControl returns the net.Dialer.Control hook that guards backend +// dials against SSRF / DNS-rebinding into private, loopback, or link-local +// ranges. It is the single policy source shared by both production dial paths — +// the per-call backend client (discoverBackends) and the session factory +// (Serve) — so neither can drift from the other. +// +// It returns nil (no guard) when cfg.BackendAllowPrivateIP is true, which is the +// opt-out for in-cluster / development deployments where backends legitimately +// resolve to private addresses. The default (false) returns the guarding hook. +func backendDialControl(cfg *config.Config) func(network, address string, c syscall.RawConn) error { + if cfg != nil && cfg.BackendAllowPrivateIP { + return nil + } + return networking.ProtectedDialerControl +} + // loadAndValidateConfig loads and validates the vMCP configuration file. func loadAndValidateConfig(configPath string) (*config.Config, error) { slog.Info(fmt.Sprintf("Loading configuration from: %s", configPath)) @@ -649,10 +682,15 @@ func discoverBackends( return nil, nil, nil, fmt.Errorf("failed to create outgoing authentication registry: %w", err) } - backendClient, err := vmcpclient.NewHTTPBackendClient( - outgoingRegistry, + clientOpts := []vmcpclient.Option{ vmcpclient.WithRequestTimeoutResolver(backendRequestTimeoutResolver(cfg)), - ) + } + // Guard per-call backend dials against SSRF / DNS-rebinding into private + // ranges, unless the operator opted out. Mirrors the session factory in Serve. + if dialControl := backendDialControl(cfg); dialControl != nil { + clientOpts = append(clientOpts, vmcpclient.WithDialControl(dialControl)) + } + backendClient, err := vmcpclient.NewHTTPBackendClient(outgoingRegistry, clientOpts...) if err != nil { return nil, nil, nil, fmt.Errorf("failed to create backend client: %w", err) } diff --git a/pkg/vmcp/cli/serve_test.go b/pkg/vmcp/cli/serve_test.go index 4054452718..481779e85d 100644 --- a/pkg/vmcp/cli/serve_test.go +++ b/pkg/vmcp/cli/serve_test.go @@ -70,6 +70,31 @@ func TestBackendRequestTimeoutResolver(t *testing.T) { } } +// TestBackendDialControl covers the guarded default and the opt-out path for the +// backend dial-control policy wired into both production dial paths in serve.go. +func TestBackendDialControl(t *testing.T) { + t.Parallel() + + t.Run("guarded by default", func(t *testing.T) { + t.Parallel() + // Nil config and BackendAllowPrivateIP=false both mean "guard". + for _, cfg := range []*config.Config{nil, {}, {BackendAllowPrivateIP: false}} { + control := backendDialControl(cfg) + require.NotNil(t, control, "guarded default must install a dial control") + // The installed control must refuse a private-range dial. + require.Error(t, control("tcp", "127.0.0.1:8080", nil)) + // ...and permit a public one. + require.NoError(t, control("tcp", "93.184.216.34:443", nil)) + } + }) + + t.Run("opt-out disables the guard", func(t *testing.T) { + t.Parallel() + control := backendDialControl(&config.Config{BackendAllowPrivateIP: true}) + require.Nil(t, control, "opt-out must return no dial control so private dials are allowed") + }) +} + // TestLoadAndValidateConfig covers all config-loading paths. func TestLoadAndValidateConfig(t *testing.T) { t.Parallel() diff --git a/pkg/vmcp/config/config.go b/pkg/vmcp/config/config.go index 86008d4f17..db7ed2a483 100644 --- a/pkg/vmcp/config/config.go +++ b/pkg/vmcp/config/config.go @@ -146,6 +146,15 @@ type Config struct { // Operational configures operational settings. Operational *OperationalConfig `json:"operational,omitempty" yaml:"operational,omitempty"` + // BackendAllowPrivateIP allows the virtual MCP server to dial backend + // endpoints that resolve to private, loopback, or link-local addresses. + // When false (the default), backend dials into those ranges are refused + // after DNS resolution to blunt SSRF / DNS-rebinding, which is the safe + // production behavior. Enable only for in-cluster or development + // deployments where backends legitimately resolve to private addresses. + // +optional + BackendAllowPrivateIP bool `json:"backendAllowPrivateIp,omitempty" yaml:"backendAllowPrivateIp,omitempty"` + // Metadata stores additional configuration metadata. Metadata map[string]string `json:"metadata,omitempty" yaml:"metadata,omitempty"` From c9b73c895f70a5346a3188ad7e6b3bd7875c0a5f Mon Sep 17 00:00:00 2001 From: Trey Date: Thu, 10 Sep 2026 08:06:35 -0700 Subject: [PATCH 2/3] Adapt backend dial-control to per-workload resolver PR #6567 generalized session.WithDialControl into session.WithDialControlResolver (per-workload). Update serve.go's session-factory wiring to the resolver shape. The policy is still global (config.BackendAllowPrivateIP), so the resolver returns the same hook for every workload; the shape leaves room for a future per-backend policy. The per-call backend client (client.WithDialControl) is unchanged. --- pkg/vmcp/cli/serve.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/vmcp/cli/serve.go b/pkg/vmcp/cli/serve.go index 254cc5f4ea..fba8f22860 100644 --- a/pkg/vmcp/cli/serve.go +++ b/pkg/vmcp/cli/serve.go @@ -372,8 +372,15 @@ func Serve(ctx context.Context, cfg ServeConfig) error { // Guard session-init dials against SSRF / DNS-rebinding into private ranges, // unless the operator opted out for in-cluster / development use. The same // policy guards the per-call backend client built in discoverBackends. + // + // The policy is currently global (config.BackendAllowPrivateIP), so the + // per-workload resolver returns the same hook for every backend; its shape + // leaves room for a future per-backend policy without re-wiring here. if dialControl := backendDialControl(vmcpCfg); dialControl != nil { - sessionFactoryOpts = append(sessionFactoryOpts, vmcpsession.WithDialControl(dialControl)) + sessionFactoryOpts = append(sessionFactoryOpts, + vmcpsession.WithDialControlResolver(func(string) func(network, address string, c syscall.RawConn) error { + return dialControl + })) } sessionFactory := vmcpsession.NewSessionFactory(outgoingRegistry, sessionFactoryOpts...) From d0f85c67786f6dbde8fcf9fbc5c6efc56bac20b2 Mon Sep 17 00:00:00 2001 From: Trey Date: Thu, 10 Sep 2026 08:17:22 -0700 Subject: [PATCH 3/3] Add missing miniredis import to auth server test #6551 added a miniredis.RunT(t) call to the embedded auth server test but omitted the github.com/alicebob/miniredis/v2 import (the dependency is already in go.mod), so the pkg/authserver/runner test package failed to compile. Add the import to restore the build. --- pkg/authserver/runner/embeddedauthserver_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/authserver/runner/embeddedauthserver_test.go b/pkg/authserver/runner/embeddedauthserver_test.go index 6a125ea470..50d49af77c 100644 --- a/pkg/authserver/runner/embeddedauthserver_test.go +++ b/pkg/authserver/runner/embeddedauthserver_test.go @@ -26,6 +26,7 @@ import ( "testing" "time" + "github.com/alicebob/miniredis/v2" "github.com/ory/fosite" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require"