Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

authres: parser rework #53

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
218 changes: 180 additions & 38 deletions authres/parse.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package authres

import (
"bufio"
"errors"
"io"
"strings"
"unicode"
)
Expand Down Expand Up @@ -222,58 +224,94 @@ var results = map[string]newResultFunc{
// Parse parses the provided Authentication-Results header field. It returns the
// authentication service identifier and authentication results.
func Parse(v string) (identifier string, results []Result, err error) {
parts := strings.Split(v, ";")
p := newParser(v)

identifier = strings.TrimSpace(parts[0])
i := strings.IndexFunc(identifier, unicode.IsSpace)
if i > 0 {
version := strings.TrimSpace(identifier[i:])
if version != "1" {
return "", nil, errors.New("msgauth: unsupported version")
}
identifier, err = p.getIndentifier()
if err != nil {
return identifier, nil, err
}

identifier = identifier[:i]
for {
result, err := p.getResult()
if result == nil {
break
}
results = append(results, result)
if err == io.EOF {
break
} else if err != nil {
return identifier, results, err
}
}

for i := 1; i < len(parts); i++ {
s := strings.TrimSpace(parts[i])
if s == "" {
return identifier, results, nil
}

type parser struct {
r *bufio.Reader
}

func newParser(v string) *parser {
return &parser{r: bufio.NewReader(strings.NewReader(v))}
}

// getIdentifier parses the authserv-id of the authres header and checks the
// version id when present. Ignore header comments in parenthesis.
func (p *parser) getIndentifier() (identifier string, err error) {
for {
c, err := p.r.ReadByte()
if err == io.EOF {
return identifier, nil
} else if err != nil {
return identifier, err
}
if c == '(' {
p.r.UnreadByte()
p.readComment()
continue
}

result, err := parseResult(s)
if err != nil {
return identifier, results, err
if c == ';' {
break
}
if result != nil {
results = append(results, result)
identifier += string(c)
}

fields := strings.Fields(identifier)
if len(fields) > 1 {
version := strings.TrimSpace(fields[1])
if version != "1" {
return "", errors.New("msgauth: unknown version")
}
} else if len(fields) == 0 {
return "", errors.New("msgauth: no identifier found")
}
return
return strings.TrimSpace(fields[0]), nil
}

func parseResult(s string) (Result, error) {
// TODO: ignore header comments in parenthesis

parts := strings.Fields(s)
if len(parts) == 0 || parts[0] == "none" {
// getResults parses the authentication part of the authres header and returns
// a Result struct. Ignore header comments in parenthesis.
func (p *parser) getResult() (result Result, err error) {
method, resultvalue, err := p.keyValue()
if method == "none" {
return nil, nil
}

k, v, err := parseParam(parts[0])
if err != nil {
return nil, err
}
method, value := k, ResultValue(strings.ToLower(v))
value := ResultValue(strings.ToLower(resultvalue))

params := make(map[string]string)
for i := 1; i < len(parts); i++ {
k, v, err := parseParam(parts[i])
if err != nil {
continue
var k, v string
for {
k, v, err = p.keyValue()
if k != "" {
params[k] = v
}
if err == io.EOF {
break
} else if err != nil {
return nil, err
}

params[k] = v
}

newResult, ok := results[method]
Expand All @@ -293,10 +331,114 @@ func parseResult(s string) (Result, error) {
return r, nil
}

func parseParam(s string) (k string, v string, err error) {
kv := strings.SplitN(s, "=", 2)
if len(kv) != 2 {
return "", "", errors.New("msgauth: malformed authentication method and value")
// keyValue parses a sequence of key=value parameters
func (p *parser) keyValue() (k, v string, err error) {
k, err = p.readKey()
if err != nil {
return
}
v, err = p.readValue()
if err != nil {
return
}
return strings.ToLower(strings.TrimSpace(kv[0])), strings.TrimSpace(kv[1]), nil
return
}

// readKey reads a method, reason or ptype.property as defined in RFC 8601
// Section 2.2. Ignore the method-version of the methodspec. Stop at EOF or the
// equal sign.
func (p *parser) readKey() (k string, err error) {
var c byte
for err != io.EOF {
c, err = p.r.ReadByte()
if err != nil {
break
}
switch c {
case ';':
err = io.EOF
break
case '=':
break
case '(':
p.r.UnreadByte()
_, err = p.readComment()
continue
case '/':
p.r.ReadBytes('=')
p.r.UnreadByte()
default:
if !unicode.IsSpace(rune(c)) {
k += string(c)
}
}
if c == '=' {
break
}
}
k = strings.TrimSpace(strings.ToLower(k))
return
}

// readValue reads a result or value as defined in RFC 8601 Section 2.2. Value
// is defined as either a token or quoted string according to RFC 2045 Section
// 5.1. Stop at EOF, white space or semi-colons.
func (p *parser) readValue() (v string, err error) {
var c byte
for err != io.EOF {
c, err = p.r.ReadByte()
if err != nil {
break
}
switch c {
case ';':
err = io.EOF
break
case '(':
p.r.UnreadByte()
_, err = p.readComment()
continue
case '"':
v, err = p.r.ReadString(c)
v = strings.TrimSuffix(v, string(c))
default:
if !unicode.IsSpace(rune(c)) {
v += string(c)
}
}
if unicode.IsSpace(rune(c)) {
if v != "" {
break
}
}
}
v = strings.TrimSpace(v)
return
}

func (p *parser) readComment() (comment string, err error) {
count := 0
var c byte
for {
c, err = p.r.ReadByte()
if err != nil {
break
}
switch c {
case '\\':
c, _ = p.r.ReadByte()
comment += "\\" + string(c)
case '(':
Copy link

@yogo1212 yogo1212 Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the CFWS referenced by https://datatracker.ietf.org/doc/html/rfc8601#section-2.2 is defined in https://www.rfc-editor.org/rfc/rfc5322#section-3.2.2 (internet message / email):

FWS             =   ([*WSP CRLF] 1*WSP) /  obs-FWS
                                       ; Folding white space

ctext           =   %d33-39 /          ; Printable US-ASCII
                    %d42-91 /          ;  characters not including
                    %d93-126 /         ;  "(", ")", or "\"
                    obs-ctext

ccontent        =   ctext / quoted-pair / comment

comment         =   "(" *([FWS] ccontent) [FWS] ")"

CFWS            =   (1*([FWS] comment) [FWS]) / FWS

this code is not entirely future-proof, though right now, i welcome everything that helps me handle my emails..
even net/mail doesn't go the whole way:

The full range of spacing (the CFWS syntax element) is not supported, such as breaking addresses across lines.

how about including the parentheses in the output string (doing comment += string(c) for both ( and )) and leaving parsing of comments up to other or future implementations?
in that case, the backslashes should also be kept.

i can think of invalid comments that would be accepted by this parser. particularly, the restrictions for ctext.
the implementation before this pr was more restrictive than the grammar. this replacement is more liberal, which could be understood as a changing interface.

count++
case ')':
count--
default:
comment += string(c)
}
if count == 0 {
break
}
}
comment = strings.TrimSpace(comment)
return
}
79 changes: 79 additions & 0 deletions authres/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,22 @@ var parseTests = []msgauthTest{
&SPFResult{Value: ResultPass, From: "example.net"},
},
},
{
value: "example.com;" +
"dkim=pass reason=\"good signature\" [email protected];",
identifier: "example.com",
results: []Result{
&DKIMResult{Value: ResultPass, Reason: "good signature", Identifier: "@mail-router.example.net"},
},
},
{
value: "example.com;" +
"dkim=pass reason=\"good; signature\" [email protected];",
identifier: "example.com",
results: []Result{
&DKIMResult{Value: ResultPass, Reason: "good; signature", Identifier: "@mail-router.example.net"},
},
},
{
value: "example.com;" +
" auth=pass (cram-md5) [email protected];",
Expand All @@ -32,6 +48,63 @@ var parseTests = []msgauthTest{
&AuthResult{Value: ResultPass, Auth: "[email protected]"},
},
},
{
value: "example.com;" +
" auth=pass (cram-md5) [email protected];" +
" spf=pass smtp.mailfrom=example.net",
identifier: "example.com",
results: []Result{
&AuthResult{Value: ResultPass, Auth: "[email protected]"},
&SPFResult{Value: ResultPass, From: "example.net"},
},
},
{
value: "example.com;" +
" auth=pass (cram-md5 (comment inside comment)) [email protected];",
identifier: "example.com",
results: []Result{
&AuthResult{Value: ResultPass, Auth: "[email protected]"},
},
},
{
value: "example.com;" +
" auth=pass (cram-md5; comment with semicolon) [email protected];",
identifier: "example.com",
results: []Result{
&AuthResult{Value: ResultPass, Auth: "[email protected]"},
},
},
{
value: "example.com;" +
" auth=pass (cram-md5 \\( comment with escaped char) [email protected];",
identifier: "example.com",
results: []Result{
&AuthResult{Value: ResultPass, Auth: "[email protected]"},
},
},
{
value: "foo.example.net (foobar) 1 (baz);" +
" dkim (Because I like it) / 1 (One yay) = (wait for it) fail" +
" policy (A dot can go here) . (like that) expired" +
" (this surprised me) = (as I wasn't expecting it) 1362471462",
identifier: "foo.example.net",
results: []Result{
&DKIMResult{Value: ResultFail, Reason: "", Domain: "", Identifier: ""},
},
},
}

var mustFailParseTests = []msgauthTest{
{
value: " ; ",
identifier: "",
results: nil,
},
{
value: "example.com 2; none",
identifier: "example.com",
results: nil,
},
}

func TestParse(t *testing.T) {
Expand All @@ -51,4 +124,10 @@ func TestParse(t *testing.T) {
}
}
}
for _, test := range mustFailParseTests {
_, _, err := Parse(test.value)
if err == nil {
t.Errorf("Expected an error when parsing header, but got none.")
}
}
}