author | James Lamb
<admin@oranged.to> 2022-01-18 08:59:44 UTC |
committer | Alberto Bertogli
<albertito@blitiri.com.ar> 2022-03-19 10:59:14 UTC |
parent | d9db5f70c02968c3f05c5c516ce520a4d1bc4397 |
spf.go | +80 | -26 |
spf_test.go | +40 | -0 |
testdata/rfc7208-tests.yml | +0 | -1 |
diff --git a/spf.go b/spf.go index 8cc061f..6a06443 100644 --- a/spf.go +++ b/spf.go @@ -92,10 +92,11 @@ var ( // Errors related to DNS lookups. // Note that the library functions may also return net.DNSError. - ErrNoResult = errors.New("no DNS record found") - ErrLookupLimitReached = errors.New("lookup limit reached") - ErrTooManyMXRecords = errors.New("too many MX records") - ErrMultipleRecords = errors.New("multiple matching DNS records") + ErrNoResult = errors.New("no DNS record found") + ErrLookupLimitReached = errors.New("lookup limit reached") + ErrVoidLookupLimitReached = errors.New("void lookup limit reached") + ErrTooManyMXRecords = errors.New("too many MX records") + ErrMultipleRecords = errors.New("multiple matching DNS records") // Errors returned on a successful match. ErrMatchedAll = errors.New("matched all") @@ -106,10 +107,18 @@ var ( ErrMatchedExists = errors.New("matched exists") ) -// Default value for the maximum number of DNS lookups while resolving SPF. -// RFC is quite clear 10 must be the maximum allowed. -// https://tools.ietf.org/html/rfc7208#section-4.6.4 -const defaultMaxLookups = 10 +const ( + // Default value for the maximum number of DNS lookups while resolving SPF. + // RFC is quite clear 10 must be the maximum allowed. + // https://tools.ietf.org/html/rfc7208#section-4.6.4 + defaultMaxLookups = 10 + + // Default value for the maximum number of DNS void lookups while + // resolving SPF. RFC suggests that implementations SHOULD limit these + // with a configurable default of 2. + // https://tools.ietf.org/html/rfc7208#section-4.6.4 + defaultMaxVoidLookups = 2 +) // TraceFunc is the type of tracing functions. type TraceFunc func(f string, a ...interface{}) @@ -138,13 +147,14 @@ type Option func(*resolution) // Deprecated: use CheckHostWithSender instead. func CheckHost(ip net.IP, domain string) (Result, error) { r := &resolution{ - ip: ip, - maxcount: defaultMaxLookups, - helo: domain, - sender: "@" + domain, - ctx: context.TODO(), - resolver: defaultResolver, - trace: defaultTrace, + ip: ip, + maxcount: defaultMaxLookups, + maxvoidcount: defaultMaxVoidLookups, + helo: domain, + sender: "@" + domain, + ctx: context.TODO(), + resolver: defaultResolver, + trace: defaultTrace, } return r.Check(domain) } @@ -168,13 +178,14 @@ func CheckHostWithSender(ip net.IP, helo, sender string, opts ...Option) (Result } r := &resolution{ - ip: ip, - maxcount: defaultMaxLookups, - helo: helo, - sender: sender, - ctx: context.TODO(), - resolver: defaultResolver, - trace: defaultTrace, + ip: ip, + maxcount: defaultMaxLookups, + maxvoidcount: defaultMaxVoidLookups, + helo: helo, + sender: sender, + ctx: context.TODO(), + resolver: defaultResolver, + trace: defaultTrace, } for _, opt := range opts { @@ -196,6 +207,18 @@ func OverrideLookupLimit(limit uint) Option { } } +// OverrideVoidLookupLimit overrides the maximum number of void DNS lookups allowed +// during SPF evaluation. A void DNS lookup is one that returns an empty +// answer, or a NXDOMAIN. Note that as per RFC, the default value of 2 SHOULD +// be used. Please use with care. +// +// This is EXPERIMENTAL for now, and the API is subject to change. +func OverrideVoidLookupLimit(limit uint) Option { + return func(r *resolution) { + r.maxvoidcount = limit + } +} + // WithContext is an option to set the context for this operation, which will // be passed along to the resolver functions and other external calls if // needed. @@ -255,9 +278,11 @@ func split(addr string) (string, string) { } type resolution struct { - ip net.IP - count uint - maxcount uint + ip net.IP + count uint + maxcount uint + voidcount uint + maxvoidcount uint helo string sender string @@ -281,7 +306,7 @@ var ptrField = regexp.MustCompile(`^(ptr$|ptr:)`) func (r *resolution) Check(domain string) (Result, error) { r.count++ - r.trace("check %q %d", domain, r.count) + r.trace("check %q %d %d", domain, r.count, r.voidcount) txt, err := r.getDNSRecord(domain) if err != nil { if isTemporary(err) { @@ -343,6 +368,11 @@ func (r *resolution) Check(domain string) (Result, error) { return PermError, ErrLookupLimitReached } + if r.voidcount > r.maxvoidcount { + r.trace("void lookup limit reached") + return PermError, ErrVoidLookupLimitReached + } + // See if we have a qualifier, defaulting to + (pass). // https://tools.ietf.org/html/rfc7208#section-4.6.2 result, ok := qualToResult[field[0]] @@ -454,6 +484,26 @@ func isTemporary(err error) bool { return ok && derr.Temporary() } +// Check if the given DNS error is a "void lookup" (0 answers, or nxdomain), +// and if so increment the void lookup counter. +func (r *resolution) checkVoidLookup(nanswers int, err error) { + if err == nil && nanswers == 0 { + r.voidcount++ + r.trace("void lookup: no answers") + return + } + + derr, ok := err.(*net.DNSError) + if !ok { + return + } + + if derr.IsNotFound { + r.voidcount++ + r.trace("void lookup: nxdomain") + } +} + // ipField processes an "ip" field. func (r *resolution) ipField(res Result, field string) (bool, Result, error) { fip := field[4:] @@ -499,6 +549,7 @@ func (r *resolution) ptrField(res Result, field, domain string) (bool, Result, e r.ipNames = []string{} r.count++ ns, err := r.resolver.LookupAddr(r.ctx, r.ip.String()) + r.checkVoidLookup(len(ns), err) if err != nil { // https://tools.ietf.org/html/rfc7208#section-5 if isTemporary(err) { @@ -555,6 +606,7 @@ func (r *resolution) existsField(res Result, field, domain string) (bool, Result r.count++ ips, err := r.resolver.LookupIPAddr(r.ctx, eDomain) + r.checkVoidLookup(len(ips), err) if err != nil { // https://tools.ietf.org/html/rfc7208#section-5 if isTemporary(err) { @@ -670,6 +722,7 @@ func (r *resolution) aField(res Result, field, domain string) (bool, Result, err r.count++ ips, err := r.resolver.LookupIPAddr(r.ctx, aDomain) + r.checkVoidLookup(len(ips), err) if err != nil { // https://tools.ietf.org/html/rfc7208#section-5 if isTemporary(err) { @@ -702,6 +755,7 @@ func (r *resolution) mxField(res Result, field, domain string) (bool, Result, er r.count++ mxs, err := r.resolver.LookupMX(r.ctx, mxDomain) + r.checkVoidLookup(len(mxs), err) if err != nil { // https://tools.ietf.org/html/rfc7208#section-5 if isTemporary(err) { diff --git a/spf_test.go b/spf_test.go index 93c6fba..58ee9ee 100644 --- a/spf_test.go +++ b/spf_test.go @@ -522,6 +522,46 @@ func TestOverrideLookupLimit(t *testing.T) { } } +func TestOverrideVoidLookupLimit(t *testing.T) { + dns := NewDefaultResolver() + defaultTrace = t.Logf + + nxDomainErr := &net.DNSError{ + Err: "no such domain", + IsTemporary: false, + IsNotFound: true, + } + + dns.Txt["domain1"] = []string{"v=spf1 exists:%{i}.one include:domain2"} + dns.Txt["domain2"] = []string{"v=spf1 exists:%{i}.two include:domain3"} + dns.Txt["domain3"] = []string{"v=spf1 exists:%{i}.three include:domain4"} + dns.Txt["domain4"] = []string{"v=spf1 +all"} + dns.Errors["1.1.1.1.one"] = nxDomainErr + dns.Errors["1.1.1.1.two"] = nxDomainErr + dns.Errors["1.1.1.1.three"] = nxDomainErr + + // The default of 2 + res, err := CheckHostWithSender(ip1111, "helo", "user@domain1") + if res != PermError { + t.Errorf("expected permerror, got %q / %q", res, err) + } + + // Set the limit to 10, which is excessive. + res, err = CheckHostWithSender(ip1111, "helo", "user@domain1", + OverrideVoidLookupLimit(10)) + if res != Pass { + t.Errorf("expected pass, got %q / %q", res, err) + } + + // Set the limit to 1, which is not enough. + res, err = CheckHostWithSender(ip1111, "helo", "user@domain1", + OverrideVoidLookupLimit(1)) + if res != PermError || err != ErrVoidLookupLimitReached { + t.Errorf("expected permerror/void lookup limit reached, got %q / %q", + res, err) + } +} + func TestWithContext(t *testing.T) { dns := NewDefaultResolver() defaultTrace = t.Logf diff --git a/testdata/rfc7208-tests.yml b/testdata/rfc7208-tests.yml index 9191fa2..b22dab9 100644 --- a/testdata/rfc7208-tests.yml +++ b/testdata/rfc7208-tests.yml @@ -2553,7 +2553,6 @@ tests: host: 1.2.3.4 mailfrom: foo@e11.example.com result: permerror - skip: We don't limit this separately from the total 10 limit. zonedata: mail.example.com: - A: 1.2.3.4