diff --git a/docs/modelcontextprotocol-io/authentication.mdx b/docs/modelcontextprotocol-io/authentication.mdx index 59d7218d5..bc042fc42 100644 --- a/docs/modelcontextprotocol-io/authentication.mdx +++ b/docs/modelcontextprotocol-io/authentication.mdx @@ -319,3 +319,42 @@ mcp-publisher login http azure-key-vault --domain="${MY_DOMAIN}" --vault "${MY_K ``` + +### Troubleshooting failed HTTP verification + +The Registry fetches your verification file from its own infrastructure when you run `mcp-publisher login http`, so a `curl` that succeeds on your machine does not prove that login will work. The Registry itself must be able to reach the endpoint. + + + If the Registry cannot use the endpoint, login fails with `token exchange failed with status 401` even though the file is served correctly to every other client. + + +The fetch is a plain HTTPS request with these properties: + +| Property | Requirement | +| --- | --- | +| Request | `GET https:///.well-known/mcp-registry-auth`, sent with `Accept: text/plain` and `User-Agent: mcp-registry/1.0` | +| Status | `200 OK`. Redirects are not followed, so a `3xx` fails verification | +| Body | the proof record, at most 4096 bytes. Surrounding whitespace is ignored | +| Timeout | 10 seconds for the whole request | +| Origin | the hosted Registry runs on Google Cloud, so the request arrives from Google Cloud addresses rather than a fixed address you can allowlist per request | + +Because the request originates from cloud infrastructure rather than a browser on a residential network, the usual causes of failure are edge rules that block it: + +- a WAF, bot-management, or "under attack" rule that challenges or blocks non-browser clients +- a geo-block or IP allowlist that excludes Google Cloud address ranges +- an `AAAA` (IPv6) record pointing at an address that is not reachable. Verification fails if none of the domain's public addresses can be reached, even when IPv4 works in a browser +- a `301`/`302` redirect to another host or path, which the Registry does not follow +- `/.well-known/mcp-registry-auth` answering `200 OK` with an HTML error page instead of the proof record + +To see what the Registry sees, request the file the same way it does, ideally from a network other than your own: + +```bash +curl -sS -o - -w '\n%{http_code}\n' \ + -H 'Accept: text/plain' \ + -A 'mcp-registry/1.0' \ + https://example.com/.well-known/mcp-registry-auth +``` + +The response body must be the proof record printed by the login command (`v=MCPv1; k=...; p=...`). When the Registry cannot use the endpoint, the login error carries the Registry's own fetch failure, such as `dial tcp :443: i/o timeout`, `HTTP 403: failed to fetch key from ...`, or `HTTP 301: failed to fetch key from ...`. Allow plain HTTPS clients through for `/.well-known/mcp-registry-auth`, serve the path without redirects, then retry. + +Addresses that resolve to loopback, private, link-local, or carrier-grade NAT ranges are refused by design, so the endpoint must be reachable at a public address. diff --git a/internal/api/handlers/v0/auth/http.go b/internal/api/handlers/v0/auth/http.go index 539276296..b13a090ca 100644 --- a/internal/api/handlers/v0/auth/http.go +++ b/internal/api/handlers/v0/auth/http.go @@ -38,16 +38,26 @@ func NewDefaultHTTPKeyFetcher() *DefaultHTTPKeyFetcher { transport := http.DefaultTransport.(*http.Transport).Clone() transport.DialContext = safeDialContext - return &DefaultHTTPKeyFetcher{ - client: &http.Client{ - Timeout: 10 * time.Second, - // Disable redirects for security purposes: - // Prevents people doing weird things like sending us to internal endpoints at different paths - CheckRedirect: func(_ *http.Request, _ []*http.Request) error { - return http.ErrUseLastResponse - }, - Transport: transport, + return &DefaultHTTPKeyFetcher{client: newHTTPKeyFetcherClient(transport)} +} + +// httpKeyFetchTimeout bounds the entire well-known fetch, including DNS resolution +// and the TLS handshake. It is part of the documented verification contract: an +// endpoint that cannot answer within this budget fails authentication. +const httpKeyFetchTimeout = 10 * time.Second + +// newHTTPKeyFetcherClient builds the HTTP client used to fetch a domain's +// well-known verification key. The transport is a parameter so that tests can +// exercise the production timeout and redirect policy against a local server. +func newHTTPKeyFetcherClient(transport http.RoundTripper) *http.Client { + return &http.Client{ + Timeout: httpKeyFetchTimeout, + // Disable redirects for security purposes: + // Prevents people doing weird things like sending us to internal endpoints at different paths + CheckRedirect: func(_ *http.Request, _ []*http.Request) error { + return http.ErrUseLastResponse }, + Transport: transport, } } diff --git a/internal/api/handlers/v0/auth/http_internal_test.go b/internal/api/handlers/v0/auth/http_internal_test.go index 998c8c890..1388b7716 100644 --- a/internal/api/handlers/v0/auth/http_internal_test.go +++ b/internal/api/handlers/v0/auth/http_internal_test.go @@ -1,8 +1,15 @@ package auth import ( + "bytes" + "context" + "crypto/tls" "net" + "net/http" + "net/http/httptest" + "strings" "testing" + "time" ) func TestIsBlockedIP(t *testing.T) { @@ -78,3 +85,128 @@ func TestIsBlockedIP(t *testing.T) { }) } } + +// Mirrors the constants of the same purpose in the external test package; the +// well-known path and a placeholder domain are all FetchKey needs, since the +// transport dials the local test server regardless of the request hostname. +const ( + internalWellKnownPath = "/.well-known/mcp-registry-auth" + internalTestDomain = "example.com" +) + +// newLocalTLSTransport builds a transport that reaches srv regardless of the +// hostname in the request URL, so the production client configuration can be +// exercised against a local server. +func newLocalTLSTransport(srv *httptest.Server) *http.Transport { + dialAddr := srv.Listener.Addr().String() + return &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // testing only + DialContext: func(ctx context.Context, network, _ string) (net.Conn, error) { + d := &net.Dialer{} + return d.DialContext(ctx, network, dialAddr) + }, + ForceAttemptHTTP2: false, + MaxIdleConns: 10, + IdleConnTimeout: 30 * time.Second, + TLSHandshakeTimeout: 5 * time.Second, + } +} + +func newLocalTLSFetcher(srv *httptest.Server) *DefaultHTTPKeyFetcher { + return &DefaultHTTPKeyFetcher{client: newHTTPKeyFetcherClient(newLocalTLSTransport(srv))} +} + +// The published HTTP verification contract (see +// docs/modelcontextprotocol-io/authentication.mdx) tells users the endpoint must +// answer 200 OK directly, so the client must never follow a redirect to another +// host or path. +func TestHTTPKeyFetcherClient_DoesNotFollowRedirects(t *testing.T) { + var redirectTargetHit bool + + target := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + redirectTargetHit = true + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("v=MCPv1; k=ed25519; p=REDIRECTED")) + })) + defer target.Close() + + redirector := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != internalWellKnownPath { + w.WriteHeader(http.StatusNotFound) + return + } + http.Redirect(w, r, target.URL+internalWellKnownPath, http.StatusMovedPermanently) + })) + defer redirector.Close() + + fetcher := newLocalTLSFetcher(redirector) + + _, err := fetcher.FetchKey(context.Background(), internalTestDomain) + if err == nil { + t.Fatal("expected the redirect to fail verification, got nil error") + } + if !strings.Contains(err.Error(), "HTTP 301") { + t.Fatalf("got err=%v, want it to report HTTP 301", err) + } + if redirectTargetHit { + t.Error("the redirect target was requested; redirects must not be followed") + } +} + +// Pins the timeout users are told to satisfy and the request headers the +// documented fetch sends. +func TestHTTPKeyFetcherClient_TimeoutAndRequestHeaders(t *testing.T) { + type observed struct { + accept string + userAgent string + } + var got observed + + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = observed{accept: r.Header.Get("Accept"), userAgent: r.Header.Get("User-Agent")} + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(" v=MCPv1; k=ed25519; p=PUBLIC_KEY\n")) + })) + defer srv.Close() + + fetcher := newLocalTLSFetcher(srv) + + if fetcher.client.Timeout != httpKeyFetchTimeout { + t.Errorf("client timeout = %v, want %v", fetcher.client.Timeout, httpKeyFetchTimeout) + } + + key, err := fetcher.FetchKey(context.Background(), internalTestDomain) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if want := "v=MCPv1; k=ed25519; p=PUBLIC_KEY"; key != want { + t.Errorf("key = %q, want %q (surrounding whitespace must be trimmed)", key, want) + } + if want := "text/plain"; got.accept != want { + t.Errorf("Accept = %q, want %q", got.accept, want) + } + if want := "mcp-registry/1.0"; got.userAgent != want { + t.Errorf("User-Agent = %q, want %q", got.userAgent, want) + } +} + +// A body above the documented 4096 byte limit must fail even when the server +// answers 200 OK. +func TestHTTPKeyFetcher_RejectsResponseAboveDocumentedLimit(t *testing.T) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != internalWellKnownPath { + w.WriteHeader(http.StatusNotFound) + return + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write(bytes.Repeat([]byte("A"), MaxKeyResponseSize+1)) + })) + defer srv.Close() + + fetcher := newLocalTLSFetcher(srv) + + _, err := fetcher.FetchKey(context.Background(), internalTestDomain) + if err == nil || !strings.Contains(err.Error(), "too large") { + t.Fatalf("got err=%v, want a response-too-large error", err) + } +}