krz/gitbay

A CLI-first git forge. cli forge git self-hosted https://gitbay.org

Commit 85af941e0a

85af941e0aecf7f05b353c891605b8a3c705fd06

parent: 9a44de69f1

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-29 06:20 UTC 

mailin: parse Authentication-Results per RFC 8601; DMARC relaxed alignment; fetch tries only for refusals

The header is tokenized with quoted strings and nested comments, backslash
escapes included, before it is split into results and properties, so
sender text the mail host echoes (a quoted MAIL FROM local part, a
reason, a comment) cannot read as dmarc=pass. Method, result and domain
values must be plain atoms. DKIM alignment compares organizational
domains by the public suffix list and requires header.d. A fetch counts
a try only when the server refuses that message (imapc.RefusedError);
a connection error or timeout ends the poll without counting tries.

Ref #295

Layout: unified · split

internal/imapc/imapc.go +9 −2
@@ -35,6 +35,13 @@ const (
35// poll. 35// poll.
36const MaxUnseen = 10000 36const MaxUnseen = 10000
37 37
38// RefusedError is the server answering a command NO or BAD, or
39// answering a FETCH with no message: a refusal of that command, with the
40// session still usable.
41type RefusedError struct{ Text string }
42
43func (e *RefusedError) Error() string { return e.Text }
44
38var ( 45var (
39 ErrTooLarge = errors.New("message larger than the fetch limit") 46 ErrTooLarge = errors.New("message larger than the fetch limit")
40 ErrLimit = errors.New("IMAP server exceeded a response limit; connection closed") 47 ErrLimit = errors.New("IMAP server exceeded a response limit; connection closed")
@@ -220,7 +227,7 @@ func (c *Client) Fetch(uid uint32) ([]byte, error) {
220 return []byte{}, nil 227 return []byte{}, nil
221 } 228 }
222 } 229 }
223 return nil, fmt.Errorf("UID FETCH %d: no message body in the response", uid) 230 return nil, &RefusedError{fmt.Sprintf("UID FETCH %d: no message body in the response", uid)}
224} 231}
225 232
226// MarkSeen sets \Seen. 233// MarkSeen sets \Seen.
@@ -264,7 +271,7 @@ func (c *Client) cmd(command string) ([]response, error) {
264 if strings.EqualFold(status, "OK") { 271 if strings.EqualFold(status, "OK") {
265 return untagged, nil 272 return untagged, nil
266 } 273 }
267 return nil, fmt.Errorf("%s", clip(rest)) 274 return nil, &RefusedError{clip(rest)}
268 } 275 }
269 if strings.HasPrefix(line, "* BYE") && command != "LOGOUT" { 276 if strings.HasPrefix(line, "* BYE") && command != "LOGOUT" {
270 return nil, fmt.Errorf("server closed the session: %s", clip(line)) 277 return nil, fmt.Errorf("server closed the session: %s", clip(line))
internal/imapc/imapc_test.go +20
@@ -331,3 +331,23 @@ func TestEmptyBody(t *testing.T) {
331 } 331 }
332 } 332 }
333} 333}
334
335// A NO to one FETCH is a RefusedError; the session goes on.
336func TestFetchRefused(t *testing.T) {
337 f := &fakeServer{raw: func(conn net.Conn, tag, cmd string) bool {
338 if !strings.HasPrefix(cmd, "UID FETCH 1 ") {
339 return false
340 }
341 fmt.Fprintf(conn, "%s NO [UNAVAILABLE] try later\r\n", tag)
342 return true
343 }}
344 c := session(t, f)
345 f.msgs[1], f.msgs[2] = "a", "b"
346 var re *RefusedError
347 if _, err := c.Fetch(1); !errors.As(err, &re) {
348 t.Fatalf("Fetch = %v, want a RefusedError", err)
349 }
350 if b, err := c.Fetch(2); err != nil || string(b) != "b" {
351 t.Fatalf("next Fetch = %q, %v", b, err)
352 }
353}
internal/mailin/authres.go +158 −57
@@ -3,6 +3,8 @@ package mailin
3import ( 3import (
4 "net/mail" 4 "net/mail"
5 "strings" 5 "strings"
6
7 "golang.org/x/net/publicsuffix"
6) 8)
7 9
8// authenticated checks the sender against the mail host's own verdict: 10// authenticated checks the sender against the mail host's own verdict:
@@ -10,35 +12,31 @@ import (
10// is authserv. The mail host adds its header above any the message 12// is authserv. The mail host adds its header above any the message
11// arrived with, so a lower header claiming the same id is the sender's 13// arrived with, so a lower header claiming the same id is the sender's
12// and is not read. It returns "" when the header shows dmarc=pass for 14// and is not read. It returns "" when the header shows dmarc=pass for
13// the From domain, or dkim=pass with a signing domain aligned with it, 15// the From domain, or dkim=pass with a header.d aligned with it, and the
14// and the refusal's reason otherwise. 16// refusal's reason otherwise.
15func authenticated(h mail.Header, authserv, from string) string { 17func authenticated(h mail.Header, authserv, from string) string {
16 _, fromDomain, ok := strings.Cut(strings.ToLower(from), "@") 18 _, fromDomain, ok := strings.Cut(strings.ToLower(from), "@")
17 if !ok || fromDomain == "" { 19 if !ok || fromDomain == "" {
18 return "no From domain" 20 return "no From domain"
19 } 21 }
20 for _, v := range h["Authentication-Results"] { 22 for _, v := range h["Authentication-Results"] {
21 parts := strings.Split(stripComments(v), ";") 23 segs := splitResults(tokenize(v))
22 f := strings.Fields(parts[0]) 24 if len(segs) == 0 || len(segs[0]) == 0 || segs[0][0].kind != tokAtom ||
23 if len(f) == 0 || !strings.EqualFold(f[0], authserv) { 25 !strings.EqualFold(segs[0][0].text, authserv) {
24 continue 26 continue
25 } 27 }
26 for _, r := range parts[1:] { 28 for _, seg := range segs[1:] {
27 method, result, props := resinfo(r) 29 method, result, props, ok := resinfo(seg)
28 if result != "pass" { 30 if !ok || result != "pass" {
29 continue 31 continue
30 } 32 }
31 switch method { 33 switch method {
32 case "dmarc": 34 case "dmarc":
33 if strings.EqualFold(props["header.from"], fromDomain) { 35 if props["header.from"] == fromDomain {
34 return "" 36 return ""
35 } 37 }
36 case "dkim": 38 case "dkim":
37 d := props["header.d"] 39 if aligned(props["header.d"], fromDomain) {
38 if d == "" {
39 _, d, _ = strings.Cut(props["header.i"], "@")
40 }
41 if aligned(strings.ToLower(d), fromDomain) {
42 return "" 40 return ""
43 } 41 }
44 } 42 }
@@ -48,58 +46,161 @@ func authenticated(h mail.Header, authserv, from string) string {
48 return "no Authentication-Results from " + authserv 46 return "no Authentication-Results from " + authserv
49} 47}
50 48
51// resinfo splits "method[/version]=result prop=value ..." into its 49type tokKind int
52// method, result and properties, all lower case. 50
53func resinfo(s string) (string, string, map[string]string) { 51const (
54 props := map[string]string{} 52 tokAtom tokKind = iota
55 f := strings.Fields(s) 53 tokQuoted
56 if len(f) == 0 { 54 tokEquals
57 return "", "", props 55 tokSemi
56)
57
58type token struct {
59 kind tokKind
60 text string // an atom's text, or a quoted string's content unescaped
61 // joined marks a token with no whitespace or comment before it, so
62 // "x"@example.org is one value.
63 joined bool
64}
65
66// tokenize splits a header value into atoms, quoted strings, "=" and
67// ";". Comments, nested or not, and whitespace separate tokens and are
68// dropped; backslash escapes are honoured in both comments and quoted
69// strings, so nothing inside either can end it early. An unterminated
70// quoted string or comment runs to the end of the value.
71func tokenize(s string) []token {
72 var out []token
73 joined := false
74 emit := func(t token) {
75 t.joined = joined
76 out = append(out, t)
77 joined = true
58 } 78 }
59 method, result, _ := strings.Cut(strings.ToLower(f[0]), "=") 79 for i := 0; i < len(s); {
60 method, _, _ = strings.Cut(method, "/") 80 c := s[i]
61 for _, kv := range f[1:] { 81 switch {
62 k, v, ok := strings.Cut(kv, "=") 82 case c == ' ' || c == '\t' || c == '\r' || c == '\n':
63 if ok { 83 joined = false
64 props[strings.ToLower(k)] = strings.ToLower(strings.Trim(v, `"`)) 84 i++
85 case c == '(':
86 depth := 0
87 for ; i < len(s); i++ {
88 if s[i] == '\\' {
89 i++
90 continue
91 }
92 if s[i] == '(' {
93 depth++
94 } else if s[i] == ')' {
95 depth--
96 if depth == 0 {
97 i++
98 break
99 }
100 }
101 }
102 joined = false
103 case c == '"':
104 var b strings.Builder
105 i++
106 for i < len(s) && s[i] != '"' {
107 if s[i] == '\\' && i+1 < len(s) {
108 i++
109 }
110 b.WriteByte(s[i])
111 i++
112 }
113 i++ // the closing quote
114 emit(token{kind: tokQuoted, text: b.String()})
115 case c == '=':
116 emit(token{kind: tokEquals})
117 i++
118 case c == ';':
119 emit(token{kind: tokSemi})
120 i++
121 default:
122 j := i
123 for j < len(s) && !strings.ContainsRune(" \t\r\n()\";=\\", rune(s[j])) {
124 j++
125 }
126 if j == i { // a stray backslash
127 j++
128 }
129 emit(token{kind: tokAtom, text: s[i:j]})
130 i = j
65 } 131 }
66 } 132 }
67 return method, result, props 133 return out
68} 134}
69 135
70// aligned reports relaxed alignment, kept simple: the signing domain is 136// splitResults splits tokens at each ";".
71// the From domain, or one is a subdomain of the other. 137func splitResults(ts []token) [][]token {
72func aligned(d, from string) bool { 138 segs := [][]token{nil}
73 if !strings.Contains(d, ".") { 139 for _, t := range ts {
74 return false 140 if t.kind == tokSemi {
141 segs = append(segs, nil)
142 continue
143 }
144 segs[len(segs)-1] = append(segs[len(segs)-1], t)
75 } 145 }
76 return d == from || strings.HasSuffix(from, "."+d) || strings.HasSuffix(d, "."+from) 146 return segs
77} 147}
78 148
79// stripComments removes RFC 5322 comments, "(...)", nested or not, 149// resinfo reads "method[/version]=result" and the "name=value" pairs
80// outside quoted strings. 150// after it. Method, result and each value must be plain atoms; a quoted
81func stripComments(s string) string { 151// value (a reason, or a quoted local part) is kept only as a quoted
82 var b strings.Builder 152// value and never read as a domain. ok is false when the method does
83 depth, quoted := 0, false 153// not parse.
84 for i := 0; i < len(s); i++ { 154func resinfo(ts []token) (method, result string, props map[string]string, ok bool) {
85 c := s[i] 155 props = map[string]string{}
86 switch { 156 if len(ts) < 3 || ts[0].kind != tokAtom || ts[1].kind != tokEquals || ts[2].kind != tokAtom {
87 case c == '\\' && i+1 < len(s): 157 return "", "", props, false
88 if depth == 0 { 158 }
89 b.WriteByte(c) 159 method, _, _ = strings.Cut(strings.ToLower(ts[0].text), "/")
90 b.WriteByte(s[i+1]) 160 result = strings.ToLower(ts[2].text)
91 } 161 i := 3
162 // A value continues through tokens joined to it: "x"@example.org.
163 for i < len(ts) && ts[i].joined && ts[i].kind != tokEquals {
164 i++
165 }
166 for i < len(ts) {
167 if ts[i].kind != tokAtom || i+2 >= len(ts) || ts[i+1].kind != tokEquals {
92 i++ 168 i++
93 case c == '"' && depth == 0: 169 continue
94 quoted = !quoted
95 b.WriteByte(c)
96 case c == '(' && !quoted:
97 depth++
98 case c == ')' && !quoted && depth > 0:
99 depth--
100 case depth == 0:
101 b.WriteByte(c)
102 } 170 }
171 name := strings.ToLower(ts[i].text)
172 v := ts[i+2]
173 j := i + 3
174 plain := v.kind == tokAtom
175 for j < len(ts) && ts[j].joined && ts[j].kind != tokEquals {
176 plain = false
177 j++
178 }
179 // The first value for a name is the one the mail host wrote
180 // beside the result.
181 if _, seen := props[name]; !seen {
182 if plain {
183 props[name] = strings.ToLower(v.text)
184 } else {
185 props[name] = "" // present, but not a plain domain
186 }
187 }
188 i = j
189 }
190 return method, result, props, true
191}
192
193// aligned is DMARC relaxed alignment: the signing domain and the From
194// domain have the same organizational domain (public suffix plus one
195// label). A domain that is itself a public suffix aligns with nothing.
196func aligned(d, from string) bool {
197 if d == "" {
198 return false
199 }
200 od, err := publicsuffix.EffectiveTLDPlusOne(d)
201 if err != nil {
202 return false
103 } 203 }
104 return b.String() 204 of, err := publicsuffix.EffectiveTLDPlusOne(from)
205 return err == nil && od == of
105} 206}
internal/mailin/authres_test.go added +92
@@ -0,0 +1,92 @@
1package mailin
2
3import (
4 "net/mail"
5 "strings"
6 "testing"
7)
8
9func arHeader(values ...string) mail.Header {
10 return mail.Header{"Authentication-Results": values}
11}
12
13func TestAuthenticatedCrafted(t *testing.T) {
14 const id = "mx.example.net"
15 for _, tc := range []struct {
16 name, value, from string
17 pass bool
18 }{
19 {"plain dmarc pass", `mx.example.net; dmarc=pass header.from=victim.example`, "a@victim.example", true},
20 {"quoted local part in smtp.mailfrom",
21 `mx.example.net; spf=pass smtp.mailfrom="x; dmarc=pass header.from=victim.example y"@evil.example; dmarc=pass header.from=evil.example`,
22 "a@victim.example", false},
23 {"quoted reason",
24 `mx.example.net; spf=fail reason="bad; dmarc=pass header.from=victim.example"; dmarc=fail header.from=victim.example`,
25 "a@victim.example", false},
26 {"comment containing a fake result",
27 `mx.example.net; spf=none (sender says; dmarc=pass header.from=victim.example) smtp.mailfrom=evil.example; dmarc=fail header.from=victim.example`,
28 "a@victim.example", false},
29 {"escaped quote inside a quoted string",
30 `mx.example.net; spf=pass smtp.mailfrom="x\"; dmarc=pass header.from=victim.example; \"y"@evil.example`,
31 "a@victim.example", false},
32 {"nested comment with an escaped paren",
33 `mx.example.net; spf=none (a (b\) ; dmarc=pass header.from=victim.example) c); dmarc=fail header.from=victim.example`,
34 "a@victim.example", false},
35 {"quoted header.from value", `mx.example.net; dmarc=pass header.from="victim.example"`, "a@victim.example", false},
36 {"dkim on a public suffix", `mx.example.net; dkim=pass header.d=github.io`, "bob@user.github.io", false},
37 {"dkim relaxed alignment", `mx.example.net; dkim=pass header.d=example.com`, "a@mail.example.com", true},
38 {"dkim unrelated domain", `mx.example.net; dkim=pass header.d=example.org`, "a@example.com", false},
39 {"dkim header.i only", `mx.example.net; dkim=pass header.i=@example.com`, "a@example.com", false},
40 {"property named like a method",
41 `mx.example.net; spf=pass dmarc=pass header.from=victim.example`, "a@victim.example", false},
42 } {
43 t.Run(tc.name, func(t *testing.T) {
44 got := authenticated(arHeader(tc.value), id, tc.from)
45 if (got == "") != tc.pass {
46 t.Fatalf("authenticated = %q, want pass %v", got, tc.pass)
47 }
48 })
49 }
50}
51
52// FuzzAuthResults: no input panics, and a header whose only mention of
53// the victim domain is inside a quoted string or a comment never
54// passes. Prefix and suffix are arbitrary text with the characters that
55// could open or close a quote or comment removed, so the wrapped payload
56// stays wrapped.
57func FuzzAuthResults(f *testing.F) {
58 f.Add("spf=pass smtp.mailfrom=", "@evil.example; dmarc=pass header.from=evil.example", 0, "x; dmarc=pass header.from=victim.example y")
59 f.Add("spf=none ", "; dkim=pass header.d=evil.example", 1, "dmarc=pass header.from=victim.example")
60 f.Add("", "", 2, `a\"; dkim=pass header.d=victim.example; \"b`)
61 strip := strings.NewReplacer(`"`, "", "(", "", ")", "", `\`, "")
62 f.Fuzz(func(t *testing.T, prefix, suffix string, wrap int, payload string) {
63 authenticated(arHeader(prefix+payload+suffix), "mx.example.net", "a@victim.example")
64 prefix, suffix = strip.Replace(prefix), strip.Replace(suffix)
65 if strings.Contains(strings.ToLower(prefix+suffix), "victim") {
66 return
67 }
68 var wrapped string
69 switch wrap % 3 {
70 case 0: // a quoted string, escapes kept balanced
71 wrapped = `"` + strings.NewReplacer(`\`, `\\`, `"`, `\"`).Replace(payload) + `"`
72 case 1: // a comment, parentheses escaped
73 wrapped = "(" + strings.NewReplacer(`\`, `\\`, "(", `\(`, ")", `\)`).Replace(payload) + ")"
74 default: // payload already escaped by the fuzzer, inside quotes
75 for i := 0; i < len(payload); i++ {
76 if payload[i] == '\\' {
77 if i+1 == len(payload) {
78 return // it would escape the closing quote
79 }
80 i++
81 } else if payload[i] == '"' {
82 return // an unescaped quote ends the string early
83 }
84 }
85 wrapped = `"` + payload + `"`
86 }
87 v := "mx.example.net; " + prefix + wrapped + suffix
88 if authenticated(arHeader(v), "mx.example.net", "a@victim.example") == "" {
89 t.Fatalf("passed with the victim domain only inside a quote or comment: %q", v)
90 }
91 })
92}
internal/mailin/mailin.go +10 −3
@@ -98,12 +98,19 @@ func (p *Processor) Drain(mb Mailbox) error {
98 res = refused("message larger than %d bytes", imapc.MaxMessage) 98 res = refused("message larger than %d bytes", imapc.MaxMessage)
99 p.audit(0, "", res.Reason) 99 p.audit(0, "", res.Reason)
100 case errors.Is(err, imapc.ErrLimit): 100 case errors.Is(err, imapc.ErrLimit):
101 // The connection is closed; the message counts a try and 101 // The server sent more than the limits allow for this
102 // the poll ends. 102 // message: it counts a try, and the closed connection ends
103 // the poll.
103 p.tries[uid]++ 104 p.tries[uid]++
104 return err 105 return err
105 case err != nil: 106 case errors.As(err, new(*imapc.RefusedError)):
107 // The server refused this message; the session goes on.
106 res = Result{Retry: true, Reason: "fetch: " + err.Error()} 108 res = Result{Retry: true, Reason: "fetch: " + err.Error()}
109 case err != nil:
110 // A connection failure or a timeout says nothing about this
111 // message or the ones after it: the poll ends, no try is
112 // counted.
113 return err
107 default: 114 default:
108 res = p.Handle(raw) 115 res = p.Handle(raw)
109 } 116 }
internal/mailin/mailin_test.go +29 −3
@@ -3,6 +3,8 @@ package mailin
3import ( 3import (
4 "errors" 4 "errors"
5 "fmt" 5 "fmt"
6 "os"
7 "slices"
6 "strings" 8 "strings"
7 "testing" 9 "testing"
8 "time" 10 "time"
@@ -272,6 +274,7 @@ func (m *fakeMailbox) Unseen() ([]uint32, error) {
272 out = append(out, uid) 274 out = append(out, uid)
273 } 275 }
274 } 276 }
277 slices.Sort(out)
275 return out, nil 278 return out, nil
276} 279}
277 280
@@ -403,7 +406,7 @@ func TestDrainFetchErrors(t *testing.T) {
403 f := setup(t) 406 f := setup(t)
404 mb := &fakeMailbox{seen: map[uint32]bool{}, 407 mb := &fakeMailbox{seen: map[uint32]bool{},
405 msgs: map[uint32][]byte{1: []byte("x"), 2: []byte(f.message(t, "bob@example.test", "hi"))}, 408 msgs: map[uint32][]byte{1: []byte("x"), 2: []byte(f.message(t, "bob@example.test", "hi"))},
406 errs: map[uint32]error{1: errors.New("NO [UNAVAILABLE] try later")}} 409 errs: map[uint32]error{1: &imapc.RefusedError{Text: "NO [UNAVAILABLE] try later"}}}
407 for i := 1; i < maxTries; i++ { 410 for i := 1; i < maxTries; i++ {
408 if err := f.p.Drain(mb); err != nil { 411 if err := f.p.Drain(mb); err != nil {
409 t.Fatal(err) 412 t.Fatal(err)
@@ -445,9 +448,12 @@ func TestAuthenticationResults(t *testing.T) {
445 {"dmarc pass", 448 {"dmarc pass",
446 "Authentication-Results: mx.example.net; spf=pass smtp.mailfrom=example.test; dmarc=pass (p=REJECT) header.from=example.test\r\n", 449 "Authentication-Results: mx.example.net; spf=pass smtp.mailfrom=example.test; dmarc=pass (p=REJECT) header.from=example.test\r\n",
447 "bob@example.test", ""}, 450 "bob@example.test", ""},
448 {"aligned dkim pass, gmail header.i", 451 {"aligned dkim pass",
449 "Authentication-Results: mx.example.net;\r\n dkim=pass header.i=@mail.example.test header.s=s1 header.b=abc\r\n", 452 "Authentication-Results: mx.example.net;\r\n dkim=pass header.d=mail.example.test header.s=s1 header.b=abc\r\n",
450 "bob@example.test", ""}, 453 "bob@example.test", ""},
454 {"dkim header.i without header.d",
455 "Authentication-Results: mx.example.net; dkim=pass header.i=@example.test\r\n",
456 "bob@example.test", "sender not authenticated"},
451 {"dmarc fail", 457 {"dmarc fail",
452 "Authentication-Results: mx.example.net; dkim=fail header.d=example.test; dmarc=fail header.from=example.test\r\n", 458 "Authentication-Results: mx.example.net; dkim=fail header.d=example.test; dmarc=fail header.from=example.test\r\n",
453 "bob@example.test", "sender not authenticated"}, 459 "bob@example.test", "sender not authenticated"},
@@ -484,3 +490,23 @@ func TestAuthenticationResults(t *testing.T) {
484 }) 490 })
485 } 491 }
486} 492}
493
494// A timeout mid-poll ends the poll and counts no try, neither for the
495// message being fetched nor for the ones after it.
496func TestDrainTimeoutCountsNoTries(t *testing.T) {
497 f := setup(t)
498 mb := &fakeMailbox{seen: map[uint32]bool{}, msgs: map[uint32][]byte{
499 1: []byte(f.message(t, "bob@example.test", "first")),
500 2: []byte("x"),
501 3: []byte(f.message(t, "bob@example.test", "third")),
502 }, errs: map[uint32]error{2: fmt.Errorf("read: %w", os.ErrDeadlineExceeded)}}
503 if err := f.p.Drain(mb); !errors.Is(err, os.ErrDeadlineExceeded) {
504 t.Fatalf("Drain = %v", err)
505 }
506 if !mb.seen[1] || mb.seen[2] || mb.seen[3] {
507 t.Fatalf("seen = %v", mb.seen)
508 }
509 if f.p.tries[2] != 0 || f.p.tries[3] != 0 {
510 t.Fatalf("tries = %v", f.p.tries)
511 }
512}