lib/spf: continuing work on parseModifier

7 files changed, 149 insertions, 91 deletions

diff --git a/doc/SPF.adoc b/doc/SPF.adoc
index b3368519..10c984fc 100644
--- a/doc/SPF.adoc
+++ b/doc/SPF.adoc
@@ -163,7 +163,7 @@ text of all the records of a given type is under 450 octets, then DNS answers
 ought to fit in UDP packets.
 Records that are too long to fit in a single UDP packet could be silently
 ignored by SPF verifiers due to firewall and other issues that interfere with
-the operation of DNS over TCP or using ENDS0.
+the operation of DNS over TCP or using EDNS0.
 
 Note that when computing the sizes for replies to queries of the TXT format,
 one has to take into account any other TXT records published at the domain
diff --git a/lib/spf/directive.go b/lib/spf/directive.go
index 4d2220a6..417384f1 100644
--- a/lib/spf/directive.go
+++ b/lib/spf/directive.go
@@ -6,26 +6,26 @@ package spf
 
 // List of known qualifier for directive.
 const (
-	qualifierPass     int = iota // "+"
-	qualifierFail                // "-"
-	qualifierSoftfail            // "~"
-	qualifierNeutral             // "?"
+	qualifierPass     byte = '+'
+	qualifierFail     byte = '-'
+	qualifierSoftfail byte = '~'
+	qualifierNeutral  byte = '?'
 )
 
 // List of mechanism for directive.
 const (
-	mechanismAll int = iota
-	mechanismInclude
-	mechanismA
-	mechanismMx
-	mechanismPtr
-	mechanismIp4
-	mechanismIp6
-	mechanismExists
+	mechanismAll     = "all"
+	mechanismInclude = "include"
+	mechanismA       = "a"
+	mechanismMx      = "mx"
+	mechanismPtr     = "ptr"
+	mechanismIP4     = "ip4"
+	mechanismIP6     = "ip6"
+	mechanismExist   = "exist"
 )
 
 type directive struct {
-	qual  int
-	mech  int
+	qual  byte
+	mech  string
 	value []byte
 }
