From 6e687a517d2a00bc54b0b560c44e48bcff27e029 Mon Sep 17 00:00:00 2001 From: Radoslav Dimitrov Date: Sat, 25 Apr 2026 02:13:07 +0300 Subject: [PATCH 1/2] fix(auth): better DNS verification errors and wrong-selector hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DNS auth users repeatedly hit two failure modes that produce identical, unhelpful "Ed25519 signature verification failed" errors (#385, #1103, #1126): 1. TXT record placed under a selector (_mcp-auth./_mcp-registry.) instead of the apex — DKIM intuition, but MCP DNS auth uses SPF-style apex placement. 2. Stale TXT record left at the apex after a key rotation, silently tried first and rejected. Server changes: - When the apex has no MCPv1 record, probe common wrong selectors and return a targeted error pointing the user at the apex. - When verification fails on records that *are* present, include short fingerprints of every key the registry tried so the user can tell a stale record from a real signing problem. Structural errors (wrong signature size, etc.) still pass through unchanged via a new ErrSignatureMismatch sentinel. Docs: - Anti-pattern callout in authentication.mdx warning explicitly against selector placement and stale records. Refs #845, #385, #1103 Fixes #1126 --- .../authentication.mdx | 6 ++ internal/api/handlers/v0/auth/common.go | 73 +++++++++++++-- internal/api/handlers/v0/auth/dns.go | 57 ++++++++++++ internal/api/handlers/v0/auth/dns_test.go | 90 ++++++++++++++++++- 4 files changed, 219 insertions(+), 7 deletions(-) diff --git a/docs/modelcontextprotocol-io/authentication.mdx b/docs/modelcontextprotocol-io/authentication.mdx index c18268c19..f5af3ad85 100644 --- a/docs/modelcontextprotocol-io/authentication.mdx +++ b/docs/modelcontextprotocol-io/authentication.mdx @@ -51,6 +51,12 @@ Successfully authenticated! DNS authentication is a domain-based authentication method that relies on a DNS TXT record. + + The TXT record must be placed on the **apex** of your domain (e.g. `example.com`), **not** under a selector like `_mcp-auth.example.com` or `_mcp-registry.example.com`. MCP DNS auth follows SPF-style placement (apex), not DKIM-style (selector). If you put the record under a selector, the registry will not see it and authentication will fail with a generic signature error. + + If you rotate keys, also remember to remove the previous TXT record from the apex — a stale record left behind will be tried first and cause verification to fail. + + To perform DNS authentication using the `mcp-publisher` CLI tool, run the following commands in your server project directory to generate a TXT record based on a public/private key pair: diff --git a/internal/api/handlers/v0/auth/common.go b/internal/api/handlers/v0/auth/common.go index 40a6ba7e5..b4471948a 100644 --- a/internal/api/handlers/v0/auth/common.go +++ b/internal/api/handlers/v0/auth/common.go @@ -8,6 +8,7 @@ import ( "crypto/sha512" "encoding/base64" "encoding/hex" + "errors" "fmt" "math/big" "regexp" @@ -18,6 +19,12 @@ import ( "github.com/modelcontextprotocol/registry/internal/config" ) +// ErrSignatureMismatch is returned by VerifySignature when the signature is structurally +// valid but does not verify against the public key. Distinguishing this from structural +// failures (wrong size, bad key format) lets the caller add fingerprint hints only when +// the failure is actually a "wrong key" situation. +var ErrSignatureMismatch = errors.New("signature does not match public key") + // CryptoAlgorithm represents the cryptographic algorithm used for a public key type CryptoAlgorithm string @@ -90,18 +97,72 @@ func DecodeAndValidateSignature(signedTimestamp string) ([]byte, error) { } func VerifySignatureWithKeys(publicKeys []PublicKeyInfo, messageBytes []byte, signature []byte) error { + var lastErr error + allMismatch := true for _, publicKeyInfo := range publicKeys { err := publicKeyInfo.VerifySignature(messageBytes, signature) if err == nil { return nil } - - if len(publicKeys) == 1 { - return err + lastErr = err + if !errors.Is(err, ErrSignatureMismatch) { + allMismatch = false } } - return fmt.Errorf("signature verification failed") + // If at least one key failed for a structural reason (wrong size, unsupported algorithm), + // surface that error directly — it's more actionable than a generic "didn't match" message. + if !allMismatch { + return lastErr + } + + // Every key was tried and produced a clean cryptographic mismatch. Include short + // fingerprints of every key that was tried so users can tell which published keys the + // registry actually saw — the most common cause of this error is a stale record left + // behind after a key rotation, which is otherwise indistinguishable from a generic + // crypto failure. + fingerprints := make([]string, 0, len(publicKeys)) + for _, publicKeyInfo := range publicKeys { + fingerprints = append(fingerprints, publicKeyInfo.Fingerprint()) + } + if len(publicKeys) == 1 { + return fmt.Errorf( + "signature verification failed (tried published key %s); "+ + "if this is not the key you are signing with, the published record may be stale", + fingerprints[0], + ) + } + return fmt.Errorf( + "signature verification failed against all %d published keys (tried: %s); "+ + "if you recently rotated keys, remove any stale records from the apex domain", + len(publicKeys), strings.Join(fingerprints, ", "), + ) +} + +// Fingerprint returns a short, human-readable identifier for the public key. +// Format: ":". Public keys are not secret, +// but truncating keeps error messages readable. +func (pki *PublicKeyInfo) Fingerprint() string { + const prefixLen = 8 + var raw []byte + switch pki.Algorithm { + case AlgorithmEd25519: + if k, ok := pki.Key.(ed25519.PublicKey); ok { + raw = k + } + case AlgorithmECDSAP384: + if k, ok := pki.Key.(ecdsa.PublicKey); ok { + raw = elliptic.MarshalCompressed(k.Curve, k.X, k.Y) //nolint:staticcheck // SA1019: matches the encoding used in DNS records + } + } + if len(raw) == 0 { + return fmt.Sprintf("%s:unknown", pki.Algorithm) + } + encoded := base64.StdEncoding.EncodeToString(raw) + if len(encoded) > prefixLen { + encoded = encoded[:prefixLen] + } + return fmt.Sprintf("%s:%s", pki.Algorithm, encoded) } // VerifySignature verifies a signature using the appropriate algorithm @@ -113,7 +174,7 @@ func (pki *PublicKeyInfo) VerifySignature(message, signature []byte) error { return fmt.Errorf("invalid signature size for Ed25519") } if !ed25519.Verify(ed25519Key, message, signature) { - return fmt.Errorf("Ed25519 signature verification failed") + return fmt.Errorf("Ed25519: %w", ErrSignatureMismatch) } return nil } @@ -126,7 +187,7 @@ func (pki *PublicKeyInfo) VerifySignature(message, signature []byte) error { s := new(big.Int).SetBytes(signature[48:]) digest := sha512.Sum384(message) if !ecdsa.Verify(&ecdsaKey, digest[:], r, s) { - return fmt.Errorf("ECDSA P-384 signature verification failed") + return fmt.Errorf("ECDSA P-384: %w", ErrSignatureMismatch) } return nil } diff --git a/internal/api/handlers/v0/auth/dns.go b/internal/api/handlers/v0/auth/dns.go index 5129486da..e735a1f6d 100644 --- a/internal/api/handlers/v0/auth/dns.go +++ b/internal/api/handlers/v0/auth/dns.go @@ -75,6 +75,11 @@ func RegisterDNSEndpoint(api huma.API, pathPrefix string, cfg *config.Config) { }) } +// commonWrongSelectors lists subdomain prefixes that users frequently mistake for the +// MCP DNS auth record location (DKIM-style intuition). MCP DNS auth uses the apex, +// like SPF — see #385, #1103, #1126 for the recurring confusion. +var commonWrongSelectors = []string{"_mcp-auth", "_mcp-registry"} + // ExchangeToken exchanges DNS signature for a Registry JWT token func (h *DNSAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, signedTimestamp string) (*auth.TokenResponse, error) { keyFetcher := func(ctx context.Context, domain string) ([]string, error) { @@ -90,9 +95,61 @@ func (h *DNSAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, s if err != nil { return nil, fmt.Errorf("failed to lookup DNS TXT records: %w", err) } + + if !hasMCPRecord(txtRecords) { + if found := h.findMisplacedSelector(timeoutCtx, domain); found != "" { + return nil, fmt.Errorf( + "no MCPv1 TXT record at %q, but one was found at %q — "+ + "MCP DNS auth requires the record at the apex domain, not under a selector", + domain, found, + ) + } + } + return txtRecords, nil } allowSubdomains := true return h.CoreAuthHandler.ExchangeToken(ctx, domain, timestamp, signedTimestamp, keyFetcher, allowSubdomains, auth.MethodDNS) } + +// hasMCPRecord reports whether any of the supplied TXT records contains an MCPv1 proof record. +func hasMCPRecord(records []string) bool { + for _, r := range records { + if strings.Contains(r, "v=MCPv1") { + return true + } + } + return false +} + +// findMisplacedSelector probes a small fixed set of common wrong selectors and returns the +// first one that holds an MCPv1 record, or "" if none do. Lookups run in parallel with a +// short individual timeout so a slow/missing zone never delays the response by much. +func (h *DNSAuthHandler) findMisplacedSelector(ctx context.Context, domain string) string { + type result struct { + name string + found bool + } + results := make(chan result, len(commonWrongSelectors)) + for _, selector := range commonWrongSelectors { + name := selector + "." + domain + go func(name string) { + lookupCtx, cancel := context.WithTimeout(ctx, 2*time.Second) + defer cancel() + records, err := h.resolver.LookupTXT(lookupCtx, name) + if err != nil { + results <- result{name: name, found: false} + return + } + results <- result{name: name, found: hasMCPRecord(records)} + }(name) + } + for range commonWrongSelectors { + r := <-results + if r.found { + return r.name + } + } + return "" +} diff --git a/internal/api/handlers/v0/auth/dns_test.go b/internal/api/handlers/v0/auth/dns_test.go index f0f3a566f..c7e191ac8 100644 --- a/internal/api/handlers/v0/auth/dns_test.go +++ b/internal/api/handlers/v0/auth/dns_test.go @@ -546,7 +546,7 @@ func TestDNSAuthHandler_ExchangeToken_ECDSAP384(t *testing.T) { timestamp: time.Now().UTC().Format(time.RFC3339), signedTimestamp: "abcdef1234", // too short for ECDSA P-384 expectError: true, - errorContains: "signature verification failed", // general error when trying all keys + errorContains: "invalid signature size for ECDSA P-384", }, { name: "wrong ECDSA P-384 key for signature", @@ -990,3 +990,91 @@ func TestDNSAuthHandler_Mixed_Algorithm_Support(t *testing.T) { assert.NotNil(t, result) }) } + +// TestDNSAuthHandler_WrongSelectorProbe covers the case where the user mistakenly placed +// the MCPv1 TXT record under a selector (e.g. _mcp-auth.) instead of the apex, +// which has been a recurring source of confusion (#385, #1103, #1126). +func TestDNSAuthHandler_WrongSelectorProbe(t *testing.T) { + cfg := &config.Config{ + JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + } + + publicKey, _, err := ed25519.GenerateKey(nil) + require.NoError(t, err) + publicKeyB64 := base64.StdEncoding.EncodeToString(publicKey) + mcpRecord := fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", publicKeyB64) + + tests := []struct { + name string + txtRecords map[string][]string + expectInError string + }{ + { + name: "record placed at _mcp-auth selector", + txtRecords: map[string][]string{ + "_mcp-auth." + testDomain: {mcpRecord}, + }, + expectInError: "_mcp-auth." + testDomain, + }, + { + name: "record placed at _mcp-registry selector", + txtRecords: map[string][]string{ + "_mcp-registry." + testDomain: {mcpRecord}, + }, + expectInError: "_mcp-registry." + testDomain, + }, + { + name: "no record anywhere falls through to standard error", + txtRecords: map[string][]string{ + testDomain: {"v=spf1 ~all"}, + }, + expectInError: "no MCP public key found in DNS TXT records", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + handler := auth.NewDNSAuthHandler(cfg) + handler.SetResolver(&MockDNSResolver{txtRecords: tt.txtRecords}) + + timestamp := time.Now().UTC().Format(time.RFC3339) + _, err := handler.ExchangeToken(context.Background(), testDomain, timestamp, hex.EncodeToString(make([]byte, 64))) + + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectInError) + }) + } +} + +// TestDNSAuthHandler_StaleKeyFingerprintInError covers #1126: when only one apex record is +// published and it doesn't match the key being signed with (the typical "rotated and forgot +// to update DNS" failure), the error message should include a fingerprint of the published +// key so the user can tell their CLI is signing with a different key than what's published. +func TestDNSAuthHandler_StaleKeyFingerprintInError(t *testing.T) { + cfg := &config.Config{ + JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + } + + stalePublicKey, _, err := ed25519.GenerateKey(nil) + require.NoError(t, err) + stalePublicKeyB64 := base64.StdEncoding.EncodeToString(stalePublicKey) + + _, currentPrivateKey, err := ed25519.GenerateKey(nil) + require.NoError(t, err) + + handler := auth.NewDNSAuthHandler(cfg) + handler.SetResolver(&MockDNSResolver{ + txtRecords: map[string][]string{ + testDomain: {fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", stalePublicKeyB64)}, + }, + }) + + timestamp := time.Now().UTC().Format(time.RFC3339) + signature := ed25519.Sign(currentPrivateKey, []byte(timestamp)) + _, err = handler.ExchangeToken(context.Background(), testDomain, timestamp, hex.EncodeToString(signature)) + + require.Error(t, err) + expectedFingerprint := "ed25519:" + stalePublicKeyB64[:8] + assert.Contains(t, err.Error(), expectedFingerprint, "error should expose the published key's fingerprint") + assert.Contains(t, err.Error(), "stale", "error should hint at the stale-record cause") +} From 17595ed40127123cda987dc582b554eedc14fa07 Mon Sep 17 00:00:00 2001 From: Radoslav Dimitrov Date: Sat, 25 Apr 2026 02:29:30 +0300 Subject: [PATCH 2/2] fix(auth): tighten MCPv1 record detection in selector probe A loose strings.Contains(r, "v=MCPv1") check at the apex would suppress the misplaced-selector probe whenever a malformed MCPv1 string was present, hiding the helpful "record is at _mcp-auth" hint from the user. Share the strict proof-record regex with the parser so presence detection and parsing agree on what counts as an MCPv1 record. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/api/handlers/v0/auth/common.go | 10 ++++++---- internal/api/handlers/v0/auth/dns.go | 6 ++++-- internal/api/handlers/v0/auth/dns_test.go | 9 +++++++++ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/internal/api/handlers/v0/auth/common.go b/internal/api/handlers/v0/auth/common.go index b4471948a..2f763d66e 100644 --- a/internal/api/handlers/v0/auth/common.go +++ b/internal/api/handlers/v0/auth/common.go @@ -25,6 +25,11 @@ import ( // the failure is actually a "wrong key" situation. var ErrSignatureMismatch = errors.New("signature does not match public key") +// MCPProofRecordPattern matches a well-formed MCPv1 DNS/HTTP proof record: +// "v=MCPv1; k=; p=". Shared so callers checking for the +// presence of a valid record see exactly what the parser will accept. +var MCPProofRecordPattern = regexp.MustCompile(`v=MCPv1;\s*k=([^;]+);\s*p=([A-Za-z0-9+/=]+)`) + // CryptoAlgorithm represents the cryptographic algorithm used for a public key type CryptoAlgorithm string @@ -307,11 +312,8 @@ func ParseMCPKeysFromStrings(inputs []string) []struct { error } - // proof record pattern: v=MCPv1; k=; p= - cryptoPattern := regexp.MustCompile(`v=MCPv1;\s*k=([^;]+);\s*p=([A-Za-z0-9+/=]+)`) - for _, record := range inputs { - if matches := cryptoPattern.FindStringSubmatch(record); len(matches) == 3 { + if matches := MCPProofRecordPattern.FindStringSubmatch(record); len(matches) == 3 { publicKey, err := ParsePublicKey(matches[1], matches[2]) publicKeys = append(publicKeys, struct { *PublicKeyInfo diff --git a/internal/api/handlers/v0/auth/dns.go b/internal/api/handlers/v0/auth/dns.go index e735a1f6d..cc1808218 100644 --- a/internal/api/handlers/v0/auth/dns.go +++ b/internal/api/handlers/v0/auth/dns.go @@ -113,10 +113,12 @@ func (h *DNSAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, s return h.CoreAuthHandler.ExchangeToken(ctx, domain, timestamp, signedTimestamp, keyFetcher, allowSubdomains, auth.MethodDNS) } -// hasMCPRecord reports whether any of the supplied TXT records contains an MCPv1 proof record. +// hasMCPRecord reports whether any of the supplied TXT records contains a well-formed +// MCPv1 proof record. Uses the same strict pattern as the parser so a malformed +// "v=MCPv1" string at the apex doesn't suppress the misplaced-selector probe. func hasMCPRecord(records []string) bool { for _, r := range records { - if strings.Contains(r, "v=MCPv1") { + if MCPProofRecordPattern.MatchString(r) { return true } } diff --git a/internal/api/handlers/v0/auth/dns_test.go b/internal/api/handlers/v0/auth/dns_test.go index c7e191ac8..b4895ab57 100644 --- a/internal/api/handlers/v0/auth/dns_test.go +++ b/internal/api/handlers/v0/auth/dns_test.go @@ -1030,6 +1030,15 @@ func TestDNSAuthHandler_WrongSelectorProbe(t *testing.T) { }, expectInError: "no MCP public key found in DNS TXT records", }, + { + name: "malformed MCPv1 string at apex still triggers selector probe", + txtRecords: map[string][]string{ + // Looks like an MCPv1 record but missing the public key field. + testDomain: {"v=MCPv1; k=ed25519"}, + "_mcp-auth." + testDomain: {mcpRecord}, + }, + expectInError: "_mcp-auth." + testDomain, + }, } for _, tt := range tests {