From bc2b3b40a545f8ddbdb515c5827ba7aa5032ec74 Mon Sep 17 00:00:00 2001
From: Alberto Bertogli <albertito@blitiri.com.ar>
Date: Sat, 8 Oct 2016 18:11:01 +0100
Subject: [PATCH] chasquid: Skip SPF if the connection has authenticated

Currently, we do SPF checks for all connections.

However, authenticated users will be sending email from different
locations, applying SPF to them will result in false positives.

So this patch makes chasquid skip SPF checking if the connection is
authenticated.
---
 chasquid.go                         | 40 +++++++++++++++++++----------
 test/t-05-null_address/expected_dsr |  1 -
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/chasquid.go b/chasquid.go
index dee0ee1..16fb926 100644
--- a/chasquid.go
+++ b/chasquid.go
@@ -655,19 +655,13 @@ func (c *Conn) MAIL(params string) (code int, msg string) {
 		}
 
 		// SPF check - https://tools.ietf.org/html/rfc7208#section-2.4
-		if tcp, ok := c.netconn.RemoteAddr().(*net.TCPAddr); ok {
-			c.spfResult, c.spfError = spf.CheckHost(
-				tcp.IP, envelope.DomainOf(addr))
-			c.tr.Debugf("SPF %v (%v)", c.spfResult, c.spfError)
-			spfResultCount.Add(string(c.spfResult), 1)
-
-			// https://tools.ietf.org/html/rfc7208#section-8
-			// We opt not to fail on errors, to avoid accidents to prevent
-			// delivery.
-			if c.spfResult == spf.Fail {
-				return 550, fmt.Sprintf(
-					"SPF check failed: %v", c.spfError)
-			}
+		// We opt not to fail on errors, to avoid accidents from preventing
+		// delivery.
+		c.spfResult, c.spfError = c.checkSPF(addr)
+		if c.spfResult == spf.Fail {
+			// https://tools.ietf.org/html/rfc7208#section-8.4
+			return 550, fmt.Sprintf(
+				"SPF check failed: %v", c.spfError)
 		}
 
 		addr, err = envelope.IDNAToUnicode(addr)
@@ -680,6 +674,26 @@ func (c *Conn) MAIL(params string) (code int, msg string) {
 	return 250, "You feel like you are being watched"
 }
 
+// checkSPF for the given address, based on the current connection.
+func (c *Conn) checkSPF(addr string) (spf.Result, error) {
+	// Does not apply to authenticated connections, they're allowed regardless.
+	if c.completedAuth {
+		return "", nil
+	}
+
+	if tcp, ok := c.netconn.RemoteAddr().(*net.TCPAddr); ok {
+		res, err := spf.CheckHost(
+			tcp.IP, envelope.DomainOf(addr))
+
+		c.tr.Debugf("SPF %v (%v)", res, err)
+		spfResultCount.Add(string(res), 1)
+
+		return res, err
+	}
+
+	return "", nil
+}
+
 func (c *Conn) RCPT(params string) (code int, msg string) {
 	// params should be: "TO:<name@host>", and possibly followed by options
 	// such as "NOTIFY=SUCCESS,DELAY" (which we ignore).
diff --git a/test/t-05-null_address/expected_dsr b/test/t-05-null_address/expected_dsr
index 3585f79..b2c473b 100644
--- a/test/t-05-null_address/expected_dsr
+++ b/test/t-05-null_address/expected_dsr
@@ -20,7 +20,6 @@ Delivery to the following recipient(s) failed permanently:
 
 ----- Original message -----
 
-Received-SPF: *
 Received: from user user@testserver
 	by *
 	(envelope from "user@testserver")