From 9c31789c56ec636b1a6d9dafc2e24670ad6d84a3 Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Wed, 9 Aug 2023 18:03:29 +0200 Subject: [PATCH] add option to ruleset to accept incoming spammy messages to a configured mailbox this is based on @bobobo1618's PR #50. bobobo1618 had the right idea, i tried including an "is forwarded email" configuration option but that indeed became too tightly coupled. the "is forwarded" option is still planned, but it is separate from the "accept rejects to mailbox" config option, because one could still want to push back on forwarded spam messages. we do an actual accept, delivering to a configured mailbox, instead of storing to the rejects mailbox where messages can automatically be removed from. one of the goals of mox is not pretend to accept email while actually junking it. users can still configure delivery to a junk folder (as was already possible), but aren't deleted automatically. there is still an X-Mox-Reason header in the message, and a log line about accepting the reject, but otherwise it is registered and treated as an (smtp) accept. the ruleset mailbox is still required to keep that explicit. users can specify Inbox again. hope this is good enough for PR #50, otherwise we'll change it. --- config/config.go | 5 ++-- config/doc.go | 25 +++++++++++++------ mox-/config.go | 4 +++ smtpserver/analyze.go | 44 ++++++++++++++++++++------------- smtpserver/server.go | 2 +- smtpserver/server_test.go | 34 ++++++++++++++++++++----- testdata/smtp/junk/domains.conf | 6 +++++ webaccount/account.html | 6 ++++- webaccount/accountapi.json | 7 ++++++ 9 files changed, 98 insertions(+), 35 deletions(-) diff --git a/config/config.go b/config/config.go index 2fc1fb5..1f0685d 100644 --- a/config/config.go +++ b/config/config.go @@ -384,7 +384,8 @@ type Ruleset struct { HeadersRegexp map[string]string `sconf:"optional" sconf-doc:"Matches if these header field/value regular expressions all match (substrings of) the message headers. Header fields and valuees are converted to lower case before matching. Whitespace is trimmed from the value before matching. A header field can occur multiple times in a message, only one instance has to match. For mailing lists, you could match on ^list-id$ with the value typically the mailing list address in angled brackets with @ replaced with a dot, e.g. ."` // todo: add a SMTPRcptTo check, and MessageFrom that works on a properly parsed From header. - ListAllowDomain string `sconf:"optional" sconf-doc:"Influence the spam filtering, this does not change whether this ruleset applies to a message. If this domain matches an SPF- and/or DKIM-verified (sub)domain, the message is accepted without further spam checks, such as a junk filter or DMARC reject evaluation. DMARC rejects should not apply for mailing lists that are not configured to rewrite the From-header of messages that don't have a passing DKIM signature of the From-domain. Otherwise, by rejecting messages, you may be automatically unsubscribed from the mailing list. The assumption is that mailing lists do their own spam filtering/moderation."` + ListAllowDomain string `sconf:"optional" sconf-doc:"Influence spam filtering only, this option does not change whether a message matches this ruleset. If this domain matches an SPF- and/or DKIM-verified (sub)domain, the message is accepted without further spam checks, such as a junk filter or DMARC reject evaluation. DMARC rejects should not apply for mailing lists that are not configured to rewrite the From-header of messages that don't have a passing DKIM signature of the From-domain. Otherwise, by rejecting messages, you may be automatically unsubscribed from the mailing list. The assumption is that mailing lists do their own spam filtering/moderation."` + AcceptRejectsToMailbox string `sconf:"optional" sconf-doc:"Influence spam filtering only, this option does not change whether a message matches this ruleset. If a message is classified as spam, it isn't rejected during the SMTP transaction (the normal behaviour), but accepted during the SMTP transaction and delivered to the specified mailbox. The specified mailbox is not automatically cleaned up like the account global Rejects mailbox, unless set to that Rejects mailbox."` Mailbox string `sconf-doc:"Mailbox to deliver to if this ruleset matches."` @@ -396,7 +397,7 @@ type Ruleset struct { // Equal returns whether r and o are equal, only looking at their user-changeable fields. func (r Ruleset) Equal(o Ruleset) bool { - if r.SMTPMailFromRegexp != o.SMTPMailFromRegexp || r.VerifiedDomain != o.VerifiedDomain || r.ListAllowDomain != o.ListAllowDomain || r.Mailbox != o.Mailbox { + if r.SMTPMailFromRegexp != o.SMTPMailFromRegexp || r.VerifiedDomain != o.VerifiedDomain || r.ListAllowDomain != o.ListAllowDomain || r.AcceptRejectsToMailbox != o.AcceptRejectsToMailbox || r.Mailbox != o.Mailbox { return false } if !reflect.DeepEqual(r.HeadersRegexp, o.HeadersRegexp) { diff --git a/config/doc.go b/config/doc.go index e278013..0670eda 100644 --- a/config/doc.go +++ b/config/doc.go @@ -715,16 +715,25 @@ describe-static" and "mox config describe-domains": HeadersRegexp: x: - # Influence the spam filtering, this does not change whether this ruleset applies - # to a message. If this domain matches an SPF- and/or DKIM-verified (sub)domain, - # the message is accepted without further spam checks, such as a junk filter or - # DMARC reject evaluation. DMARC rejects should not apply for mailing lists that - # are not configured to rewrite the From-header of messages that don't have a - # passing DKIM signature of the From-domain. Otherwise, by rejecting messages, you - # may be automatically unsubscribed from the mailing list. The assumption is that - # mailing lists do their own spam filtering/moderation. (optional) + # Influence spam filtering only, this option does not change whether a message + # matches this ruleset. If this domain matches an SPF- and/or DKIM-verified + # (sub)domain, the message is accepted without further spam checks, such as a junk + # filter or DMARC reject evaluation. DMARC rejects should not apply for mailing + # lists that are not configured to rewrite the From-header of messages that don't + # have a passing DKIM signature of the From-domain. Otherwise, by rejecting + # messages, you may be automatically unsubscribed from the mailing list. The + # assumption is that mailing lists do their own spam filtering/moderation. + # (optional) ListAllowDomain: + # Influence spam filtering only, this option does not change whether a message + # matches this ruleset. If a message is classified as spam, it isn't rejected + # during the SMTP transaction (the normal behaviour), but accepted during the SMTP + # transaction and delivered to the specified mailbox. The specified mailbox is not + # automatically cleaned up like the account global Rejects mailbox, unless set to + # that Rejects mailbox. (optional) + AcceptRejectsToMailbox: + # Mailbox to deliver to if this ruleset matches. Mailbox: diff --git a/mox-/config.go b/mox-/config.go index 71315b3..e0d899f 100644 --- a/mox-/config.go +++ b/mox-/config.go @@ -1039,6 +1039,10 @@ func prepareDynamicConfig(ctx context.Context, dynamicPath string, static config for i, rs := range dest.Rulesets { checkMailboxNormf(rs.Mailbox, "account %q, destination %q, ruleset %d", accName, addrName, i+1) + checkMailboxNormf(rs.AcceptRejectsToMailbox, "account %q, destination %q, ruleset %d, rejects mailbox", accName, addrName, i+1) + if strings.EqualFold(rs.AcceptRejectsToMailbox, "inbox") { + addErrorf("account %q, destination %q, ruleset %d: AcceptRejectsToMailbox cannot be set to Inbox", accName, addrName, i+1) + } n := 0 diff --git a/smtpserver/analyze.go b/smtpserver/analyze.go index 4ab6a7d..ef84376 100644 --- a/smtpserver/analyze.go +++ b/smtpserver/analyze.go @@ -38,6 +38,7 @@ type delivery struct { type analysis struct { accept bool + mailbox string code int secode string userError bool @@ -67,27 +68,43 @@ const ( ) func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delivery) analysis { - reject := func(code int, secode string, errmsg string, err error, reason string) analysis { - return analysis{false, code, secode, err == nil, errmsg, err, nil, nil, reason} + mailbox := d.rcptAcc.destination.Mailbox + if mailbox == "" { + mailbox = "Inbox" } // If destination mailbox has a mailing list domain (for SPF/DKIM) configured, // check it for a pass. - // todo: should use this evaluation for final delivery as well rs := store.MessageRuleset(log, d.rcptAcc.destination, d.m, d.m.MsgPrefix, d.dataFile) + if rs != nil { + mailbox = rs.Mailbox + } if rs != nil && !rs.ListAllowDNSDomain.IsZero() { ld := rs.ListAllowDNSDomain // todo: on temporary failures, reject temporarily? if d.m.MailFromValidated && ld.Name() == d.m.MailFromDomain { - return analysis{accept: true, reason: reasonListAllow} + return analysis{accept: true, mailbox: mailbox, reason: reasonListAllow} } for _, r := range d.dkimResults { if r.Status == dkim.StatusPass && r.Sig.Domain == ld { - return analysis{accept: true, reason: reasonListAllow} + return analysis{accept: true, mailbox: mailbox, reason: reasonListAllow} } } } + reject := func(code int, secode string, errmsg string, err error, reason string) analysis { + accept := false + if rs != nil && rs.AcceptRejectsToMailbox != "" { + accept = true + mailbox = rs.AcceptRejectsToMailbox + d.m.IsReject = true + // Don't draw attention, but don't go so far as to mark as junk. + d.m.Seen = true + log.Info("accepting reject to configured mailbox due to ruleset") + } + return analysis{accept, mailbox, code, secode, err == nil, errmsg, err, nil, nil, reason} + } + if d.dmarcUse && d.dmarcResult.Reject { return reject(smtp.C550MailboxUnavail, smtp.SePol7MultiAuthFails26, "rejecting per dmarc policy", nil, reasonDMARCPolicy) } @@ -166,20 +183,13 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive // per-mailbox. If referenced mailbox is not found (e.g. does not yet exist), we // can still determine a reputation because we also base it on outgoing // messages and those are account-global. - mailbox := d.rcptAcc.destination.Mailbox - if mailbox == "" { - mailbox = "Inbox" - } - if rs != nil { - mailbox = rs.Mailbox - } mb, err := d.acc.MailboxFind(tx, mailbox) if err != nil { return fmt.Errorf("finding destination mailbox: %w", err) } if mb != nil { // We want to deliver to mb.ID, but this message may be rejected and sent to the - // Rejects mailbox instead, which MailboxID overwritten. Record the ID in + // Rejects mailbox instead, with MailboxID overwritten. Record the ID in // MailboxDestinedID too. If the message is later moved out of the Rejects mailbox, // we'll adjust the MailboxOrigID so it gets taken into account during reputation // calculating in future deliveries. If we end up delivering to the intended @@ -203,12 +213,12 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive log.Info("reputation analyzed", mlog.Field("conclusive", conclusive), mlog.Field("isjunk", isjunk), mlog.Field("method", string(method))) if conclusive { if !*isjunk { - return analysis{accept: true, dmarcReport: dmarcReport, tlsReport: tlsReport, reason: reason} + return analysis{accept: true, mailbox: mailbox, dmarcReport: dmarcReport, tlsReport: tlsReport, reason: reason} } return reject(smtp.C451LocalErr, smtp.SeSys3Other0, "error processing", err, string(method)) } else if dmarcReport != nil || tlsReport != nil { log.Info("accepting dmarc reporting or tlsrpt message without reputation") - return analysis{accept: true, dmarcReport: dmarcReport, tlsReport: tlsReport, reason: reasonReporting} + return analysis{accept: true, mailbox: mailbox, dmarcReport: dmarcReport, tlsReport: tlsReport, reason: reasonReporting} } // If there was no previous message from sender or its domain, and we have an SPF // (soft)fail, reject the message. @@ -244,7 +254,7 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive pass := err == nil log.Infox("pass by subject token", err, mlog.Field("pass", pass)) if pass { - return analysis{accept: true, reason: reasonSubjectpass} + return analysis{accept: true, mailbox: mailbox, reason: reasonSubjectpass} } } @@ -324,7 +334,7 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive } if accept { - return analysis{accept: true, reason: reasonNoBadSignals} + return analysis{accept: true, mailbox: mailbox, reason: reasonNoBadSignals} } if subjectpassKey != "" && d.dmarcResult.Status == dmarc.StatusPass && method == methodNone && (dnsblocklisted || junkSubjectpass) { diff --git a/smtpserver/server.go b/smtpserver/server.go index 6edd3cf..d2d21d6 100644 --- a/smtpserver/server.go +++ b/smtpserver/server.go @@ -2463,7 +2463,7 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW } } else { acc.WithWLock(func() { - if err := acc.Deliver(log, rcptAcc.destination, m, dataFile, false); err != nil { + if err := acc.DeliverMailbox(log, a.mailbox, m, dataFile, false); err != nil { log.Errorx("delivering", err) metricDelivery.WithLabelValues("delivererror", a.reason).Inc() addError(rcptAcc, smtp.C451LocalErr, smtp.SeSys3Other0, false, "error processing") diff --git a/smtpserver/server_test.go b/smtpserver/server_test.go index 06b0ca4..a301d20 100644 --- a/smtpserver/server_test.go +++ b/smtpserver/server_test.go @@ -72,6 +72,14 @@ Message-Id: test email `, "\n", "\r\n") +var deliverMessage2 = strings.ReplaceAll(`From: +To: +Subject: test +Message-Id: + +test email, unique. +`, "\n", "\r\n") + type testserver struct { t *testing.T acc *store.Account @@ -411,10 +419,10 @@ func TestSpam(t *testing.T) { tinsertmsg(t, ts.acc, "Inbox", &nm, deliverMessage) } - checkRejectsCount := func(expect int) { + checkCount := func(mailboxName string, expect int) { t.Helper() q := bstore.QueryDB[store.Mailbox](ctxbg, ts.acc.DB) - q.FilterNonzero(store.Mailbox{Name: "Rejects"}) + q.FilterNonzero(store.Mailbox{Name: mailboxName}) mb, err := q.Get() tcheck(t, err, "get rejects mailbox") qm := bstore.QueryDB[store.Message](ctxbg, ts.acc.DB) @@ -439,8 +447,21 @@ func TestSpam(t *testing.T) { t.Fatalf("delivery by bad sender, got err %v, expected smtpclient.Error with code %d", err, smtp.C451LocalErr) } - // Message should now be in Rejects mailbox. - checkRejectsCount(1) + checkCount("Rejects", 1) + }) + + // Delivery from sender with bad reputation matching AcceptRejectsToMailbox should + // result in accepted delivery to the mailbox. + ts.run(func(err error, client *smtpclient.Client) { + mailFrom := "remote@example.org" + rcptTo := "mjl2@mox.example" + if err == nil { + err = client.Deliver(ctxbg, mailFrom, rcptTo, int64(len(deliverMessage2)), strings.NewReader(deliverMessage2), false, false) + } + tcheck(t, err, "deliver") + + checkCount("mjl2junk", 1) // In ruleset rejects mailbox. + checkCount("Rejects", 1) // Same as before. }) // Mark the messages as having good reputation. @@ -458,8 +479,9 @@ func TestSpam(t *testing.T) { } tcheck(t, err, "deliver") - // Message should now be removed from Rejects mailbox. - checkRejectsCount(0) + // Message should now be removed from Rejects mailboxes. + checkCount("Rejects", 0) + checkCount("mjl2junk", 1) }) // Undo dmarc pass, mark messages as junk, and train the filter. diff --git a/testdata/smtp/junk/domains.conf b/testdata/smtp/junk/domains.conf index f628f26..1a8c40c 100644 --- a/testdata/smtp/junk/domains.conf +++ b/testdata/smtp/junk/domains.conf @@ -5,6 +5,12 @@ Accounts: Domain: mox.example Destinations: mjl@mox.example: nil + mjl2@mox.example: + Rulesets: + - + SMTPMailFromRegexp: remote@example\.org + AcceptRejectsToMailbox: mjl2junk + Mailbox: mjl2 RejectsMailbox: Rejects JunkFilter: # Spamminess score between 0 and 1 above which emails are rejected as spam. E.g. diff --git a/webaccount/account.html b/webaccount/account.html index 7688fb0..80303f2 100644 --- a/webaccount/account.html +++ b/webaccount/account.html @@ -574,6 +574,7 @@ const destination = async (name) => { dom.td(row.VerifiedDomain=dom.input(attr({value: rs.VerifiedDomain || ''}))), headersCell, dom.td(row.ListAllowDomain=dom.input(attr({value: rs.ListAllowDomain || ''}))), + dom.td(row.AcceptRejectsToMailbox=dom.input(attr({value: rs.AcceptRejectsToMailbox || ''}))), dom.td(row.Mailbox=dom.input(attr({value: rs.Mailbox || ''}))), dom.td( dom.button('Remove ruleset', function click(e) { @@ -615,13 +616,15 @@ const destination = async (name) => { dom.h2('Rulesets'), dom.p('Incoming messages are checked against the rulesets. If a ruleset matches, the message is delivered to the mailbox configured for the ruleset instead of to the default mailbox.'), dom.p('The "List allow domain" does not affect the matching, but skips the regular spam checks if one of the verified domains is a (sub)domain of the domain mentioned here.'), + dom.p('"Accept rejects to mailbox" does not affect the matching, but causes messages classified as junk to be accepted and delivered to this mailbox, instead of being rejected during the SMTP transaction. Useful for incoming forwarded messages where rejecting incoming messages may cause the forwarding server to stop forwarding.'), dom.table( dom.thead( dom.tr( dom.th('SMTP "MAIL FROM" regexp', attr({title: 'Matches if this regular expression matches (a substring of) the SMTP MAIL FROM address (not the message From-header). E.g. user@example.org.'})), dom.th('Verified domain', attr({title: 'Matches if this domain matches an SPF- and/or DKIM-verified (sub)domain.'})), dom.th('Headers regexp', attr({title: 'Matches if these header field/value regular expressions all match (substrings of) the message headers. Header fields and valuees are converted to lower case before matching. Whitespace is trimmed from the value before matching. A header field can occur multiple times in a message, only one instance has to match. For mailing lists, you could match on ^list-id$ with the value typically the mailing list address in angled brackets with @ replaced with a dot, e.g. .'})), - dom.th('List allow domain', attr({title: "Influence the spam filtering, this does not change whether this ruleset applies to a message. If this domain matches an SPF- and/or DKIM-verified (sub)domain, the message is accepted without further spam checks, such as a junk filter or DMARC reject evaluation. DMARC rejects should not apply for mailing lists that are not configured to rewrite the From-header of messages that don't have a passing DKIM signature of the From-domain. Otherwise, by rejecting messages, you may be automatically unsubscribed from the mailing list. The assumption is that mailing lists do their own spam filtering/moderation."})), + dom.th('List allow domain', attr({title: "Influence spam filtering only, this option does not change whether a message matches this ruleset. If this domain matches an SPF- and/or DKIM-verified (sub)domain, the message is accepted without further spam checks, such as a junk filter or DMARC reject evaluation. DMARC rejects should not apply for mailing lists that are not configured to rewrite the From-header of messages that don't have a passing DKIM signature of the From-domain. Otherwise, by rejecting messages, you may be automatically unsubscribed from the mailing list. The assumption is that mailing lists do their own spam filtering/moderation."})), + dom.th('Allow rejects to mailbox', attr({title: "Influence spam filtering only, this option does not change whether a message matches this ruleset. If a message is classified as spam, it isn't rejected during the SMTP transaction (the normal behaviour), but accepted during the SMTP transaction and delivered to the specified mailbox. The specified mailbox is not automatically cleaned up like the account global Rejects mailbox, unless set to that Rejects mailbox."})), dom.th('Mailbox', attr({title: 'Mailbox to deliver to if this ruleset matches.'})), dom.th('Action'), ) @@ -651,6 +654,7 @@ const destination = async (name) => { VerifiedDomain: row.VerifiedDomain.value, HeadersRegexp: Object.fromEntries(row.headers.map(h => [h.key.value, h.value.value])), ListAllowDomain: row.ListAllowDomain.value, + AcceptRejectsToMailbox: row.AcceptRejectsToMailbox.value, Mailbox: row.Mailbox.value, } }), diff --git a/webaccount/accountapi.json b/webaccount/accountapi.json index 6576ab9..fb1c408 100644 --- a/webaccount/accountapi.json +++ b/webaccount/accountapi.json @@ -176,6 +176,13 @@ "string" ] }, + { + "Name": "AcceptRejectsToMailbox", + "Docs": "", + "Typewords": [ + "string" + ] + }, { "Name": "Mailbox", "Docs": "",