From 2d9d93653b37448b0e5efe38dc10c7e55096533f Mon Sep 17 00:00:00 2001
From: Tom Limoncelli <tlimoncelli@stackexchange.com>
Date: Wed, 19 Jun 2019 18:46:56 +0100
Subject: [PATCH] SPF Optimizer: Add "redirect:" support (#506)

FYI: The support is very minimal. It only supports redirect if it is the last item in an SPF record. At that point, it is equivalent to include.

* In SFP, treat redirect like a special include.
* Document SPF redirect: limited implementation.
---
 docs/spf-optimizer.md    |  4 ++++
 pkg/spflib/parse.go      | 15 ++++++++++++---
 pkg/spflib/parse_test.go | 28 ++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/docs/spf-optimizer.md b/docs/spf-optimizer.md
index 5b3176d4b..142a074a3 100644
--- a/docs/spf-optimizer.md
+++ b/docs/spf-optimizer.md
@@ -178,6 +178,10 @@ could exceed 512 bytes, and will require EDNS or a TCP request.
 3. Dnscontrol does not warn if the number of lookups exceeds 10.
 We hope to implement this some day.
 
+4. The `redirect:` directive is only partially implemented.  We only
+handle the case where redirect is the last item in the SPF record.
+In which case, it is equivalent to `include:`.
+
 
 ## Advanced Technique: Interactive SPF Debugger
 
diff --git a/pkg/spflib/parse.go b/pkg/spflib/parse.go
index 2111c9e5a..19491a254 100644
--- a/pkg/spflib/parse.go
+++ b/pkg/spflib/parse.go
@@ -52,7 +52,7 @@ func Parse(text string, dnsres Resolver) (*SPFRecord, error) {
 	}
 	parts := strings.Split(text, " ")
 	rec := &SPFRecord{}
-	for _, part := range parts[1:] {
+	for pi, part := range parts[1:] {
 		if part == "" {
 			continue
 		}
@@ -69,9 +69,18 @@ func Parse(text string, dnsres Resolver) (*SPFRecord, error) {
 		} else if strings.HasPrefix(part, "ip4:") || strings.HasPrefix(part, "ip6:") {
 			// ip address, 0 lookups
 			continue
-		} else if strings.HasPrefix(part, "include:") {
+		} else if strings.HasPrefix(part, "include:") || strings.HasPrefix(part, "redirect:") {
+			if strings.HasPrefix(part, "redirect:") {
+				// pi + 2: because pi starts at 0 when it iterates starting on parts[1],
+				// and because len(parts) is one bigger than the highest index.
+				if (pi + 2) != len(parts) {
+					return nil, errors.Errorf("%s must be last item", part)
+				}
+				p.IncludeDomain = strings.TrimPrefix(part, "redirect:")
+			} else {
+				p.IncludeDomain = strings.TrimPrefix(part, "include:")
+			}
 			p.IsLookup = true
-			p.IncludeDomain = strings.TrimPrefix(part, "include:")
 			if dnsres != nil {
 				subRecord, err := dnsres.GetSPF(p.IncludeDomain)
 				if err != nil {
diff --git a/pkg/spflib/parse_test.go b/pkg/spflib/parse_test.go
index 8bb02e850..1b628e10a 100644
--- a/pkg/spflib/parse_test.go
+++ b/pkg/spflib/parse_test.go
@@ -40,3 +40,31 @@ func TestParseWithDoubleSpaces(t *testing.T) {
 	}
 	t.Log(rec.Print())
 }
+
+func TestParseRedirectNotLast(t *testing.T) {
+	// Make sure redirect:foo fails if it isn't the last item.
+	dnsres, err := NewCache("testdata-dns1.json")
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = Parse(strings.Join([]string{"v=spf1",
+		"redirect:servers.mcsv.net",
+		"~all"}, " "), dnsres)
+	if err == nil {
+		t.Fatal("should fail")
+	}
+}
+
+func TestParseRedirectLast(t *testing.T) {
+	dnsres, err := NewCache("testdata-dns1.json")
+	if err != nil {
+		t.Fatal(err)
+	}
+	rec, err := Parse(strings.Join([]string{"v=spf1",
+		"ip4:198.252.206.0/24",
+		"redirect:servers.mcsv.net"}, " "), dnsres)
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Log(rec.Print())
+}