From 2c9cb5b847a7708ba401351e092098048eb8297b Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Wed, 13 Mar 2024 17:35:53 +0100 Subject: [PATCH] add parser of Authentication-Results, and fix bugs it found in our generated headers we weren't always quoting the values, like dkim's header.b=abc/def. the "/" requires that the value be quoted. --- message/authresults.go | 373 +++++++++++++++++++++++++++++++++++- message/authresults_test.go | 303 ++++++++++++++++++++++++++++- message/part_test.go | 2 +- smtpserver/parse.go | 12 +- 4 files changed, 674 insertions(+), 16 deletions(-) diff --git a/message/authresults.go b/message/authresults.go index 94fccfa..e3443e2 100644 --- a/message/authresults.go +++ b/message/authresults.go @@ -2,6 +2,9 @@ package message import ( "fmt" + "strings" + + "github.com/mjl-/mox/dns" ) // ../rfc/8601:577 @@ -9,8 +12,11 @@ import ( // Authentication-Results header, see RFC 8601. type AuthResults struct { Hostname string - Comment string // If not empty, header comment without "()", added after Hostname. - Methods []AuthMethod + // Optional version of Authentication-Results header, assumed "1" when absent, + // which is common. + Version string + Comment string // If not empty, header comment without "()", added after Hostname. + Methods []AuthMethod // Can be empty, in case of "none". } // ../rfc/8601:598 @@ -21,6 +27,7 @@ type AuthResults struct { type AuthMethod struct { // E.g. "dkim", "spf", "iprev", "auth". Method string + Version string // For optional method version. "1" is implied when missing, which is common. Result string // Each method has a set of known values, e.g. "pass", "temperror", etc. Comment string // Optional, message header comment. Reason string // Optional. @@ -64,7 +71,7 @@ func (h AuthResults) Header() string { } w := &HeaderWriter{} - w.Add("", "Authentication-Results:"+optComment(h.Comment)+" "+value(h.Hostname)+";") + w.Add("", "Authentication-Results:"+optComment(h.Comment)+" "+value(h.Hostname, false)+";") for i, m := range h.Methods { w.Newline() @@ -78,13 +85,10 @@ func (h AuthResults) Header() string { addf("(%s)", m.Comment) } if m.Reason != "" { - addf("reason=%s", value(m.Reason)) + addf("reason=%s", value(m.Reason, false)) } for _, p := range m.Props { - v := p.Value - if !p.IsAddrLike { - v = value(v) - } + v := value(p.Value, p.IsAddrLike) addf("%s.%s=%s%s", p.Type, p.Property, v, optComment(p.Comment)) } for j, t := range tokens { @@ -101,11 +105,12 @@ func (h AuthResults) Header() string { return w.String() } -func value(s string) string { +func value(s string, isAddrLike bool) string { quote := s == "" for _, c := range s { // utf-8 does not have to be quoted. ../rfc/6532:242 - if c == '"' || c == '\\' || c <= ' ' || c == 0x7f { + // Characters outside of tokens do. ../rfc/2045:661 + if c <= ' ' || c == 0x7f || (c == '@' && !isAddrLike) || strings.ContainsRune(`()<>,;:\\"/[]?= `, c) { quote = true break } @@ -123,3 +128,351 @@ func value(s string) string { r += `"` return r } + +// ParseAuthResults parses a Authentication-Results header value. +// +// Comments are not populated in the returned AuthResults. +// Both crlf and lf line-endings are accepted. The input string must end with +// either crlf or lf. +func ParseAuthResults(s string) (ar AuthResults, err error) { + // ../rfc/8601:577 + lower := make([]byte, len(s)) + for i, c := range []byte(s) { + if c >= 'A' && c <= 'Z' { + c += 'a' - 'A' + } + lower[i] = c + } + p := &parser{s: s, lower: string(lower)} + defer p.recover(&err) + + p.cfws() + ar.Hostname = p.xvalue() + p.cfws() + ar.Version = p.digits() + p.cfws() + for { + p.xtake(";") + p.cfws() + // Yahoo has ";" at the end of the header value, incorrect. + if !Pedantic && p.end() { + break + } + method := p.xkeyword(false) + p.cfws() + if method == "none" { + if len(ar.Methods) == 0 { + p.xerrorf("missing results") + } + if !p.end() { + p.xerrorf(`data after "none" result`) + } + return + } + ar.Methods = append(ar.Methods, p.xresinfo(method)) + p.cfws() + if p.end() { + break + } + } + return +} + +type parser struct { + s string + lower string // Like s, but with ascii characters lower-cased (utf-8 offsets preserved). + o int +} + +type parseError struct{ err error } + +func (p *parser) recover(err *error) { + x := recover() + if x == nil { + return + } + perr, ok := x.(parseError) + if ok { + *err = perr.err + return + } + panic(x) +} + +func (p *parser) xerrorf(format string, args ...any) { + panic(parseError{fmt.Errorf(format, args...)}) +} + +func (p *parser) end() bool { + return p.s[p.o:] == "\r\n" || p.s[p.o:] == "\n" +} + +// ../rfc/5322:599 +func (p *parser) cfws() { + p.fws() + for p.prefix("(") { + p.xcomment() + } + p.fws() +} + +func (p *parser) fws() { + for p.take(" ") || p.take("\t") { + } + opts := []string{"\n ", "\n\t", "\r\n ", "\r\n\t"} + for _, o := range opts { + if p.take(o) { + break + } + } + for p.take(" ") || p.take("\t") { + } +} + +func (p *parser) xcomment() { + p.xtake("(") + p.fws() + for !p.take(")") { + if p.empty() { + p.xerrorf("unexpected end in comment") + } + if p.prefix("(") { + p.xcomment() + p.fws() + continue + } + p.take(`\`) + if c := p.s[p.o]; c > ' ' && c < 0x7f { + p.o++ + } else { + p.xerrorf("bad character %c in comment", c) + } + p.fws() + } +} + +func (p *parser) prefix(s string) bool { + return strings.HasPrefix(p.lower[p.o:], s) +} + +func (p *parser) xvalue() string { + if p.prefix(`"`) { + return p.xquotedString() + } + return p.xtakefn1("value token", func(c rune, i int) bool { + // ../rfc/2045:661 + // todo: token cannot contain utf-8? not updated in ../rfc/6532. however, we also use it for the localpart & domain parsing, so we'll allow it. + return c > ' ' && !strings.ContainsRune(`()<>@,;:\\"/[]?= `, c) + }) +} + +func (p *parser) xchar() rune { + // We are careful to track invalid utf-8 properly. + if p.empty() { + p.xerrorf("need another character") + } + var r rune + var o int + for i, c := range p.s[p.o:] { + if i > 0 { + o = i + break + } + r = c + } + if o == 0 { + p.o = len(p.s) + } else { + p.o += o + } + return r +} + +func (p *parser) xquotedString() string { + p.xtake(`"`) + var s string + var esc bool + for { + c := p.xchar() + if esc { + if c >= ' ' && c < 0x7f { + s += string(c) + esc = false + continue + } + p.xerrorf("bad escaped char %c in quoted string", c) + } + if c == '\\' { + esc = true + continue + } + if c == '"' { + return s + } + if c >= ' ' && c != '\\' && c != '"' { + s += string(c) + continue + } + p.xerrorf("invalid quoted string, invalid character %c", c) + } +} + +func (p *parser) digits() string { + o := p.o + for o < len(p.s) && p.s[o] >= '0' && p.s[o] <= '9' { + o++ + } + p.o = o + return p.s[o:p.o] +} + +func (p *parser) xdigits() string { + s := p.digits() + if s == "" { + p.xerrorf("expected digits, remaining %q", p.s[p.o:]) + } + return s +} + +func (p *parser) xtake(s string) { + if !p.prefix(s) { + p.xerrorf("expected %q, remaining %q", s, p.s[p.o:]) + } + p.o += len(s) +} + +func (p *parser) empty() bool { + return p.o >= len(p.s) +} + +func (p *parser) take(s string) bool { + if p.prefix(s) { + p.o += len(s) + return true + } + return false +} + +func (p *parser) xtakefn1(what string, fn func(c rune, i int) bool) string { + if p.empty() { + p.xerrorf("need at least one char for %s", what) + } + for i, c := range p.s[p.o:] { + if !fn(c, i) { + if i == 0 { + p.xerrorf("expected at least one char for %s, remaining %q", what, p.s[p.o:]) + } + s := p.s[p.o : p.o+i] + p.o += i + return s + } + } + s := p.s[p.o:] + p.o = len(p.s) + return s +} + +// ../rfc/5321:2287 +func (p *parser) xkeyword(isResult bool) string { + s := strings.ToLower(p.xtakefn1("keyword", func(c rune, i int) bool { + // Yahoo sends results like "dkim=perm_fail". + return c >= 'a' && c <= 'z' || c >= '0' && c <= '9' || c == '-' || isResult && !Pedantic && c == '_' + })) + if s == "-" { + p.xerrorf("missing keyword") + } else if strings.HasSuffix(s, "-") { + p.o-- + s = s[:len(s)-1] + } + return s +} + +func (p *parser) xmethodspec(methodKeyword string) (string, string, string) { + p.cfws() + var methodDigits string + if p.take("/") { + methodDigits = p.xdigits() + p.cfws() + } + p.xtake("=") + p.cfws() + result := p.xkeyword(true) + return methodKeyword, methodDigits, result +} + +func (p *parser) xpropspec() (ap AuthProp) { + ap.Type = p.xkeyword(false) + p.cfws() + p.xtake(".") + p.cfws() + if p.take("mailfrom") { + ap.Property = "mailfrom" + } else if p.take("rcptto") { + ap.Property = "rcptto" + } else { + ap.Property = p.xkeyword(false) + } + p.cfws() + p.xtake("=") + ap.IsAddrLike, ap.Value = p.xpvalue() + return +} + +// method keyword has been parsed, method-version not yet. +func (p *parser) xresinfo(methodKeyword string) (am AuthMethod) { + p.cfws() + am.Method, am.Version, am.Result = p.xmethodspec(methodKeyword) + p.cfws() + if p.take("reason") { + p.cfws() + am.Reason = p.xvalue() + } + p.cfws() + for !p.prefix(";") && !p.end() { + am.Props = append(am.Props, p.xpropspec()) + p.cfws() + } + return +} + +// todo: could keep track whether this is a localpart. +func (p *parser) xpvalue() (bool, string) { + p.cfws() + if p.take("@") { + // Bare domain. + dom, _ := p.xdomain() + return true, "@" + dom + } + s := p.xvalue() + if p.take("@") { + dom, _ := p.xdomain() + s += "@" + dom + return true, s + } + return false, s +} + +// ../rfc/5321:2291 +func (p *parser) xdomain() (string, dns.Domain) { + s := p.xsubdomain() + for p.take(".") { + s += "." + p.xsubdomain() + } + d, err := dns.ParseDomain(s) + if err != nil { + p.xerrorf("parsing domain name %q: %s", s, err) + } + if len(s) > 255 { + // ../rfc/5321:3491 + p.xerrorf("domain longer than 255 octets") + } + return s, d +} + +// ../rfc/5321:2303 +// ../rfc/5321:2303 ../rfc/6531:411 +func (p *parser) xsubdomain() string { + return p.xtakefn1("subdomain", func(c rune, i int) bool { + return c >= '0' && c <= '9' || c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || i > 0 && c == '-' || c > 0x7f + }) +} diff --git a/message/authresults_test.go b/message/authresults_test.go index 012be0b..ecab540 100644 --- a/message/authresults_test.go +++ b/message/authresults_test.go @@ -6,7 +6,7 @@ import ( "github.com/mjl-/mox/dns" ) -func TestAuthResults(t *testing.T) { +func TestAuthResultsPack(t *testing.T) { dom, err := dns.ParseDomain("møx.example") if err != nil { t.Fatalf("parsing domain: %v", err) @@ -15,7 +15,7 @@ func TestAuthResults(t *testing.T) { Hostname: dom.XName(true), Comment: dom.ASCIIExtra(true), Methods: []AuthMethod{ - {"dkim", "pass", "", "", []AuthProp{{"header", "d", dom.XName(true), true, dom.ASCIIExtra(true)}}}, + {"dkim", "", "pass", "", "", []AuthProp{{"header", "d", dom.XName(true), true, dom.ASCIIExtra(true)}}}, }, } s := authRes.Header() @@ -24,3 +24,302 @@ func TestAuthResults(t *testing.T) { t.Fatalf("got %q, expected %q", s, exp) } } + +func TestAuthResultsParse(t *testing.T) { + ar, err := ParseAuthResults("(xn--mx-lka.example) møx.example;\r\n\tdkim=pass header.d=møx.example (xn--mx-lka.example)\r\n") + tcheck(t, err, "parsing auth results header") + tcompare(t, ar, AuthResults{ + Hostname: "møx.example", + Methods: []AuthMethod{ + { + Method: "dkim", + Result: "pass", + Props: []AuthProp{ + {Type: "header", Property: "d", Value: "møx.example"}, + }, + }, + }, + }) + + const localhost = `localhost; + auth=pass smtp.mailfrom=mox+qvpVtG6ZQg-vJmN_beaGyQ@localhost +` + ar, err = ParseAuthResults(localhost) + tcheck(t, err, "parsing auth results header") + tcompare(t, ar, AuthResults{ + Hostname: "localhost", + Methods: []AuthMethod{ + { + Method: "auth", + Result: "pass", + Props: []AuthProp{ + {Type: "smtp", Property: "mailfrom", IsAddrLike: true, Value: "mox+qvpVtG6ZQg-vJmN_beaGyQ@localhost"}, + }, + }, + }, + }) + + const other = `komijn.test.xmox.nl; + iprev=pass (without dnssec) policy.iprev=198.2.145.102; + dkim=pass (2048 bit rsa, without dnssec) header.d=mandrillapp.com + header.s=mte1 header.a=rsa-sha256 header.b="CfNW8cht1/v3"; + dkim=pass (2048 bit rsa, without dnssec) header.d=letsencrypt.org + header.s=mte1 header.a=rsa-sha256 header.b=F9lCi4OC77su + header.i=expiry@letsencrypt.org; + spf=pass (without dnssec) smtp.mailfrom=delivery.letsencrypt.org; + dmarc=pass (without dnssec) header.from=letsencrypt.org +` + + ar, err = ParseAuthResults(other) + tcheck(t, err, "parsing auth results header") + tcompare(t, ar, AuthResults{ + Hostname: "komijn.test.xmox.nl", + Methods: []AuthMethod{ + { + Method: "iprev", + Result: "pass", + Props: []AuthProp{ + {Type: "policy", Property: "iprev", Value: "198.2.145.102"}, + }, + }, + { + Method: "dkim", + Result: "pass", + Props: []AuthProp{ + {Type: "header", Property: "d", Value: "mandrillapp.com"}, + {Type: "header", Property: "s", Value: "mte1"}, + {Type: "header", Property: "a", Value: "rsa-sha256"}, + {Type: "header", Property: "b", Value: "CfNW8cht1/v3"}, + }, + }, + { + Method: "dkim", + Result: "pass", + Props: []AuthProp{ + {Type: "header", Property: "d", Value: "letsencrypt.org"}, + {Type: "header", Property: "s", Value: "mte1"}, + {Type: "header", Property: "a", Value: "rsa-sha256"}, + {Type: "header", Property: "b", Value: "F9lCi4OC77su"}, + {Type: "header", Property: "i", IsAddrLike: true, Value: "expiry@letsencrypt.org"}, + }, + }, + { + Method: "spf", + Result: "pass", + Props: []AuthProp{ + {Type: "smtp", Property: "mailfrom", Value: "delivery.letsencrypt.org"}, + }, + }, + { + Method: "dmarc", + Result: "pass", + Props: []AuthProp{ + {Type: "header", Property: "from", Value: "letsencrypt.org"}, + }, + }, + }, + }) + + const google = `mx.google.com; + dkim=pass header.i=@test.xmox.nl header.s=2022b header.b="Z9k/yZIA"; + spf=pass (google.com: domain of mjl@test.xmox.nl designates 2a02:2770::21a:4aff:feba:bde0 as permitted sender) smtp.mailfrom=mjl@test.xmox.nl; + dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=test.xmox.nl +` + + ar, err = ParseAuthResults(google) + tcheck(t, err, "parsing auth results header") + tcompare(t, ar, AuthResults{ + Hostname: "mx.google.com", + Methods: []AuthMethod{ + { + Method: "dkim", + Result: "pass", + Props: []AuthProp{ + {Type: "header", Property: "i", IsAddrLike: true, Value: "@test.xmox.nl"}, + {Type: "header", Property: "s", Value: "2022b"}, + {Type: "header", Property: "b", Value: "Z9k/yZIA"}, + }, + }, + { + Method: "spf", + Result: "pass", + Props: []AuthProp{ + {Type: "smtp", Property: "mailfrom", IsAddrLike: true, Value: "mjl@test.xmox.nl"}, + }, + }, + { + Method: "dmarc", + Result: "pass", + Props: []AuthProp{ + {Type: "header", Property: "from", Value: "test.xmox.nl"}, + }, + }, + }, + }) + + const yahoo = `atlas220.free.mail.bf1.yahoo.com; + dkim=perm_fail header.i=@ueber.net header.s=2023a; + dkim=pass header.i=@ueber.net header.s=2023b; + spf=pass smtp.mailfrom=ueber.net; + dmarc=pass(p=REJECT) header.from=ueber.net; +` + ar, err = ParseAuthResults(yahoo) + tcheck(t, err, "parsing auth results header") + tcompare(t, ar, AuthResults{ + Hostname: "atlas220.free.mail.bf1.yahoo.com", + Methods: []AuthMethod{ + { + Method: "dkim", + Result: "perm_fail", + Props: []AuthProp{ + {Type: "header", Property: "i", IsAddrLike: true, Value: "@ueber.net"}, + {Type: "header", Property: "s", Value: "2023a"}, + }, + }, + { + Method: "dkim", + Result: "pass", + Props: []AuthProp{ + {Type: "header", Property: "i", IsAddrLike: true, Value: "@ueber.net"}, + {Type: "header", Property: "s", Value: "2023b"}, + }, + }, + { + Method: "spf", + Result: "pass", + Props: []AuthProp{ + {Type: "smtp", Property: "mailfrom", Value: "ueber.net"}, + }, + }, + { + Method: "dmarc", + Result: "pass", + Props: []AuthProp{ + {Type: "header", Property: "from", Value: "ueber.net"}, + }, + }, + }, + }) + + const proton0 = `mail.protonmail.ch; dkim=pass (Good + ed25519-sha256 signature) header.d=ueber.net header.i=mechiel@ueber.net + header.a=ed25519-sha256; dkim=pass (Good 2048 bit rsa-sha256 signature) + header.d=ueber.net header.i=mechiel@ueber.net header.a=rsa-sha256 +` + ar, err = ParseAuthResults(proton0) + tcheck(t, err, "parsing auth results header") + tcompare(t, ar, AuthResults{ + Hostname: "mail.protonmail.ch", + Methods: []AuthMethod{ + { + Method: "dkim", + Result: "pass", + Props: []AuthProp{ + {Type: "header", Property: "d", Value: "ueber.net"}, + {Type: "header", Property: "i", IsAddrLike: true, Value: "mechiel@ueber.net"}, + {Type: "header", Property: "a", Value: "ed25519-sha256"}, + }, + }, + { + Method: "dkim", + Result: "pass", + Props: []AuthProp{ + {Type: "header", Property: "d", Value: "ueber.net"}, + {Type: "header", Property: "i", IsAddrLike: true, Value: "mechiel@ueber.net"}, + {Type: "header", Property: "a", Value: "rsa-sha256"}, + }, + }, + }, + }) + + const proton1 = `mail.protonmail.ch; dmarc=pass (p=reject dis=none) + header.from=ueber.net +` + ar, err = ParseAuthResults(proton1) + tcheck(t, err, "parsing auth results header") + tcompare(t, ar, AuthResults{ + Hostname: "mail.protonmail.ch", + Methods: []AuthMethod{ + { + Method: "dmarc", + Result: "pass", + Props: []AuthProp{ + {Type: "header", Property: "from", Value: "ueber.net"}, + }, + }, + }, + }) + const proton2 = `mail.protonmail.ch; spf=pass smtp.mailfrom=ueber.net +` + ar, err = ParseAuthResults(proton2) + tcheck(t, err, "parsing auth results header") + tcompare(t, ar, AuthResults{ + Hostname: "mail.protonmail.ch", + Methods: []AuthMethod{ + { + Method: "spf", + Result: "pass", + Props: []AuthProp{ + {Type: "smtp", Property: "mailfrom", Value: "ueber.net"}, + }, + }, + }, + }) + const proton3 = `mail.protonmail.ch; arc=none smtp.remote-ip=84.22.96.237 +` + ar, err = ParseAuthResults(proton3) + tcheck(t, err, "parsing auth results header") + tcompare(t, ar, AuthResults{ + Hostname: "mail.protonmail.ch", + Methods: []AuthMethod{ + { + Method: "arc", + Result: "none", + Props: []AuthProp{ + {Type: "smtp", Property: "remote-ip", Value: "84.22.96.237"}, + }, + }, + }, + }) + const proton4 = `mail.protonmail.ch; dkim=permerror (0-bit key) header.d=ueber.net + header.i=mechiel@ueber.net header.b="a4SMWyJ7"; dkim=pass (2048-bit key) + header.d=ueber.net header.i=mechiel@ueber.net header.b="mQickWQ7" +` + ar, err = ParseAuthResults(proton4) + tcheck(t, err, "parsing auth results header") + tcompare(t, ar, AuthResults{ + Hostname: "mail.protonmail.ch", + Methods: []AuthMethod{ + { + Method: "dkim", + Result: "permerror", + Props: []AuthProp{ + {Type: "header", Property: "d", Value: "ueber.net"}, + {Type: "header", Property: "i", IsAddrLike: true, Value: "mechiel@ueber.net"}, + {Type: "header", Property: "b", Value: "a4SMWyJ7"}, + }, + }, + { + Method: "dkim", + Result: "pass", + Props: []AuthProp{ + {Type: "header", Property: "d", Value: "ueber.net"}, + {Type: "header", Property: "i", IsAddrLike: true, Value: "mechiel@ueber.net"}, + {Type: "header", Property: "b", Value: "mQickWQ7"}, + }, + }, + }, + }) + + // Outlook adds an invalid line, missing required hostname at the start. And their + // dmarc "action=none" is invalid. Nothing to be done. + const outlook = `x; spf=pass (sender IP is 84.22.96.237) + smtp.mailfrom=ueber.net; dkim=pass (signature was verified) + header.d=ueber.net;dmarc=pass action=none header.from=ueber.net;compauth=pass + reason=100 +` + _, err = ParseAuthResults(outlook) + if err == nil { + t.Fatalf("missing error while parsing authresults header from outlook") + } +} diff --git a/message/part_test.go b/message/part_test.go index 4bf1334..6827377 100644 --- a/message/part_test.go +++ b/message/part_test.go @@ -26,7 +26,7 @@ func tcheck(t *testing.T, err error, msg string) { func tcompare(t *testing.T, got, exp any) { t.Helper() if !reflect.DeepEqual(got, exp) { - t.Fatalf("got %q, expected %q", got, exp) + t.Fatalf("got %v, expected %v", got, exp) } } diff --git a/smtpserver/parse.go b/smtpserver/parse.go index 9a6f91d..334de52 100644 --- a/smtpserver/parse.go +++ b/smtpserver/parse.go @@ -261,7 +261,6 @@ func (p *parser) xdomain() dns.Domain { return d } -// ../rfc/5321:2303 // ../rfc/5321:2303 ../rfc/6531:411 func (p *parser) xsubdomain() string { return p.xtakefn1("subdomain", func(c rune, i int) bool { @@ -278,9 +277,16 @@ func (p *parser) xmailbox() smtp.Path { // ../rfc/5321:2307 func (p *parser) xldhstr() string { - return p.xtakefn1("ldh-str", func(c rune, i int) bool { - return c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || i == 0 && c == '-' + s := p.xtakefn1("ldh-str", func(c rune, i int) bool { + return c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '-' }) + if s == "-" { + p.xerrorf("empty ldh-str") + } else if strings.HasSuffix(s, "-") { + p.o-- + s = s[:len(s)-1] + } + return s } // parse address-literal or domain.