diff --git a/docs/protocol.md b/docs/protocol.md index f60371b..06c1b77 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -639,24 +639,24 @@ provided to ### Server-Side Request Forgery -The [mitigations](https://modelcontextprotocol.io/docs/tutorials/security/security_best_practices#mitigation-3) are as follows: +The OAuth discovery helpers apply these [mitigations](https://modelcontextprotocol.io/docs/tutorials/security/security_best_practices#mitigation-3) by default: -- _Enforce HTTPS_. The OAuth helpers provided by the SDK reject the `http://` URLs -except loopback addresses (`localhost`, `127.0.0.1`, `::1`). +- _Enforce HTTPS_. Reject `http://` URLs (except loopback: `localhost`, `127.0.0.1`, `::1`) +and redirects that downgrade `https` to a non-`https` scheme. -- _Block Private IP Ranges_. The OAuth helpers provided by the SDK allow passing -a custom `http.Client`. Developers are advised to customize the client it with -appropriate network protections, including IP range blocking. The SDK does not provide -this capability out of the box. +- _Block Private IP Ranges_. Reject targets that resolve to a non-public address +(private, link-local including the `169.254.169.254` metadata endpoint, CGNAT, multicast, +unspecified). The check runs on the initial URL and at dial time, guarding against DNS rebinding. -- _Validate Redirect Targets_. Similarly to previous point, customized `http.Client` -can be used to validate network hops. The SDK does not provide this capability out -of the box. +- _Validate Redirect Targets_. Reject redirects to non-public addresses and cap the redirect count. -- _Use Egress Proxies_. This is out of scope for the SDK and can be configured separately. +- _Use Egress Proxies_. Out of scope for the SDK; configure separately. -- _DNS Resolution Considerations_. The SDK has DNS rebinding protection on the server side which is enabled by default. For the client side, consider providing -a custom `http.Client` that would implement DNS pinning. +**Opting out.** Passing a custom `http.Client` can bypass these checks, making SSRF +protection your responsibility. Specifically: a custom `Transport.DialContext`/`DialTLSContext`, +a non-`*http.Transport`, or a configured `Proxy` disables the dial-time IP check; setting +`CheckRedirect` replaces all redirect validation. In these cases, implement your own IP +blocking, redirect validation, and DNS pinning. ### Session Hijacking diff --git a/internal/docs/protocol.src.md b/internal/docs/protocol.src.md index dbc8c6e..7d6b01b 100644 --- a/internal/docs/protocol.src.md +++ b/internal/docs/protocol.src.md @@ -534,24 +534,24 @@ provided to ### Server-Side Request Forgery -The [mitigations](https://modelcontextprotocol.io/docs/tutorials/security/security_best_practices#mitigation-3) are as follows: +The OAuth discovery helpers apply these [mitigations](https://modelcontextprotocol.io/docs/tutorials/security/security_best_practices#mitigation-3) by default: -- _Enforce HTTPS_. The OAuth helpers provided by the SDK reject the `http://` URLs -except loopback addresses (`localhost`, `127.0.0.1`, `::1`). +- _Enforce HTTPS_. Reject `http://` URLs (except loopback: `localhost`, `127.0.0.1`, `::1`) +and redirects that downgrade `https` to a non-`https` scheme. -- _Block Private IP Ranges_. The OAuth helpers provided by the SDK allow passing -a custom `http.Client`. Developers are advised to customize the client it with -appropriate network protections, including IP range blocking. The SDK does not provide -this capability out of the box. +- _Block Private IP Ranges_. Reject targets that resolve to a non-public address +(private, link-local including the `169.254.169.254` metadata endpoint, CGNAT, multicast, +unspecified). The check runs on the initial URL and at dial time, guarding against DNS rebinding. -- _Validate Redirect Targets_. Similarly to previous point, customized `http.Client` -can be used to validate network hops. The SDK does not provide this capability out -of the box. +- _Validate Redirect Targets_. Reject redirects to non-public addresses and cap the redirect count. -- _Use Egress Proxies_. This is out of scope for the SDK and can be configured separately. +- _Use Egress Proxies_. Out of scope for the SDK; configure separately. -- _DNS Resolution Considerations_. The SDK has DNS rebinding protection on the server side which is enabled by default. For the client side, consider providing -a custom `http.Client` that would implement DNS pinning. +**Opting out.** Passing a custom `http.Client` can bypass these checks, making SSRF +protection your responsibility. Specifically: a custom `Transport.DialContext`/`DialTLSContext`, +a non-`*http.Transport`, or a configured `Proxy` disables the dial-time IP check; setting +`CheckRedirect` replaces all redirect validation. In these cases, implement your own IP +blocking, redirect validation, and DNS pinning. ### Session Hijacking diff --git a/internal/util/net.go b/internal/util/net.go index 6858614..593012d 100644 --- a/internal/util/net.go +++ b/internal/util/net.go @@ -24,3 +24,26 @@ func IsLoopback(addr string) bool { } return ip.IsLoopback() } + +// cgnat is the RFC 6598 carrier-grade NAT range (100.64.0.0/10). netip's +// IsPrivate does not report it, but it is shared ISP space that is not publicly +// routable and could point at another tenant behind the same NAT. +var cgnat = netip.MustParsePrefix("100.64.0.0/10") + +// IsPrivateOrReserved reports whether ip belongs to a range that must not be +// reachable through a server-controlled URL, as a defense against SSRF: private +// (RFC 1918 / RFC 4193 ULA), link-local (including the 169.254.169.254 cloud +// metadata endpoint), carrier-grade NAT, multicast, and the unspecified address. +func IsPrivateOrReserved(ip netip.Addr) bool { + ip = ip.Unmap() + if !ip.IsValid() { + return true + } + if ip.IsPrivate() || + ip.IsLinkLocalUnicast() || + ip.IsMulticast() || + ip.IsUnspecified() { + return true + } + return cgnat.Contains(ip) +} diff --git a/internal/util/net_test.go b/internal/util/net_test.go index fd3187b..21b7179 100644 --- a/internal/util/net_test.go +++ b/internal/util/net_test.go @@ -3,7 +3,10 @@ // that can be found in the LICENSE file. package util -import "testing" +import ( + "net/netip" + "testing" +) // TestIsLoopback tests the IsLoopback helper function. func TestIsLoopback(t *testing.T) { @@ -33,3 +36,52 @@ func TestIsLoopback(t *testing.T) { }) } } + +func TestIsPrivateOrReserved(t *testing.T) { + tests := []struct { + ip string + want bool + }{ + // Private (RFC 1918) and IPv6 ULA (RFC 4193). + {"10.0.0.1", true}, + {"172.16.5.4", true}, + {"192.168.1.1", true}, + {"fd00::1", true}, + // Link-local, including the cloud metadata endpoint. + {"169.254.169.254", true}, + {"169.254.0.1", true}, + {"fe80::1", true}, + // Carrier-grade NAT (RFC 6598). + {"100.64.0.1", true}, + {"100.127.255.255", true}, + // Multicast and unspecified. + {"224.0.0.1", true}, + {"ff02::1", true}, + {"0.0.0.0", true}, + {"::", true}, + // IPv4-mapped IPv6 of a private address must also be caught. + {"::ffff:10.0.0.1", true}, + {"::ffff:192.168.0.1", true}, + // Loopback is intentionally NOT reserved here (policy handled elsewhere). + {"127.0.0.1", false}, + {"::1", false}, + // Public addresses are allowed. + {"8.8.8.8", false}, + {"1.1.1.1", false}, + {"2606:4700:4700::1111", false}, + // Just outside CGNAT. + {"100.63.255.255", false}, + {"100.128.0.0", false}, + } + for _, tt := range tests { + t.Run(tt.ip, func(t *testing.T) { + ip, err := netip.ParseAddr(tt.ip) + if err != nil { + t.Fatalf("ParseAddr(%q): %v", tt.ip, err) + } + if got := IsPrivateOrReserved(ip); got != tt.want { + t.Errorf("IsPrivateOrReserved(%q) = %v, want %v", tt.ip, got, tt.want) + } + }) + } +} diff --git a/oauthex/discovery_test.go b/oauthex/discovery_test.go new file mode 100644 index 0000000..8b64d0c --- /dev/null +++ b/oauthex/discovery_test.go @@ -0,0 +1,158 @@ +// Copyright 2025 The Go MCP SDK Authors. All rights reserved. +// Use of this source code is governed by an MIT-style +// license that can be found in the LICENSE file. + +package oauthex + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestCheckHTTPSOrLoopback(t *testing.T) { + tests := []struct { + name string + addr string + wantErr bool + }{ + {name: "empty", addr: "", wantErr: false}, + {name: "https dns allowed", addr: "https://example.com/prm", wantErr: false}, + {name: "cleartext dns disallowed", addr: "http://example.com/prm", wantErr: true}, + {name: "https loopback ip allowed", addr: "http://127.0.0.1:8080/prm", wantErr: false}, + {name: "http localhost allowed", addr: "http://localhost/prm", wantErr: false}, + {name: "https private ip disallowed", addr: "https://10.0.0.1/prm", wantErr: true}, + {name: "link local metadata disallowed", addr: "https://169.254.169.254/", wantErr: true}, + {name: "mapped private address disallowed", addr: "https://[::ffff:10.0.0.1]/prm", wantErr: true}, + {name: "https public ip allowed", addr: "https://8.8.8.8/prm", wantErr: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := checkHTTPSOrLoopback(tt.addr) + if (err != nil) != tt.wantErr { + t.Errorf("checkHTTPSOrLoopback(%q) error = %v, wantErr %v", tt.addr, err, tt.wantErr) + } + }) + } +} + +func TestDiscoveryClientRedirect(t *testing.T) { + newRequestTo := func(u string) *http.Request { + t.Helper() + r, err := http.NewRequest(http.MethodGet, u, nil) + if err != nil { + t.Fatalf("NewRequest(%q): %v", u, err) + } + return r + } + testCases := []struct { + name string + custom *http.Client + req *http.Request + via []*http.Request + wantBlocked bool + }{ + { + name: "redirect to public allowed", + req: newRequestTo("https://8.8.8.8/"), + via: []*http.Request{newRequestTo("https://93.184.216.34/")}, + wantBlocked: false, + }, + { + name: "downgrade blocked", + req: newRequestTo("http://93.184.216.34/"), + via: []*http.Request{newRequestTo("https://93.184.216.34/")}, + wantBlocked: true, + }, + { + name: "redirect to loopback blocked", + req: newRequestTo("https://127.0.0.1/"), + via: []*http.Request{newRequestTo("https://93.184.216.34/")}, + wantBlocked: true, + }, + { + name: "redirect to hostname resolving to loopback blocked", + req: newRequestTo("https://localhost/"), + via: []*http.Request{newRequestTo("https://93.184.216.34/")}, + wantBlocked: true, + }, + { + name: "custom client with no custom check", + custom: &http.Client{}, + req: newRequestTo("https://127.0.0.1/"), + via: []*http.Request{newRequestTo("https://93.184.216.34/")}, + wantBlocked: true, + }, + { + name: "custom client check disable", + custom: &http.Client{ + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return nil + }, + }, + req: newRequestTo("https://127.0.0.1/"), + via: []*http.Request{newRequestTo("https://93.184.216.34/")}, + wantBlocked: false, + }, + { + name: "too many redirects", + req: newRequestTo("https://8.8.8.8/"), + via: func() []*http.Request { + via := make([]*http.Request, maxDiscoveryRedirects) + for i := range via { + via[i] = newRequestTo("https://8.8.8.8/") + } + return via + }(), + wantBlocked: true, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + client := newDiscoveryClient(tc.custom) + gotErr := client.CheckRedirect(tc.req, tc.via) + if gotErr != nil && !tc.wantBlocked { + t.Fatalf("client.CheckRedirect() error = %v", gotErr) + } + if gotErr == nil && tc.wantBlocked { + t.Fatal("client.CheckRedirect() error = nil, want blocked") + } + }) + } +} + +func TestDiscoveryBlocksInternalTarget(t *testing.T) { + ctx := t.Context() + for _, target := range []string{ + "https://169.254.169.254/latest/meta-data/", + "https://10.0.0.1/.well-known/oauth-protected-resource", + } { + t.Run(target, func(t *testing.T) { + _, err := GetProtectedResourceMetadata(ctx, target, target, nil) + if err == nil { + t.Fatalf("GetProtectedResourceMetadata(%q) succeeded, want SSRF rejection", target) + } + if !strings.Contains(err.Error(), "non-public") { + t.Errorf("error = %v, want it to mention a non-public address", err) + } + }) + } +} + +func TestDiscoveryAllowsLoopbackInitial(t *testing.T) { + ctx := t.Context() + h := &fakeResourceHandler{} + server := httptest.NewTLSServer(h) + defer server.Close() + h.installHandlers(server.URL) + + metadataURL := server.URL + "/.well-known/oauth-protected-resource" + prm, err := GetProtectedResourceMetadata(ctx, metadataURL, server.URL, server.Client()) + if err != nil { + t.Fatalf("GetProtectedResourceMetadata on loopback failed: %v", err) + } + if prm == nil { + t.Fatal("nil prm") + } +} diff --git a/oauthex/oauth2.go b/oauthex/oauth2.go index 689f17a..caed92f 100644 --- a/oauthex/oauth2.go +++ b/oauthex/oauth2.go @@ -11,14 +11,22 @@ import ( "fmt" "io" "mime" + "net" "net/http" + "net/netip" "net/url" "strings" + "syscall" + "time" "github.com/modelcontextprotocol/go-sdk/internal/json" "github.com/modelcontextprotocol/go-sdk/internal/util" ) +const maxDiscoveryRedirects = 10 + +var defaultDiscoveryTransport = newDiscoveryTransport(http.DefaultTransport) + type httpStatusError struct { StatusCode int } @@ -35,10 +43,7 @@ func getJSON[T any](ctx context.Context, c *http.Client, url string, limit int64 if err != nil { return nil, err } - if c == nil { - c = http.DefaultClient - } - res, err := c.Do(req) + res, err := newDiscoveryClient(c).Do(req) if err != nil { return nil, err } @@ -91,5 +96,112 @@ func checkHTTPSOrLoopback(addr string) error { if !util.IsLoopback(u.Host) && u.Scheme != "https" { return fmt.Errorf("URL %q does not use HTTPS or is not a loopback address", addr) } + if ip, err := netip.ParseAddr(u.Hostname()); err == nil && util.IsPrivateOrReserved(ip) { + return fmt.Errorf("URL %q refers to a non-public address", addr) + } + return nil +} + +func newDiscoveryClient(c *http.Client) *http.Client { + transport := defaultDiscoveryTransport + if c != nil && c.Transport != nil { + // a caller that has taken over dialing or TLS opts out of the checks + if t, ok := c.Transport.(*http.Transport); ok && t.DialContext == nil && t.DialTLSContext == nil { + transport = newDiscoveryTransport(t) + } else { + transport = c.Transport + } + } + discoveryClient := &http.Client{ + Transport: transport, + + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if c != nil && c.CheckRedirect != nil { + return c.CheckRedirect(req, via) + } + if len(via) >= maxDiscoveryRedirects { + return fmt.Errorf("max redirects exceeded (%d)", maxDiscoveryRedirects) + } + if prev := via[len(via)-1]; prev.URL.Scheme == "https" && req.URL.Scheme != "https" { + return fmt.Errorf("redirect to non-HTTPS scheme %q is a security downgrade", req.URL.Scheme) + } + return checkLoopbackRedirect(req.Context(), req.URL.Hostname()) + }, + } + if c != nil { + discoveryClient.Timeout = c.Timeout + discoveryClient.Jar = c.Jar + } + return discoveryClient +} + +// newDiscoveryTransport creates an http.RoundTripper with extra protections enabled, +// unless the provided RoundTripper is not *http.Transport. +func newDiscoveryTransport(rt http.RoundTripper) http.RoundTripper { + base, ok := rt.(*http.Transport) + if !ok { + return rt + } + // If a proxy is configured Control checks should be disabled, because they will + // basically be running for proxy dials. + if proxyConfigured(base) { + return rt + } + result := base.Clone() + result.DialContext = (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + Control: func(_, address string, _ syscall.RawConn) error { + host, _, err := net.SplitHostPort(address) + if err != nil { + host = address + } + ip, err := netip.ParseAddr(host) + if err != nil { + return fmt.Errorf("non-ip address: %w", err) + } + if util.IsPrivateOrReserved(ip) { + return fmt.Errorf("non-public ip address: %q", ip) + } + return nil + }, + }).DialContext + return result +} + +func proxyConfigured(t *http.Transport) bool { + if t.Proxy == nil { + return false + } + req, err := http.NewRequest(http.MethodGet, "https://check-proxy-existence.com", nil) + if err != nil { + return false + } + u, err := t.Proxy(req) + return err == nil && u != nil +} + +func checkLoopbackRedirect(ctx context.Context, host string) error { + doCheck := func(ip netip.Addr) error { + if ip.Unmap().IsLoopback() { + return fmt.Errorf("redirect into loopback address %q", host) + } + if util.IsPrivateOrReserved(ip) { + return fmt.Errorf("redirect into non-public address %q", host) + } + return nil + } + if ip, err := netip.ParseAddr(host); err == nil { + return doCheck(ip) + } + ips, err := net.DefaultResolver.LookupNetIP(ctx, "ip", host) + if err != nil { + return fmt.Errorf("cannot resolve redirect host %q: %w", host, err) + } + for _, ip := range ips { + if err := doCheck(ip); err != nil { + return err + } + } return nil }