Skip to content

Commit 55d82f5

Browse files
JAORMXclaude
andauthored
Add SSRF protection to RemoteURL validation (#4697)
ValidateRemoteURL now rejects URLs targeting internal and metadata endpoints to prevent SSRF vectors when downstream components fetch user-supplied URLs in-cluster. Blocked ranges: loopback (127.0.0.0/8, ::1), link-local/cloud metadata (169.254.0.0/16), RFC 1918 private (10/8, 172.16/12, 192.168/16), IPv6 ULA (fc00::/7), and the unspecified address (0.0.0.0/8, ::). IPv4-mapped IPv6 addresses (::ffff:127.0.0.1) are normalized to prevent bypass. Blocked hostnames: localhost, kubernetes.default.svc(.cluster.local), cluster.local, metadata.google.internal (with subdomain matching). The MCPServerEntry controller now calls ValidateRemoteURL and reports results through a RemoteURLValidated status condition, consistent with the existing MCPRemoteProxy controller pattern. Closes #4695 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1f658a2 commit 55d82f5

5 files changed

Lines changed: 331 additions & 1 deletion

File tree

cmd/thv-operator/api/v1alpha1/mcpserverentry_types.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ const (
9898
// ConditionTypeMCPServerEntryCABundleRefValidated indicates whether the referenced
9999
// CA bundle ConfigMap exists (when configured).
100100
ConditionTypeMCPServerEntryCABundleRefValidated = ConditionCABundleRefValidated
101+
102+
// ConditionTypeMCPServerEntryRemoteURLValidated indicates whether the RemoteURL passes
103+
// format and SSRF safety checks.
104+
ConditionTypeMCPServerEntryRemoteURLValidated = "RemoteURLValidated"
101105
)
102106

103107
// Condition reasons for MCPServerEntry.
@@ -136,6 +140,13 @@ const (
136140

137141
// ConditionReasonMCPServerEntryCABundleRefNotConfigured indicates no CA bundle ref is set.
138142
ConditionReasonMCPServerEntryCABundleRefNotConfigured = "CABundleRefNotConfigured"
143+
144+
// ConditionReasonMCPServerEntryRemoteURLValid indicates the RemoteURL passed all checks.
145+
ConditionReasonMCPServerEntryRemoteURLValid = "RemoteURLValid"
146+
147+
// ConditionReasonMCPServerEntryRemoteURLInvalid indicates the RemoteURL is malformed or
148+
// targets a blocked internal/metadata endpoint.
149+
ConditionReasonMCPServerEntryRemoteURLInvalid = ConditionReasonRemoteURLInvalid
139150
)
140151

141152
//+kubebuilder:object:root=true

cmd/thv-operator/controllers/mcpserverentry_controller.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2121

2222
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
23+
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/validation"
2324
)
2425