diff --git a/lib/spf/macro.go b/lib/spf/macro.go
index 918e5b18..7768829f 100644
--- a/lib/spf/macro.go
+++ b/lib/spf/macro.go
@@ -25,9 +25,11 @@ const (
 	macroPtr          byte = 'p'
 	macroInAddr       byte = 'v'
 	macroEhloDomain   byte = 'h'
-	macroExpClientIP  byte = 'c'
-	macroExpDomain    byte = 'r'
-	macroExpCurTime   byte = 't'
+
+	// The following macro letters are allowed only in "exp".
+	macroExpClientIP byte = 'c'
+	macroExpDomain   byte = 'r'
+	macroExpCurTime  byte = 't'
 )
 
 type macro struct {
@@ -36,14 +38,14 @@ type macro struct {
 
 	// mode is either mechanism "include", modifier "redirect", or
 	// modifier "exp".
-	mode       int
-	letter     byte
+	mode       string
 	nright     int
 	dels       []byte
+	letter     byte
 	isReversed bool
 }
 
-func macroExpand(ref *Result, mode int, data []byte) (out []byte, err error) {
+func macroExpand(ref *Result, mode string, data []byte) (out []byte, err error) {
 	m := &macro{
 		ref:  ref,
 		mode: mode,
@@ -79,7 +81,7 @@ func isDelimiter(c byte) bool {
 //
 // Letter "c", "r", and "t" only valid if modifier is "exp".
 //
-func isMacroLetter(mode int, c byte) bool {
+func isMacroLetter(mode string, c byte) bool {
 	if c == macroSender || c == macroSenderLocal ||
 		c == macroSenderDomain || c == macroDomain || c == macroIP ||
 		c == macroPtr || c == macroEhloDomain || c == macroInAddr {
@@ -259,7 +261,7 @@ func (m *macro) expandLetter() (value []byte) {
 	case macroExpClientIP:
 		value = toDotIP(m.ref.IP)
 	case macroExpDomain:
-		value = []byte(m.ref.Hostname)
+		value = m.ref.Hostname
 	case macroExpCurTime:
 		now := time.Now()
 		strNow := fmt.Sprintf("%d", now.Unix())
diff --git a/lib/spf/macro_test.go b/lib/spf/macro_test.go
index 8221e23d..21a42080 100644
--- a/lib/spf/macro_test.go
+++ b/lib/spf/macro_test.go
@@ -16,7 +16,7 @@ func TestMacroExpandIPv4(t *testing.T) {
 		"strong-bad@email.example.com", "mail.localhost")
 
 	cases := []struct {
-		mode   int
+		mode   string
 		data   string
 		exp    string
 		expErr string
@@ -121,7 +121,7 @@ func TestMacroExpandIPv4(t *testing.T) {
 
 		got, err := macroExpand(ref, c.mode, []byte(c.data))
 		if err != nil {
-			test.Assert(t, "error", string(c.expErr), err.Error(), true)
+			test.Assert(t, "error", c.expErr, err.Error(), true)
 			continue
 		}
 
@@ -134,7 +134,7 @@ func TestMacroExpandIPv6(t *testing.T) {
 		"strong-bad@email.example.com", "email.localhost")
 
 	cases := []struct {
-		mode   int
+		mode   string
 		data   string
 		exp    string
 		expErr string
@@ -148,7 +148,7 @@ func TestMacroExpandIPv6(t *testing.T) {
 
 		got, err := macroExpand(ref, c.mode, []byte(c.data))
 		if err != nil {
-			test.Assert(t, "error", string(c.expErr), string(got), true)
+			test.Assert(t, "error", c.expErr, string(got), true)
 			continue
 		}
 
diff --git a/lib/spf/modifier.go b/lib/spf/modifier.go
index 935fb73c..8b82d2e7 100644
--- a/lib/spf/modifier.go
+++ b/lib/spf/modifier.go
@@ -8,13 +8,11 @@ package spf
 // List of modifiers.
 //
 const (
-	modifierRedirect int = iota
-	modifierExp
-	modifierUnknown
+	modifierExp      = "exp"
+	modifierRedirect = "redirect"
 )
 
 type modifier struct {
-	kind  byte
 	name  string
 	value string
 }
diff --git a/lib/spf/result.go b/lib/spf/result.go
index d0a6f020..08a9d3ec 100644
--- a/lib/spf/result.go
+++ b/lib/spf/result.go
@@ -17,16 +17,16 @@ import (
 // Result contains the output of CheckHost function.
 //
 type Result struct {
-	IP       net.IP
-	Domain   []byte
-	Sender   []byte
+	IP       net.IP // The IP address of sender.
+	Domain   []byte // The domain address of sender from SMTP EHLO or MAIL FROM command.
+	Sender   []byte // The email address of sender.
 	Hostname []byte
-	Code     byte
+	Code     byte // Result of check host.
 	Err      string
 
-	senderLocal  []byte
-	senderDomain []byte
-	rdata        []byte // rdata contains raw DNS resource record data that have SPF record.
+	senderLocal  []byte // The local part of sender.
+	senderDomain []byte // The domain part of sender.
+	terms        []byte // terms contains raw DNS RR that have SPF record.
 	dirs         []*directive
 	mods         []*modifier
 }
@@ -34,11 +34,11 @@ type Result struct {
 //
 // newResult initialize new SPF result on single domain.
 //
-func newResult(ip net.IP, domain, sender, hostname string) (r *Result) {
+func newResult(ip net.IP, domain, sender, hostname string) (result *Result) {
 	bsender := []byte(sender)
 
 	at := bytes.Index(bsender, []byte{'@'})
-	r = &Result{
+	result = &Result{
 		IP:           ip,
 		Domain:       []byte(domain),
 		Sender:       []byte(sender),
@@ -47,9 +47,9 @@ func newResult(ip net.IP, domain, sender, hostname string) (r *Result) {
 		senderDomain: bsender[at+1:],
 	}
 
-	if !libnet.IsHostnameValid(r.Domain, true) {
-		r.Code = ResultCodeNone
-		r.Err = "invalid domain name"
+	if !libnet.IsHostnameValid(result.Domain, true) {
+		result.Code = ResultCodeNone
+		result.Err = "invalid domain name"
 		return
 	}
 
@@ -59,14 +59,14 @@ func newResult(ip net.IP, domain, sender, hostname string) (r *Result) {
 //
 // Error return the string representation of the result as error message.
 //
-func (r *Result) Error() string {
-	return fmt.Sprintf("spf: %q %s", r.Domain, r.Err)
+func (result *Result) Error() string {
+	return fmt.Sprintf("spf: %q %s", result.Domain, result.Err)
 }
 
 //
 // lookup the TXT record that contains SPF record on domain name.
 //
-func (r *Result) lookup() {
+func (result *Result) lookup() {
 	var (
 		dnsMsg *libdns.Message
 		err    error
@@ -74,29 +74,29 @@ func (r *Result) lookup() {
 	)
 
 	dnsMsg, err = dnsClient.Lookup(true, libdns.QueryTypeTXT,
-		libdns.QueryClassIN, r.Domain)
+		libdns.QueryClassIN, result.Domain)
 	if err != nil {
-		r.Code = ResultCodeTempError
-		r.Err = err.Error()
+		result.Code = ResultCodeTempError
+		result.Err = err.Error()
 		return
 	}
 
 	switch dnsMsg.Header.RCode {
 	case libdns.RCodeOK:
 	case libdns.RCodeErrName:
-		r.Code = ResultCodeNone
-		r.Err = "domain name does not exist"
+		result.Code = ResultCodeNone
+		result.Err = "domain name does not exist"
 		return
 	default:
-		r.Code = ResultCodeTempError
-		r.Err = "server failure"
+		result.Code = ResultCodeTempError
+		result.Err = "server failure"
 		return
 	}
 
 	txts = dnsMsg.FilterAnswers(libdns.QueryTypeTXT)
 	if len(txts) == 0 {
-		r.Code = ResultCodeNone
-		r.Err = "no SPF record found"
+		result.Code = ResultCodeNone
+		result.Err = "no SPF record found"
 		return
 	}
 
@@ -111,39 +111,68 @@ func (r *Result) lookup() {
 		if bytes.HasPrefix(rdata, []byte("v=spf1")) {
 			found++
 			if found == 1 {
-				r.rdata = rdata
+				result.terms = rdata
 			}
 		}
 	}
 	if found == 0 {
-		r.Code = ResultCodeNone
-		r.Err = "no SPF record found"
+		result.Code = ResultCodeNone
+		result.Err = "no SPF record found"
 		return
 	}
 	if found > 1 {
-		r.Code = ResultCodePermError
-		r.Err = "multiple SPF records found"
+		result.Code = ResultCodePermError
+		result.Err = "multiple SPF records found"
 		return
 	}
 
-	r.rdata = bytes.ToLower(r.rdata)
+	result.terms = bytes.ToLower(result.terms)
 }
 
 //
 // evaluateSPFRecord parse and evaluate each directive with its modifiers
 // in the SPF record.
 //
-func (r *Result) evaluateSPFRecord(rdata []byte) {
-	terms := bytes.Fields(rdata)
+//	terms            = *( 1*SP ( directive / modifier ) )
+//
+//	directive        = [ qualifier ] mechanism
+//	qualifier        = "+" / "-" / "?" / "~"
+//	mechanism        = ( all / include
+//	                 / a / mx / ptr / ip4 / ip6 / exists )
+//	modifier         = redirect / explanation / unknown-modifier
+//	unknown-modifier = name "=" macro-string
+//	                 ; where name is not any known modifier
+//
+//	name             = ALPHA *( ALPHA / DIGIT / "-" / "_" / "." )
+//
+func (result *Result) evaluateSPFRecord() {
+	terms := bytes.Fields(result.terms)
 
 	// Ignore the first field "v=spf1".
 
 	for x := 1; x < len(terms); x++ {
-		dir := r.parseDirective(terms[x])
+		dir := result.parseDirective(terms[x])
 		if dir != nil {
+			result.dirs = append(result.dirs, dir)
+
+			// Mechanisms after "all" will never be tested and MUST be
+			// ignored -- RFC 7208 section 5.1.
+			if dir.mech == mechanismAll {
+				return
+			}
 			continue
 		}
-		r.dirs = append(r.dirs, dir)
+
+		if result.Code != 0 {
+			return
+		}
+
+		mod := result.parseModifier(terms[x])
+		if mod == nil {
+			return
+		}
+
+		result.mods = append(result.mods, mod)
 	}
 }
 
@@ -152,55 +181,79 @@ func (r *Result) evaluateSPFRecord(rdata []byte) {
 // It will return non-nil if term is a directive, otherwise it will return
 // nil.
 //
-func (r *Result) parseDirective(term []byte) (dir *directive) {
+func (result *Result) parseDirective(term []byte) (dir *directive) {
 	var (
-		qual int = -1
+		qual byte
 		err  error
 	)
 
 	switch term[0] {
-	case '+':
-		qual = qualifierPass
-	case '-':
-		qual = qualifierFail
-	case '~':
-		qual = qualifierSoftfail
-	case '?':
-		qual = qualifierNeutral
-	}
-
-	if qual == -1 {
-		qual = qualifierPass
-	} else {
+	case qualifierPass, qualifierNeutral, qualifierSoftfail, qualifierFail:
+		qual = term[0]
 		term = term[1:]
+	default:
+		qual = qualifierPass
 	}
 
 	// Try to split the term to get the mechanism and domain-spec.
 	kv := bytes.Split(term, []byte{':'})
 
-	switch {
-	case bytes.Equal(kv[0], []byte("all")):
+	mech := string(kv[0])
+
+	switch mech {
+	case mechanismAll:
 		dir = &directive{
 			qual: qual,
-			mech: mechanismAll,
+			mech: mech,
 		}
 		return dir
 
-	case bytes.Equal(kv[0], []byte("include")):
+	case mechanismInclude:
 		dir = &directive{
 			qual: qual,
-			mech: mechanismInclude,
+			mech: mech,
 		}
 		if len(kv) >= 2 {
-			dir.value, err = macroExpand(r, mechanismInclude, kv[1])
+			dir.value, err = macroExpand(result, mechanismInclude, kv[1])
 			if err != nil {
-				r.Code = ResultCodePermError
-				r.Err = err.Error()
+				result.Code = ResultCodePermError
+				result.Err = err.Error()
 				return nil
 			}
 		}
 		return dir
+
+	case mechanismA:
+
+	case mechanismMx:
+
+	case mechanismPtr:
+
+	case mechanismIP4:
+
+	case mechanismIP6:
+
+	case mechanismExist:
 	}
 
 	return nil
 }
+
+func (result *Result) parseModifier(term []byte) (mod *modifier) {
+	kv := bytes.Split(term, []byte{'='})
+
+	mod = &modifier{
+		name: string(kv[0]),
+	}
+	if len(kv) >= 2 {
+		mod.value = string(kv[1])
+	}
+
+	switch mod.name {
+	case modifierExp:
+		return mod
+	case modifierRedirect:
+		return mod
+	}
+	return mod
+}
diff --git a/lib/spf/spf.go b/lib/spf/spf.go
index 16d28af9..bd49bd5d 100644
--- a/lib/spf/spf.go
+++ b/lib/spf/spf.go
@@ -86,13 +86,18 @@ func CheckHost(ip net.IP, domain, sender, hostname string) (result *Result) {
 
 	result = newResult(ip, domain, sender, hostname)
 	if result.Code != 0 {
-		return
+		return result
 	}
 
 	result.lookup()
 	if result.Code != 0 {
-		return
+		return result
 	}
 
-	return
+	result.evaluateSPFRecord()
+	if result.Code != 0 {
+		return result
+	}
+
+	return result
 }