diff --git a/internal/api/handlers/v0/auth/dns.go b/internal/api/handlers/v0/auth/dns.go index 991f5132..db06fd54 100644 --- a/internal/api/handlers/v0/auth/dns.go +++ b/internal/api/handlers/v0/auth/dns.go @@ -40,6 +40,12 @@ func (r *DefaultDNSResolver) LookupTXT(ctx context.Context, name string) ([]stri return (&net.Resolver{}).LookupTXT(ctx, name) } +// DNSAuthRecord represents a DNS TXT authentication record with optional name pattern +type DNSAuthRecord struct { + PublicKey ed25519.PublicKey + NamePattern string // Defaults to "*" for wildcard access +} + // DNSAuthHandler handles DNS-based authentication type DNSAuthHandler struct { config *config.Config @@ -120,29 +126,34 @@ func (h *DNSAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, s return nil, fmt.Errorf("failed to lookup DNS TXT records: %w", err) } - // Parse public keys from TXT records - publicKeys := h.parsePublicKeysFromTXT(txtRecords) + // Parse auth records from TXT records + authRecords := h.parseAuthRecordsFromTXT(txtRecords) - if len(publicKeys) == 0 { + if len(authRecords) == 0 { return nil, fmt.Errorf("no valid MCP public keys found in DNS TXT records") } - // Verify signature with any of the public keys + // Verify signature and collect all valid records messageBytes := []byte(timestamp) - signatureValid := false - for _, publicKey := range publicKeys { - if ed25519.Verify(publicKey, messageBytes, signature) { - signatureValid = true - break + var validRecords []DNSAuthRecord + for _, record := range authRecords { + if ed25519.Verify(record.PublicKey, messageBytes, signature) { + validRecords = append(validRecords, record) } } - if !signatureValid { + if len(validRecords) == 0 { return nil, fmt.Errorf("signature verification failed") } - // Build permissions for domain and subdomains - permissions := h.buildPermissions(domain) + // Build permissions from all valid records + var permissions []auth.Permission + for _, record := range validRecords { + permissions = append(permissions, h.buildPermissions(domain, record.NamePattern)...) + } + if len(permissions) == 0 { + return nil, fmt.Errorf("no valid permissions found for the given name pattern") + } // Create JWT claims jwtClaims := auth.JWTClaims{ @@ -160,14 +171,16 @@ func (h *DNSAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, s return tokenResponse, nil } -// parsePublicKeysFromTXT parses Ed25519 public keys from DNS TXT records -func (h *DNSAuthHandler) parsePublicKeysFromTXT(txtRecords []string) []ed25519.PublicKey { - var publicKeys []ed25519.PublicKey - mcpPattern := regexp.MustCompile(`v=MCPv1;\s*k=ed25519;\s*p=([A-Za-z0-9+/=]+)`) +// parseAuthRecordsFromTXT parses DNS authentication records from TXT records +// Supports optional n= parameter for name scoping +func (h *DNSAuthHandler) parseAuthRecordsFromTXT(txtRecords []string) []DNSAuthRecord { + var authRecords []DNSAuthRecord + // Updated pattern to capture optional n= parameter + mcpPattern := regexp.MustCompile(`v=MCPv1;\s*k=ed25519;\s*p=([A-Za-z0-9+/=]+)(?:;\s*n=([^;]+))?`) for _, record := range txtRecords { matches := mcpPattern.FindStringSubmatch(record) - if len(matches) == 2 { + if len(matches) >= 2 { // Decode base64 public key publicKeyBytes, err := base64.StdEncoding.DecodeString(matches[1]) if err != nil { @@ -178,29 +191,79 @@ func (h *DNSAuthHandler) parsePublicKeysFromTXT(txtRecords []string) []ed25519.P continue // Skip invalid key sizes } - publicKeys = append(publicKeys, ed25519.PublicKey(publicKeyBytes)) + // Extract name pattern or default to wildcard + namePattern := "*" + if len(matches) > 2 && matches[2] != "" { + namePattern = strings.TrimSpace(matches[2]) + } + + authRecords = append(authRecords, DNSAuthRecord{ + PublicKey: ed25519.PublicKey(publicKeyBytes), + NamePattern: namePattern, + }) } } - return publicKeys + return authRecords } -// buildPermissions builds permissions for a domain and its subdomains using reverse DNS notation -func (h *DNSAuthHandler) buildPermissions(domain string) []auth.Permission { +// buildPermissions builds permissions based on domain and name pattern +// namePattern defaults to "*" for wildcard access (backward compatible) +func (h *DNSAuthHandler) buildPermissions(domain string, namePattern string) []auth.Permission { reverseDomain := reverseString(domain) + // If namePattern is "*", grant traditional wildcard permissions + if namePattern == "*" { + permissions := []auth.Permission{ + // Grant permissions for the exact domain (e.g., com.example/*) + { + Action: auth.PermissionActionPublish, + ResourcePattern: fmt.Sprintf("%s/*", reverseDomain), + }, + // DNS implies a hierarchy where subdomains are treated as part of the parent domain, + // therefore we grant permissions for all subdomains (e.g., com.example.*) + // This is in line with other DNS-based authentication methods e.g. ACME DNS-01 challenges + { + Action: auth.PermissionActionPublish, + ResourcePattern: fmt.Sprintf("%s.*", reverseDomain), + }, + } + return permissions + } + + // For specific name patterns, grant permission only for the specified pattern + // This allows DNS controllers to scope permissions to specific prefixes + // The name pattern MUST be scoped to the domain it is on. + // We need to ensure proper delimiter checking to prevent prefix attacks + // e.g., micro.com should not be able to claim com.microsoft/* + if !strings.HasPrefix(namePattern, reverseDomain) { + return []auth.Permission{} + } + + // Check that after the reverse domain, there's either: + // - nothing (exact match) + // - a '.' (subdomain like com.example.api) + // - a '/' (path like com.example/foo) + if len(namePattern) > len(reverseDomain) { + delimiter := namePattern[len(reverseDomain)] + if delimiter != '.' && delimiter != '/' { + // Invalid pattern - doesn't have proper delimiter after domain + return []auth.Permission{} + } + } + + // Validate server name format: should have exactly one slash + // This aligns with PR #476 requirements + slashCount := strings.Count(namePattern, "/") + if slashCount > 1 { + // Invalid pattern - multiple slashes not allowed in server names + return []auth.Permission{} + } + permissions := []auth.Permission{ - // Grant permissions for the exact domain (e.g., com.example/*) - { - Action: auth.PermissionActionPublish, - ResourcePattern: fmt.Sprintf("%s/*", reverseDomain), - }, - // DNS implies a hierarchy where subdomains are treated as part of the parent domain, - // therefore we grant permissions for all subdomains (e.g., com.example.*) - // This is in line with other DNS-based authentication methods e.g. ACME DNS-01 challenges { Action: auth.PermissionActionPublish, - ResourcePattern: fmt.Sprintf("%s.*", reverseDomain), + ResourcePattern: namePattern, }, } diff --git a/internal/api/handlers/v0/auth/dns_test.go b/internal/api/handlers/v0/auth/dns_test.go index e9c10d52..3d93b5ae 100644 --- a/internal/api/handlers/v0/auth/dns_test.go +++ b/internal/api/handlers/v0/auth/dns_test.go @@ -194,3 +194,235 @@ func TestDNSAuthHandler_ExchangeToken(t *testing.T) { }) } } + +func TestDNSAuthHandler_NamePatternScoping(t *testing.T) { + cfg := &config.Config{ + JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + } + handler := auth.NewDNSAuthHandler(cfg) + + // Generate test key pairs + publicKey1, privateKey1, err := ed25519.GenerateKey(nil) + require.NoError(t, err) + publicKey2, privateKey2, err := ed25519.GenerateKey(nil) + require.NoError(t, err) + + publicKeyB64_1 := base64.StdEncoding.EncodeToString(publicKey1) + publicKeyB64_2 := base64.StdEncoding.EncodeToString(publicKey2) + + tests := []struct { + name string + domain string + txtRecords []string + usePrivateKey ed25519.PrivateKey + expectedPatterns []string + expectError bool + errorContains string + expectedNumPerms int + }{ + { + name: "wildcard pattern (backward compatible)", + domain: "example.com", + txtRecords: []string{ + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", publicKeyB64_1), + }, + usePrivateKey: privateKey1, + expectedPatterns: []string{"com.example/*", "com.example.*"}, + expectedNumPerms: 2, + expectError: false, + }, + { + name: "specific name pattern", + domain: "microsoft.com", + txtRecords: []string{ + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n=com.microsoft/team-foo-*", publicKeyB64_1), + }, + usePrivateKey: privateKey1, + expectedPatterns: []string{"com.microsoft/team-foo-*"}, + expectedNumPerms: 1, + expectError: false, + }, + { + name: "multiple keys with different patterns", + domain: "example.com", + txtRecords: []string{ + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n=com.example/app1-*", publicKeyB64_1), + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n=com.example/app2-*", publicKeyB64_2), + }, + usePrivateKey: privateKey2, + expectedPatterns: []string{"com.example/app2-*"}, + expectedNumPerms: 1, + expectError: false, + }, + { + name: "explicit wildcard in n parameter", + domain: "test.com", + txtRecords: []string{ + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n=*", publicKeyB64_1), + }, + usePrivateKey: privateKey1, + expectedPatterns: []string{"com.test/*", "com.test.*"}, + expectedNumPerms: 2, + expectError: false, + }, + { + name: "subdomain scoping pattern", + domain: "example.com", + txtRecords: []string{ + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n=com.example.api/*", publicKeyB64_1), + }, + usePrivateKey: privateKey1, + expectedPatterns: []string{"com.example.api/*"}, + expectedNumPerms: 1, + expectError: false, + }, + { + name: "pattern with spaces (should trim)", + domain: "example.com", + txtRecords: []string{ + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n= com.example/test-* ", publicKeyB64_1), + }, + usePrivateKey: privateKey1, + expectedPatterns: []string{"com.example/test-*"}, + expectedNumPerms: 1, + expectError: false, + }, + { + name: "mixing patterns - one with, one without", + domain: "example.com", + txtRecords: []string{ + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", publicKeyB64_1), + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n=com.example/limited-*", publicKeyB64_2), + }, + usePrivateKey: privateKey1, + expectedPatterns: []string{"com.example/*", "com.example.*"}, + expectedNumPerms: 2, + expectError: false, + }, + { + name: "pattern with multiple slashes should be rejected", + domain: "example.com", + txtRecords: []string{ + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n=com.example/services/auth/*", publicKeyB64_1), + }, + usePrivateKey: privateKey1, + expectError: true, + errorContains: "no valid permissions found", + }, + { + name: "valid single slash pattern", + domain: "example.com", + txtRecords: []string{ + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n=com.example/my-server", publicKeyB64_1), + }, + usePrivateKey: privateKey1, + expectedPatterns: []string{"com.example/my-server"}, + expectedNumPerms: 1, + expectError: false, + }, + { + name: "multiple valid records with the same key", + domain: "example.com", + txtRecords: []string{ + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n=com.example/foo-*", publicKeyB64_1), + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n=com.example/bar-*", publicKeyB64_1), + }, + usePrivateKey: privateKey1, + expectedPatterns: []string{"com.example/foo-*", "com.example/bar-*"}, + expectedNumPerms: 2, + expectError: false, + }, + { + name: "name pattern does not start with reverse domain", + domain: "example.com", + txtRecords: []string{ + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n=org.example/foo-*", publicKeyB64_1), + }, + usePrivateKey: privateKey1, + expectError: true, + errorContains: "no valid permissions found", + }, + { + name: "malicious prefix overlap - micro.com trying to claim microsoft", + domain: "micro.com", + txtRecords: []string{ + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n=com.microsoft/*", publicKeyB64_1), + }, + usePrivateKey: privateKey1, + expectError: true, + errorContains: "no valid permissions found", + }, + { + name: "malicious prefix overlap - example.com trying to claim example.org", + domain: "example.com", + txtRecords: []string{ + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n=com.examplecorp/*", publicKeyB64_1), + }, + usePrivateKey: privateKey1, + expectError: true, + errorContains: "no valid permissions found", + }, + { + name: "name pattern is a subdomain", + domain: "example.com", + txtRecords: []string{ + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s; n=com.example.sub/*", publicKeyB64_1), + }, + usePrivateKey: privateKey1, + expectedPatterns: []string{"com.example.sub/*"}, + expectedNumPerms: 1, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create mock DNS resolver + mockResolver := &MockDNSResolver{ + txtRecords: map[string][]string{ + tt.domain: tt.txtRecords, + }, + } + handler.SetResolver(mockResolver) + + // Create timestamp and signature + timestamp := time.Now().UTC().Format(time.RFC3339) + signature := ed25519.Sign(tt.usePrivateKey, []byte(timestamp)) + signedTimestamp := hex.EncodeToString(signature) + + // Call the handler + result, err := handler.ExchangeToken(context.Background(), tt.domain, timestamp, signedTimestamp) + + if tt.expectError { + assert.Error(t, err) + if tt.errorContains != "" { + assert.Contains(t, err.Error(), tt.errorContains) + } + assert.Nil(t, result) + } else { + assert.NoError(t, err) + assert.NotNil(t, result) + assert.NotEmpty(t, result.RegistryToken) + + // Verify the token contains expected claims + jwtManager := intauth.NewJWTManager(cfg) + claims, err := jwtManager.ValidateToken(context.Background(), result.RegistryToken) + require.NoError(t, err) + + assert.Equal(t, intauth.MethodDNS, claims.AuthMethod) + assert.Equal(t, tt.domain, claims.AuthMethodSubject) + assert.Len(t, claims.Permissions, tt.expectedNumPerms) + + // Check permissions match expected patterns + patterns := make([]string, len(claims.Permissions)) + for i, perm := range claims.Permissions { + patterns[i] = perm.ResourcePattern + } + + for _, expectedPattern := range tt.expectedPatterns { + assert.Contains(t, patterns, expectedPattern, "Expected pattern %s not found in permissions", expectedPattern) + } + } + }) + } +}