diff --git a/pkg/analyzer/authentication.go b/pkg/analyzer/authentication.go index 02f8b28..07f7794 100644 --- a/pkg/analyzer/authentication.go +++ b/pkg/analyzer/authentication.go @@ -50,13 +50,6 @@ func (a *AuthenticationAnalyzer) AnalyzeAuthentication(email *EmailMessage) *api results.Spf = a.parseLegacySPF(email) } - if results.Dkim == nil || len(*results.Dkim) == 0 { - dkimResults := a.parseLegacyDKIM(email) - if len(dkimResults) > 0 { - results.Dkim = &dkimResults - } - } - // Parse ARC headers if not already parsed from Authentication-Results if results.Arc == nil { results.Arc = a.parseARCHeaders(email) diff --git a/pkg/analyzer/authentication_dkim.go b/pkg/analyzer/authentication_dkim.go index 9f1774b..b6cf5f8 100644 --- a/pkg/analyzer/authentication_dkim.go +++ b/pkg/analyzer/authentication_dkim.go @@ -59,40 +59,6 @@ func (a *AuthenticationAnalyzer) parseDKIMResult(part string) *api.AuthResult { return result } -// parseLegacyDKIM attempts to parse DKIM from DKIM-Signature header -func (a *AuthenticationAnalyzer) parseLegacyDKIM(email *EmailMessage) []api.AuthResult { - var results []api.AuthResult - - // Get all DKIM-Signature headers - dkimHeaders := email.Header[textprotoCanonical("DKIM-Signature")] - for _, dkimHeader := range dkimHeaders { - result := api.AuthResult{ - Result: api.AuthResultResultNone, // We can't determine pass/fail from signature alone - } - - // Extract domain (d=) - domainRe := regexp.MustCompile(`d=([^\s;]+)`) - if matches := domainRe.FindStringSubmatch(dkimHeader); len(matches) > 1 { - domain := matches[1] - result.Domain = &domain - } - - // Extract selector (s=) - selectorRe := regexp.MustCompile(`s=([^\s;]+)`) - if matches := selectorRe.FindStringSubmatch(dkimHeader); len(matches) > 1 { - selector := matches[1] - result.Selector = &selector - } - - details := "DKIM signature present (verification status unknown)" - result.Details = &details - - results = append(results, result) - } - - return results -} - func (a *AuthenticationAnalyzer) calculateDKIMScore(results *api.AuthenticationResults) (score int) { // Expect at least one passing signature if results.Dkim != nil && len(*results.Dkim) > 0 { diff --git a/pkg/analyzer/authentication_dkim_test.go b/pkg/analyzer/authentication_dkim_test.go index 323e421..2aab530 100644 --- a/pkg/analyzer/authentication_dkim_test.go +++ b/pkg/analyzer/authentication_dkim_test.go @@ -22,7 +22,6 @@ package analyzer import ( - "strings" "testing" "git.happydns.org/happyDeliver/internal/api" @@ -85,246 +84,3 @@ func TestParseDKIMResult(t *testing.T) { }) } } - -func TestParseLegacyDKIM(t *testing.T) { - tests := []struct { - name string - dkimSignatures []string - expectedCount int - expectedDomains []string - expectedSelector []string - }{ - { - name: "Single DKIM signature with domain and selector", - dkimSignatures: []string{ - "v=1; a=rsa-sha256; c=relaxed/relaxed; d=example.com; s=selector1; h=from:to:subject:date; bh=xyz; b=abc", - }, - expectedCount: 1, - expectedDomains: []string{"example.com"}, - expectedSelector: []string{"selector1"}, - }, - { - name: "Multiple DKIM signatures", - dkimSignatures: []string{ - "v=1; a=rsa-sha256; d=example.com; s=selector1; b=abc123", - "v=1; a=rsa-sha256; d=example.com; s=selector2; b=def456", - }, - expectedCount: 2, - expectedDomains: []string{"example.com", "example.com"}, - expectedSelector: []string{"selector1", "selector2"}, - }, - { - name: "DKIM signature with different domain", - dkimSignatures: []string{ - "v=1; a=rsa-sha256; d=mail.example.org; s=default; b=xyz789", - }, - expectedCount: 1, - expectedDomains: []string{"mail.example.org"}, - expectedSelector: []string{"default"}, - }, - { - name: "DKIM signature with subdomain", - dkimSignatures: []string{ - "v=1; a=rsa-sha256; d=newsletters.example.com; s=marketing; b=aaa", - }, - expectedCount: 1, - expectedDomains: []string{"newsletters.example.com"}, - expectedSelector: []string{"marketing"}, - }, - { - name: "Multiple signatures from different domains", - dkimSignatures: []string{ - "v=1; a=rsa-sha256; d=example.com; s=s1; b=abc", - "v=1; a=rsa-sha256; d=relay.com; s=s2; b=def", - }, - expectedCount: 2, - expectedDomains: []string{"example.com", "relay.com"}, - expectedSelector: []string{"s1", "s2"}, - }, - { - name: "No DKIM signatures", - dkimSignatures: []string{}, - expectedCount: 0, - expectedDomains: []string{}, - expectedSelector: []string{}, - }, - { - name: "DKIM signature without selector", - dkimSignatures: []string{ - "v=1; a=rsa-sha256; d=example.com; b=abc123", - }, - expectedCount: 1, - expectedDomains: []string{"example.com"}, - expectedSelector: []string{""}, - }, - { - name: "DKIM signature without domain", - dkimSignatures: []string{ - "v=1; a=rsa-sha256; s=selector1; b=abc123", - }, - expectedCount: 1, - expectedDomains: []string{""}, - expectedSelector: []string{"selector1"}, - }, - { - name: "DKIM signature with whitespace in parameters", - dkimSignatures: []string{ - "v=1; a=rsa-sha256; d=example.com ; s=selector1 ; b=abc123", - }, - expectedCount: 1, - expectedDomains: []string{"example.com"}, - expectedSelector: []string{"selector1"}, - }, - { - name: "DKIM signature with multiline format", - dkimSignatures: []string{ - "v=1; a=rsa-sha256; c=relaxed/relaxed;\r\n\td=example.com; s=selector1;\r\n\th=from:to:subject:date;\r\n\tb=abc123def456ghi789", - }, - expectedCount: 1, - expectedDomains: []string{"example.com"}, - expectedSelector: []string{"selector1"}, - }, - { - name: "DKIM signature with ed25519 algorithm", - dkimSignatures: []string{ - "v=1; a=ed25519-sha256; d=example.com; s=ed25519; b=xyz", - }, - expectedCount: 1, - expectedDomains: []string{"example.com"}, - expectedSelector: []string{"ed25519"}, - }, - { - name: "Complex real-world DKIM signature", - dkimSignatures: []string{ - "v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1234567890; x=1234567950; darn=example.com; h=to:subject:message-id:date:from:mime-version:from:to:cc:subject:date:message-id:reply-to; bh=abc123def456==; b=longsignaturehere==", - }, - expectedCount: 1, - expectedDomains: []string{"google.com"}, - expectedSelector: []string{"20230601"}, - }, - } - - analyzer := NewAuthenticationAnalyzer() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Create a mock email message with DKIM-Signature headers - email := &EmailMessage{ - Header: make(map[string][]string), - } - if len(tt.dkimSignatures) > 0 { - email.Header["Dkim-Signature"] = tt.dkimSignatures - } - - results := analyzer.parseLegacyDKIM(email) - - // Check count - if len(results) != tt.expectedCount { - t.Errorf("Expected %d results, got %d", tt.expectedCount, len(results)) - return - } - - // Check each result - for i, result := range results { - // All legacy DKIM results should have Result = none - if result.Result != api.AuthResultResultNone { - t.Errorf("Result[%d].Result = %v, want %v", i, result.Result, api.AuthResultResultNone) - } - - // Check domain - if i < len(tt.expectedDomains) { - expectedDomain := tt.expectedDomains[i] - if expectedDomain != "" { - if result.Domain == nil { - t.Errorf("Result[%d].Domain = nil, want %v", i, expectedDomain) - } else if strings.TrimSpace(*result.Domain) != expectedDomain { - t.Errorf("Result[%d].Domain = %v, want %v", i, *result.Domain, expectedDomain) - } - } - } - - // Check selector - if i < len(tt.expectedSelector) { - expectedSelector := tt.expectedSelector[i] - if expectedSelector != "" { - if result.Selector == nil { - t.Errorf("Result[%d].Selector = nil, want %v", i, expectedSelector) - } else if strings.TrimSpace(*result.Selector) != expectedSelector { - t.Errorf("Result[%d].Selector = %v, want %v", i, *result.Selector, expectedSelector) - } - } - } - - // Check that Details is set - if result.Details == nil { - t.Errorf("Result[%d].Details = nil, expected non-nil", i) - } else { - expectedDetails := "DKIM signature present (verification status unknown)" - if *result.Details != expectedDetails { - t.Errorf("Result[%d].Details = %v, want %v", i, *result.Details, expectedDetails) - } - } - } - }) - } -} - -func TestParseLegacyDKIM_Integration(t *testing.T) { - hostname = "" - - // Test that parseLegacyDKIM is properly integrated into AnalyzeAuthentication - t.Run("Legacy DKIM is used when no Authentication-Results", func(t *testing.T) { - analyzer := NewAuthenticationAnalyzer() - email := &EmailMessage{ - Header: make(map[string][]string), - } - email.Header["Dkim-Signature"] = []string{ - "v=1; a=rsa-sha256; d=example.com; s=selector1; b=abc", - } - - results := analyzer.AnalyzeAuthentication(email) - - if results.Dkim == nil { - t.Fatal("Expected DKIM results, got nil") - } - if len(*results.Dkim) != 1 { - t.Errorf("Expected 1 DKIM result, got %d", len(*results.Dkim)) - } - if (*results.Dkim)[0].Result != api.AuthResultResultNone { - t.Errorf("Expected DKIM result to be 'none', got %v", (*results.Dkim)[0].Result) - } - if (*results.Dkim)[0].Domain == nil || *(*results.Dkim)[0].Domain != "example.com" { - t.Error("Expected domain to be 'example.com'") - } - }) - - t.Run("Legacy DKIM is NOT used when Authentication-Results present", func(t *testing.T) { - analyzer := NewAuthenticationAnalyzer() - email := &EmailMessage{ - Header: make(map[string][]string), - } - // Both Authentication-Results and DKIM-Signature headers - email.Header["Authentication-Results"] = []string{ - "mx.example.com; dkim=pass header.d=verified.com header.s=s1", - } - email.Header["Dkim-Signature"] = []string{ - "v=1; a=rsa-sha256; d=example.com; s=selector1; b=abc", - } - - results := analyzer.AnalyzeAuthentication(email) - - // Should use the Authentication-Results DKIM (pass from verified.com), not the legacy signature - if results.Dkim == nil { - t.Fatal("Expected DKIM results, got nil") - } - if len(*results.Dkim) != 1 { - t.Errorf("Expected 1 DKIM result, got %d", len(*results.Dkim)) - } - if (*results.Dkim)[0].Result != api.AuthResultResultPass { - t.Errorf("Expected DKIM result to be 'pass', got %v", (*results.Dkim)[0].Result) - } - if (*results.Dkim)[0].Domain == nil || *(*results.Dkim)[0].Domain != "verified.com" { - t.Error("Expected domain to be 'verified.com' from Authentication-Results, not 'example.com' from legacy") - } - }) -}