git » spf » commit 1bd7bc8

Fix "h" macro expansion: use HELO instead of sender domain

author Gianni Ceccarelli
2021-11-01 10:25:40 UTC
committer Alberto Bertogli
2021-11-20 17:24:26 UTC
parent e2d699fe620dec1371f16e04d894a659df245e4a
Fix "h" macro expansion: use HELO instead of sender domain

The %{h} macro currently expands to the sender's domain (like %{o}), but
RFC 7208 section 7.2 clearly says it should be expanded to the HELO/EHLO
domain.

https://datatracker.ietf.org/doc/html/rfc7208#section-7.2

This patch fixes the bug by making the macro expansion use the HELO/EHLO
domain as per the RFC.

Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
  Updated commit message, auto-formatted code, added yml test.
spf.go +4 -1
spf_test.go +2 -0
testdata/blitirispf-tests.yml +16 -0 
diff --git a/spf.go b/spf.go
index 23aee29..8cc061f 100644
--- a/spf.go
+++ b/spf.go
@@ -140,6 +140,7 @@ func CheckHost(ip net.IP, domain string) (Result, error) {
 	r := &resolution{
 		ip:       ip,
 		maxcount: defaultMaxLookups,
+		helo:     domain,
 		sender:   "@" + domain,
 		ctx:      context.TODO(),
 		resolver: defaultResolver,
@@ -169,6 +170,7 @@ 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,
@@ -257,6 +259,7 @@ type resolution struct {
 	count    uint
 	maxcount uint
 
+	helo   string
 	sender string
 
 	// Result of doing a reverse lookup for ip (so we only do it once).
@@ -872,7 +875,7 @@ func (r *resolution) expandMacros(s, domain string) (string, error) {
 					str = "ip6"
 				}
 			case "h":
-				str = domain
+				str = r.helo
 			default:
 				// c, r, t are allowed in exp only, and we don't expand macros
 				// in exp so they are just as invalid as the rest.
diff --git a/spf_test.go b/spf_test.go
index 5e4a595..93c6fba 100644
--- a/spf_test.go
+++ b/spf_test.go
@@ -347,6 +347,7 @@ func TestMacros(t *testing.T) {
 		{"v=spf1 +a:ooo-%{o}-ooo", Pass, ErrMatchedA},
 		{"v=spf1 +a:OOO-%{O}-OOO", Pass, ErrMatchedA},
 		{"v=spf1 +a:ppp-%{p}-ppp", Pass, ErrMatchedA},
+		{"v=spf1 +a:hhh-%{h}-hhh", Pass, ErrMatchedA},
 		{"v=spf1 +a:vvv-%{v}-vvv", Pass, ErrMatchedA},
 		{"v=spf1 a:%{x}", PermError, ErrInvalidMacro},
 		{"v=spf1 +a:ooo-%{o7}-ooo", Pass, ErrMatchedA},
@@ -357,6 +358,7 @@ func TestMacros(t *testing.T) {
 	dns.Ip["ooo-domain-ooo"] = []net.IP{ip6666}
 	dns.Ip["ppp-unknown-ppp"] = []net.IP{ip6666}
 	dns.Ip["vvv-ip6-vvv"] = []net.IP{ip6666}
+	dns.Ip["hhh-helo-hhh"] = []net.IP{ip6666}
 	dns.Ip["8.6.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.vvv"] = []net.IP{ip1111}
 
 	for _, c := range cases {
diff --git a/testdata/blitirispf-tests.yml b/testdata/blitirispf-tests.yml
index 24c9926..81addc3 100644
--- a/testdata/blitirispf-tests.yml
+++ b/testdata/blitirispf-tests.yml
@@ -114,3 +114,19 @@ zonedata:
     - SPF: v=spf1 exists:lalala.com ~all
   lalala.com:
     - SERVFAIL: true
+---
+description: Resolve H macros correctly
+tests:
+  resolve-h-macros:
+    description: |
+      Check that '%{h}' macros are correctly resolved to the HELO/EHLO and not
+      the sender domain.
+    mailfrom: "foo@domain.net"
+    helo: holahola
+    host: 1.2.3.4
+    result: pass
+zonedata:
+  domain.net:
+    - SPF: v=spf1 exists:%{h}.com ~all
+  holahola.com:
+    - A: 127.0.0.2