diff --git a/.gitignore b/.gitignore index 2b9ef76..e2df41c 100644 --- a/.gitignore +++ b/.gitignore @@ -8,4 +8,5 @@ tests/ CLAUDE.md graphify-out/ -*.sum \ No newline at end of file +*.sum +cache/ diff --git a/internal/db/crud_mailbox_cascade_test.go b/internal/db/crud_mailbox_cascade_test.go new file mode 100644 index 0000000..77070e6 --- /dev/null +++ b/internal/db/crud_mailbox_cascade_test.go @@ -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", "

hi

") + 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) + } +} diff --git a/internal/db/crud_mailboxes.go b/internal/db/crud_mailboxes.go index dcdc7ba..52ddb84 100644 --- a/internal/db/crud_mailboxes.go +++ b/internal/db/crud_mailboxes.go @@ -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 = ?`, } { diff --git a/internal/smtpserver/mailbox_autoreply_test.go b/internal/smtpserver/mailbox_autoreply_test.go index 01a12ee..488c4fb 100644 --- a/internal/smtpserver/mailbox_autoreply_test.go +++ b/internal/smtpserver/mailbox_autoreply_test.go @@ -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) diff --git a/internal/smtpserver/null_sender_test.go b/internal/smtpserver/null_sender_test.go new file mode 100644 index 0000000..2f2e1fe --- /dev/null +++ b/internal/smtpserver/null_sender_test.go @@ -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 \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)") + } +} diff --git a/internal/smtpserver/session.go b/internal/smtpserver/session.go index 052762c..138304f 100644 --- a/internal/smtpserver/session.go +++ b/internal/smtpserver/session.go @@ -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 ", 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)