From ea6757f557c89ad6581709da0172a12c22caa5c6 Mon Sep 17 00:00:00 2001 From: Dmitry Motylev <156671+dmotylev@users.noreply.github.com> Date: Wed, 20 Mar 2024 17:33:37 +0000 Subject: [PATCH 1/2] BED-591/expose-all-found-SPF-policies --- depaware.txt | 6 ++++-- parser.go | 20 +++++++++++++++----- parser_test.go | 25 ++++++++++++++----------- spf.go | 34 ++++++++++++++++++++++++++-------- 4 files changed, 59 insertions(+), 26 deletions(-) diff --git a/depaware.txt b/depaware.txt index c0f0044..5c45d28 100644 --- a/depaware.txt +++ b/depaware.txt @@ -14,7 +14,6 @@ github.com/redsift/spf/v2 dependencies: (generated by github.com/tailscale/depaw golang.org/x/crypto/chacha20poly1305 from crypto/tls golang.org/x/crypto/cryptobyte from crypto/ecdsa+ golang.org/x/crypto/cryptobyte/asn1 from crypto/ecdsa+ - golang.org/x/crypto/curve25519 from crypto/tls golang.org/x/crypto/hkdf from crypto/tls LD golang.org/x/net/bpf from golang.org/x/net/ipv4+ golang.org/x/net/dns/dnsmessage from net @@ -25,6 +24,7 @@ github.com/redsift/spf/v2 dependencies: (generated by github.com/tailscale/depaw LD golang.org/x/sys/unix from github.com/miekg/dns+ bufio from github.com/miekg/dns bytes from bufio+ + cmp from net/netip+ container/list from crypto/tls context from crypto/tls+ crypto from crypto/ecdsa+ @@ -32,6 +32,7 @@ github.com/redsift/spf/v2 dependencies: (generated by github.com/tailscale/depaw crypto/cipher from crypto/aes+ crypto/des from crypto/tls+ crypto/dsa from crypto/x509 + crypto/ecdh from crypto/ecdsa+ crypto/ecdsa from crypto/tls+ crypto/ed25519 from crypto/tls+ crypto/elliptic from crypto/ecdsa+ @@ -61,7 +62,7 @@ github.com/redsift/spf/v2 dependencies: (generated by github.com/tailscale/depaw hash from crypto+ io from bufio+ io/fs from crypto/x509+ - io/ioutil from golang.org/x/sys/cpu+ + io/ioutil from github.com/outcaste-io/ristretto/z math from crypto/rsa+ math/big from crypto/dsa+ math/bits from crypto/internal/edwards25519/field+ @@ -76,6 +77,7 @@ github.com/redsift/spf/v2 dependencies: (generated by github.com/tailscale/depaw reflect from crypto/x509+ regexp from github.com/dustin/go-humanize+ regexp/syntax from regexp + slices from encoding/base32+ sort from encoding/asn1+ strconv from crypto+ strings from bufio+ diff --git a/parser.go b/parser.go index 90a2db5..952bb7c 100644 --- a/parser.go +++ b/parser.go @@ -95,6 +95,10 @@ func Cause(e error) (string, error) { return lastToken.String(), e } +func (e SpfError) Unwrap() error { + return e.err +} + func (e SpfError) Cause() error { return e.err } @@ -204,14 +208,20 @@ func (p *parser) checkHost(ip net.IP, domain, sender string) (r Result, expl str // If the resultant record set includes no records, check_host() // produces the "none" result. If the resultant record set includes // more than one record, check_host() produces the "permerror" result. - spf, err = filterSPF(txts) - if err != nil { - return Permerror, "", "", NewSpfError(spferr.KindValidation, err, nil) + policies := filterSPF(txts) + + if len(policies) == 0 { + return None, "", "", NewSpfError(spferr.KindValidation, + &PolicyDeploymentError{Err: ErrSPFNotFound, Domain: domain}, nil) } - if spf == "" { - return None, "", "", NewSpfError(spferr.KindValidation, ErrSPFNotFound, nil) + + if len(policies) > 1 { + return Permerror, "", "", NewSpfError(spferr.KindValidation, + &PolicyDeploymentError{Err: ErrTooManySPFRecords, Domain: domain, Policies: policies}, nil) } + spf = policies[0] + r, expl, u, err = newParserWithVisited(p.visited, p.fireFirstMatchOnce, p.options...).with(spf, sender, domain, ip).check() return } diff --git a/parser_test.go b/parser_test.go index 5be754f..e528f12 100644 --- a/parser_test.go +++ b/parser_test.go @@ -1329,25 +1329,28 @@ func TestSelectingRecord(t *testing.T) { })) defer dns.HandleRemove("many-records.") - samples := []struct { + tests := []struct { d string r Result e error }{ - {"notexists", None, SpfError{kind: spferr.KindDNS, err: ErrDNSPermerror}}, - {"v-spf2", None, SpfError{kind: spferr.KindValidation, err: ErrSPFNotFound}}, - {"v-spf10", None, SpfError{kind: spferr.KindValidation, err: ErrSPFNotFound}}, - {"no-record", None, SpfError{kind: spferr.KindValidation, err: ErrSPFNotFound}}, - {"many-records", Permerror, SpfError{kind: spferr.KindValidation, err: ErrTooManySPFRecords}}, + {"notexists", None, ErrDNSPermerror}, + {"v-spf2", None, ErrSPFNotFound}, + {"v-spf10", None, ErrSPFNotFound}, + {"no-record", None, ErrSPFNotFound}, + {"many-records", Permerror, ErrTooManySPFRecords}, {"mixed-records", Pass, nil}, } ip := net.ParseIP("10.0.0.1") - for i, s := range samples { - r, _, _, e := CheckHost(ip, s.d, s.d, WithResolver(testResolver)) - if r != s.r || e != s.e { - t.Errorf("#%d `%s` want [`%v` `%v`], got [`%v` `%v`]", i, s.d, s.r, s.e, r, e) - } + for _, test := range tests { + tt := test + t.Run(tt.d, func(t *testing.T) { + r, _, _, e := CheckHost(ip, tt.d, tt.d, WithResolver(testResolver)) + if r != test.r || !errors.Is(e, tt.e) { + t.Errorf("CheckHost(...)\n\twant [`%#v` `%#v`]\n\tgot [`%#v` `%#v`]", tt.r, tt.e, r, e) + } + }) } } diff --git a/spf.go b/spf.go index cfc71c3..21d4e00 100644 --- a/spf.go +++ b/spf.go @@ -29,6 +29,26 @@ var ( ErrTooManyErrors = errors.New("too many errors") ) +type PolicyDeploymentError struct { + // Err is the error that occurred during the policy lookup. + Err error + // Domain is the domain that was being looked up. + Domain string + // Policies is the list of policies that were found as part of the lookup. + Policies []string +} + +func (e *PolicyDeploymentError) Error() string { + if e == nil { + return "" + } + return e.Err.Error() +} + +func (e *PolicyDeploymentError) Unwrap() error { + return e.Err +} + // DomainError represents a domain check error type DomainError struct { Err string // description of the error @@ -337,13 +357,13 @@ func CheckHost(ip net.IP, domain, sender string, opts ...Option) (Result, string // "v=spf1". Note that the version section is terminated by either an // SP character or the end of the record. As an example, a record with // a version section of "v=spf10" does not match and is discarded. -func filterSPF(txt []string) (string, error) { +func filterSPF(txt []string) []string { const ( v = "v=spf1" vLen = 6 ) var ( - spf string + spf []string n int ) @@ -353,7 +373,7 @@ func filterSPF(txt []string) (string, error) { } if len(s) == vLen { if s == v { - spf = s + spf = append(spf, s) n++ } continue @@ -364,13 +384,11 @@ func filterSPF(txt []string) (string, error) { if !strings.HasPrefix(s, v) { continue } - spf = s + spf = append(spf, s) n++ } - if n > 1 { - return "", ErrTooManySPFRecords - } - return spf, nil + + return spf } // isDomainName checks if a string is a presentation-format domain name From ecf8aafa5a58910d9448c2846d3e8907f9f92b64 Mon Sep 17 00:00:00 2001 From: Dmitry Motylev <156671+dmotylev@users.noreply.github.com> Date: Wed, 20 Mar 2024 17:47:20 +0000 Subject: [PATCH 2/2] tidy up --- spf.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/spf.go b/spf.go index 21d4e00..623b657 100644 --- a/spf.go +++ b/spf.go @@ -362,10 +362,7 @@ func filterSPF(txt []string) []string { v = "v=spf1" vLen = 6 ) - var ( - spf []string - n int - ) + var spf []string for _, s := range txt { if len(s) < vLen { @@ -374,7 +371,6 @@ func filterSPF(txt []string) []string { if len(s) == vLen { if s == v { spf = append(spf, s) - n++ } continue } @@ -385,7 +381,6 @@ func filterSPF(txt []string) []string { continue } spf = append(spf, s) - n++ } return spf