Skip to content

Commit c5f50e8

Browse files
rdimitrovclaude
andauthored
fix(auth): better DNS verification errors and wrong-selector hint (#1202)
## Summary DNS auth users have repeatedly hit two failure modes that both produce the same opaque "Ed25519 signature verification failed" error, making them indistinguishable to debug. This PR makes both diagnosable end-to-end without changing the auth protocol or any successful path. The two recurring failure modes: 1. **Wrong placement** — TXT record put under a selector (`_mcp-auth.<domain>` or `_mcp-registry.<domain>`) instead of the apex. DKIM-style intuition, but MCP DNS auth uses SPF-style apex placement. See #385 (PR #388 fixed the docs but the failure recurred), #1103, #1126. 2. **Stale apex record** — old TXT record left behind after a key rotation, silently tried first and rejected. ## Changes **`internal/api/handlers/v0/auth/dns.go`** — when the apex has no `v=MCPv1` record, probe `_mcp-auth.<domain>` and `_mcp-registry.<domain>` in parallel (2s individual timeout). If a record is found there, return a targeted error pointing the user at the apex. Costs zero extra DNS work in the success path. **`internal/api/handlers/v0/auth/common.go`** — when verification fails against records that *are* present, include short fingerprints of every key the registry tried (`ed25519:YJLsHFhu`-style 8-char prefix). A new `ErrSignatureMismatch` sentinel keeps structural errors (wrong signature size, unsupported algorithm) passing through unchanged via `errors.Is`, so existing assertions on those messages still hold. **`docs/modelcontextprotocol-io/authentication.mdx`** — anti-pattern `<Warning>` callout in the DNS section explicitly warning against selector placement and stale records. **Tests** — two new tests: - `TestDNSAuthHandler_WrongSelectorProbe` — verifies the probe surfaces records found at `_mcp-auth.` / `_mcp-registry.` and falls through to the standard error when nothing is found anywhere - `TestDNSAuthHandler_StaleKeyFingerprintInError` — verifies the fingerprint hint appears in the failure message for #1126's exact scenario One existing test assertion was updated from a generic `"signature verification failed"` substring to the more specific `"invalid signature size for ECDSA P-384"` it should always have been asserting; the new sentinel-based logic surfaces structural errors verbatim. ## Test plan - [x] `make lint` — clean - [x] `make test-unit` — all auth tests pass (existing + 2 new) - [x] Verified existing single-key and multi-key paths preserve their original (more specific) error messages where they were already specific - [ ] Reviewer to sanity-check the docs callout renders well in the Mintlify theme ## Out of scope The publisher-side preflight DNS check (cross-link in #845) is intentionally **not** in this PR — that belongs in the broader CLI UX work tracked by #845. Refs #845, #385, #1103 Fixes #1126 --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8b893ae commit c5f50e8

4 files changed

Lines changed: 236 additions & 11 deletions

File tree

docs/modelcontextprotocol-io/authentication.mdx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ Successfully authenticated!
5151

5252
DNS authentication is a domain-based authentication method that relies on a DNS TXT record.
5353

54+
<Warning>
55+
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.
56+
57+
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.
58+
</Warning>
59+
5460
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:
5561

5662
<CodeGroup>

internal/api/handlers/v0/auth/common.go

Lines changed: 73 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"crypto/sha512"
99
"encoding/base64"
1010
"encoding/hex"
11+
"errors"
1112
"fmt"
1213
"math/big"
1314
"regexp"
@@ -18,6 +19,17 @@ import (
1819
"github.com/modelcontextprotocol/registry/internal/config"
1920
)
2021

22+
// ErrSignatureMismatch is returned by VerifySignature when the signature is structurally
23+
// valid but does not verify against the public key. Distinguishing this from structural
24+
// failures (wrong size, bad key format) lets the caller add fingerprint hints only when
25+
// the failure is actually a "wrong key" situation.
26+
var ErrSignatureMismatch = errors.New("signature does not match public key")
27+
28+
// MCPProofRecordPattern matches a well-formed MCPv1 DNS/HTTP proof record:
29+
// "v=MCPv1; k=<algo>; p=<base64-public-key>". Shared so callers checking for the
30+
// presence of a valid record see exactly what the parser will accept.
31+
var MCPProofRecordPattern = regexp.MustCompile(`v=MCPv1;\s*k=([^;]+);\s*p=([A-Za-z0-9+/=]+)`)
32+
2133
// CryptoAlgorithm represents the cryptographic algorithm used for a public key
2234
type CryptoAlgorithm string
2335

@@ -90,18 +102,72 @@ func DecodeAndValidateSignature(signedTimestamp string) ([]byte, error) {
90102
}
91103

92104
func VerifySignatureWithKeys(publicKeys []PublicKeyInfo, messageBytes []byte, signature []byte) error {
105+
var lastErr error
106+
allMismatch := true
93107
for _, publicKeyInfo := range publicKeys {
94108
err := publicKeyInfo.VerifySignature(messageBytes, signature)
95109
if err == nil {
96110
return nil
97111
}
98-
99-
if len(publicKeys) == 1 {
100-
return err
112+
lastErr = err
113+
if !errors.Is(err, ErrSignatureMismatch) {
114+
allMismatch = false
101115
}
102116
}
103117

104-
return fmt.Errorf("signature verification failed")
118+
// If at least one key failed for a structural reason (wrong size, unsupported algorithm),
119+
// surface that error directly — it's more actionable than a generic "didn't match" message.
120+
if !allMismatch {
121+
return lastErr
122+
}
123+
124+
// Every key was tried and produced a clean cryptographic mismatch. Include short
125+
// fingerprints of every key that was tried so users can tell which published keys the
126+
// registry actually saw — the most common cause of this error is a stale record left
127+
// behind after a key rotation, which is otherwise indistinguishable from a generic
128+
// crypto failure.
129+
fingerprints := make([]string, 0, len(publicKeys))
130+
for _, publicKeyInfo := range publicKeys {
131+
fingerprints = append(fingerprints, publicKeyInfo.Fingerprint())
132+
}
133+
if len(publicKeys) == 1 {
134+
return fmt.Errorf(
135+
"signature verification failed (tried published key %s); "+
136+
"if this is not the key you are signing with, the published record may be stale",
137+
fingerprints[0],
138+
)
139+
}
140+
return fmt.Errorf(
141+
"signature verification failed against all %d published keys (tried: %s); "+
142+
"if you recently rotated keys, remove any stale records from the apex domain",
143+
len(publicKeys), strings.Join(fingerprints, ", "),
144+
)
145+
}
146+
147+
// Fingerprint returns a short, human-readable identifier for the public key.
148+
// Format: "<algorithm>:<first 8 base64 chars of the raw key>". Public keys are not secret,
149+
// but truncating keeps error messages readable.
150+
func (pki *PublicKeyInfo) Fingerprint() string {
151+
const prefixLen = 8
152+
var raw []byte
153+
switch pki.Algorithm {
154+
case AlgorithmEd25519:
155+
if k, ok := pki.Key.(ed25519.PublicKey); ok {
156+
raw = k
157+
}
158+
case AlgorithmECDSAP384:
159+
if k, ok := pki.Key.(ecdsa.PublicKey); ok {
160+
raw = elliptic.MarshalCompressed(k.Curve, k.X, k.Y) //nolint:staticcheck // SA1019: matches the encoding used in DNS records
161+
}
162+
}
163+
if len(raw) == 0 {
164+
return fmt.Sprintf("%s:unknown", pki.Algorithm)
165+
}
166+
encoded := base64.StdEncoding.EncodeToString(raw)
167+
if len(encoded) > prefixLen {
168+
encoded = encoded[:prefixLen]
169+
}
170+
return fmt.Sprintf("%s:%s", pki.Algorithm, encoded)
105171
}
106172

107173
// VerifySignature verifies a signature using the appropriate algorithm
@@ -113,7 +179,7 @@ func (pki *PublicKeyInfo) VerifySignature(message, signature []byte) error {
113179
return fmt.Errorf("invalid signature size for Ed25519")
114180
}
115181
if !ed25519.Verify(ed25519Key, message, signature) {
116-
return fmt.Errorf("Ed25519 signature verification failed")
182+
return fmt.Errorf("Ed25519: %w", ErrSignatureMismatch)
117183
}
118184
return nil
119185
}
@@ -126,7 +192,7 @@ func (pki *PublicKeyInfo) VerifySignature(message, signature []byte) error {
126192
s := new(big.Int).SetBytes(signature[48:])
127193
digest := sha512.Sum384(message)
128194
if !ecdsa.Verify(&ecdsaKey, digest[:], r, s) {
129-
return fmt.Errorf("ECDSA P-384 signature verification failed")
195+
return fmt.Errorf("ECDSA P-384: %w", ErrSignatureMismatch)
130196
}
131197
return nil
132198
}
@@ -246,11 +312,8 @@ func ParseMCPKeysFromStrings(inputs []string) []struct {
246312
error
247313
}
248314

249-
// proof record pattern: v=MCPv1; k=<algo>; p=<base64-public-key>
250-
cryptoPattern := regexp.MustCompile(`v=MCPv1;\s*k=([^;]+);\s*p=([A-Za-z0-9+/=]+)`)
251-
252315
for _, record := range inputs {
253-
if matches := cryptoPattern.FindStringSubmatch(record); len(matches) == 3 {
316+
if matches := MCPProofRecordPattern.FindStringSubmatch(record); len(matches) == 3 {
254317
publicKey, err := ParsePublicKey(matches[1], matches[2])
255318
publicKeys = append(publicKeys, struct {
256319
*PublicKeyInfo

internal/api/handlers/v0/auth/dns.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ func RegisterDNSEndpoint(api huma.API, pathPrefix string, cfg *config.Config) {
7575
})
7676
}
7777

78+
// commonWrongSelectors lists subdomain prefixes that users frequently mistake for the
79+
// MCP DNS auth record location (DKIM-style intuition). MCP DNS auth uses the apex,
80+
// like SPF — see #385, #1103, #1126 for the recurring confusion.
81+
var commonWrongSelectors = []string{"_mcp-auth", "_mcp-registry"}
82+
7883
// ExchangeToken exchanges DNS signature for a Registry JWT token
7984
func (h *DNSAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, signedTimestamp string) (*auth.TokenResponse, error) {
8085
keyFetcher := func(ctx context.Context, domain string) ([]string, error) {
@@ -90,9 +95,63 @@ func (h *DNSAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, s
9095
if err != nil {
9196
return nil, fmt.Errorf("failed to lookup DNS TXT records: %w", err)
9297
}
98+
99+
if !hasMCPRecord(txtRecords) {
100+
if found := h.findMisplacedSelector(timeoutCtx, domain); found != "" {
101+
return nil, fmt.Errorf(
102+
"no MCPv1 TXT record at %q, but one was found at %q — "+
103+
"MCP DNS auth requires the record at the apex domain, not under a selector",
104+
domain, found,
105+
)
106+
}
107+
}
108+
93109
return txtRecords, nil
94110
}
95111

96112
allowSubdomains := true
97113
return h.CoreAuthHandler.ExchangeToken(ctx, domain, timestamp, signedTimestamp, keyFetcher, allowSubdomains, auth.MethodDNS)
98114
}
115+
116+
// hasMCPRecord reports whether any of the supplied TXT records contains a well-formed
117+
// MCPv1 proof record. Uses the same strict pattern as the parser so a malformed
118+
// "v=MCPv1" string at the apex doesn't suppress the misplaced-selector probe.
119+
func hasMCPRecord(records []string) bool {
120+
for _, r := range records {
121+
if MCPProofRecordPattern.MatchString(r) {
122+
return true
123+
}
124+
}
125+
return false
126+
}
127+
128+
// findMisplacedSelector probes a small fixed set of common wrong selectors and returns the
129+
// first one that holds an MCPv1 record, or "" if none do. Lookups run in parallel with a
130+
// short individual timeout so a slow/missing zone never delays the response by much.
131+
func (h *DNSAuthHandler) findMisplacedSelector(ctx context.Context, domain string) string {
132+
type result struct {
133+
name string
134+
found bool
135+
}
136+
results := make(chan result, len(commonWrongSelectors))
137+
for _, selector := range commonWrongSelectors {
138+
name := selector + "." + domain
139+
go func(name string) {
140+
lookupCtx, cancel := context.WithTimeout(ctx, 2*time.Second)
141+
defer cancel()
142+
records, err := h.resolver.LookupTXT(lookupCtx, name)
143+
if err != nil {
144+
results <- result{name: name, found: false}
145+
return
146+
}
147+
results <- result{name: name, found: hasMCPRecord(records)}
148+
}(name)
149+
}
150+
for range commonWrongSelectors {
151+
r := <-results
152+
if r.found {
153+
return r.name
154+
}
155+
}
156+
return ""
157+
}

internal/api/handlers/v0/auth/dns_test.go

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,7 @@ func TestDNSAuthHandler_ExchangeToken_ECDSAP384(t *testing.T) {
546546
timestamp: time.Now().UTC().Format(time.RFC3339),
547547
signedTimestamp: "abcdef1234", // too short for ECDSA P-384
548548
expectError: true,
549-
errorContains: "signature verification failed", // general error when trying all keys
549+
errorContains: "invalid signature size for ECDSA P-384",
550550
},
551551
{
552552
name: "wrong ECDSA P-384 key for signature",
@@ -990,3 +990,100 @@ func TestDNSAuthHandler_Mixed_Algorithm_Support(t *testing.T) {
990990
assert.NotNil(t, result)
991991
})
992992
}
993+
994+
// TestDNSAuthHandler_WrongSelectorProbe covers the case where the user mistakenly placed
995+
// the MCPv1 TXT record under a selector (e.g. _mcp-auth.<domain>) instead of the apex,
996+
// which has been a recurring source of confusion (#385, #1103, #1126).
997+
func TestDNSAuthHandler_WrongSelectorProbe(t *testing.T) {
998+
cfg := &config.Config{
999+
JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
1000+
}
1001+
1002+
publicKey, _, err := ed25519.GenerateKey(nil)
1003+
require.NoError(t, err)
1004+
publicKeyB64 := base64.StdEncoding.EncodeToString(publicKey)
1005+
mcpRecord := fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", publicKeyB64)
1006+
1007+
tests := []struct {
1008+
name string
1009+
txtRecords map[string][]string
1010+
expectInError string
1011+
}{
1012+
{
1013+
name: "record placed at _mcp-auth selector",
1014+
txtRecords: map[string][]string{
1015+
"_mcp-auth." + testDomain: {mcpRecord},
1016+
},
1017+
expectInError: "_mcp-auth." + testDomain,
1018+
},
1019+
{
1020+
name: "record placed at _mcp-registry selector",
1021+
txtRecords: map[string][]string{
1022+
"_mcp-registry." + testDomain: {mcpRecord},
1023+
},
1024+
expectInError: "_mcp-registry." + testDomain,
1025+
},
1026+
{
1027+
name: "no record anywhere falls through to standard error",
1028+
txtRecords: map[string][]string{
1029+
testDomain: {"v=spf1 ~all"},
1030+
},
1031+
expectInError: "no MCP public key found in DNS TXT records",
1032+
},
1033+
{
1034+
name: "malformed MCPv1 string at apex still triggers selector probe",
1035+
txtRecords: map[string][]string{
1036+
// Looks like an MCPv1 record but missing the public key field.
1037+
testDomain: {"v=MCPv1; k=ed25519"},
1038+
"_mcp-auth." + testDomain: {mcpRecord},
1039+
},
1040+
expectInError: "_mcp-auth." + testDomain,
1041+
},
1042+
}
1043+
1044+
for _, tt := range tests {
1045+
t.Run(tt.name, func(t *testing.T) {
1046+
handler := auth.NewDNSAuthHandler(cfg)
1047+
handler.SetResolver(&MockDNSResolver{txtRecords: tt.txtRecords})
1048+
1049+
timestamp := time.Now().UTC().Format(time.RFC3339)
1050+
_, err := handler.ExchangeToken(context.Background(), testDomain, timestamp, hex.EncodeToString(make([]byte, 64)))
1051+
1052+
require.Error(t, err)
1053+
assert.Contains(t, err.Error(), tt.expectInError)
1054+
})
1055+
}
1056+
}
1057+
1058+
// TestDNSAuthHandler_StaleKeyFingerprintInError covers #1126: when only one apex record is
1059+
// published and it doesn't match the key being signed with (the typical "rotated and forgot
1060+
// to update DNS" failure), the error message should include a fingerprint of the published
1061+
// key so the user can tell their CLI is signing with a different key than what's published.
1062+
func TestDNSAuthHandler_StaleKeyFingerprintInError(t *testing.T) {
1063+
cfg := &config.Config{
1064+
JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
1065+
}
1066+
1067+
stalePublicKey, _, err := ed25519.GenerateKey(nil)
1068+
require.NoError(t, err)
1069+
stalePublicKeyB64 := base64.StdEncoding.EncodeToString(stalePublicKey)
1070+
1071+
_, currentPrivateKey, err := ed25519.GenerateKey(nil)
1072+
require.NoError(t, err)
1073+
1074+
handler := auth.NewDNSAuthHandler(cfg)
1075+
handler.SetResolver(&MockDNSResolver{
1076+
txtRecords: map[string][]string{
1077+
testDomain: {fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", stalePublicKeyB64)},
1078+
},
1079+
})
1080+
1081+
timestamp := time.Now().UTC().Format(time.RFC3339)
1082+
signature := ed25519.Sign(currentPrivateKey, []byte(timestamp))
1083+
_, err = handler.ExchangeToken(context.Background(), testDomain, timestamp, hex.EncodeToString(signature))
1084+
1085+
require.Error(t, err)
1086+
expectedFingerprint := "ed25519:" + stalePublicKeyB64[:8]
1087+
assert.Contains(t, err.Error(), expectedFingerprint, "error should expose the published key's fingerprint")
1088+
assert.Contains(t, err.Error(), "stale", "error should hint at the stale-record cause")
1089+
}

0 commit comments

Comments
 (0)