diff --git a/pkg/analyzer/dns.go b/pkg/analyzer/dns.go index 542d704..e75b3ac 100644 --- a/pkg/analyzer/dns.go +++ b/pkg/analyzer/dns.go @@ -269,6 +269,18 @@ func (d *DNSAnalyzer) resolveSPFRecords(domain string, visited map[string]bool, Error: errMsg, }) + // Check for redirect= modifier first (it replaces the entire SPF policy) + redirectDomain := d.extractSPFRedirect(spfRecord) + if redirectDomain != "" { + // redirect= replaces the current domain's policy entirely + // Only follow if no other mechanisms matched (per RFC 7208) + redirectRecords := d.resolveSPFRecords(redirectDomain, visited, depth+1) + if redirectRecords != nil { + results = append(results, *redirectRecords...) + } + return &results + } + // Extract and resolve include: directives includes := d.extractSPFIncludes(spfRecord) for _, includeDomain := range includes { @@ -294,6 +306,17 @@ func (d *DNSAnalyzer) extractSPFIncludes(record string) []string { return includes } +// extractSPFRedirect extracts the redirect= domain from an SPF record +// The redirect= modifier replaces the current domain's SPF policy with that of the target domain +func (d *DNSAnalyzer) extractSPFRedirect(record string) string { + re := regexp.MustCompile(`redirect=([^\s]+)`) + matches := re.FindStringSubmatch(record) + if len(matches) > 1 { + return matches[1] + } + return "" +} + // validateSPF performs basic SPF record validation func (d *DNSAnalyzer) validateSPF(record string) bool { // Must start with v=spf1 @@ -301,6 +324,11 @@ func (d *DNSAnalyzer) validateSPF(record string) bool { return false } + // Check for redirect= modifier (which replaces the need for an 'all' mechanism) + if strings.Contains(record, "redirect=") { + return true + } + // Check for common syntax issues // Should have a final mechanism (all, +all, -all, ~all, ?all) validEndings := []string{" all", " +all", " -all", " ~all", " ?all"} @@ -752,8 +780,27 @@ func (d *DNSAnalyzer) CalculateDNSScore(results *api.DNSResults, senderIP string // SPF Records: 20 points // SPF is essential for email authentication if results.SpfRecords != nil && len(*results.SpfRecords) > 0 { - // Check the main domain's SPF record (first in the list) - mainSPF := (*results.SpfRecords)[0] + // Find the main SPF record by skipping redirects + // Loop through records to find the last redirect or the first non-redirect + mainSPFIndex := 0 + for i := 0; i < len(*results.SpfRecords); i++ { + spfRecord := (*results.SpfRecords)[i] + if spfRecord.Record != nil && strings.Contains(*spfRecord.Record, "redirect=") { + // This is a redirect, check if there's a next record + if i+1 < len(*results.SpfRecords) { + mainSPFIndex = i + 1 + } else { + // Redirect exists but no target record found + break + } + } else { + // Found a non-redirect record + mainSPFIndex = i + break + } + } + + mainSPF := (*results.SpfRecords)[mainSPFIndex] if mainSPF.Valid { // Full points for valid SPF score += 15 diff --git a/pkg/analyzer/dns_test.go b/pkg/analyzer/dns_test.go index d3deb20..10b7b98 100644 --- a/pkg/analyzer/dns_test.go +++ b/pkg/analyzer/dns_test.go @@ -82,13 +82,23 @@ func TestValidateSPF(t *testing.T) { record: "v=spf1 mx ?all", expected: true, }, + { + name: "Valid SPF with redirect", + record: "v=spf1 redirect=_spf.example.com", + expected: true, + }, + { + name: "Valid SPF with redirect and mechanisms", + record: "v=spf1 ip4:192.0.2.0/24 redirect=_spf.example.com", + expected: true, + }, { name: "Invalid SPF - no version", record: "include:_spf.example.com -all", expected: false, }, { - name: "Invalid SPF - no all mechanism", + name: "Invalid SPF - no all mechanism or redirect", record: "v=spf1 include:_spf.example.com", expected: false, }, @@ -111,6 +121,51 @@ func TestValidateSPF(t *testing.T) { } } +func TestExtractSPFRedirect(t *testing.T) { + tests := []struct { + name string + record string + expectedRedirect string + }{ + { + name: "SPF with redirect", + record: "v=spf1 redirect=_spf.example.com", + expectedRedirect: "_spf.example.com", + }, + { + name: "SPF with redirect and other mechanisms", + record: "v=spf1 ip4:192.0.2.0/24 redirect=_spf.google.com", + expectedRedirect: "_spf.google.com", + }, + { + name: "SPF without redirect", + record: "v=spf1 include:_spf.example.com -all", + expectedRedirect: "", + }, + { + name: "SPF with only all mechanism", + record: "v=spf1 -all", + expectedRedirect: "", + }, + { + name: "Empty record", + record: "", + expectedRedirect: "", + }, + } + + analyzer := NewDNSAnalyzer(5 * time.Second) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := analyzer.extractSPFRedirect(tt.record) + if result != tt.expectedRedirect { + t.Errorf("extractSPFRedirect(%q) = %q, want %q", tt.record, result, tt.expectedRedirect) + } + }) + } +} + func TestValidateDKIM(t *testing.T) { tests := []struct { name string diff --git a/web/src/lib/components/SpfRecordsDisplay.svelte b/web/src/lib/components/SpfRecordsDisplay.svelte index 20419b6..6d7d621 100644 --- a/web/src/lib/components/SpfRecordsDisplay.svelte +++ b/web/src/lib/components/SpfRecordsDisplay.svelte @@ -11,6 +11,9 @@ const spfIsValid = $derived( spfRecords?.reduce((acc, r) => acc && r.valid, true) ?? false ); + const spfCanBeImprove = $derived( + spfRecords.length > 0 && spfRecords.filter((r) => !r.record.includes(" redirect="))[0]?.all_qualifier != "-" + ); {#if spfRecords && spfRecords.length > 0} @@ -19,8 +22,10 @@
@@ -62,7 +67,7 @@ {:else if spf.all_qualifier === '?'} Neutral (?all) {/if} - {#if index === 0} + {#if index === 0 || (index === 1 && spfRecords[0].record?.includes('redirect='))}
{#if spf.all_qualifier === '-'} All unauthorized servers will be rejected. This is the recommended strict policy.