Skip to content

Commit

Permalink
[BED-375] Feature/ignore matchs fix (#22)
Browse files Browse the repository at this point in the history 
- Added FireFirstMatch, which sends the first result to the listener
when we use ignoreMatches

- Needed to make sure FireFirstMatch is only called once, so added
firstMatchFound, which is just a flag to indicate if it is or not. As
the calls to checkhost are recursive and create a new parser each time,
the firstMatchFound flag is initialised outside the scope of recursion,
so it can be shared with all calls, as we do with the visited Stack.
  • Loading branch information
@csucu
csucu authored Aug 30, 2023
2 parents 9b9eacf + 95c6057 commit 4c16e59
Show file tree
Hide file tree
Showing 5 changed files with 176 additions and 44 deletions.
1 change: 1 addition & 0 deletions listener.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ type Listener interface {
Directive(unused bool, qualifier, mechanism, value, effectiveValue string)
NonMatch(qualifier, mechanism, value string, result Result, err error)
Match(qualifier, mechanism, value string, result Result, explanation string, ttl time.Duration, err error)
FirstMatch(r Result, err error)
MatchingIP(qualifier, mechanism, value string, fqdn string, ipn net.IPNet, host string, ip net.IP)
}
70 changes: 37 additions & 33 deletions parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net"
"strconv"
"strings"
"sync"
"time"
)

