from-header-binding: what the reviews had left as found, and address extensions
test / test (push) Canceled after 0s
test / test (push) Canceled after 0s
The owner on the three things the reviews' commit left as found: can they be fixed? They can. - Downtime that crossed a UTC midnight was all booked on the day of the restart: it is cut at the midnights between (byDay). - A message with the null envelope sender - a bounce an application writes itself - left a refusal row without a domain, which no domain administrator would see: the row takes the domain the login is bound to. - A log read again after an unclean stop counted its deferrals again: while the tailer catches up, a warning no later than the last one counted is passed over. The mark is kept to the fraction of a second the log gives. And on the check being stricter than Postfix for an address with an extension (invoices+x@ passed in the envelope, was refused in the header): it makes Postfix's second try now, the address without its extension. recipient_delimiter was the distribution's default; it is set in postfix-config.sh, since the check depends on it. e2e sends one through. Test server, this tree: go vet ok, go test 23 packages ok, make e2e ok (339 s). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -153,7 +153,7 @@ func Run(ctx context.Context, path string, st StatusStore, retention RetentionDa
|
||||
journal.handle(line, now, catchingUp)
|
||||
}
|
||||
if deferred != nil {
|
||||
deferred.handle(line, now)
|
||||
deferred.handle(line, now, catchingUp)
|
||||
}
|
||||
keeper.add(line, now, catchingUp)
|
||||
queueID, recipient, status, ok := parseDelivery(line)
|
||||
|
||||
@@ -33,9 +33,11 @@ import (
|
||||
// "milter unix:<socket>: can't read … reply packet header" (Postfix's
|
||||
// milter8.c; not seen on the stand).
|
||||
|
||||
// SenderCheckStore is where the count goes. *store.Store satisfies it.
|
||||
// SenderCheckStore is where the count goes, and where the time of the last
|
||||
// counted line is kept. *store.Store satisfies it.
|
||||
type SenderCheckStore interface {
|
||||
AddSenderCheckDeferred(at time.Time, n int64) error
|
||||
SenderCheckDeferredAt() (time.Time, error)
|
||||
}
|
||||
|
||||
// SenderCheck turns the counting on: the store and the socket Postfix was
|
||||
@@ -60,10 +62,22 @@ type deferredCounter struct {
|
||||
// listening, and a reply that did not come.
|
||||
connect, broken string
|
||||
failed map[string]bool // smtpd pid -> its current client was counted already
|
||||
// last is the time of the latest line counted, by this run or an earlier
|
||||
// one. While the tailer catches up after a start it may be reading lines
|
||||
// it had read before an unclean stop; a warning no later than this was
|
||||
// counted then. mail.log's stamps are whole seconds, so a warning of the
|
||||
// very second of the last one counted can be passed over - once, at a
|
||||
// restart - which is the cheaper mistake than counting a stretch twice.
|
||||
last time.Time
|
||||
}
|
||||
|
||||
func newDeferredCounter(sc *SenderCheck) *deferredCounter {
|
||||
last, err := sc.Store.SenderCheckDeferredAt()
|
||||
if err != nil {
|
||||
log.Printf("log-tailer: sender check statistics: %v", err)
|
||||
}
|
||||
return &deferredCounter{
|
||||
last: last,
|
||||
st: sc.Store,
|
||||
connect: "warning: connect to Milter service unix:" + sc.Socket + ":",
|
||||
broken: "warning: milter unix:" + sc.Socket + ":",
|
||||
@@ -71,8 +85,9 @@ func newDeferredCounter(sc *SenderCheck) *deferredCounter {
|
||||
}
|
||||
}
|
||||
|
||||
// handle looks at one mail.log line.
|
||||
func (c *deferredCounter) handle(line string, now time.Time) {
|
||||
// handle looks at one mail.log line. catchingUp is true until the tailer has
|
||||
// read to the end of the log for the first time.
|
||||
func (c *deferredCounter) handle(line string, now time.Time, catchingUp bool) {
|
||||
at, rest := lineTime(line, now)
|
||||
m := smtpdLineRe.FindStringSubmatch(rest)
|
||||
if m == nil {
|
||||
@@ -94,6 +109,12 @@ func (c *deferredCounter) handle(line string, now time.Time) {
|
||||
c.failed = make(map[string]bool)
|
||||
}
|
||||
c.failed[pid] = true
|
||||
if catchingUp && !at.After(c.last) {
|
||||
return
|
||||
}
|
||||
if at.After(c.last) {
|
||||
c.last = at
|
||||
}
|
||||
if err := c.st.AddSenderCheckDeferred(at, 1); err != nil {
|
||||
log.Printf("log-tailer: sender check statistics: %v", err)
|
||||
}
|
||||
|
||||
@@ -7,13 +7,19 @@ import (
|
||||
|
||||
type deferredStore struct {
|
||||
days map[string]int64
|
||||
last time.Time
|
||||
}
|
||||
|
||||
func (d *deferredStore) SenderCheckDeferredAt() (time.Time, error) { return d.last, nil }
|
||||
|
||||
func (d *deferredStore) AddSenderCheckDeferred(at time.Time, n int64) error {
|
||||
if d.days == nil {
|
||||
d.days = make(map[string]int64)
|
||||
}
|
||||
d.days[at.UTC().Format("2006-01-02")] += n
|
||||
if at.After(d.last) {
|
||||
d.last = at
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -65,9 +71,41 @@ func TestDeferredCounter(t *testing.T) {
|
||||
// cleanup that writes it.
|
||||
`2026-10-13T00:01:00.000000+00:00 mail postfix/cleanup[1195]: B6B484831946: milter-reject: END-OF-MESSAGE from unknown[10.77.0.1]: 5.7.1 From: director@other.e2e.test - not an address this application sends as; from=<alerts@sender.e2e.test> to=<rcpt@sink.e2e.test> proto=ESMTP helo=<x>`,
|
||||
} {
|
||||
c.handle(line, now)
|
||||
c.handle(line, now, false)
|
||||
}
|
||||
if len(st.days) != 2 || st.days["2026-10-12"] != 1 || st.days["2026-10-13"] != 1 {
|
||||
t.Fatalf("deferred per day = %v, want one on each of the two days", st.days)
|
||||
}
|
||||
}
|
||||
|
||||
// After an unclean stop the tailer reads again some lines it had read. What
|
||||
// was counted then is not counted now; what was written while the panel was
|
||||
// down - the very lines this count exists for - is.
|
||||
func TestDeferredCounterDoesNotCountALogReadAgain(t *testing.T) {
|
||||
st := &deferredStore{}
|
||||
now := time.Date(2026, 10, 12, 18, 0, 0, 0, time.UTC)
|
||||
outage := func(pid, at string) []string {
|
||||
return []string{
|
||||
at + `0+00:00 mail postfix/smtpd[` + pid + `]: connect from unknown[10.77.0.1]`,
|
||||
at + `1+00:00 mail postfix/smtpd[` + pid + `]: warning: connect to Milter service unix:/run/selfpost/sender.sock: Connection refused`,
|
||||
}
|
||||
}
|
||||
first := append(outage("11", "2026-10-12T16:00:00.00000"), outage("12", "2026-10-12T16:00:05.00000")...)
|
||||
|
||||
c := newDeferredCounter(&SenderCheck{Store: st, Socket: "/run/selfpost/sender.sock"})
|
||||
for _, line := range first {
|
||||
c.handle(line, now, false)
|
||||
}
|
||||
if st.days["2026-10-12"] != 2 {
|
||||
t.Fatalf("first run counted %d, want 2", st.days["2026-10-12"])
|
||||
}
|
||||
|
||||
// A new run: the same lines again, then what the outage added.
|
||||
c = newDeferredCounter(&SenderCheck{Store: st, Socket: "/run/selfpost/sender.sock"})
|
||||
for _, line := range append(first, outage("13", "2026-10-12T16:00:09.00000")...) {
|
||||
c.handle(line, now, true)
|
||||
}
|
||||
if st.days["2026-10-12"] != 3 {
|
||||
t.Fatalf("after the restart the day has %d, want 3", st.days["2026-10-12"])
|
||||
}
|
||||
}
|
||||
|
||||
@@ -31,9 +31,9 @@ import (
|
||||
// answer, Postfix defers mail. It therefore depends on as little as it can.
|
||||
// It reads no database: what an application may send as comes from the file
|
||||
// of sender bindings Postfix itself reads for the envelope, so the two go by
|
||||
// one table. Where the lookups differ the check is the stricter: Postfix
|
||||
// also tries an address without its "+extension" and compares logins without
|
||||
// regard to case. The send-log row of a refusal is written by another goroutine and
|
||||
// one table, by one lookup: the address, the address without its
|
||||
// "+extension", the domain. (Postfix compares logins without regard to case;
|
||||
// the check compares them as written, which is how the panel writes them.) The send-log row of a refusal is written by another goroutine and
|
||||
// is not waited for.
|
||||
|
||||
// tableFile is a table of the mail path that the panel rewrites and the check
|
||||
@@ -69,9 +69,10 @@ func (f *tableFile) changed() (lines []string, again bool, err error) {
|
||||
// sender - an address, or "@domain" for any address of a domain - with the
|
||||
// logins that may use it.
|
||||
type Bindings struct {
|
||||
mu sync.Mutex
|
||||
file tableFile
|
||||
logins map[string]map[string]bool // sender -> logins
|
||||
mu sync.Mutex
|
||||
file tableFile
|
||||
logins map[string]map[string]bool // sender -> logins
|
||||
domainOf map[string]string // login -> the domain it sends as
|
||||
}
|
||||
|
||||
// NewBindings reads the file at path on first use.
|
||||
@@ -86,6 +87,7 @@ func (b *Bindings) fresh() error {
|
||||
return err
|
||||
}
|
||||
logins := make(map[string]map[string]bool)
|
||||
domains := make(map[string]string)
|
||||
for _, line := range lines {
|
||||
// texthash: <key><whitespace><value>, the value a comma-separated
|
||||
// list of logins (internal/postfix renderSenderLoginMaps).
|
||||
@@ -102,10 +104,13 @@ func (b *Bindings) fresh() error {
|
||||
for _, l := range strings.Split(strings.Join(fields[1:], ""), ",") {
|
||||
if l != "" {
|
||||
set[l] = true
|
||||
if _, known := domains[l]; !known {
|
||||
domains[l] = domainOf(sender)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
b.logins = logins
|
||||
b.logins, b.domainOf = logins, domains
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -122,12 +127,48 @@ func (b *Bindings) Allowed(login, addr string) (bool, error) {
|
||||
if b.logins[addr][login] {
|
||||
return true, nil
|
||||
}
|
||||
// Postfix's second try: the address without its extension, so that an
|
||||
// application listed as invoices@ may also send as invoices+march@
|
||||
// (recipient_delimiter in build/postfix-config.sh).
|
||||
if bare := withoutExtension(addr); bare != addr && b.logins[bare][login] {
|
||||
return true, nil
|
||||
}
|
||||
if d := domainOf(addr); d != "" && b.logins["@"+d][login] {
|
||||
return true, nil
|
||||
}
|
||||
return false, nil
|
||||
}
|
||||
|
||||
// extensionDelimiter is Postfix's recipient_delimiter, which
|
||||
// build/postfix-config.sh sets to this.
|
||||
const extensionDelimiter = "+"
|
||||
|
||||
// withoutExtension drops the extension of an address's local part: what
|
||||
// follows its first delimiter. "user+tag@example.org" is "user@example.org";
|
||||
// an address whose local part begins with the delimiter has no extension.
|
||||
func withoutExtension(addr string) string {
|
||||
at := strings.LastIndexByte(addr, '@')
|
||||
if at < 0 {
|
||||
return addr
|
||||
}
|
||||
if i := strings.Index(addr[:at], extensionDelimiter); i > 0 {
|
||||
return addr[:i] + addr[at:]
|
||||
}
|
||||
return addr
|
||||
}
|
||||
|
||||
// DomainOf returns the domain login sends as, from the first of its bindings
|
||||
// in the file's order; "" for a login that has none. An application belongs
|
||||
// to one domain, so all of its bindings name the same one.
|
||||
func (b *Bindings) DomainOf(login string) string {
|
||||
b.mu.Lock()
|
||||
defer b.mu.Unlock()
|
||||
if b.fresh() != nil {
|
||||
return ""
|
||||
}
|
||||
return b.domainOf[login]
|
||||
}
|
||||
|
||||
// Signers is OpenDKIM's SigningTable as the check reads it: the names this
|
||||
// server signs for - every sending domain, with applications or without, and
|
||||
// the server's own name.
|
||||
@@ -319,7 +360,7 @@ func (s *senderSession) record(shown string) {
|
||||
to += fmt.Sprintf(" and %d more", more)
|
||||
}
|
||||
e := store.SendLogEntry{
|
||||
Domain: domainOf(s.from),
|
||||
Domain: s.domain(),
|
||||
AppLogin: s.login,
|
||||
From: s.from,
|
||||
To: to,
|
||||
@@ -365,6 +406,18 @@ func unquoted(v string) string {
|
||||
return sb.String()
|
||||
}
|
||||
|
||||
// domain is the sending domain of the refused message for its row in the
|
||||
// log: the envelope sender's, which Postfix has held against the login - or,
|
||||
// for the null sender of a bounce an application writes itself, which Postfix
|
||||
// holds against nobody, the domain the login is bound to. Without it the row
|
||||
// would belong to no domain and its administrator would never see it.
|
||||
func (s *senderSession) domain() string {
|
||||
if d := domainOf(s.from); d != "" {
|
||||
return d
|
||||
}
|
||||
return s.bind.DomainOf(s.login)
|
||||
}
|
||||
|
||||
// mentionedDomains returns, lower-cased, what follows every "@" of v as far
|
||||
// as it reads as a host name.
|
||||
func mentionedDomains(v string) []string {
|
||||
|
||||
@@ -94,6 +94,8 @@ func TestSenderCheck(t *testing.T) {
|
||||
{"own domain, any address, with a name", "shop", [][2]string{from(`"The Shop" <Orders@Shop.Example.Org>`)}, true, ""},
|
||||
{"second application of the domain", "shop2", [][2]string{from("x@shop.example.org")}, true, ""},
|
||||
{"a listed address", "billing", [][2]string{from("Invoices <invoices@example.org>")}, true, ""},
|
||||
{"a listed address with an extension", "billing", [][2]string{from("invoices+2026-march@example.org")}, true, ""},
|
||||
{"an extension, any address of the domain", "shop", [][2]string{from("orders+48213@shop.example.org")}, true, ""},
|
||||
{"header name in another case", "shop", [][2]string{{"FROM", "a@shop.example.org"}}, true, ""},
|
||||
{"its own name inside the display name", "shop", [][2]string{from(`"shop@shop.example.org" <a@shop.example.org>`)}, true, ""},
|
||||
{"an encoded display name", "shop", [][2]string{from("=?utf-8?b?0JzQsNCz0LDQt9C40L0=?= <a@shop.example.org>")}, true, ""},
|
||||
@@ -107,6 +109,8 @@ func TestSenderCheck(t *testing.T) {
|
||||
{"the server's own name", "shop", [][2]string{from("postmaster@mail.example.org")}, false, "not an address this application sends as"},
|
||||
{"a stranger's domain", "shop", [][2]string{from("someone@gmail.example")}, false, "not an address this application sends as"},
|
||||
{"an address off the list", "billing", [][2]string{from("director@example.org")}, false, "not an address this application sends as"},
|
||||
{"an address off the list, with an extension", "billing", [][2]string{from("director+invoices@example.org")}, false, "not an address this application sends as"},
|
||||
{"a listed name as the extension of another", "billing", [][2]string{from("+invoices@example.org")}, false, "not an address this application sends as"},
|
||||
{"a subdomain of its own", "shop", [][2]string{from("a@sub.shop.example.org")}, false, "not an address this application sends as"},
|
||||
{"no From: line", "shop", [][2]string{{"Subject", "hello"}}, false, "no From: line"},
|
||||
{"two From: lines", "shop", [][2]string{from("a@shop.example.org"), from("director@example.org")}, false, "more than one From: line"},
|
||||
@@ -254,3 +258,16 @@ func TestSenderCheckRefusalIsOneRow(t *testing.T) {
|
||||
t.Fatalf("rows = %+v", rows)
|
||||
}
|
||||
}
|
||||
|
||||
// A message with the null envelope sender - a bounce an application writes
|
||||
// itself - is judged like any other, and its row belongs to the domain the
|
||||
// login is bound to.
|
||||
func TestSenderCheckRefusalWithNullSenderHasItsDomain(t *testing.T) {
|
||||
bind := NewBindings(writeMaps(t, senderMaps))
|
||||
for login, want := range map[string]string{"shop": "shop.example.org", "billing": "example.org"} {
|
||||
_, rows := offer(t, bind, login, "", [2]string{"From", "someone@gmail.example"})
|
||||
if len(rows) != 1 || rows[0].Domain != want || rows[0].From != "" {
|
||||
t.Errorf("%s: rows = %+v, want one of domain %s", login, rows, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -88,13 +88,26 @@ func (s *Store) AddSenderCheckDeferred(at time.Time, n int64) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
stamp := at.UTC().Format(time.RFC3339)
|
||||
// To the fraction of a second the log gives, and of a fixed width so that
|
||||
// the strings compare as the times do.
|
||||
stamp := at.UTC().Format("2006-01-02T15:04:05.000000Z")
|
||||
if stamp > last {
|
||||
return s.SetSetting(settingSenderCheckDeferredAt, stamp)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// SenderCheckDeferredAt is when a message was last counted as deferred; zero
|
||||
// if none was.
|
||||
func (s *Store) SenderCheckDeferredAt() (time.Time, error) {
|
||||
v, err := s.GetSetting(settingSenderCheckDeferredAt)
|
||||
if err != nil || v == "" {
|
||||
return time.Time{}, err
|
||||
}
|
||||
at, _ := time.Parse(time.RFC3339Nano, v)
|
||||
return at, nil
|
||||
}
|
||||
|
||||
// SenderCheckStats sums the days up to now: today, the last 30 days and the
|
||||
// last 12 months, each including today.
|
||||
func (s *Store) SenderCheckStats(now time.Time) (SenderCheckStats, error) {
|
||||
|
||||
Reference in New Issue
Block a user