Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions cmd/thv-operator/api/v1alpha1/mcpserverentry_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ const (
// ConditionTypeMCPServerEntryCABundleRefValidated indicates whether the referenced
// CA bundle ConfigMap exists (when configured).
ConditionTypeMCPServerEntryCABundleRefValidated = ConditionCABundleRefValidated

// ConditionTypeMCPServerEntryRemoteURLValidated indicates whether the RemoteURL passes
// format and SSRF safety checks.
ConditionTypeMCPServerEntryRemoteURLValidated = "RemoteURLValidated"
)

// Condition reasons for MCPServerEntry.
Expand Down Expand Up @@ -136,6 +140,13 @@ const (

// ConditionReasonMCPServerEntryCABundleRefNotConfigured indicates no CA bundle ref is set.
ConditionReasonMCPServerEntryCABundleRefNotConfigured = "CABundleRefNotConfigured"

// ConditionReasonMCPServerEntryRemoteURLValid indicates the RemoteURL passed all checks.
ConditionReasonMCPServerEntryRemoteURLValid = "RemoteURLValid"

// ConditionReasonMCPServerEntryRemoteURLInvalid indicates the RemoteURL is malformed or
// targets a blocked internal/metadata endpoint.
ConditionReasonMCPServerEntryRemoteURLInvalid = ConditionReasonRemoteURLInvalid
)

//+kubebuilder:object:root=true
Expand Down
29 changes: 29 additions & 0 deletions cmd/thv-operator/controllers/mcpserverentry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/validation"
)

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

allValid = r.validateRemoteURL(entry) && allValid