Expand Down Expand Up @@ -90,42 +91,42 @@ func (e SyntaxError) TokenString() string {
// level CheckHost method as well as tokenized terms from TXT RR. One should
// call parser.Parse() for a proper SPF evaluation.
type parser struct {
sender string
domain string
heloDomain string
ip net.IP
query string
resolver Resolver
listener Listener
ignoreMatches bool
options []Option
visited *stringsStack
evaluatedOn time.Time
receivingFQDN string
stopAtError func(error) bool
partialMacros bool
firstMatchResult Result
sender string
domain string
heloDomain string
ip net.IP
query string
resolver Resolver
listener Listener
ignoreMatches bool
options []Option
visited *stringsStack
evaluatedOn time.Time
receivingFQDN string
stopAtError func(error) bool
partialMacros bool
fireFirstMatchOnce *sync.Once
}

// newParser creates new Parser objects and returns its reference.
// It accepts CheckHost() parameters as well as SPF query (fetched from TXT RR
// during initial DNS lookup.
func newParser(opts ...Option) *parser {
return newParserWithVisited(newStringsStack(), opts...)
return newParserWithVisited(newStringsStack(), new(sync.Once), opts...)
}

// newParserWithVisited creates new Parser objects with prepopulated map of visited domains and returns its reference.
// It accepts CheckHost() parameters as well as SPF query (fetched from TXT RR
// during initial DNS lookup.
func newParserWithVisited(visited *stringsStack, opts ...Option) *parser {
func newParserWithVisited(visited *stringsStack, fireFirstMatchOnce *sync.Once, opts ...Option) *parser {
p := &parser{
// mechanisms: make([]*token, 0, 10),
resolver: NewLimitedResolver(&DNSResolver{}, 10, 10),
options: opts,
visited: visited,
receivingFQDN: "unknown",
evaluatedOn: time.Now().UTC(),
firstMatchResult: None,
resolver: NewLimitedResolver(&DNSResolver{}, 10, 10),
options: opts,
visited: visited,
receivingFQDN: "unknown",
evaluatedOn: time.Now().UTC(),
fireFirstMatchOnce: fireFirstMatchOnce,
}
for _, opt := range opts {
opt(p)
Expand Down Expand Up @@ -188,7 +189,7 @@ func (p *parser) checkHost(ip net.IP, domain, sender string) (r Result, expl str
return None, "", "", ErrSPFNotFound
}

r, expl, u, err = newParserWithVisited(p.visited, p.options...).with(spf, sender, domain, ip).check()
r, expl, u, err = newParserWithVisited(p.visited, p.fireFirstMatchOnce, p.options...).with(spf, sender, domain, ip).check()
return
}

Expand Down Expand Up @@ -266,18 +267,14 @@ func (p *parser) check() (Result, string, unused, error) {
}

// Store the first match result if not already set
if p.ignoreMatches && matches && p.firstMatchResult == None {
p.firstMatchResult = result
if p.ignoreMatches && matches {
p.fireFirstMatch(result, err)
}

p.fireNonMatch(token, result, err)

// in walker-mode we want to count number of errors and check the counter against some threshold
if p.ignoreMatches && p.stopAtError != nil && p.stopAtError(err) {
if p.firstMatchResult != None {
return p.firstMatchResult, "", unused{mechanisms[i+1:], redirect}, ErrTooManyErrors
}

return unreliableResult, "", unused{mechanisms[i+1:], redirect}, ErrTooManyErrors
}

Expand All @@ -290,9 +287,6 @@ func (p *parser) check() (Result, string, unused, error) {
}

if p.ignoreMatches {
if p.firstMatchResult != None {
return p.firstMatchResult, "", unused{}, nil
}
return unreliableResult, "", unused{}, ErrUnreliableResult
}

Expand Down Expand Up @@ -355,6 +349,16 @@ func (p *parser) fireMatch(t *token, r Result, explanation string, ttl time.Dura
p.listener.Match(t.qualifier.String(), t.mechanism.String(), t.value, r, explanation, ttl, e)
}

func (p *parser) fireFirstMatch(r Result, e error) {
if p.listener == nil || p.fireFirstMatchOnce == nil {
return
}

p.fireFirstMatchOnce.Do(func() {
p.listener.FirstMatch(r, e)
})
}

func sortTokens(tokens []*token) (mechanisms []*token, redirect, explanation *token, err error) {
mechanisms = make([]*token, 0, len(tokens))
for _, token := range tokens {
Expand Down
14 changes: 3 additions & 11 deletions parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -705,24 +705,16 @@ func TestParseInclude(t *testing.T) {
{173, 20, 21, 1},
}

p := newParser(WithResolver(testResolver)).with(stub, "matching.net", "matching.net", net.IP{0, 0, 0, 0})
testcases := []TokenTestCase{
{&token{tInclude, qPlus, "_spf.matching.net"}, Pass, true, false},
{&token{tInclude, qMinus, "_spf.matching.net"}, Fail, true, false},
{&token{tInclude, qTilde, "_spf.matching.net"}, Softfail, true, false},
{&token{tInclude, qQuestionMark, "_spf.matching.net"}, Neutral, true, false},

{&token{tInclude, qPlus, "_spf.matching.net"}, Pass, true, true},
{&token{tInclude, qMinus, "_spf.matching.net"}, Fail, true, true},
}

for i, testcase := range testcases {
for j, ip := range ips {
opts := []Option{WithResolver(testResolver)}
if testcase.ignoreMatches {
opts = append(opts, IgnoreMatches())
}
p := newParser(opts...).with(stub, "matching.net", "matching.net", net.IP{0, 0, 0, 0})

p.ip = ip
match, result, _ := p.parseInclude(testcase.Input)
if testcase.Match != match {
Expand Down Expand Up @@ -1414,8 +1406,8 @@ func TestCheckHost_Loops(t *testing.T) {
},
[]Option{WithResolver(testResolver)},
},
{"walker mode, errors below threshold", "example.com", Permerror, nil, []Option{WithResolver(testResolver), IgnoreMatches(), ErrorsThreshold(4)}},
{"walker mode, errors above threshold", "example.com", Permerror, ErrTooManyErrors, []Option{WithResolver(testResolver), IgnoreMatches(), ErrorsThreshold(2)}},
{"walker mode, errors below threshold", "example.com", unreliableResult, ErrUnreliableResult, []Option{WithResolver(testResolver), IgnoreMatches(), ErrorsThreshold(4)}},
{"walker mode, errors above threshold", "example.com", unreliableResult, ErrTooManyErrors, []Option{WithResolver(testResolver), IgnoreMatches(), ErrorsThreshold(2)}},
}

ip := net.ParseIP("10.0.0.1")
Expand Down
4 changes: 4 additions & 0 deletions printer/printer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ func (p *Printer) Match(qualifier, mechanism, value string, result spf.Result, e
// fmt.Fprintf(p.w, "%sMATCH: %s, %q, %v\n", strings.Repeat(" ", p.c), result, explanation, err)
}

func (p *Printer) FirstMatch(r spf.Result, err error) {
fmt.Fprintf(p.w, "%sFIRST-MATCH: %s, %v\n", strings.Repeat(" ", p.c), r, err)
}

func (p *Printer) LookupTXT(name string) ([]string, time.Duration, error) {
fmt.Fprintf(p.w, "%s lookup(TXT) %s\n", strings.Repeat(" ", p.c), name)
atomic.AddInt64(&p.lc, 1)
Expand Down
131 changes: 131 additions & 0 deletions printer/printer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,3 +214,134 @@ func ExamplePrinter_ipv6nil() {
// = softfail, "59s", , <nil>
//
}

func ExamplePrinter_ignoreMatches() {
dump := []byte(`[
";aspmx2.googlemail.com. IN AAAA", "tZyBgAABAAEAAAAABmFzcG14Mgpnb29nbGVtYWlsA2NvbQAAHAABBmFzcG14Mgpnb29nbGVtYWlsA2NvbQAAHAABAAABJAAQKgAUUEAQDAUAAAAAAAAAGw==",
";alt2.aspmx.l.google.com. IN A", "rgaBgAABAAEAAAAABGFsdDIFYXNwbXgBbAZnb29nbGUDY29tAAABAAEEYWx0MgVhc3BteAFsBmdvb2dsZQNjb20AAAEAAQAAASQABEp9yBo=",
";alt1.aspmx.l.google.com. IN A", "t8eBgAABAAEAAAAABGFsdDEFYXNwbXgBbAZnb29nbGUDY29tAAABAAEEYWx0MQVhc3BteAFsBmdvb2dsZQNjb20AAAEAAQAAAJgABEDpoRo=",
";spf.mailjet.com. IN TXT", "HUKBgAABAAEAAAAAA3NwZgdtYWlsamV0A2NvbQAAEAABA3NwZgdtYWlsamV0A2NvbQAAEAABAAAF0wCZmHY9c3BmMSBpcDQ6MTc4LjMzLjExMS4xNDQgaXA0OjE3OC4zMy4xMzcuMjA4LzI4IGlwNDoxNzguMzMuMjIxLjAvMjQgaXA0OjM3LjU5LjY5LjEyOC8yNSBpcDQ6MzcuNTkuMjQ5LjAvMjQgaXA0Ojg3LjI1My4yMzIuMC8yMSBpcDQ6MTg1LjE4OS4yMzYuMC8yMiA/YWxs",
";servers.mcsv.net. IN TXT", "xruBgAABAAEAAAAAB3NlcnZlcnMEbWNzdgNuZXQAABAAAQdzZXJ2ZXJzBG1jc3YDbmV0AAAQAAEAAACYAEdGdj1zcGYxIGlwNDoyMDUuMjAxLjEyOC4wLzIwIGlwNDoxOTguMi4xMjguMC8xOCBpcDQ6MTQ4LjEwNS44LjAvMjEgP2FsbA==",
";alt2.aspmx.l.google.com. IN AAAA", "duCBgAABAAEAAAAABGFsdDIFYXNwbXgBbAZnb29nbGUDY29tAAAcAAEEYWx0MgVhc3BteAFsBmdvb2dsZQNjb20AABwAAQAAASQAECQEaABAAwwAAAAAAAAAABo=",
";blocket.se. IN MX", "sMSBgAABAAUAAAAAB2Jsb2NrZXQCc2UAAA8AAQdibG9ja2V0AnNlAAAPAAEAAAG7ABkAHgZhc3BteDMKZ29vZ2xlbWFpbANjb20AB2Jsb2NrZXQCc2UAAA8AAQAAAbsAGwAUBGFsdDEFYXNwbXgBbAZnb29nbGUDY29tAAdibG9ja2V0AnNlAAAPAAEAAAG7ABsAFARhbHQyBWFzcG14AWwGZ29vZ2xlA2NvbQAHYmxvY2tldAJzZQAADwABAAABuwAZAB4GYXNwbXgyCmdvb2dsZW1haWwDY29tAAdibG9ja2V0AnNlAAAPAAEAAAG7ABYACgVhc3BteAFsBmdvb2dsZQNjb20A",
";aspmx.l.google.com. IN AAAA", "ul2BgAABAAEAAAAABWFzcG14AWwGZ29vZ2xlA2NvbQAAHAABBWFzcG14AWwGZ29vZ2xlA2NvbQAAHAABAAABJAAQKgAUUEAMDAsAAAAAAAAAGw==",
";aspmx.l.google.com. IN A", "IG+BgAABAAEAAAAABWFzcG14AWwGZ29vZ2xlA2NvbQAAAQABBWFzcG14AWwGZ29vZ2xlA2NvbQAAAQABAAABJAAEQOm4Gg==",
";alt1.aspmx.l.google.com. IN AAAA", "Pp2BgAABAAEAAAAABGFsdDEFYXNwbXgBbAZnb29nbGUDY29tAAAcAAEEYWx0MQVhc3BteAFsBmdvb2dsZQNjb20AABwAAQAAASQAECoAFFBAEAwFAAAAAAAAABo=",
";aspmx2.googlemail.com. IN A", "hJGBgAABAAEAAAAABmFzcG14Mgpnb29nbGVtYWlsA2NvbQAAAQABBmFzcG14Mgpnb29nbGVtYWlsA2NvbQAAAQABAAAAzQAEQOmhGw==",
";aspmx3.googlemail.com. IN AAAA", "3H2BgAABAAEAAAAABmFzcG14Mwpnb29nbGVtYWlsA2NvbQAAHAABBmFzcG14Mwpnb29nbGVtYWlsA2NvbQAAHAABAAABJAAQJARoAEADDAAAAAAAAAAAGg==",
";subito.it. IN TXT", "R0aBgAABAAIAAAAABnN1Yml0bwJpdAAAEAABBnN1Yml0bwJpdAAAEAABAAABDQGr/3Y9c3BmMSBteDpibG9ja2V0LnNlIGluY2x1ZGU6c3BmLm1haWxqZXQuY29tIGluY2x1ZGU6c2VydmVycy5tY3N2Lm5ldCBpcDQ6MTA5LjE2OC4xMjcuMTYwLzI3IGlwNDoyMTIuMzEuMjUyLjY0LzI3IGlwNDoyMTIuNzcuNjguNiBpcDQ6NjIuMjEyLjEuMTYwIGlwNDo2Mi4yMTIuMC4xNjAgaXA0OjkzLjk0LjMyLjAvMjEgaXA0OjkzLjk0LjM3LjI1MyBpcDQ6MTA5LjE2OC4xMjEuNDgvMjggaXA0OjM3LjIwMi4yMC4yMy8zMiBpcDQ6MjEzLjIxNS4xNaoyLjI1NC8zMiBpcDQ6MjEzLjIxNS4xNTIuMjUzLzMyIGlwNDoyMTMuMjE1LjE1Mi4yNTIvMzIgaXA0OjIxMy4yMTUuMTUyLjI1MS8zMiBpcDQ6MTA5LjE2OC4xMjEuNTQvMzIgaXA0OjEwOS4xNjguMTIxLjU1LzMyIGlwNDoxMDkuMTY4LjEyMS41Ny8zMiBpcDQ6MTA5LjE2OC4xMjEuNTgvMzIgLWFsbAZzdWJpdG8CaXQAABAAAQAAAQ0ARURnb29nbGUtc2l0ZS12ZXJpZmljYXRpb249NXZqME5OR2FXZGtDaUJCd01EcUF5WE90aWsxejR1SF9Wc0dKbDNfY3djOA==",
";ptr.test.redsift.io. IN TXT", "O7yBgAABAAEAAAAAA3B0cgR0ZXN0B3JlZHNpZnQCaW8AABAAAQNwdHIEdGVzdAdyZWRzaWZ0AmlvAAAQAAEAAAErABAPdj1zcGYxIHB0ciB+YWxs",
";aspmx3.googlemail.com. IN A", "j2WBgAABAAEAAAAABmFzcG14Mwpnb29nbGVtYWlsA2NvbQAAAQABBmFzcG14Mwpnb29nbGVtYWlsA2NvbQAAAQABAAABJAAESn3IGg=="
]`)

// fill dns cache
var d spf.CacheDump
if err := json.Unmarshal(dump, &d); err != nil {
log.Fatal(err)
}

c := z.MustRistrettoCache(&ristretto.Config{
NumCounters: int64(100 * 10),
MaxCost: 1 << 20,
BufferItems: 64,
Metrics: true,
KeyToHash: z.QuestionToHash,
Cost: z.MsgCost,
})
// use resolver with cache and no parallelism
r, err := spf.NewMiekgDNSResolver("8.8.8.8:53", spf.MiekgDNSParallelism(1), spf.MiekgDNSCache(c))
if err != nil {
log.Fatalf("error creating resolver: %s", err)
}

d.ForEach(r.CacheResponse)

c.Wait()

// create a printer
p := New(os.Stdout, r)

_, _, _, err = spf.CheckHost(net.ParseIP("0.0.0.0"), "subito.it", "aspmx.l.google.com",
spf.IgnoreMatches(),
spf.WithResolver(p),
spf.WithListener(p),
)
_, _, _, err = spf.CheckHost(net.ParseIP("0.0.0.0"), "ptr.test.redsift.io", "aspmx.l.google.com",
spf.IgnoreMatches(),
spf.WithResolver(p),
spf.WithListener(p),
)

fmt.Printf("## of lookups: %d\n", p.LookupsCount())

// output:
// CHECK_HOST("0.0.0.0", "subito.it.", "aspmx.l.google.com")
// lookup(TXT:strict) subito.it.
// SPF: v=spf1 mx:blocket.se include:spf.mailjet.com include:servers.mcsv.net ip4:109.168.127.160/27 ip4:212.31.252.64/27 ip4:212.77.68.6 ip4:62.212.1.160 ip4:62.212.0.160 ip4:93.94.32.0/21 ip4:93.94.37.253 ip4:109.168.121.48/28 ip4:37.202.20.23/32 ip4:213.215.152.254/32 ip4:213.215.152.253/32 ip4:213.215.152.252/32 ip4:213.215.152.251/32 ip4:109.168.121.54/32 ip4:109.168.121.55/32 ip4:109.168.121.57/32 ip4:109.168.121.58/32 -all
// v=spf1
// mx:blocket.se (blocket.se.)
// lookup(mx:blocket.se.) aspmx3.googlemail.com. -> (74.125.200.26/32 has? 0.0.0.0) = false
// lookup(mx:blocket.se.) aspmx3.googlemail.com. -> (2404:6800:4003:c00::1a/128 has? 0.0.0.0) = false
// lookup(mx:blocket.se.) alt1.aspmx.l.google.com. -> (64.233.161.26/32 has? 0.0.0.0) = false
// lookup(mx:blocket.se.) alt1.aspmx.l.google.com. -> (2a00:1450:4010:c05::1a/128 has? 0.0.0.0) = false
// lookup(mx:blocket.se.) alt2.aspmx.l.google.com. -> (74.125.200.26/32 has? 0.0.0.0) = false
// lookup(mx:blocket.se.) alt2.aspmx.l.google.com. -> (2404:6800:4003:c00::1a/128 has? 0.0.0.0) = false
// lookup(mx:blocket.se.) aspmx2.googlemail.com. -> (64.233.161.27/32 has? 0.0.0.0) = false
// lookup(mx:blocket.se.) aspmx2.googlemail.com. -> (2a00:1450:4010:c05::1b/128 has? 0.0.0.0) = false
// lookup(mx:blocket.se.) aspmx.l.google.com. -> (64.233.184.26/32 has? 0.0.0.0) = false
// lookup(mx:blocket.se.) aspmx.l.google.com. -> (2a00:1450:400c:c0b::1b/128 has? 0.0.0.0) = false
// include:spf.mailjet.com (spf.mailjet.com.)
// CHECK_HOST("0.0.0.0", "spf.mailjet.com.", "aspmx.l.google.com")
// lookup(TXT:strict) spf.mailjet.com.
// SPF: v=spf1 ip4:178.33.111.144 ip4:178.33.137.208/28 ip4:178.33.221.0/24 ip4:37.59.69.128/25 ip4:37.59.249.0/24 ip4:87.253.232.0/21 ip4:185.189.236.0/22 ?all
// v=spf1
// ip4:178.33.111.144 (178.33.111.144)
// ip4:178.33.137.208/28 (178.33.137.208/28)
// ip4:178.33.221.0/24 (178.33.221.0/24)
// ip4:37.59.69.128/25 (37.59.69.128/25)
// ip4:37.59.249.0/24 (37.59.249.0/24)
// ip4:87.253.232.0/21 (87.253.232.0/21)
// ip4:185.189.236.0/22 (185.189.236.0/22)
// ?all
// FIRST-MATCH: neutral, <nil>
// = 8, "24m51s", , result is unreliable with IgnoreMatches option enabled
// include:servers.mcsv.net (servers.mcsv.net.)
// CHECK_HOST("0.0.0.0", "servers.mcsv.net.", "aspmx.l.google.com")
// lookup(TXT:strict) servers.mcsv.net.
// SPF: v=spf1 ip4:205.201.128.0/20 ip4:198.2.128.0/18 ip4:148.105.8.0/21 ?all
// v=spf1
// ip4:205.201.128.0/20 (205.201.128.0/20)
// ip4:198.2.128.0/18 (198.2.128.0/18)
// ip4:148.105.8.0/21 (148.105.8.0/21)
// ?all
// = 8, "2m32s", , result is unreliable with IgnoreMatches option enabled
// ip4:109.168.127.160/27 (109.168.127.160/27)
// ip4:212.31.252.64/27 (212.31.252.64/27)
// ip4:212.77.68.6 (212.77.68.6)
// ip4:62.212.1.160 (62.212.1.160)
// ip4:62.212.0.160 (62.212.0.160)
// ip4:93.94.32.0/21 (93.94.32.0/21)
// ip4:93.94.37.253 (93.94.37.253)
// ip4:109.168.121.48/28 (109.168.121.48/28)
// ip4:37.202.20.23/32 (37.202.20.23/32)
// ip4:213.215.152.254/32 (213.215.152.254/32)
// ip4:213.215.152.253/32 (213.215.152.253/32)
// ip4:213.215.152.252/32 (213.215.152.252/32)
// ip4:213.215.152.251/32 (213.215.152.251/32)
// ip4:109.168.121.54/32 (109.168.121.54/32)
// ip4:109.168.121.55/32 (109.168.121.55/32)
// ip4:109.168.121.57/32 (109.168.121.57/32)
// ip4:109.168.121.58/32 (109.168.121.58/32)
// -all
// = 8, "4m29s", , result is unreliable with IgnoreMatches option enabled
// CHECK_HOST("0.0.0.0", "ptr.test.redsift.io.", "aspmx.l.google.com")
// lookup(TXT:strict) ptr.test.redsift.io.
// SPF: v=spf1 ptr ~all
// v=spf1
// ptr (ptr.test.redsift.io.)
// lookup(PTR) 0.0.0.0
// ~all
// FIRST-MATCH: softfail, <nil>
// = 8, "4m59s", , result is unreliable with IgnoreMatches option enabled
// ## of lookups: 15
}

0 comments on commit 4c16e59

Please sign in to comment.