sender check: the sender map read and looked up as Postfix does it

Postfix 3.7.11 takes the owners of the first key mail_addr_find() finds - the address, then without its extension, then @domain, each tried only when the one before is absent (src/global/mail_addr_find.c, mail_addr_find_opt()) - and compares the login with them by strcasecmp_utf8() (src/smtpd/smtpd_check.c, reject_auth_sender_login_mismatch()). The check accepted a From: address when any of the three keys named the application: an application holding @domain could write an address listed for another application of the domain, which Postfix refuses it in the envelope, and have it signed (assumptions B-PS1). The table is now read as dict_thash_open() reads it: continuation lines, owners split on commas and blanks (B-PS4), the key ending at the first blank outside quotes, keys folded; a key given twice makes Postfix refuse the whole table (no DICT_FLAG_DUP_* flag), and the check then defers too. The writer folds keys, so two cases of one address cannot be two lines. e2e on the stand (2026-10-14, make e2e, exit 0): the wildcard application is refused billing@ - listed for app-list - in From: (550 5.7.1), and in the envelope by Postfix at RCPT ("553 5.7.1 <billing@...>: Sender address rejected: not owned by user"). go vet and go test ./... pass on the server (23 packages). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-14 20:03:34 +03:00
parent 31c2b50457
commit 55aef6e721
8 changed files with 262 additions and 53 deletions
@@ -17,6 +17,18 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
  both exceptions off; tried against rspamd 3.11.1 before and after
  (audit 2026-10-12, stage 2c; assumptions C-R7). Copy the new
  `antispam/rspamd/` files next to your compose fragment.
+- **An address listed for one application could be put in `From:` by
+  another of its domain.** The sender check accepted a `From:` address when
+  any of its keys in the sender map - the address, the address without its
+  extension, the domain - named the application; Postfix takes the first
+  key it finds and asks only its owners. An application set to any address
+  of a domain could therefore write in `From:` an address listed for
+  another application of that domain - which Postfix refuses it in the
+  envelope - and have it signed. The check now reads the map as Postfix
+  does: the first key found decides, logins are compared without regard to
+  case, owners may be separated by blanks or continue on the next line, and
+  a key written twice makes the check defer, as it makes Postfix refuse the
+  whole table (assumptions B-PS1, B-PS4).

 ### Fixed

@@ -242,10 +242,16 @@ server itself.
  its domain may write any address of that domain, and one with a list only
  the listed ones; everything else is refused - another registered domain,
  the server's name, a domain the server knows nothing of. One table for
-  both senders and one lookup: the address, the address without its
-  `+extension` (`recipient_delimiter`, which `build/postfix-config.sh` sets
-  for that reason), then the domain. Postfix compares logins without regard
-  to case and the check as written, which is how the panel writes them.
+  both senders, read as Postfix reads a texthash table and looked up as
+  Postfix looks it up: the address, the address without its `+extension`
+  (`recipient_delimiter`, which `build/postfix-config.sh` sets for that
+  reason), then `@domain` - and the **first of these keys in the table
+  decides**, its owners alone: an address listed for one application is not
+  open to another that holds the whole domain. Logins are compared without
+  regard to case, as Postfix compares them (Postfix 3.7.11,
+  `src/smtpd/smtpd_check.c`, `reject_auth_sender_login_mismatch()`, over
+  `src/global/mail_addr_find.c`, `mail_addr_find_opt()`). A key written
+  twice makes Postfix refuse the whole table, and the check defers then too.
 - **How the header is read.** Exactly one `From:` field (none, or more than
  one, is refused), parsed as an address list that must hold exactly one
  address. Then a cruder second look at the raw value: OpenDKIM finds the
@@ -1122,6 +1122,10 @@ An address with an extension counts as the address: an application listed as
 `invoices@…` may also send as `invoices+march@…`, in the envelope and in
 `From:` alike.

+An address listed for an application is that application's alone: another
+application of the same domain, set to any address, may not send as it -
+Postfix asks who owns the address before it asks who owns the domain.
+
 A message whose `From:` is anything else is refused with `550 5.7.1` and a
 text naming the line: another of your domains, the server's own name, an
 address at a domain that is not on this server, a line with two addresses,
