mix/selfpost
1
0
Fork 0
Code Issues Pull Requests Actions 21 Packages Projects Releases Wiki Activity

sender check: a Sender: line is held to the rule From: is held to
test / test (push) Waiting to run
Details

Browse Source 
The owner's decision (2026-10-16) on the security review's F1: a Sender:
line - the sending agent's, shown beside From: as "on behalf of", and
signed since 2.8.0 - was neither held to the login nor, before, signed: an
application could write its own From: and Sender: ceo@bank.com under this
server's signature. Now, when a message has one, it must be one address
the application may send as, and at most one such line (RFC 5322 § 3.6);
From: is judged first.

The refusal is "550 5.7.1 Sender: <address> - <why>", a send-log row of
reason "sender" (store.ReasonSender), tagged "wrong Sender"; the message
page names the line. The log tailer recovers such a row from cleanup's line
as it does a From: refusal (fromRefusalRe reads either). Help text as in
the design commit d9d9af3; guide, architecture, security and CHANGELOG say
it, security.md keeps Resent-From: (signed, not judged) as an accepted
risk.

Tests: TestSenderLineHeldToTheSameRule (own address, a name, another
application's, a stranger's, two addresses, two lines, From: first), the
recovery of a Sender: refusal line, the tag, the message page; e2e
sender_check_holds_the_sender_line (own Sender: passes and is signed,
another domain's refused at DATA and logged).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
mix
2026-10-16 07:41:42 +03:00
co-authored by Claude Opus 5.5
parent d9d9af3932
commit 0cceb549cc
18 changed files with 268 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/milter/sender.go
+48 -15
View File
@@ -284,6 +284,7 @@ type senderSession struct {
rcpts []string
subject string
froms []string // the value of every From: field of the message
senders []string // and of every Sender: field
}
// MailFrom starts a message and forgets the one before it: a connection may
@@ -297,7 +298,7 @@ type senderSession struct {
func (s *senderSession) MailFrom(from string, m *milter.Modifier) (milter.Response, error) {
s.from = cleanAddress(from)
s.login = macro(m, "auth_authen")
s.rcpts, s.subject, s.froms = nil, "", nil
s.rcpts, s.subject, s.froms, s.senders = nil, "", nil, nil
return milter.RespContinue, nil
}
@@ -311,6 +312,8 @@ func (s *senderSession) Header(name, value string, m *milter.Modifier) (milter.R
switch textproto.CanonicalMIMEHeaderKey(strings.TrimSpace(name)) {
case "From":
s.froms = append(s.froms, value)
case "Sender":
s.senders = append(s.senders, value)
case "Subject":
if s.subject == "" {
s.subject = mailhdr.DecodeSubject(value)
@@ -333,7 +336,7 @@ func (s *senderSession) Header(name, value string, m *milter.Modifier) (milter.R
// second message it can still hold the first one's From:. The session keeps
// its own, emptied at each MAIL FROM (audit 2026-10-12, L9).
func (s *senderSession) Headers(h textproto.MIMEHeader, m *milter.Modifier) (milter.Response, error) {
shown, why, err := s.judge()
field, shown, why, err := s.judge()
if err != nil {
// Strict: what cannot be checked is not let through.
log.Printf("sender-check: %v - deferring mail of %q", err, s.login)
@@ -342,27 +345,41 @@ func (s *senderSession) Headers(h textproto.MIMEHeader, m *milter.Modifier) (mil
if why == "" {
return milter.RespContinue, nil
}
log.Printf("sender-check: application %q refused: %s (From: %s)", s.login, why, shown)
log.Printf("sender-check: application %q refused: %s (%s: %s)", s.login, why, field, shown)
// cleanup has given the message a queue id by the end of its headers
// (milter_end_of_header_macros has {i}); Postfix's lines about it - the
// refusal among them - are found by it on the message's page.
// Postfix 3.10.13, src/global/mail_params.h: DEF_MILT_EOH_MACROS "i",
// answered by src/cleanup/cleanup_milter.c, cleanup_milter_eval().
s.record(shown, macro(m, "i"))
reason := store.ReasonFrom
if field == "Sender" {
reason = store.ReasonSender
}
s.record(reason, shown, macro(m, "i"))
// A reply code of its own, so that the application is told what is wrong
// and, the refusal being permanent, does not send the message again.
return milter.NewResponseStr('y', "550 5.7.1 From: "+shown+" - "+why), nil
return milter.NewResponseStr('y', "550 5.7.1 "+field+": "+shown+" - "+why), nil
}
func (s *senderSession) Abort(m *milter.Modifier) error {
s.rcpts, s.subject, s.froms = nil, "", nil
s.rcpts, s.subject, s.froms, s.senders = nil, "", nil, nil
return nil
}
// judge decides about the message at hand. why is empty when it may pass;
// otherwise it says what is wrong, and shown is the From: line in a form that
// is safe to print - both go into the reply to the client.
func (s *senderSession) judge() (shown, why string, err error) {
// otherwise it says what is wrong, field is the header it is about ("From"
// or "Sender"), and shown is that line in a form that is safe to print - all
// go into the reply to the client.
//
// A Sender: line - the agent that sent a message written by the From:
// line's author (RFC 5322 § 3.6.2), which a mail program shows beside From:
// as "on behalf of" - is held to the same rule as From: when the message has
// one, and at most one is allowed (§ 3.6, its table). The panel's signer
// signs it (sign.go), so an address of another sender there would be shown
// under this server's signature (security review of panel-dkim-signing, F1;
// the owner's decision, 2026-10-16).
func (s *senderSession) judge() (field, shown, why string, err error) {
field = "From"
if s.login == "" {
// Not reachable while the submission ports demand authentication; a
// message nobody answers for is not signed either way.
@@ -370,15 +387,32 @@ func (s *senderSession) judge() (shown, why string, err error) {
// smtpd_delay_reject (default yes) smtpd_client_restrictions
// (permit_sasl_authenticated,reject on 465 and 587) refuse every RCPT;
// src/smtpd/smtpd.c, data_cmd(): no valid recipients, no message.
return "(none)", "no authenticated application", nil
return field, "(none)", "no authenticated application", nil
}
switch len(s.froms) {
case 0:
return "(none)", "the message has no From: line", nil
return field, "(none)", "the message has no From: line", nil
case 1:
default:
return printable(strings.Join(s.froms, " | ")), "the message has more than one From: line", nil
return field, printable(strings.Join(s.froms, " | ")), "the message has more than one From: line", nil
}
if shown, why, err = s.judgeLine(s.froms[0]); err != nil || why != "" {
return field, shown, why, err
}
field = "Sender"
switch len(s.senders) {
case 0:
return field, "", "", nil
case 1:
default:
return field, printable(strings.Join(s.senders, " | ")), "the message has more than one Sender: line", nil
}
shown, why, err = s.judgeLine(s.senders[0])
return field, shown, why, err
}
// judgeLine holds one From: or Sender: line against the application.
func (s *senderSession) judgeLine(raw string) (shown, why string, err error) {
// The line must be one address in RFC 5322's grammar (net/mail), the
// address a mail program shows; a display name may say anything. A line
// a lenient program could show as another address - "ceo@bank.com
@@ -389,7 +423,6 @@ func (s *senderSession) judge() (shown, why string, err error) {
// 3.10.13, src/global/tok822_parse.c), is the one held against the
// bindings: "x"@mine.com is x@mine.com, which may be another
// application's (F2).
raw := s.froms[0]
shown = printable(raw)
list, perr := fromParser.ParseList(unfold(raw))
if perr != nil || len(list) != 1 {
@@ -439,7 +472,7 @@ func asciiLower(v string) string {
// each recipient as an accepted message has: nothing was sent to anybody, and
// a refused message with a thousand recipients would otherwise be a thousand
// rows at the price of one.
func (s *senderSession) record(shown, queueID string) {
func (s *senderSession) record(reason, shown, queueID string) {
if s.refused == nil {
return
}
@@ -457,7 +490,7 @@ func (s *senderSession) record(shown, queueID string) {
From: s.from,
To: to,
Subject: s.subject,
Reason: store.ReasonFrom,
Reason: reason,
HeaderFrom: shown,
}
select {
internal/milter/sender_test.go
+45
View File
@@ -415,3 +415,48 @@ func TestSenderCheckPassesTheSharedLines(t *testing.T) {
}
}
}
// A Sender: line, when the message has one, is held to the rule From: is
// held to; the From: line is judged first (the owner's decision, 2026-10-16).
func TestSenderLineHeldToTheSameRule(t *testing.T) {
bind := NewBindings(writeMaps(t, senderMaps))
from := [2]string{"From", "a@shop.example.org"}
sender := func(v string) [2]string { return [2]string{"Sender", v} }
for _, c := range []struct {
name string
headers [][2]string
field string // the line refused; "" when the message passes
wantText string
}{
{"no Sender: line", [][2]string{from}, "", ""},
{"its own address", [][2]string{from, sender("shop@shop.example.org")}, "", ""},
{"its own address, with a name", [][2]string{from, sender(`"The Shop's mailer" <mailer@shop.example.org>`)}, "", ""},
{"header name in another case", [][2]string{from, {"SENDER", "ceo@bank.example"}}, "Sender", "not an address this application sends as"},
{"a stranger's address", [][2]string{from, sender("ceo@bank.example")}, "Sender", "not an address this application sends as"},
{"another application's address", [][2]string{from, sender("desk@shop.example.org")}, "Sender", "not an address this application sends as"},
{"two addresses", [][2]string{from, sender("a@shop.example.org, ceo@bank.example")}, "Sender", "one address"},
{"two Sender: lines", [][2]string{from, sender("a@shop.example.org"), sender("b@shop.example.org")}, "Sender", "more than one Sender: line"},
{"a wrong From: and a wrong Sender:", [][2]string{{"From", "ceo@bank.example"}, sender("x@bank.example")}, "From", "not an address this application sends as"},
} {
t.Run(c.name, func(t *testing.T) {
resp, rows := offer(t, bind, "shop", "shop@shop.example.org", c.headers...)
if c.field == "" {
if resp != milter.RespContinue || len(rows) != 0 {
t.Fatalf("refused: %s", replyText(resp))
}
return
}
text := replyText(resp)
if !strings.HasPrefix(text, "y 550 5.7.1 "+c.field+": ") || !strings.Contains(text, c.wantText) {
t.Fatalf("reply %q, want a 550 5.7.1 about %s: saying %q", text, c.field, c.wantText)
}
reason := store.ReasonFrom
if c.field == "Sender" {
reason = store.ReasonSender
}
if len(rows) != 1 || rows[0].Reason != reason {
t.Fatalf("log rows: %+v, want one of reason %q", rows, reason)
}
})
}
}