valid, err := r.validateGroupRef(ctx, entry)
if err != nil {
return ctrl.Result{}, err
Expand Down Expand Up @@ -296,6 +299,32 @@ func (r *MCPServerEntryReconciler) validateCABundleRef(
return true, nil
}

// validateRemoteURL checks that the RemoteURL is well-formed and does not target
// a blocked internal or metadata endpoint (SSRF protection).
func (*MCPServerEntryReconciler) validateRemoteURL(
entry *mcpv1alpha1.MCPServerEntry,
) bool {
if err := validation.ValidateRemoteURL(entry.Spec.RemoteURL); err != nil {
meta.SetStatusCondition(&entry.Status.Conditions, metav1.Condition{
Type: mcpv1alpha1.ConditionTypeMCPServerEntryRemoteURLValidated,
Status: metav1.ConditionFalse,
Reason: mcpv1alpha1.ConditionReasonMCPServerEntryRemoteURLInvalid,
Message: err.Error(),
ObservedGeneration: entry.Generation,
})
return false
}

meta.SetStatusCondition(&entry.Status.Conditions, metav1.Condition{
Type: mcpv1alpha1.ConditionTypeMCPServerEntryRemoteURLValidated,
Status: metav1.ConditionTrue,
Reason: mcpv1alpha1.ConditionReasonMCPServerEntryRemoteURLValid,
Message: "Remote URL is valid",
ObservedGeneration: entry.Generation,
})
return true
}

// updateOverallStatus sets the phase and Valid condition based on validation results.
func (*MCPServerEntryReconciler) updateOverallStatus(
entry *mcpv1alpha1.MCPServerEntry,
Expand Down
54 changes: 54 additions & 0 deletions cmd/thv-operator/controllers/mcpserverentry_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,60 @@ func TestMCPServerEntryReconciler_Reconcile(t *testing.T) {
{mcpv1alpha1.ConditionTypeMCPServerEntryValid, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonMCPServerEntryInvalid},
},
},
{
name: "SSRF - loopback IP rejected",
entry: func() *mcpv1alpha1.MCPServerEntry {
e := newMCPServerEntry(testEntryGroupRef, nil, nil)
e.Spec.RemoteURL = "http://127.0.0.1:8080/"
return e
}(),
objects: []client.Object{newMCPGroup(mcpv1alpha1.MCPGroupPhaseReady)},
wantPhase: mcpv1alpha1.MCPServerEntryPhaseFailed,
conditions: []struct {
condType string
status metav1.ConditionStatus
reason string
}{
{mcpv1alpha1.ConditionTypeMCPServerEntryRemoteURLValidated, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonMCPServerEntryRemoteURLInvalid},
{mcpv1alpha1.ConditionTypeMCPServerEntryValid, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonMCPServerEntryInvalid},
},
},
{
name: "SSRF - metadata endpoint rejected",
entry: func() *mcpv1alpha1.MCPServerEntry {
e := newMCPServerEntry(testEntryGroupRef, nil, nil)
e.Spec.RemoteURL = "http://169.254.169.254/latest/meta-data/"
return e
}(),
objects: []client.Object{newMCPGroup(mcpv1alpha1.MCPGroupPhaseReady)},
wantPhase: mcpv1alpha1.MCPServerEntryPhaseFailed,
conditions: []struct {
condType string
status metav1.ConditionStatus
reason string
}{
{mcpv1alpha1.ConditionTypeMCPServerEntryRemoteURLValidated, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonMCPServerEntryRemoteURLInvalid},
{mcpv1alpha1.ConditionTypeMCPServerEntryValid, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonMCPServerEntryInvalid},
},
},
{
name: "SSRF - kubernetes.default.svc rejected",
entry: func() *mcpv1alpha1.MCPServerEntry {
e := newMCPServerEntry(testEntryGroupRef, nil, nil)
e.Spec.RemoteURL = "http://kubernetes.default.svc/"
return e
}(),
objects: []client.Object{newMCPGroup(mcpv1alpha1.MCPGroupPhaseReady)},
wantPhase: mcpv1alpha1.MCPServerEntryPhaseFailed,
conditions: []struct {
condType string
status metav1.ConditionStatus
reason string
}{
{mcpv1alpha1.ConditionTypeMCPServerEntryRemoteURLValidated, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonMCPServerEntryRemoteURLInvalid},
{mcpv1alpha1.ConditionTypeMCPServerEntryValid, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonMCPServerEntryInvalid},
},
},
{
name: "entry not found returns no error and no requeue",
entry: nil, // no entry seeded
Expand Down
80 changes: 79 additions & 1 deletion cmd/thv-operator/pkg/validation/url_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,59 @@ package validation

import (
"fmt"
"net"
"net/url"
"strings"
)

const (
schemeHTTP = "http"
schemeHTTPS = "https"
)

// internalCIDRs are IP ranges that should never appear in RemoteURL fields.
// These cover loopback, link-local (including cloud metadata), RFC 1918
// private ranges, and the unspecified address.
var internalCIDRs = func() []*net.IPNet {
cidrs := []string{
"0.0.0.0/8", // RFC 1122 "this network" (often resolves to localhost)
"127.0.0.0/8", // IPv4 loopback
"169.254.0.0/16", // IPv4 link-local (cloud metadata lives here)
"10.0.0.0/8", // RFC 1918 class A
"172.16.0.0/12", // RFC 1918 class B
"192.168.0.0/16", // RFC 1918 class C
"::/128", // IPv6 unspecified
"::1/128", // IPv6 loopback
"fe80::/10", // IPv6 link-local
"fc00::/7", // IPv6 unique-local (ULA)
}

nets := make([]*net.IPNet, 0, len(cidrs))
for _, cidr := range cidrs {
_, ipNet, err := net.ParseCIDR(cidr)
if err != nil {
panic(fmt.Sprintf("bad CIDR in internalCIDRs: %s", cidr))
}
nets = append(nets, ipNet)
}
return nets
}()

// blockedHostnames are well-known internal hostnames that must be rejected.
// Subdomain matching (via HasSuffix) ensures that e.g. "api.kubernetes.default.svc"
// is also blocked.
var blockedHostnames = []string{
"localhost",
"kubernetes.default.svc.cluster.local",
"kubernetes.default.svc",
"kubernetes.default",
"cluster.local",
"metadata.google.internal",
}

// ValidateRemoteURL validates that rawURL is a well-formed HTTP or HTTPS URL
// with a non-empty host. No network calls are made.
// with a non-empty host. It also rejects URLs targeting internal/metadata
// endpoints to prevent SSRF. No network calls or DNS resolution is performed.
func ValidateRemoteURL(rawURL string) error {
if rawURL == "" {
return fmt.Errorf("remote URL must not be empty")
Expand All @@ -33,6 +76,41 @@ func ValidateRemoteURL(rawURL string) error {
return fmt.Errorf("remote URL must have a valid host")
}

if err := validateHostNotInternal(u.Hostname()); err != nil {
return fmt.Errorf("remote URL host is not allowed: %w", err)
}

return nil
}

// validateHostNotInternal checks that the host is not a known internal address.
// It rejects literal IPs in private/loopback/link-local ranges and well-known
// internal hostnames. Hostnames that are not on the blocklist are allowed
// because we do not perform DNS resolution.
func validateHostNotInternal(host string) error {
ip := net.ParseIP(host)
if ip != nil {
// Normalize IPv4-mapped IPv6 addresses (e.g. ::ffff:127.0.0.1) to their
// 4-byte IPv4 form so that IPv4 CIDRs match correctly.
if v4 := ip.To4(); v4 != nil {
ip = v4
}
for _, cidr := range internalCIDRs {
if cidr.Contains(ip) {
return fmt.Errorf("IP address %s falls within blocked range %s", host, cidr)
}
}
return nil
}

// Host is a hostname -- check against blocked names.
lower := strings.ToLower(host)
for _, blocked := range blockedHostnames {
if lower == blocked || strings.HasSuffix(lower, "."+blocked) {
return fmt.Errorf("hostname %q matches blocked internal hostname %q", host, blocked)
}
}

return nil
}

Expand Down
Loading
Loading