git » spf » commit 5a188ab

Fix DNS lookup limits in MX field processing

author Alberto Bertogli
2022-06-18 22:52:10 UTC
committer Alberto Bertogli
2022-06-18 22:52:10 UTC
parent 48ee7003328e0d0cd8ce851f356fb4be0ae4364d
Fix DNS lookup limits in MX field processing

The RFC text for what to do with MX fields in the context of DNS lookup
limits is:

> When evaluating the "mx" mechanism, the number of "MX" resource
> records queried is included in the overall limit of 10 mechanisms/
> modifiers that cause DNS lookups as described above.  In addition to
> that limit, the evaluation of each "MX" record MUST NOT result in
> querying more than 10 address records -- either "A" or "AAAA"
> resource records.  If this limit is exceeded, the "mx" mechanism MUST
> produce a "permerror" result.

Today, we count each individual MX record (which triggers a DNS lookup)
against the total term counter. That is contrary to the RFC, and what
other libraries do.

The correct behaviour, which this patch implements, is to count each
"mx" term as 1, regardless of how many MX records it produces. This
enables some level of DNS amplification, but because there is a second
limit of 10 MX records per term, it is bounded.

This makes our behaviour match RFC, and also other popular
implementations.
spf.go +0 -1
testdata/blitirispf-tests.yml +32 -0 
diff --git a/spf.go b/spf.go
index 8a75d71..be9d585 100644
--- a/spf.go
+++ b/spf.go
@@ -777,7 +777,6 @@ func (r *resolution) mxField(res Result, field, domain string) (bool, Result, er
 
 	mxips := []net.IP{}
 	for _, mx := range mxs {
-		r.count++
 		ips, err := r.resolver.LookupIPAddr(r.ctx, mx.Host)
 		if err != nil {
 			// https://tools.ietf.org/html/rfc7208#section-5
diff --git a/testdata/blitirispf-tests.yml b/testdata/blitirispf-tests.yml
index cb6047e..5730dc4 100644
--- a/testdata/blitirispf-tests.yml
+++ b/testdata/blitirispf-tests.yml
@@ -225,3 +225,35 @@ zonedata:
     - SPF: v=spf1 -all
   d25:
     - SPF: v=spf1 all
+---
+description: MX resolution limits
+tests:
+  mx-resolution-10-terms:
+    description: |
+      Check that a resolution with 10 "mx" terms works, because it's within
+      the limit. Each term will resolve to multiple records, but those
+      shouldn't be individually counted (there's a limit of 10 MX records per
+      MX lookup, but that's tested separately).
+    mailfrom: "foo@mx10"
+    host: 1.2.3.4
+    result: pass
+  mx-resolution-11-terms:
+    description: |
+      Check that a resolution with 11 "mx" terms, causes a permerror due to
+      exceeding lookup limits.
+    mailfrom: "foo@mx11"
+    host: 1.2.3.4
+    result: permerror
+zonedata:
+  mx10:
+    - SPF: v=spf1 mx:domain mx:domain mx:domain mx:domain mx:domain
+                  mx:domain mx:domain mx:domain mx:domain mx:domain
+                  all
+  mx11:
+    - SPF: v=spf1 mx:domain mx:domain mx:domain mx:domain mx:domain
+                  mx:domain mx:domain mx:domain mx:domain mx:domain
+                  mx:domain all
+  domain:
+    - MX: [1, blah1]
+    - MX: [2, blah2]
+    - MX: [3, blah3]