in smtp submission, if a fromid is present in the mailfrom command, u…

…se it when queueing

it's the responsibility of the sender to use unique fromid's.
we do check if that's the case, and return an error if not.

also make it more clear that "unique smtp mail from addresses" map to the
"FromIDLoginAddresses" account config field.

based on feedback from cuu508 for #31, thanks!

config/config.go

@@ -410,7 +410,7 @@ type AutomaticJunkFlags struct {
 type Account struct {
 	OutgoingWebhook          *OutgoingWebhook `sconf:"optional" sconf-doc:"Webhooks for events about outgoing deliveries."`
 	IncomingWebhook          *IncomingWebhook `sconf:"optional" sconf-doc:"Webhooks for events about incoming deliveries over SMTP."`
-	FromIDLoginAddresses     []string         `sconf:"optional" sconf-doc:"Login addresses that cause outgoing email to be sent with SMTP MAIL FROM addresses with a unique id after the localpart catchall separator (which must be enabled when addresses are specified here). Any delivery status notifications (DSN, e.g. for bounces), can be related to the original message and recipient with unique id's. You can login to an account with any valid email address, including variants with the localpart catchall separator. You can use this mechanism to both send outgoing messages both with and without unique fromid for a given address."`
+	FromIDLoginAddresses     []string         `sconf:"optional" sconf-doc:"Login addresses that cause outgoing email to be sent with SMTP MAIL FROM addresses with a unique id after the localpart catchall separator (which must be enabled when addresses are specified here). Any delivery status notifications (DSN, e.g. for bounces), can be related to the original message and recipient with unique id's. You can login to an account with any valid email address, including variants with the localpart catchall separator. You can use this mechanism to both send outgoing messages with and without unique fromid for a given email address. With the webapi and webmail, a unique id will be generated. For submission, the id from the SMTP MAIL FROM command is used if present, and a unique id is generated otherwise."`
 	KeepRetiredMessagePeriod time.Duration    `sconf:"optional" sconf-doc:"Period to keep messages retired from the queue (delivered or failed) around. Keeping retired messages is useful for maintaining the suppression list for transactional email, for matching incoming DSNs to sent messages, and for debugging. The time at which to clean up (remove) is calculated at retire time. E.g. 168h (1 week)."`
 	KeepRetiredWebhookPeriod time.Duration    `sconf:"optional" sconf-doc:"Period to keep webhooks retired from the queue (delivered or failed) around. Useful for debugging. The time at which to clean up (remove) is calculated at retire time. E.g. 168h (1 week)."`
 

config/doc.go

@@ -975,8 +975,10 @@ See https://pkg.go.dev/github.com/mjl-/sconf for details.
 			# (DSN, e.g. for bounces), can be related to the original message and recipient
 			# with unique id's. You can login to an account with any valid email address,
 			# including variants with the localpart catchall separator. You can use this
-			# mechanism to both send outgoing messages both with and without unique fromid for
-			# a given address. (optional)
+			# mechanism to both send outgoing messages with and without unique fromid for a
+			# given email address. With the webapi and webmail, a unique id will be generated.
+			# For submission, the id from the SMTP MAIL FROM command is used if present, and a
+			# unique id is generated otherwise. (optional)
 			FromIDLoginAddresses:
 				-
 

queue/queue.go

@@ -41,6 +41,10 @@ import (
 	"github.com/mjl-/mox/webhook"
 )
 
+// ErrFromID indicate a fromid was present when adding a message to the queue, but
+// it wasn't unique.
+var ErrFromID = errors.New("fromid not unique")
+
 var (
 	metricConnection = promauto.NewCounterVec(
 		prometheus.CounterOpts{
@@ -662,6 +666,22 @@ func Add(ctx context.Context, log mlog.Log, senderAccount string, msgFile *os.Fi
 	// message inserted.
 	var baseID int64
 	for i := range qml {
+		// FromIDs must be unique if present. We don't have a unique index because values
+		// can be the empty string. We check in both Msg and MsgRetired, both are relevant
+		// for uniquely identifying a message sent in the past.
+		if fromID := qml[i].FromID; fromID != "" {
+			if exists, err := bstore.QueryTx[Msg](tx).FilterNonzero(Msg{FromID: fromID}).Exists(); err != nil {
+				return fmt.Errorf("looking up fromid: %v", err)
+			} else if exists {
+				return fmt.Errorf("%w: fromid %q already present in message queue", ErrFromID, fromID)
+			}
+			if exists, err := bstore.QueryTx[MsgRetired](tx).FilterNonzero(MsgRetired{FromID: fromID}).Exists(); err != nil {
+				return fmt.Errorf("looking up fromid: %v", err)
+			} else if exists {
+				return fmt.Errorf("%w: fromid %q already present in retired message queue", ErrFromID, fromID)
+			}
+		}
+
 		qml[i].SenderAccount = senderAccount
 		qml[i].BaseID = baseID
 		for _, hr := range holdRules {

smtpserver/server.go

@@ -2097,8 +2097,20 @@ func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWr
 	xcheckf(err, "parsing login address")
 	useFromID := slices.Contains(accConf.ParsedFromIDLoginAddresses, loginAddr)
 	var localpartBase string
+	var fromID string
+	var genFromID bool
 	if useFromID {
-		localpartBase = strings.SplitN(string(c.mailFrom.Localpart), confDom.LocalpartCatchallSeparator, 2)[0]
+		// With submission, user can bring their own fromid.
+		t := strings.SplitN(string(c.mailFrom.Localpart), confDom.LocalpartCatchallSeparator, 2)
+		localpartBase = t[0]
+		if len(t) == 2 {
+			fromID = t[1]
+			if fromID != "" && len(c.recipients) > 1 {
+				xsmtpServerErrorf(codes{smtp.C554TransactionFailed, smtp.SeProto5TooManyRcpts3}, "cannot send to multiple recipients with chosen fromid")
+			}
+		} else {
+			genFromID = true
+		}
 	}
 	now := time.Now()
 	qml := make([]queue.Msg, len(c.recipients))
@@ -2116,9 +2128,10 @@ func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWr
 		}
 
 		fp := *c.mailFrom
-		var fromID string
 		if useFromID {
-			fromID = xrandomID(16)
+			if genFromID {
+				fromID = xrandomID(16)
+			}
 			fp.Localpart = smtp.Localpart(localpartBase + confDom.LocalpartCatchallSeparator + fromID)
 		}
 
@@ -2142,7 +2155,11 @@ func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWr
 	}
 
 	// todo: it would be good to have a limit on messages (count and total size) a user has in the queue. also/especially with futurerelease. ../rfc/4865:387
-	if err := queue.Add(ctx, c.log, c.account.Name, dataFile, qml...); err != nil {
+	if err := queue.Add(ctx, c.log, c.account.Name, dataFile, qml...); err != nil && errors.Is(err, queue.ErrFromID) && !genFromID {
+		// todo: should we return this error during the "rcpt to" command?
+		// secode is not an exact match, but seems closest.
+		xsmtpServerErrorf(errCodes(smtp.C554TransactionFailed, smtp.SeAddr1SenderSyntax7, err), "bad fromid in smtp mail from address: %s", err)
+	} else if err != nil {
 		// Aborting the transaction is not great. But continuing and generating DSNs will
 		// probably result in errors as well...
 		metricSubmission.WithLabelValues("queueerror").Inc()

smtpserver/server_test.go

@@ -1898,3 +1898,48 @@ test email
 		ts.smtpErr(err, &smtpclient.Error{Permanent: true, Code: smtp.C554TransactionFailed, Secode: smtp.SeMsg6Other0})
 	})
 }
+
+// FromID can be specified during submission, but must be unique, with single recipient.
+func TestUniqueFromID(t *testing.T) {
+	ts := newTestServer(t, filepath.FromSlash("../testdata/smtpfromid/mox.conf"), dns.MockResolver{})
+	defer ts.close()
+
+	ts.user = "[email protected]"
+	ts.pass = password0
+	ts.submission = true
+
+	extraMsg := strings.ReplaceAll(`From: <[email protected]>
+To: <[email protected]>
+Subject: test
+
+test email
+`, "\n", "\r\n")
+
+	// Specify our own unique id when queueing.
+	ts.run(func(err error, client *smtpclient.Client) {
+		tcheck(t, err, "init client")
+		mailFrom := "[email protected]"
+		rcptTo := "[email protected]"
+		err = client.Deliver(ctxbg, mailFrom, rcptTo, int64(len(extraMsg)), strings.NewReader(extraMsg), true, true, false)
+		ts.smtpErr(err, nil)
+	})
+
+	// But we can only use it once.
+	ts.run(func(err error, client *smtpclient.Client) {
+		tcheck(t, err, "init client")
+		mailFrom := "[email protected]"
+		rcptTo := "[email protected]"
+		err = client.Deliver(ctxbg, mailFrom, rcptTo, int64(len(extraMsg)), strings.NewReader(extraMsg), true, true, false)
+		ts.smtpErr(err, &smtpclient.Error{Permanent: true, Code: smtp.C554TransactionFailed, Secode: smtp.SeAddr1SenderSyntax7})
+	})
+
+	// We cannot use our own fromid with multiple recipients.
+	ts.run(func(err error, client *smtpclient.Client) {
+		tcheck(t, err, "init client")
+		mailFrom := "[email protected]"
+		rcptTo := []string{"[email protected]", "[email protected]"}
+		_, err = client.DeliverMultiple(ctxbg, mailFrom, rcptTo, int64(len(extraMsg)), strings.NewReader(extraMsg), true, true, false)
+		ts.smtpErr(err, &smtpclient.Error{Permanent: true, Code: smtp.C554TransactionFailed, Secode: smtp.SeProto5TooManyRcpts3})
+	})
+
+}

testdata/smtpfromid/domains.conf

@@ -0,0 +1,10 @@
+Domains:
+	mox.example:
+		LocalpartCatchallSeparator: +
+Accounts:
+	mjl:
+		Domain: mox.example
+		Destinations:
+			[email protected]: nil
+		FromIDLoginAddresses:
+			- [email protected]

testdata/smtpfromid/mox.conf

@@ -0,0 +1,9 @@
+DataDir: data
+User: 1000
+LogLevel: trace
+Hostname: mox.example
+Postmaster:
+	Account: mjl
+	Mailbox: postmaster
+Listeners:
+	local: nil

webaccount/account.js

@@ -1449,7 +1449,7 @@ const index = async () => {
 		e.preventDefault();
 		e.stopPropagation();
 		await check(keepRetiredPeriodsFieldset, (async () => await client.KeepRetiredPeriodsSave(parseDuration(keepRetiredMessagePeriod.value), parseDuration(keepRetiredWebhookPeriod.value)))());
-	}, keepRetiredPeriodsFieldset = dom.fieldset(dom.div(style({ display: 'flex', gap: '1em', alignItems: 'flex-end' }), dom.div(dom.label('Messages deliveries', dom.br(), keepRetiredMessagePeriod = dom.input(attr.value(formatDuration(acc.KeepRetiredMessagePeriod))))), dom.div(dom.label('Webhook deliveries', dom.br(), keepRetiredWebhookPeriod = dom.input(attr.value(formatDuration(acc.KeepRetiredWebhookPeriod))))), dom.div(dom.submitbutton('Save'))))), dom.br(), dom.h2('Unique SMTP MAIL FROM login addresses', attr.title('Outgoing messages are normally sent using your email address in the SMTP MAIL FROM command. By using unique addresses (by using the localpart catchall separator, e.g. addresses of the form "localpart+<uniquefromid>@domain"), future incoming DSNs can be related to the original outgoing messages and recipients, which allows for automatic management of recipient suppression lists when keeping retired messages for as long as you expect DSNs to come in as configured above. Configure the addresses used for logging in with SMTP submission, the webapi or webmail for which unique SMTP MAIL FROM addesses should be enabled. Note: These are addresses used for authenticating, not the address in the message "From" header.')), (() => {
+	}, keepRetiredPeriodsFieldset = dom.fieldset(dom.div(style({ display: 'flex', gap: '1em', alignItems: 'flex-end' }), dom.div(dom.label('Messages deliveries', dom.br(), keepRetiredMessagePeriod = dom.input(attr.value(formatDuration(acc.KeepRetiredMessagePeriod))))), dom.div(dom.label('Webhook deliveries', dom.br(), keepRetiredWebhookPeriod = dom.input(attr.value(formatDuration(acc.KeepRetiredWebhookPeriod))))), dom.div(dom.submitbutton('Save'))))), dom.br(), dom.h2('Unique SMTP MAIL FROM login addresses ("FromID")', attr.title('Login addresses that cause outgoing email to be sent with SMTP MAIL FROM addresses with a unique id after the localpart catchall separator (which must be enabled when addresses are specified here). Any delivery status notifications (DSN, e.g. for bounces), can be related to the original message and recipient with unique id\'s. You can login to an account with any valid email address, including variants with the localpart catchall separator. You can use this mechanism to both send outgoing messages with and without unique fromid for a given email address. With the webapi and webmail, a unique id will be generated. For submission, the id from the SMTP MAIL FROM command is used if present, and a unique id is generated otherwise. Corresponds to field FromIDLoginAddresses in the Account configuration in domains.conf.')), (() => {
 		let inputs = [];
 		let elem;
 		const render = () => {

webaccount/account.ts

@@ -1063,7 +1063,7 @@ const index = async () => {
 		),
 		dom.br(),
 
-		dom.h2('Unique SMTP MAIL FROM login addresses', attr.title('Outgoing messages are normally sent using your email address in the SMTP MAIL FROM command. By using unique addresses (by using the localpart catchall separator, e.g. addresses of the form "localpart+<uniquefromid>@domain"), future incoming DSNs can be related to the original outgoing messages and recipients, which allows for automatic management of recipient suppression lists when keeping retired messages for as long as you expect DSNs to come in as configured above. Configure the addresses used for logging in with SMTP submission, the webapi or webmail for which unique SMTP MAIL FROM addesses should be enabled. Note: These are addresses used for authenticating, not the address in the message "From" header.')),
+		dom.h2('Unique SMTP MAIL FROM login addresses ("FromID")', attr.title('Login addresses that cause outgoing email to be sent with SMTP MAIL FROM addresses with a unique id after the localpart catchall separator (which must be enabled when addresses are specified here). Any delivery status notifications (DSN, e.g. for bounces), can be related to the original message and recipient with unique id\'s. You can login to an account with any valid email address, including variants with the localpart catchall separator. You can use this mechanism to both send outgoing messages with and without unique fromid for a given email address. With the webapi and webmail, a unique id will be generated. For submission, the id from the SMTP MAIL FROM command is used if present, and a unique id is generated otherwise. Corresponds to field FromIDLoginAddresses in the Account configuration in domains.conf.')),
 		(() => {
 			let inputs: HTMLInputElement[] = []
 			let elem: HTMLElement

webapi/doc.go

@@ -27,10 +27,12 @@ An HTTP POST to /webapi/v0/<method> calls a method.The form can be either
 "application/x-www-form-urlencoded" or "multipart/form-data".  Form field
 "request" must contain the request parameters, encoded as JSON.
 
-HTTP basic authentication is required for calling methods, with an email
-address as user name. Use a login address configured for "unique SMTP MAIL
-FROM" addresses, and configure a period to "keep retired messages delivered
-from the queue" for automatic suppression list management.
+HTTP basic authentication is required for calling methods, with an email address
+as user name. Use a login address configured for "unique SMTP MAIL FROM"
+addresses ("FromIDLoginAddresses" in the account configuration), and configure
+an interval to "keep retired messages delivered from the queue". This allows
+incoming DSNs to be matched to the original outgoing messages, and enables
+automatic suppression list management.
 
 HTTP response status 200 OK indicates a successful method call, status 400
 indicates an error.  The response body of an error is a JSON object with a

webapi/gendoc.sh

@@ -37,10 +37,12 @@ An HTTP POST to /webapi/v0/<method> calls a method.The form can be either
 "application/x-www-form-urlencoded" or "multipart/form-data".  Form field
 "request" must contain the request parameters, encoded as JSON.
 
-HTTP basic authentication is required for calling methods, with an email
-address as user name. Use a login address configured for "unique SMTP MAIL
-FROM" addresses, and configure a period to "keep retired messages delivered
-from the queue" for automatic suppression list management.
+HTTP basic authentication is required for calling methods, with an email address
+as user name. Use a login address configured for "unique SMTP MAIL FROM"
+addresses ("FromIDLoginAddresses" in the account configuration), and configure
+an interval to "keep retired messages delivered from the queue". This allows
+incoming DSNs to be matched to the original outgoing messages, and enables
+automatic suppression list management.
 
 HTTP response status 200 OK indicates a successful method call, status 400
 indicates an error.  The response body of an error is a JSON object with a

website/features/index.md

@@ -315,8 +315,8 @@ composing email messages (internet message format, IMF), SMTP for submission,
 and IMAP for handling incoming messages including delivery status notifications
 (DSNs).
 
-Outgoing webhooks notify about events for outgoing deliveries (such as
-"delivered", "delayed", "failed", "suppressed").
+Outgoing webhooks notify about events for outgoing deliveries, such as
+"delivered", "delayed", "failed", "suppressed".
 
 Incoming webhooks notify about incoming deliveries.