From e2924af8d28acf7fa2d9d071efd8671adea6aa69 Mon Sep 17 00:00:00 2001
From: Mechiel Lukkien <mechiel@ueber.net>
Date: Sun, 28 Apr 2024 11:03:47 +0200
Subject: [PATCH] ensure senderaccount is always set for messages in queue

before, the smtpserver that queued a dsn would set an empty senderaccount,
which was interpreted in a few places as the globally configured postmaster
cacount. the empty senderaccount would be used by the smtpserver that queued a
dsn with null return path. we now set the postmaster account when we add a
message to the queue. more code in the queue pretty much needs a non-empty
senderaccount, such as the filters when listing, and the suppression list.
---
 queue/hook.go     |  8 +++-----
 queue/queue.go    | 36 +++++++++++++++++-------------------
 smtpserver/dsn.go |  2 +-
 3 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/queue/hook.go b/queue/hook.go
index 0a66f7c..4a028c0 100644
--- a/queue/hook.go
+++ b/queue/hook.go
@@ -204,11 +204,9 @@ func cleanupHookRetiredSingle(log mlog.Log) {
 
 func hookRetiredKeep(account string) time.Duration {
 	keep := 24 * 7 * time.Hour
-	if account != "" {
-		accConf, ok := mox.Conf.Account(account)
-		if ok {
-			keep = accConf.KeepRetiredWebhookPeriod
-		}
+	accConf, ok := mox.Conf.Account(account)
+	if ok {
+		keep = accConf.KeepRetiredWebhookPeriod
 	}
 	return keep
 }
diff --git a/queue/queue.go b/queue/queue.go
index 567b4d1..e6b40f4 100644
--- a/queue/queue.go
+++ b/queue/queue.go
@@ -1366,26 +1366,24 @@ func deliver(log mlog.Log, resolver dns.Resolver, m0 Msg) {
 	var remoteMTA dsn.NameIP // Zero value, will not be included in DSN. ../rfc/3464:1027
 
 	// Check if recipient is on suppression list. If so, fail delivery.
-	if m0.SenderAccount != "" {
-		path := smtp.Path{Localpart: m0.RecipientLocalpart, IPDomain: m0.RecipientDomain}
-		baseAddr := baseAddress(path).XString(true)
-		qsup := bstore.QueryTx[webapi.Suppression](xtx)
-		qsup.FilterNonzero(webapi.Suppression{Account: m0.SenderAccount, BaseAddress: baseAddr})
-		exists, err := qsup.Exists()
-		if err != nil || exists {
-			if err != nil {
-				qlog.Errorx("checking whether recipient address is in suppression list", err)
-			} else {
-				err := fmt.Errorf("not delivering to recipient address %s: %w", path.XString(true), errSuppressed)
-				err = smtpclient.Error{Permanent: true, Err: err}
-				failMsgsTx(qlog, xtx, []*Msg{&m0}, m0.DialedIPs, backoff, remoteMTA, err)
-			}
-			err = xtx.Commit()
-			qlog.Check(err, "commit processing failure to deliver messages")
-			xtx = nil
-			kick()
-			return
+	path := smtp.Path{Localpart: m0.RecipientLocalpart, IPDomain: m0.RecipientDomain}
+	baseAddr := baseAddress(path).XString(true)
+	qsup := bstore.QueryTx[webapi.Suppression](xtx)
+	qsup.FilterNonzero(webapi.Suppression{Account: m0.SenderAccount, BaseAddress: baseAddr})
+	exists, err := qsup.Exists()
+	if err != nil || exists {
+		if err != nil {
+			qlog.Errorx("checking whether recipient address is in suppression list", err)
+		} else {
+			err := fmt.Errorf("not delivering to recipient address %s: %w", path.XString(true), errSuppressed)
+			err = smtpclient.Error{Permanent: true, Err: err}
+			failMsgsTx(qlog, xtx, []*Msg{&m0}, m0.DialedIPs, backoff, remoteMTA, err)
 		}
+		err = xtx.Commit()
+		qlog.Check(err, "commit processing failure to deliver messages")
+		xtx = nil
+		kick()
+		return
 	}
 
 	resolveTransport := func(mm Msg) (string, config.Transport, bool) {
diff --git a/smtpserver/dsn.go b/smtpserver/dsn.go
index 4c2dcce..77b4bae 100644
--- a/smtpserver/dsn.go
+++ b/smtpserver/dsn.go
@@ -56,7 +56,7 @@ func queueDSN(ctx context.Context, log mlog.Log, c *conn, rcptTo smtp.Path, m ds
 	}
 	qm := queue.MakeMsg(smtp.Path{}, rcptTo, has8bit, smtputf8, int64(len(buf)), m.MessageID, nil, reqTLS, time.Now(), m.Subject)
 	qm.DSNUTF8 = bufUTF8
-	if err := queue.Add(ctx, c.log, "", f, qm); err != nil {
+	if err := queue.Add(ctx, c.log, mox.Conf.Static.Postmaster.Account, f, qm); err != nil {
 		return err
 	}
 	return nil