2526
const (
@@ -64,6 +65,8 @@ func (r *MCPServerEntryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
6465
// to force a requeue rather than persisting a misleading condition.
6566
allValid := true
6667

68+
allValid = r.validateRemoteURL(entry) && allValid
69+
6770
valid, err := r.validateGroupRef(ctx, entry)
6871
if err != nil {
6972
return ctrl.Result{}, err
@@ -296,6 +299,32 @@ func (r *MCPServerEntryReconciler) validateCABundleRef(
296299
return true, nil
297300
}
298301

302+
// validateRemoteURL checks that the RemoteURL is well-formed and does not target
303+
// a blocked internal or metadata endpoint (SSRF protection).
304+
func (*MCPServerEntryReconciler) validateRemoteURL(
305+
entry *mcpv1alpha1.MCPServerEntry,
306+
) bool {
307+
if err := validation.ValidateRemoteURL(entry.Spec.RemoteURL); err != nil {
308+
meta.SetStatusCondition(&entry.Status.Conditions, metav1.Condition{
309+
Type: mcpv1alpha1.ConditionTypeMCPServerEntryRemoteURLValidated,
310+
Status: metav1.ConditionFalse,
311+
Reason: mcpv1alpha1.ConditionReasonMCPServerEntryRemoteURLInvalid,
312+
Message: err.Error(),
313+
ObservedGeneration: entry.Generation,
314+
})
315+
return false
316+
}
317+
318+
meta.SetStatusCondition(&entry.Status.Conditions, metav1.Condition{
319+
Type: mcpv1alpha1.ConditionTypeMCPServerEntryRemoteURLValidated,
320+
Status: metav1.ConditionTrue,
321+
Reason: mcpv1alpha1.ConditionReasonMCPServerEntryRemoteURLValid,
322+
Message: "Remote URL is valid",
323+
ObservedGeneration: entry.Generation,
324+
})
325+
return true
326+
}
327+
299328
// updateOverallStatus sets the phase and Valid condition based on validation results.
300329
func (*MCPServerEntryReconciler) updateOverallStatus(
301330
entry *mcpv1alpha1.MCPServerEntry,

cmd/thv-operator/controllers/mcpserverentry_controller_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,60 @@ func TestMCPServerEntryReconciler_Reconcile(t *testing.T) {
252252
{mcpv1alpha1.ConditionTypeMCPServerEntryValid, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonMCPServerEntryInvalid},
253253
},
254254
},
255+
{
256+
name: "SSRF - loopback IP rejected",
257+
entry: func() *mcpv1alpha1.MCPServerEntry {
258+
e := newMCPServerEntry(testEntryGroupRef, nil, nil)
259+
e.Spec.RemoteURL = "http://127.0.0.1:8080/"
260+
return e
261+
}(),
262+
objects: []client.Object{newMCPGroup(mcpv1alpha1.MCPGroupPhaseReady)},
263+
wantPhase: mcpv1alpha1.MCPServerEntryPhaseFailed,
264+
conditions: []struct {
265+
condType string
266+
status metav1.ConditionStatus
267+
reason string
268+
}{
269+
{mcpv1alpha1.ConditionTypeMCPServerEntryRemoteURLValidated, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonMCPServerEntryRemoteURLInvalid},
270+
{mcpv1alpha1.ConditionTypeMCPServerEntryValid, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonMCPServerEntryInvalid},
271+
},
272+
},
273+
{
274+
name: "SSRF - metadata endpoint rejected",
275+
entry: func() *mcpv1alpha1.MCPServerEntry {
276+
e := newMCPServerEntry(testEntryGroupRef, nil, nil)
277+
e.Spec.RemoteURL = "http://169.254.169.254/latest/meta-data/"
278+
return e
279+
}(),
280+
objects: []client.Object{newMCPGroup(mcpv1alpha1.MCPGroupPhaseReady)},
281+
wantPhase: mcpv1alpha1.MCPServerEntryPhaseFailed,
282+
conditions: []struct {
283+
condType string
284+
status metav1.ConditionStatus
285+
reason string
286+
}{
287+
{mcpv1alpha1.ConditionTypeMCPServerEntryRemoteURLValidated, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonMCPServerEntryRemoteURLInvalid},
288+
{mcpv1alpha1.ConditionTypeMCPServerEntryValid, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonMCPServerEntryInvalid},
289+
},
290+
},
291+
{
292+
name: "SSRF - kubernetes.default.svc rejected",
293+
entry: func() *mcpv1alpha1.MCPServerEntry {
294+
e := newMCPServerEntry(testEntryGroupRef, nil, nil)
295+
e.Spec.RemoteURL = "http://kubernetes.default.svc/"
296+
return e
297+
}(),
298+
objects: []client.Object{newMCPGroup(mcpv1alpha1.MCPGroupPhaseReady)},
299+
wantPhase: mcpv1alpha1.MCPServerEntryPhaseFailed,
300+
conditions: []struct {
301+
condType string
302+
status metav1.ConditionStatus
303+
reason string
304+
}{
305+
{mcpv1alpha1.ConditionTypeMCPServerEntryRemoteURLValidated, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonMCPServerEntryRemoteURLInvalid},
306+
{mcpv1alpha1.ConditionTypeMCPServerEntryValid, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonMCPServerEntryInvalid},
307+
},
308+
},
255309
{
256310
name: "entry not found returns no error and no requeue",
257311
entry: nil, // no entry seeded

cmd/thv-operator/pkg/validation/url_validation.go

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,59 @@ package validation
55

66
import (
77
"fmt"
8+
"net"
89
"net/url"
10+
"strings"
911
)
1012

1113
const (
1214
schemeHTTP = "http"
1315
schemeHTTPS = "https"
1416
)
1517

18+
// internalCIDRs are IP ranges that should never appear in RemoteURL fields.
19+
// These cover loopback, link-local (including cloud metadata), RFC 1918
20+
// private ranges, and the unspecified address.
21+
var internalCIDRs = func() []*net.IPNet {
22+
cidrs := []string{
23+
"0.0.0.0/8", // RFC 1122 "this network" (often resolves to localhost)
24+
"127.0.0.0/8", // IPv4 loopback
25+
"169.254.0.0/16", // IPv4 link-local (cloud metadata lives here)
26+
"10.0.0.0/8", // RFC 1918 class A
27+
"172.16.0.0/12", // RFC 1918 class B
28+
"192.168.0.0/16", // RFC 1918 class C
29+
"::/128", // IPv6 unspecified
30+
"::1/128", // IPv6 loopback
31+
"fe80::/10", // IPv6 link-local
32+
"fc00::/7", // IPv6 unique-local (ULA)
33+
}
34+
35+
nets := make([]*net.IPNet, 0, len(cidrs))
36+
for _, cidr := range cidrs {
37+
_, ipNet, err := net.ParseCIDR(cidr)
38+
if err != nil {
39+
panic(fmt.Sprintf("bad CIDR in internalCIDRs: %s", cidr))
40+
}
41+
nets = append(nets, ipNet)
42+
}
43+
return nets
44+
}()
45+
46+
// blockedHostnames are well-known internal hostnames that must be rejected.
47+
// Subdomain matching (via HasSuffix) ensures that e.g. "api.kubernetes.default.svc"
48+
// is also blocked.
49+
var blockedHostnames = []string{
50+
"localhost",
51+
"kubernetes.default.svc.cluster.local",
52+
"kubernetes.default.svc",
53+
"kubernetes.default",
54+
"cluster.local",
55+
"metadata.google.internal",
56+
}
57+
1658
// ValidateRemoteURL validates that rawURL is a well-formed HTTP or HTTPS URL
17-
// with a non-empty host. No network calls are made.
59+
// with a non-empty host. It also rejects URLs targeting internal/metadata
60+
// endpoints to prevent SSRF. No network calls or DNS resolution is performed.
1861
func ValidateRemoteURL(rawURL string) error {
1962
if rawURL == "" {
2063
return fmt.Errorf("remote URL must not be empty")
@@ -33,6 +76,41 @@ func ValidateRemoteURL(rawURL string) error {
3376
return fmt.Errorf("remote URL must have a valid host")
3477
}
3578

79+
if err := validateHostNotInternal(u.Hostname()); err != nil {
80+
return fmt.Errorf("remote URL host is not allowed: %w", err)
81+
}
82+
83+
return nil
84+
}
85+
86+
// validateHostNotInternal checks that the host is not a known internal address.
87+
// It rejects literal IPs in private/loopback/link-local ranges and well-known
88+
// internal hostnames. Hostnames that are not on the blocklist are allowed
89+
// because we do not perform DNS resolution.
90+
func validateHostNotInternal(host string) error {
91+
ip := net.ParseIP(host)
92+
if ip != nil {
93+
// Normalize IPv4-mapped IPv6 addresses (e.g. ::ffff:127.0.0.1) to their
94+
// 4-byte IPv4 form so that IPv4 CIDRs match correctly.
95+
if v4 := ip.To4(); v4 != nil {
96+
ip = v4
97+
}
98+
for _, cidr := range internalCIDRs {
99+
if cidr.Contains(ip) {
100+
return fmt.Errorf("IP address %s falls within blocked range %s", host, cidr)
101+
}
102+
}
103+
return nil
104+
}
105+
106+
// Host is a hostname -- check against blocked names.
107+
lower := strings.ToLower(host)
108+
for _, blocked := range blockedHostnames {
109+
if lower == blocked || strings.HasSuffix(lower, "."+blocked) {
110+
return fmt.Errorf("hostname %q matches blocked internal hostname %q", host, blocked)
111+
}
112+
}
113+
36114
return nil
37115
}
38116

0 commit comments

Comments
 (0)