@@ -31,10 +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, 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.
+// one table, read the way Postfix reads it and looked up the way Postfix looks
+// it up (Bindings). The send-log row of a refusal is written by another
+// goroutine and is not waited for.
 //
 // Postfix 3.7.11, src/smtpd/smtpd_check.c, reject_auth_sender_login_mismatch():
 // the SASL login is compared with an owner by strcasecmp_utf8();
@@ -78,8 +77,9 @@ func (f *tableFile) changed() (lines []string, again bool, err error) {
 type Bindings struct {
 	mu       sync.Mutex
 	file     tableFile
-	logins   map[string]map[string]bool // sender -> logins
+	logins   map[string]map[string]bool // sender -> logins, both in lower case
 	domainOf map[string]string          // login -> the domain it sends as
+	broken   error                      // why Postfix would not use the table
 }

 // NewBindings reads the file at path on first use.
@@ -90,63 +90,136 @@ func NewBindings(path string) *Bindings {
 // fresh makes the table match the file.
 func (b *Bindings) fresh() error {
 	lines, again, err := b.file.changed()
-	if err != nil || !again {
+	if err != nil {
 		return err
 	}
-	logins := make(map[string]map[string]bool)
-	domains := make(map[string]string)
+	if again {
+		b.logins, b.domainOf, b.broken = parseSenderMap(lines)
+	}
+	return b.broken
+}
+
+// parseSenderMap reads the lines of a sender_login_maps file as Postfix 3.7.11
+// reads a texthash table (src/util/dict_thash.c, dict_thash_open(), over
+// src/util/readlline.c, readllines()):
+//   - a line that starts with a blank continues the one before; an empty
+//     line, or one whose first non-blank is "#", is skipped;
+//   - the key ends at the first blank outside double quotes, a backslash
+//     escaping the next character; a line with an unbalanced quote, or
+//     without a key or a value, is ignored (with a warning, in Postfix);
+//   - the key is folded to lower case (DICT_FLAG_FOLD_FIX, which
+//     src/smtpd/smtpd_check.c, pre_jail_init(), gives
+//     smtpd_sender_login_maps);
+//   - a key given twice makes the whole table unusable: no DICT_FLAG_DUP_*
+//     flag is given, so dict_thash_open() returns a surrogate that fails
+//     every lookup, and smtpd answers 451 (reject_server_error()). The check
+//     then defers mail as well.
+//
+// The owners are split on commas and blanks (smtpd_check.c,
+// reject_auth_sender_login_mismatch(), mystrtok(CHARS_COMMA_SP)).
+func parseSenderMap(lines []string) (logins map[string]map[string]bool, domains map[string]string, broken error) {
+	var logical []string
 	for _, line := range lines {
-		// texthash: <key><whitespace><value>, the value a comma-separated
-		// list of logins (internal/postfix renderSenderLoginMaps).
-		// Postfix 3.7.11, src/util/dict_thash.c, dict_thash_open(): the key
-		// ends at the first blank; src/util/readlline.c skips a line whose
-		// first non-blank is "#"; src/util/dict_ht.c folds the key to lower
-		// case (DICT_FLAG_FOLD_FIX, set for smtpd_sender_login_maps in
-		// src/smtpd/smtpd_check.c).
-		fields := strings.Fields(line)
-		if len(fields) < 2 || strings.HasPrefix(fields[0], "#") {
+		line = strings.TrimRight(line, "\r")
+		trimmed := strings.TrimLeft(line, " \t")
+		switch {
+		case trimmed == "" || trimmed[0] == '#':
+			continue
+		case len(trimmed) < len(line) && len(logical) > 0:
+			logical[len(logical)-1] += line
+		default:
+			logical = append(logical, trimmed)
+		}
+	}
+	logins = make(map[string]map[string]bool)
+	domains = make(map[string]string)
+	for _, line := range logical {
+		key, value, ok := splitTableLine(line)
+		if !ok {
 			continue
 		}
-		sender := strings.ToLower(fields[0])
-		set := logins[sender]
-		if set == nil {
-			set = make(map[string]bool)
-			logins[sender] = set
+		sender := strings.ToLower(key)
+		if _, dup := logins[sender]; dup {
+			return nil, nil, fmt.Errorf("sender map: duplicate entry %q, which Postfix refuses the whole table for", sender)
 		}
-		for _, l := range strings.Split(strings.Join(fields[1:], ""), ",") {
-			if l != "" {
-				set[l] = true
-				if _, known := domains[l]; !known {
-					domains[l] = domainOf(sender)
-				}
+		set := make(map[string]bool)
+		logins[sender] = set
+		for _, l := range strings.FieldsFunc(value, func(r rune) bool {
+			return r == ',' || r == ' ' || r == '\t' || r == '\r' || r == '\n'
+		}) {
+			l = strings.ToLower(l)
+			set[l] = true
+			if _, known := domains[l]; !known {
+				domains[l] = domainOf(sender)
 			}
 		}
 	}
-	b.logins, b.domainOf = logins, domains
-	return nil
+	return logins, domains, nil
 }

-// Allowed reports whether login may send as addr: the address itself is bound
-// to it, or its whole domain is. This is the lookup Postfix makes for the
-// envelope sender.
+// splitTableLine splits a logical texthash line into its key and its value,
+// as dict_thash_open() does: the key ends at the first blank outside double
+// quotes (a backslash escaping the character after it), and the value is the
+// rest without the blanks around it. ok is false where Postfix ignores the
+// line: an unbalanced quote, no key, or no value.
+func splitTableLine(line string) (key, value string, ok bool) {
+	inQuotes := false
+	i := 0
+	for ; i < len(line); i++ {
+		c := line[i]
+		if c == '\\' {
+			i++
+			continue
+		}
+		if c == '"' {
+			inQuotes = !inQuotes
+			continue
+		}
+		if (c == ' ' || c == '\t') && !inQuotes {
+			break
+		}
+	}
+	if inQuotes {
+		return "", "", false
+	}
+	if i > len(line) {
+		i = len(line)
+	}
+	key, value = line[:i], strings.Trim(line[i:], " \t")
+	return key, value, key != "" && value != ""
+}
+
+// Allowed reports whether login may send as addr - the lookup Postfix makes
+// for the envelope sender. Postfix 3.7.11, src/smtpd/smtpd_check.c,
+// reject_auth_sender_login_mismatch(), takes the owners of the first key
+// mail_addr_find() finds (src/global/mail_addr_find.c, mail_addr_find_opt()
+// with MA_FIND_DEFAULT): the address, then the address without its
+// "+extension", then "@domain" - each tried only when the one before is not
+// in the table - and compares the login with them by strcasecmp_utf8(). So
+// an address bound to one application is not open to another that holds its
+// whole domain (assumptions B-PS1). The bare local part MA_FIND_DEFAULT also
+// tries for a local domain is never a key here: every key the panel writes
+// has an "@" (internal/postfix renderSenderLoginMaps).
 func (b *Bindings) Allowed(login, addr string) (bool, error) {
 	b.mu.Lock()
 	defer b.mu.Unlock()
 	if err := b.fresh(); err != nil {
 		return false, err
 	}
-	addr = strings.ToLower(addr)
-	if b.logins[addr][login] {
-		return true, nil
+	login, addr = strings.ToLower(login), strings.ToLower(addr)
+	keys := []string{addr}
+	// recipient_delimiter is "+" (build/postfix-config.sh), so an
+	// application listed as invoices@ may also send as invoices+march@.
+	if bare := withoutExtension(addr); bare != addr {
+		keys = append(keys, bare)
 	}
-	// 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 != "" {
+		keys = append(keys, "@"+d)
 	}
-	if d := domainOf(addr); d != "" && b.logins["@"+d][login] {
-		return true, nil
+	for _, k := range keys {
+		if owners, found := b.logins[k]; found {
+			return owners[login], nil
+		}
 	}
 	return false, nil
 }
@@ -182,7 +255,7 @@ func (b *Bindings) DomainOf(login string) string {
 	if b.fresh() != nil {
 		return ""
 	}
-	return b.domainOf[login]
+	return b.domainOf[strings.ToLower(login)]
 }

 // Signers is OpenDKIM's SigningTable as the check reads it: the names this
@@ -216,6 +216,80 @@ func TestBindingsFollowTheFile(t *testing.T) {
 	}
 }

+// The owners of the first key Postfix finds decide, not any key that lists
+// the login: an address bound to one application is not open to another that
+// holds its whole domain (Postfix 3.7.11, smtpd_check.c,
+// reject_auth_sender_login_mismatch(), over mail_addr_find(); assumptions
+// B-PS1). Composed by hand in internal/postfix's format.
+func TestBindingsFirstKeyWins(t *testing.T) {
+	bind := NewBindings(writeMaps(t, `@example.org shop
+billing@example.org billing
+invoices+march@example.org other
+`))
+	for _, c := range []struct {
+		login, addr string
+		want        bool
+	}{
+		{"shop", "a@example.org", true},
+		{"billing", "billing@example.org", true},
+		{"shop", "billing@example.org", false},        // the address's own key decides
+		{"billing", "billing+q3@example.org", true},   // without the extension
+		{"shop", "billing+q3@example.org", false},     // ... before @domain
+		{"shop", "invoices+march@example.org", false}, // the full key first
+		{"other", "invoices+march@example.org", true},
+		{"shop", "invoices+april@example.org", true}, // no key without "+": @domain
+		{"billing", "a@example.org", false},
+		{"SHOP", "A@Example.ORG", true}, // strcasecmp_utf8, a folded key
+	} {
+		if ok, err := bind.Allowed(c.login, c.addr); err != nil || ok != c.want {
+			t.Errorf("Allowed(%q, %q) = %v, %v; want %v", c.login, c.addr, ok, err, c.want)
+		}
+	}
+}
+
+// The table is read the way dict_thash_open() reads it: continuation lines,
+// owners split on blanks as well as commas, comments, lines Postfix ignores.
+// Composed by hand.
+func TestBindingsReadAsPostfixReadsThem(t *testing.T) {
+	bind := NewBindings(writeMaps(t, "# a comment\n"+
+		"@shop.example.org shop,\n"+
+		"  shop2 shop3\n"+
+		"\n"+
+		"\"a b\"@x.example.org quoted\n"+
+		"nokey-value-only\n"+
+		"\"open@x.example.org broken\n"+
+		"@news.example.net\tnews\r\n"))
+	for _, c := range []struct {
+		login, addr string
+		want        bool
+	}{
+		{"shop", "a@shop.example.org", true},
+		{"shop2", "a@shop.example.org", true},
+		{"shop3", "a@shop.example.org", true},
+		{"news", "a@news.example.net", true},
+		{"quoted", "\"a b\"@x.example.org", true},
+		{"broken", "\"open@x.example.org", false},
+	} {
+		if ok, err := bind.Allowed(c.login, c.addr); err != nil || ok != c.want {
+			t.Errorf("Allowed(%q, %q) = %v, %v; want %v", c.login, c.addr, ok, err, c.want)
+		}
+	}
+}
+
+// A key given twice - in any case, since Postfix folds keys - makes Postfix
+// refuse the whole table (dict_thash_open(), no DICT_FLAG_DUP_* flag), and
+// the check defers as smtpd does. Composed by hand.
+func TestBindingsDuplicateKeyBreaksTheTable(t *testing.T) {
+	bind := NewBindings(writeMaps(t, "@shop.example.org shop\nA@Shop.example.org other\n"))
+	if _, err := bind.Allowed("shop", "b@shop.example.org"); err != nil {
+		t.Fatalf("two different keys: %v", err)
+	}
+	bind = NewBindings(writeMaps(t, "@shop.example.org shop\n@SHOP.example.org other\n"))
+	if ok, err := bind.Allowed("shop", "b@shop.example.org"); err == nil || ok {
+		t.Fatalf("duplicate key: %v, %v; want an error", ok, err)
+	}
+}
+
 // Without the signing table the second look cannot be taken, and a line it
 // would have to judge is deferred; a line that names nobody else is not held
 // up by it.
@@ -100,10 +100,17 @@ func renderSenderLoginMaps(bindings []Binding) ([]byte, error) {
 		if err := assertMapSafe(b.Address, b.Login); err != nil {
 			return nil, err
 		}
-		if _, seen := byAddr[b.Address]; !seen {
-			order = append(order, b.Address)
+		// One line per key as Postfix folds it: smtpd opens the map with
+		// DICT_FLAG_FOLD_FIX and no DICT_FLAG_DUP_* flag, so two keys that
+		// differ only in case would make the whole table unusable (Postfix
+		// 3.7.11, src/util/dict_thash.c, dict_thash_open()). The panel stores
+		// addresses in lower case already; this keeps the file safe if a row
+		// ever is not.
+		addr := strings.ToLower(b.Address)
+		if _, seen := byAddr[addr]; !seen {
+			order = append(order, addr)
 		}
-		byAddr[b.Address] = appendUnique(byAddr[b.Address], b.Login)
+		byAddr[addr] = appendUnique(byAddr[addr], b.Login)
 	}
 	sort.Strings(order)

@@ -50,6 +50,22 @@ func TestRenderSenderLoginMapsDedupesLogin(t *testing.T) {
 	}
 }

+// Keys differing only in case are one key to Postfix, which folds them, and
+// two of them would make it refuse the whole table (dict_thash_open() with no
+// DICT_FLAG_DUP_* flag): they are written as one line.
+func TestRenderSenderLoginMapsFoldsKeys(t *testing.T) {
+	got, err := renderSenderLoginMaps([]Binding{
+		{"Billing@Example.org", "billing"},
+		{"billing@example.org", "billing2"},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(got) != "billing@example.org billing,billing2\n" {
+		t.Errorf("map = %q, want one folded key", got)
+	}
+}
+
 func TestAssertMapSafeRejectsInjection(t *testing.T) {
 	bad := []struct{ addr, login string }{
 		{"@exa mple.com", "log"},
@@ -62,6 +62,10 @@ func testSenderCheck(t *testing.T, sc *scenario) {
 		{"the server's own name", wildcard, "postmaster@" + selfpostHostname, false},
 		{"a domain the server knows nothing of", wildcard, "someone@elsewhere.e2e.test", false},
 		{"an address off the list", list, "director@" + senderDomain, false},
+		// The first key Postfix finds decides: billing@ is listed for
+		// app-list, so the wildcard application may not have it, in the
+		// header as in the envelope (assumptions B-PS1; the envelope below).
+		{"the address listed for another application", wildcard, "billing@" + senderDomain, false},
 		{"another sender in the display name", wildcard, `"director@` + otherDomain + `" <alerts@` + senderDomain + `>`, false},
 	} {
 		token := uniqueToken("from")
@@ -98,14 +102,27 @@ func testSenderCheck(t *testing.T, sc *scenario) {
 		}
 	}

+	// What the header case above holds to: Postfix itself refuses the
+	// wildcard application billing@ as envelope sender, at RCPT, because the
+	// address's own key names app-list only (Postfix 3.7.11,
+	// src/smtpd/smtpd_check.c, reject_auth_sender_login_mismatch(): "553
+	// 5.7.1 <%s>: Sender address rejected: not owned by user %s").
+	if res := attemptSend(sendAttempt{
+		authLogin: sc.appLogin, authPassword: sc.appPassword,
+		from: "billing@" + senderDomain, headerFrom: "alerts@" + senderDomain, to: recipient,
+		subject: "e2e " + uniqueToken("envelope"), body: "sender check: another application's address in the envelope",
+	}); res.refusedAt("RCPT", 553, "not owned by user") != nil {
+		t.Errorf("the address listed for another application, in the envelope: %v", res.refusedAt("RCPT", 553, "not owned by user"))
+	}
+
 	// A refusal is a row of the Outbound log whose tag says why.
 	if err := waitFor("the refusals to appear in the Outbound log", 15*time.Second, 500*time.Millisecond, func() (bool, error) {
 		rows, err := sc.panel.sendLogRows(senderDomain, "")
 		if err != nil {
 			return false, err
 		}
-		if n := strings.Count(rows, `">wrong From</span>`); n < 5 {
-			return false, fmt.Errorf("%d rows tagged wrong From, want 5", n)
+		if n := strings.Count(rows, `">wrong From</span>`); n < 6 {
+			return false, fmt.Errorf("%d rows tagged wrong From, want 6", n)
 		}
 		return true, nil
 	}); err != nil {