null sender bounce

This commit is contained in:
2026-08-20 19:55:11 +01:00
parent fdb9cbc2ac
commit a9828e7883
6 changed files with 194 additions and 7 deletions
+2 -1
View File
@@ -8,4 +8,5 @@ tests/
CLAUDE.md
graphify-out/
*.sum
*.sum
cache/
+95
View File
@@ -0,0 +1,95 @@
package db
import (
"testing"
"time"
)
// TestRemoveMailboxCascadeDeletesEveryOwnedTable seeds one row in every table that
// references esrv_mailboxes.mailbox_id (plus esrv_mailbox_calendar_reminders, which
// only references an event owned by the mailbox) and confirms RemoveMailboxCascade
// leaves none of them behind — this DB never enables PRAGMA foreign_keys, so nothing
// but this function's own explicit DELETEs cleans these up.
func TestRemoveMailboxCascadeDeletesEveryOwnedTable(t *testing.T) {
database := openTestDB(t)
domainID, err := database.CreateDomain("cascade.example")
if err != nil {
t.Fatal(err)
}
hash, _ := HashPassword("irrelevant")
mailboxID, err := database.CreateMailbox("cascade@cascade.example", hash, domainID, 1024, []byte("wrapped"), []byte("nonce"))
if err != nil {
t.Fatal(err)
}
if _, err := database.CreateContact(mailboxID, "friend@example.com", "Friend", ""); err != nil {
t.Fatal(err)
}
if err := database.AddTrustedImageSender(mailboxID, "trusted@example.com"); err != nil {
t.Fatal(err)
}
sigID, err := database.CreateSignature(mailboxID, "Default", "<p>hi</p>")
if err != nil {
t.Fatal(err)
}
if err := database.SetSignatureAliasDefault(mailboxID, sigID, "alias@cascade.example", false); err != nil {
t.Fatal(err)
}
if err := database.RecordAutoReply(mailboxID, "someone@example.com"); err != nil {
t.Fatal(err)
}
start := time.Now()
eventID, err := database.CreateEvent(mailboxID, "Standup", "", "", start, start.Add(time.Hour), false, "", "")
if err != nil {
t.Fatal(err)
}
m := 10
if err := database.SetEventReminder(mailboxID, eventID, &m); err != nil {
t.Fatal(err)
}
tables := []string{
"esrv_mailbox_contacts",
"esrv_mailbox_trusted_image_senders",
"esrv_mailbox_signatures",
"esrv_mailbox_signature_alias_defaults",
"esrv_mailbox_autoreply_log",
"esrv_mailbox_calendar_events",
}
for _, table := range tables {
var n int
if err := database.QueryRow(`SELECT COUNT(*) FROM `+table+` WHERE mailbox_id = ?`, mailboxID).Scan(&n); err != nil {
t.Fatal(err)
}
if n == 0 {
t.Fatalf("setup failed: expected a seeded row in %s", table)
}
}
var reminderCount int
if err := database.QueryRow(`SELECT COUNT(*) FROM esrv_mailbox_calendar_reminders WHERE event_id = ?`, eventID).Scan(&reminderCount); err != nil {
t.Fatal(err)
}
if reminderCount == 0 {
t.Fatal("setup failed: expected a seeded calendar reminder")
}
if err := database.RemoveMailboxCascade(mailboxID); err != nil {
t.Fatal(err)
}
for _, table := range tables {
var n int
if err := database.QueryRow(`SELECT COUNT(*) FROM `+table+` WHERE mailbox_id = ?`, mailboxID).Scan(&n); err != nil {
t.Fatal(err)
}
if n != 0 {
t.Errorf("expected %s cleaned up by RemoveMailboxCascade, found %d row(s) left", table, n)
}
}
if err := database.QueryRow(`SELECT COUNT(*) FROM esrv_mailbox_calendar_reminders WHERE event_id = ?`, eventID).Scan(&reminderCount); err != nil {
t.Fatal(err)
}
if reminderCount != 0 {
t.Errorf("expected the calendar reminder cleaned up by RemoveMailboxCascade, found %d row(s) left", reminderCount)
}
}
+10
View File
@@ -237,6 +237,7 @@ func (d *DB) RemoveMailboxCascade(id int64) error {
`DELETE FROM esrv_mailbox_aliases WHERE mailbox_id = ?`,
`DELETE FROM esrv_mailbox_allowblock WHERE mailbox_id = ?`,
`DELETE FROM esrv_mailbox_filter_rules WHERE mailbox_id = ?`,
`DELETE FROM esrv_mailbox_autoreply_log WHERE mailbox_id = ?`,
`DELETE FROM esrv_mailbox_sessions WHERE mailbox_id = ?`,
`DELETE FROM esrv_mailbox_webauthn_credentials WHERE mailbox_id = ?`,
`DELETE FROM esrv_mailbox_folders WHERE mailbox_id = ?`,
@@ -244,6 +245,15 @@ func (d *DB) RemoveMailboxCascade(id int64) error {
`DELETE FROM esrv_mailbox_smime_contacts WHERE mailbox_id = ?`,
`DELETE FROM esrv_mailbox_pgp_identities WHERE mailbox_id = ?`,
`DELETE FROM esrv_mailbox_pgp_contacts WHERE mailbox_id = ?`,
`DELETE FROM esrv_mailbox_contacts WHERE mailbox_id = ?`,
`DELETE FROM esrv_mailbox_trusted_image_senders WHERE mailbox_id = ?`,
`DELETE FROM esrv_mailbox_signature_alias_defaults WHERE mailbox_id = ?`,
`DELETE FROM esrv_mailbox_signatures WHERE mailbox_id = ?`,
// esrv_mailbox_calendar_reminders has no mailbox_id column of its own (only
// event_id) — must go before the events themselves, via subquery, same
// reasoning as DeleteEvent's own reminder cleanup.
`DELETE FROM esrv_mailbox_calendar_reminders WHERE event_id IN (SELECT id FROM esrv_mailbox_calendar_events WHERE mailbox_id = ?)`,
`DELETE FROM esrv_mailbox_calendar_events WHERE mailbox_id = ?`,
`DELETE FROM esrv_mailbox_messages WHERE mailbox_id = ?`,
`DELETE FROM esrv_mailboxes WHERE id = ?`,
} {
@@ -77,12 +77,11 @@ func TestAutoReplyRuleFiresOnceThenSuppressedFor24h(t *testing.T) {
// TestAutoReplyNeverFiresForNullSender confirms sendAutoReply itself refuses a
// null-sender message — the classic bounce-loop bug this codebase already avoids for
// SendBounce. A white-box unit test, not a live SMTP round trip: Session.Mail()
// already hard-rejects MAIL FROM:<> for every sender category before DATA is ever
// reached (a separate, pre-existing gap — this server currently can't accept a
// genuine bounce/DSN from another real MTA at all — out of scope for this feature),
// so sendAutoReply's own guard is a defense-in-depth check that isn't reachable
// through the live protocol today; this test exercises it directly instead.
// SendBounce. A white-box unit test, not a live SMTP round trip, purely for
// convenience (no need for a real filter rule/mailbox delivery round trip just to
// exercise this one guard) — Session.Mail() now genuinely accepts MAIL FROM:<> for
// local delivery (see null_sender_test.go), so this guard is reachable through the
// live protocol too, not just here.
func TestAutoReplyNeverFiresForNullSender(t *testing.T) {
backend, mailboxID := newTestBackendWithMailbox(t)
mbox, err := backend.DB.GetMailboxByID(mailboxID)
+67
View File
@@ -0,0 +1,67 @@
package smtpserver
import (
"net/smtp"
"testing"
)
// TestNullSenderDeliversToLocalMailbox confirms a genuine MAIL FROM:<> bounce/DSN —
// RFC 5321 §4.5.5 requires every MTA accept this — can now be delivered to a real
// local mailbox, from an entirely unauthenticated connection (matching how a real
// remote MTA sending us a bounce actually behaves: no AUTH, no IP whitelist entry for
// its own address). Previously validateSenderAuthorization hard-rejected MAIL FROM:<>
// unconditionally, meaning this server could never receive a bounce from anywhere.
func TestNullSenderDeliversToLocalMailbox(t *testing.T) {
backend, mailboxID := newTestBackendWithMailbox(t)
addr := startTestServer(t, backend)
c, err := smtp.Dial(addr)
if err != nil {
t.Fatal(err)
}
defer c.Close()
if err := c.Mail(""); err != nil {
t.Fatalf("expected MAIL FROM:<> to be accepted, got: %v", err)
}
if err := c.Rcpt("inbox@example.com"); err != nil {
t.Fatalf("expected RCPT to a real local mailbox to succeed for a null sender, got: %v", err)
}
w, err := c.Data()
if err != nil {
t.Fatal(err)
}
if _, err := w.Write([]byte("From: Mail Delivery System <mailer-daemon@example.org>\r\nSubject: Undelivered Mail\r\n\r\nbounce body")); err != nil {
t.Fatal(err)
}
if err := w.Close(); err != nil {
t.Fatalf("expected DATA to succeed for a null-sender local delivery, got: %v", err)
}
msgs, err := backend.DB.ListMessagesInFolder(mailboxID, "INBOX")
if err != nil || len(msgs) != 1 {
t.Fatalf("expected the bounce delivered to INBOX, got %d messages (err=%v)", len(msgs), err)
}
}
// TestNullSenderCannotRelayExternally confirms a null sender is still refused relay
// to a non-local recipient (550 "Relay access denied") — provisional acceptance for
// local delivery must never extend to using this server as an open relay, since
// there's no sender identity behind a null sender to hold accountable.
func TestNullSenderCannotRelayExternally(t *testing.T) {
backend, _ := newTestBackendWithMailbox(t)
addr := startTestServer(t, backend)
c, err := smtp.Dial(addr)
if err != nil {
t.Fatal(err)
}
defer c.Close()
if err := c.Mail(""); err != nil {
t.Fatalf("expected MAIL FROM:<> to be accepted, got: %v", err)
}
if err := c.Rcpt("someone@not-configured-here.example"); err == nil {
t.Fatal("expected RCPT to a non-local recipient to be refused for a null sender (open-relay guard)")
}
}
+15
View File
@@ -103,6 +103,21 @@ func (s *Session) Mail(from string, opts *smtp.MailOptions) error {
// onward, so this cannot be used as an open relay.
func (s *Session) validateSenderAuthorization(mailFrom string) (accept, authorized bool, message string) {
if mailFrom == "" {
// A null sender (MAIL FROM:<>) is how every real MTA sends bounce/DSN
// messages (RFC 5321 §4.5.5 requires accepting it) — this server previously
// hard-rejected it unconditionally, meaning it could never receive a genuine
// bounce from anywhere. There's no sender identity to authorize (nothing can
// be "authorized to send as no one"), so this is provisionally accepted the
// same way a not-configured-here domain already is just below
// (accept=true, authorized=false) — Rcpt's mailFromAuthorized check still
// forbids using it to relay onward, so a null sender can only ever deliver to
// a local mailbox here, never open-relay. Checked before any
// authenticated-sender/mailbox/IP branch below, on purpose: authentication
// grants "authorized to send as <this address>", which is meaningless for an
// address that doesn't exist, so it must never inherit relay authorization.
if s.backend.Mailstore != nil {
return true, false, "Null sender (bounce/DSN) accepted for possible local delivery only"
}
return false, false, "No sender address provided"
}
fromDomain := domainOfAddr(mailFrom)