diff --git a/README.rst b/README.rst index 9e8b273..af53ffd 100644 --- a/README.rst +++ b/README.rst @@ -162,7 +162,7 @@ A few especially important configuration files: * :code:`transfers.conf` - File transfer configuration -Additionally, the MailScript rules engine uses a script file called :code:`.rules` in the root maildir and the user's root maildir for manipulating messages. +Additionally, the MailScript rules engine uses a script file called :code:`.rules` in the user's root maildir (and :code:`before.rules` and :code:`after.rules` in the root maildir for global filtering) for manipulating messages. A sample MailScript rules file is in :code:`configs/.rules` (though this is not a config file, but a sample rule script file). User Configuration @@ -464,7 +464,7 @@ slightly more complicated syntax, such as Sieve. MailScript also allows for basi independent of the filtering language used, which can be useful for testing. MailScript was added before Sieve support was added due to the easier implementation. -Currently, some capabilities, such as executing system commands or processing outgoing emails, that are only possible with MailScript, not with Sieve. +Currently, some capabilities, such as executing system commands or processing outgoing emails, are only possible with MailScript, not with Sieve. Although there are Sieve extensions to do this, the Sieve implementation in the BBS does not yet support this (or rather, the underlying library does not). Eventually the goal is to have full feature parity. @@ -485,52 +485,139 @@ There is a builtin module for SpamAssassin integration. SpamAssassin installatio Installation ------------ -* Install SpamAssassin: :code:`apt-get install -y spamassassin`. You do not need :code:`spamass-milter` since milters are not currently supported. +Install SpamAssassin: :code:`apt-get install -y spamassassin`. You do not need :code:`spamass-milter` since milters are not currently supported. -* Create your preference file, e.g. :code:`/etc/spamassassin/config.cf`:: +TIP: If you have multiple mail servers in an internal hierarchy, we recommend installing SpamAssassin on the "outermost" SMTP server, i.e. the one that receives mail directly from other MTAs on the Internet. This way, you have the ability to refuse acceptance of certain spam emails during the SMTP transaction itself (which is "cheap"), rather than accepting it, relaying it to a downstream server, determining the email should be rejected, and then having to generate a "bounce" messages, since the original connection has already been closed (which is "expensive", and not as reliable). The first server can run SpamAssassin, and downstream servers with user mailboxes can then actually do filtering based on the headers added previously by SpamAssassin. - # Required score to be considered spam (5 is the default, and should generally be left alone) +Note that if the incoming mail server running SpamAssassin is hosted on DigitalOcean, you will need to `sign up for a DQS key and follow the instructions in order to make Spamhaus's DNSBLs functional `_. + +Deployment Considerations +------------------------- +SpamAssassin is best used before-queue, since this prevents backscatter by ensuring spam results are available for filtering rules to use (allowing recipients to outright reject highly suspected spam, for instance). :code:`mod_spamassassin` invokes SpamAssassin during the SMTP delivery process to allow this. + +When invoked directly (e.g. as :code:`/usr/bin/spamassassin`), SpamAssassin will read the message from the BBS on STDIN and output the modified message on STDOUT. Because the BBS only needs SpamAssassin to prepend headers at the top, it will *not* use the entire returned body from SpamAssassin. Instead, it will prepend all of the SpamAssassin headers and ignore everything else, since that would just involve copying the remainder of the message back again for no reason. This contrasts with with more conventional facilities that mail transfer agents provide for modifying message bodies on delivery. + +Configuration +------------- + +* Load the language plugin by adding :code:`loadplugin Mail::SpamAssassin::Plugin::TextCat` to :code:`/etc/spamassassin/local.pre` + +* Create your custom preference file, e.g. :code:`/etc/spamassassin/config.cf`:: + + # Required score to be considered spam (5 is the default, and should generally be left alone, fine tune your Junk threshold using mail filtering rules instead) required_score 5 - # Heavily penalize HTML only emails - score MIME_HTML_ONLY 2.10 + # English is the only language that won't trigger the UNWANTED_LANGUAGE_BODY rule + ok_languages en + + # SPF hard fail, always reject + score SPF_FAIL 10.0 + + # SPF soft fail, always send to Junk + score SPF_SOFTFAIL 5.0 + + # Heavily penalize mail from domains with no SPF record + score SPF_NONE 3.0 + + # Email is not in English + score UNWANTED_LANGUAGE_BODY 3.5 + + # Penalize HTML only emails + score MIME_HTML_ONLY 1.8 + + # Penalized heavily abused freemail + score FREEMAIL_FROM 0.5 # Don't modify original message (apart from adding headers) report_safe 0 + # Add X-Spam-Report to all emails, including ham, not just spam + add_header all Report _REPORT_ + + # Add X-Spam-Score to all emails, including ham, not just spam + add_header all Score _SCORE_ + # Bayes DB (specify a path and sa-learn will create the DB for you) bayes_path /var/lib/spamassassin/bayesdb/bayes -* Go ahead and run `sa-compile` to compile your rule set into a more efficient form for runtime. +* Go ahead and run :code:`sa-compile` to compile your rule set into a more efficient form for runtime (if you modify :code:`config.cf` in the future, rerun this command). -Training --------- +To regularly update SpamAssassin with the latest rules, enable the cron job by adding :code:`CRON=1` to :code:`/etc/default/spamd`. -SpamAssassin needs to be trained for optimal filtering results. It is best trained on real spam (and ham, or non-spam) messages. You can tell SpamAssassin about actual spam (:code:`sa-learn --spam /path/to/spam/folder`) or ham (:code:`sa-learn --ham /path/to/ham/folder`). +Filtering Spam +-------------- -SpamAssassin can work reasonably well out of the box, but will get better with training. If you receive spam, don't delete them - put them in a special folder (e.g. Junk) and rerun :code:`sa-learn` periodically. +SpamAssassin will tag spam appropriately, but not do anything to it. That's where filtering rules can help filter spam to the right place (or even reject it during the SMTP session). There are a few headers that SpamAssassin will add, e.g. :code:`X-Spam-Level`. Users can customize what they want to do with spam and their threshold for spam filtering using a filter. The most common rule is to move suspected spam to the user's Junk folder. -You can also run on multiple folders - careful though, if users have a Sieve rule to move suspected spam to Junk, this could train on false positives if this is run before they react and correct that. Therefore, if your mail server is small, you may just want to do this manually periodically after receiving Spam:: +Our recommendation is to ignore the :code:`X-Spam-Flag` header entirely. Instead, you can use the :code:`X-Spam-Level` header in mail filtering rules to handle spam, by either moving them to Junk (at a lower threshold) and outright rejecting them (at a higher threshold). This gives you much more fine-grained control, and allows different users to customize their filtering. - sa-learn --spam /home/bbs/maildir/*/Junk/{cur,new} - sa-learn --ham /home/bbs/maildir/*/cur +The :code:`X-Spam-Level` header contains one asterisk for each whole positive spam score level (i.e. it is the value of the spam score (also available directly in the :code:`X-Spam-Score` header), rounded down, and empty if less than 1.0, including negative. For instance, :code:`****` denotes the message has a spam score of between 4.0 and 4.9. Since spammier messages have more :code:`*`s, you can easily use a simple substring match on this header value, for example:: -Once you've trained the Bayes model, you can delete the spam messages if you wish. Rerunning the model on existing messages is fine too - the model will skip messages it's already seen, so there's no harm in not deleting them immediately, if you have the disk space. + # This MailScript rule will outright reject any messages with a spam score of 10.0 or greater (and set a custom refusal message) + RULE + MATCH DIRECTION IN + MATCH HEADER X-Spam-Level CONTAINS ********** + ACTION REJECT Message refused, appears to be spam + ENDRULE -Adding Spam Headers -------------------- + # This MailScript rule will move any messages with a spam score of 5.0 or greater (and implicitly 9.9 or less, if the above rule is present) to the user's Junk folder + RULE + MATCH DIRECTION IN + MATCH HEADER X-Spam-Level CONTAINS ***** + ACTION MOVETO Junk + ENDRULE -SpamAssassin can be called by the SMTP server on incoming emails delivered from external recipients. This should be done automatically provided that :code:`mod_spamassassin` is loaded and SpamAssassin is installed and configured properly. -SpamAssassin will add some headers to each message, which can then be used in a Sieve script or MailScript rule to filter suspected spam into the Junk folder (but SpamAssassin on its own will not filter mail, just identify messages it thinks are spam). +You could also use a standard Sieve rule instead of a MailScript rule:: -SpamAssassin is best used before-queue, since this prevents backscatter by ensuring spam results are available for filtering rules to use (allowing recipients to outright reject highly suspected spam, for instance). :code:`mod_spamassassin` invokes SpamAssassin during the SMTP delivery process to allow this. + require "fileinto"; + if header :contains "X-Spam-Level" "*****" { + fileinto "Junk"; + } -When invoked directly (e.g. as :code:`/usr/bin/spamassassin`), SpamAssassin will read the message from the BBS on STDIN and output the modified message on STDOUT. Because the BBS only needs SpamAssassin to prepend headers at the top, it will *not* use the entire returned body from SpamAssassin. Instead, it will prepend all of the SpamAssassin headers and ignore everything else, since that would just involve copying the remainder of the message back again for no reason. This contrasts with with more conventional facilities that mail transfer agents provide for modifying message bodies on delivery. +Note that :code:`X-Spam-Level` only gives you the ability to filter by intervals of 1. If you want more granular control than that, you should use the :code:`X-Spam-Score` header instead:: -Filtering Spam --------------- + # This MailScript rule will reject any messages with a spam score of 7.7 or greater + RULE + MATCH DIRECTION IN + MATCH HEADER X-Spam-Score >= 7.7 + ACTION REJECT + ENDRULE + +Both Sieve and MailScript rules can also be configured globally (system-wide), in addition to per-mailbox. This is useful if you as the postmaster want to reject all mail above a certain spam level. There are two global Sieve scripts that can be configured and one global MailScript script. All of these files must be named as follows and placed in the root maildir: + +* :code:`before.sieve`: Sieve rules that will be executed before any per-mailbox rules are executed. This is usually better for default settings that users may override, such as moving spam to Junk. +* :code:`after.sieve`: Sieve rules that will be executed after any per-mailbox rules are executed. This is usually better for settings that you do not want users to override. +* :code:`before.rules`: MailScript rules that will be executed before any per-mailbox rules. This is usually better for default settings that users may override, such as moving spam to Junk. +* :code:`after.rules`: MailScript rules that will be executed after any per-mailbox rules. This is usually better for settings that you do not want users to override. + +The order with which Sieve and MailScript rules run with respect to each other is consistent between rule engines, e.g. both global Sieve and MailScript "before" rules will run before any per-mailbox rules, which will run before any global "after" rules. The order with which Sieve and MailScript rules are evaluated within a single pass (e.g. the before rules) is not defined and should not be relied upon. + +One special case that can only be handled in MailScript is filtering outbound mail. The Sieve implementation does not currently support this. A special case of outbound filtering that may be useful is refusing acceptance of spam in a multi-server mail network. If your primary incoming mail server runs SpamAssassin (as recommended), but user mailboxes reside on another server downstream, then normal user mail filtering to outright refuse definite spam messages normally wouldn't be performed until the message is delivered to the local mail server, by which time the incoming server has already accepted the message, only for it later to be rejected, requiring a bounce to be generated. To work around this, you can configure a global MailScript rule to refuse acceptance of confirmed spam. The downside to this approach is users no longer have the ability to override this, since the rule is being run on a different server. Therefore, use caution and only refuse messages with a very high probability of being spam (e.g. spam score of 10 or greater). This could be done as follows:: + + # This MailScript rule will reject any messages with a spam score of 10 or greater + RULE + MATCH DIRECTION OUT + MATCH HEADER X-Spam-Score >= 10 + ACTION REJECT + ENDRULE + +Note this is similar to an above rule, except the direction is :code:`OUT`. For incoming mail, this rule will only be executed for mail that is accepted and sent onwards to another server. (It could also apply for local submissions that are sent to external parties, though such mail shouldn't have an :code:`X-Spam-Score` header at this point, so this is unlikely to cause an issue.) Note, however, that not all filter actions apply to all mail; for example, in the case of mail accepted by an edge server and then relayed to another server housing the actual mailbox, no mailbox exists locally on the edge server for the message while it is being processed. Mailbox rules thus cannot be run on these messages, but global rules (before/after Sieve and MailScript rules) can still be run. Certain actions, like REJECT, can be used without issue, while some actions, such as :code:`fileinto` (Sieve) or :code:`MOVETO` (MailScript) cannot be used since there is no corresponding mailbox within which to move messages (if such rules are, they will be ignored and trigger a warning). Currently, in Sieve, it is not actually possible to target such mail; in MailScript, the condition :code:`MATCH DIRECTION IN` should currently suffice to ensure the rule is skipped for non-mailbox mail. -SpamAssassin will tag spam appropriately, but not do anything to it. That's where Sieve rules can help filter spam to the right place (or even reject it during the SMTP session). There are a few headers that SpamAssassin will add, e.g. :code:`X-Spam-Status`. Users can customize what they want to do with spam and their threshold for spam filtering using a Sieve rule. The most common rule is to move suspected spam to the user's Junk folder. +The :code:`mod_smtp_recipient_monitor` plugin module also performs outbound filtering. If the :code:`.config/.recipientmap` file exists in a user's home directory, this module will automatically screen outbound mail and warn the user if sending mail to a brand-new from/recipient combination. This can help prevent mail from accidentally being sent to the wrong users, or from the wrong email address. + +Training +-------- + +SpamAssassin can work reasonably well out of the box, but will get better with training. It is best trained on real spam (and ham, or non-spam) messages. You can tell SpamAssassin about actual spam (:code:`sa-learn --spam /path/to/spam/folder`) or ham (:code:`sa-learn --ham /path/to/ham/folder`). + +If you receive spam, don't delete them - put them in a special folder (e.g. Junk) and rerun :code:`sa-learn` periodically. + +You can also run on multiple folders - careful though, if users have a filter to move suspected spam to Junk, this could train on false positives if this is run before they react and correct that. Therefore, if your mail server is small, you may just want to do this manually periodically after receiving Spam:: + + sa-learn --spam /home/bbs/maildir/*/Junk/{cur,new} + sa-learn --ham /home/bbs/maildir/*/cur + +Once you've trained the Bayes model, you can delete the spam messages if you wish. Rerunning the model on existing messages is fine too - the model will skip messages it's already seen, so there's no harm in not deleting them immediately, if you have the disk space. Email sent from the BBS keeps going to people's spam! ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -558,7 +645,7 @@ You *could* use RFC 4468 BURL, but this is not supported by virtually any mail c The recommended setting is to use MailScript rules to "filter" your outgoing emails. You can define a rule for each account to save a copy in your IMAP server's Sent folder. -For your local BBS email account, you can use :code:`MOVETO .Sent`; for remote IMAP servers, +For your local BBS email account, you can use :code:`MOVETO Sent`; for remote IMAP servers, you can specify an IMAP URL like :code:`MOVETO imaps://username@domain.com:password@imap.example.com:993/Sent`. The BBS's SMTP server will then save a copy of the message in the designated location before relaying or sending it. diff --git a/configs/.rules b/configs/.rules index 1eeeb5d..140bcac 100644 --- a/configs/.rules +++ b/configs/.rules @@ -5,8 +5,9 @@ # By default, rules apply to ALL messages, and you need to use MATCH directives to explicitly # filter to the rules you want. # Rules are evaluated in a single ordered pass from top to bottom. -# The global rules file (e.g. /home/bbs/maildir/.rules) is executed first, followed by -# any .rules file in each user's individual maildir (e.g. /home/bbs/maildir/1/.rules) +# The global before rules file (e.g. /home/bbs/maildir/before.rules ) is executed first, followed by +# any .rules file in each user's individual maildir (e.g. /home/bbs/maildir/1/.rules), +# followed by the global after rules file (e.g. /home/bbs/maildir/after.rules). # Rule processing occurs once the entire message has been received and before delivery is attempted. # WARNING: DO NOT ALLOW USERS to directly create or modify these rules. @@ -40,15 +41,23 @@ # DIRECTION - direction of message. IN=incoming message for a local recipient, OUT=message sent by a local user for delivery (either locally or externally) # MAILFROM - envelope MAIL FROM (envelope sender) # RECIPIENT - envelope RCPT TO (envelope recipient) -# HEADER
- header in the message. Since certain headers may be duplicated (To:, Cc:, etc.), this is a match on ANY of these headers. +# HEADER
=|>|<|<=|==> - header in the message. Since certain headers may be duplicated (To:, Cc:, etc.), this is a match on ANY of these headers. # FILE - whether a named file exists in the user's maildir. May be used for both incoming and outgoing messages. # RETVAL (>,>=,==,<,<=) - check return value of last command # SIZE (>,>=,==,<,<=) - size of message, in bytes # Condition Keywords: +# EXISTS - specified value exists # EQUALS - exact string match +# CONTAINS - contains substring # LIKE - regular expression match -# EXISTS - specified value exists +# >= - greater than or equal to +# > - greater than +# <= - less than or equal to +# < less than +# == equal to + +# Note that floating point comparisons are supported for HEADER conditions, but not for RETVAL or SIZE. # Other Keywords: # RETVAL - return code of previous TEST or ACTION statement, useful for conditional execution of certain MATCH or ACTION statements. @@ -57,15 +66,18 @@ # List of actions # BREAK - Stop executing the current rule # RETURN - Stop executing all rules in the current rules file -# EXIT - Stop executing all rules in all rules files -# BOUNCE - Reject SMTP acceptance of the message and return an SMTP error code to the sending server/client. Optionally, a custom bounce message may be specified as an argument. -# DROP - Drop the message (prevent delivery). Typically used in conjunction with BOUNCE. If you wish to delete the message but retain it temporarily in the Trash folder, you should use MOVETO .Trash instead. -# FORWARD - Forward the message to another address. Can be used to implement conditional or unconditional forwarding. +# EXIT - Stop executing all rules in all rules files. May not be used in mailbox rules, can only be used in the global rules. Use with caution, you probably should use BREAK or RETURN instead! +# BOUNCE - Return an SMTP error code to the sending server/client. Optionally, a custom bounce message may be specified as an argument. This rule does not reject acceptance on its own. Use with caution, you probably want to use REJECT instead unless you are trying to be clever. +# REJECT - Same as Sieve 'reject' action. Reject SMTP acceptance of the message and return an SMTP error code to the sending server/client. Optionally, a custom bounce message may be specified as an argument. +# DISCARD - Same as Sieve 'discard' action. Drop and discard the message (prevent delivery). This rule does not trigger a reject message on its own. If you wish to delete the message but retain it temporarily in the Trash folder, you should use MOVETO Trash instead. This rule is typically not implicit, only the REJECT action will automatically discard a message as well. +# REDIRECT - Similar to the Sieve 'redirect' action. Forward the message to another address. Can be used to implement conditional or unconditional forwarding. +# Unlike the Sieve 'redirect' action, "keep copy" is implicit. Additionally, it can be used multiple times to forward copies to multiple destinations. +# To drop the message after forwarding it, also use the DISCARD action afterwards. # RELAY - Outgoing messages only. Relay the message via another SMTP server. Useful for SMTP proxying. Format is smtp:// or smtps://user:password@host:port # Note that STARTTLS is always attempted for smtp://, while smtps:// will force Implicit TLS to be used, and smtp:// will use Explicit TLS if possible. -# Currently, RELAY implicitly results in a DROP, normal message processing will not continue afterwards. +# Currently, RELAY implicitly results in a DISCARD, normal message processing will not continue afterwards and no copy of the message is retained. # REPLY - Reply to the sent message, to the original sender -# MOVETO - Move the message to a specified folder in the maildir, e.g. to .Junk, .Trash, etc. Can be used to implement filtering. If this action is executed multiple times, the last one wins. For outgoing messages, an IMAP URI may also be used, e.g. to APPEND the sent message to a remote IMAP mailbox. +# MOVETO - Move the message to a specified folder in the maildir, e.g. to Junk, Trash, Folder.subfolder, etc. Can be used to implement filtering. If this action is executed multiple times, the last one wins. For outgoing messages, an IMAP URI may also be used, e.g. to APPEND the sent message to a remote IMAP mailbox. # EXEC - Execute a system program. # NOOP - Does nothing and always returns 0. Possibly useful for debugging rule execution with debug enabled. @@ -77,11 +89,11 @@ COMMENT # Reject all messages, incoming and outgoing RULE -# You can use BOUNCE or DROP in isolation but typically these are used together -# BOUNCE used by itself would send a bounce but actually deliver the message -# DROP used by itself would drop the message but not send a bounce -ACTION BOUNCE This message is not allowed # Send a custom bounce message -ACTION DROP # Drop the message +# You can use REJECT or DISCARD in isolation but typically these are used together +# REJECT used by itself would send a bounce and not deliver the message +# DISCARD used by itself would silently drop the message and not send a bounce +ACTION REJECT This message is not allowed # Send a custom bounce message +ACTION DISCARD # Drop and discard the message ENDRULE RULE @@ -90,8 +102,8 @@ MATCH SIZE >= 700 # greater than 700 bytes MATCH NOT HEADER Precedence EQUALS Bulk # if Precedence header is not bulk MATCH HEADER Precedence EXISTS # ... and it's present (not missing) # if all these rules match: -ACTION FORWARD # forward the message to sysop@example.com -ACTION MOVETO .Trash # delete the message after forwarding it +ACTION REDIRECT # forward the message to sysop@example.com +ACTION MOVETO Trash # delete the message after forwarding it ACTION BREAK # stop processing the CURRENT rules. In this example, this isn't meaningful, but if there were further actions it would be. ENDRULE @@ -108,19 +120,38 @@ ENDIF IF RETVAL == 0 ACTION BREAK ENDIF -# This rule will bounce and drop the message if it came from <1|2|3>@example.com -ACTION BOUNCE -ACTION DROP +# This rule will reject the message if it came from <1|2|3>@example.com +ACTION REJECT +ENDRULE + +# Forward any messages to forwarder@example.com smaller than 1 KB to secret@example.net +RULE +MATCH RECIPIENT EQUALS forwarder@example.com +IF SIZE > 1024 + ACTION REJECT "Message is too large" + ACTION BREAK # This is necessary! If the explicit BREAK is missing, we will end up forwarding a copy nonetheless! REJECT is implicit drop, it does NOT implicitly stop processing rules! +ENDIF +ACTION REDIRECT ENDRULE # A simple spam filter RULE MATCH DIRECTION IN # all incoming email (you don't want to spam filter the mail you send, right?) +MATCH HEADER X-Spam-Level CONTAINS ***** +IF NOT RETVAL 0 + ACTION MOVETO Junk + ACTION RETURN # stop processing ALL rules immediately (why bother if it's spam?) +ENDIF +ENDRULE + +# Alternate simple spam filter +RULE +MATCH DIRECTION IN # all incoming email (you don't want to spam filter the mail you send, right?) MATCH NOT FILE .nospamfilter MISSING # if .nospamfilter file is present in a user's root maildir folder, don't do any spam filtering MATCH NOT HEADER From LIKE ^[A-Za-z0-9._%+-]+@safe\.example\.com$ # safe/allowed sender ACTION EXEC spamassasin -e ${MAILFILE} # run spamassasin on this file, which will return nonzero if it's spam IF NOT RETVAL 0 - ACTION MOVETO .Junk + ACTION MOVETO Junk ACTION RETURN # stop processing ALL rules immediately (why bother if it's spam?) ENDIF ENDRULE @@ -150,12 +181,12 @@ ENDRULE # Prevent accidentally sending emails to the wrong people. e.g. the same message should never contain a@example.com and b@example.org. # You can see how you could use this to prevent accidentally replying all to email lists you never intend to post to, etc. which could be quite handy! +# mod_smtp_recipient_monitor does more in-depth sender/recipient message analysis RULE MATCH DIRECTION OUT MATCH HEADER To EQUALS a@example.com MATCH HEADER To EQUALS b@example.org -ACTION BOUNCE # send bounce message -ACTION DROP # drop the message, prevent it from being sent. +ACTION REJECT # reject message, prevent it from being sent ENDRULE ENDCOMMENT diff --git a/include/net_smtp.h b/include/net_smtp.h index 98934f3..2eccd13 100644 --- a/include/net_smtp.h +++ b/include/net_smtp.h @@ -230,6 +230,12 @@ int smtp_message_quarantinable(struct smtp_session *smtp); #define SMTP_MSG_DIRECTION_IN 0 #define SMTP_MSG_DIRECTION_OUT 1 +enum msg_process_iteration { + FILTER_BEFORE_MAILBOX = 0, /*!< Execute before the mailbox filters */ + FILTER_MAILBOX, /*!< Mailbox filter execution */ + FILTER_AFTER_MAILBOX, /*!< Execute after the mailbox filters */ +}; + struct smtp_msg_process { /* Inputs */ struct smtp_session *smtp; /*!< SMTP session. Not originally included, so try to avoid using this! */ @@ -246,8 +252,9 @@ struct smtp_msg_process { unsigned int userid; /*!< User ID (outgoing only) */ enum smtp_direction dir; /*!< Full direction (IN, OUT, or SUBMIT) */ unsigned int direction:1; /*!< 0 = incoming, 1 = outgoing */ + enum msg_process_iteration iteration; /*!< Which processing pass this is */ /* Outputs */ - unsigned int bounce:1; /*!< Whether to send a bounce */ + unsigned int bounce:1; /*!< Whether to send a bounce. This on its own does not also implicitly drop the message, that bit must be explicitly set. */ unsigned int drop:1; /*!< Whether message should be dropped */ int res; /*!< General return code */ char *newdir; /*!< New message location (incoming only) */ @@ -260,7 +267,7 @@ struct smtp_msg_process { void smtp_mproc_init(struct smtp_session *smtp, struct smtp_msg_process *mproc); /*! - * \brief Register an SMTP processor callback to run on each message received or sent + * \brief Register an SMTP processor callback to run on each message received or sent. Callback will be called 3x, once for each msg_process_order. * \param cb Callback that should return nonzero to stop processing further callbacks */ #define smtp_register_processor(cb) __smtp_register_processor(cb, BBS_MODULE_SELF) @@ -272,6 +279,8 @@ int smtp_unregister_processor(int (*cb)(struct smtp_msg_process *mproc)); /*! * \brief Run SMTP callbacks for a message (only called by net_smtp) + * \param mproc + * \retval 0 to continue, -1 to abort transaction immediately */ int smtp_run_callbacks(struct smtp_msg_process *mproc); @@ -282,6 +291,20 @@ struct smtp_response { const char *reply; }; +/*! + * \brief Wrapper around smtp_run_callbacks, for use by delivery handlers + * \param smtp + * \param mproc Does not need to be (and should not be) initialized + * \param mbox Mailbox or NULL + * \param resp + * \param dir + * \param recipient Recipient, with <> + * \param datalen Message size + * \param freedata + * \retval 0 to continue, nonzero to return the return value + */ +int smtp_run_delivery_callbacks(struct smtp_session *smtp, struct smtp_msg_process *mproc, struct mailbox *mbox, struct smtp_response **restrict resp, enum smtp_direction dir, const char *recipient, size_t datalen, void **freedata); + #define smtp_abort(r, c, sub, msg) \ r->code = c; \ r->subcode = #sub; \ diff --git a/modules/mod_mailscript.c b/modules/mod_mailscript.c index 92e74c8..a12902b 100644 --- a/modules/mod_mailscript.c +++ b/modules/mod_mailscript.c @@ -23,6 +23,7 @@ #include #include #include +#include #include "include/module.h" #include "include/system.h" @@ -69,8 +70,19 @@ static int numcmp(char *s, int num) return match; } +enum match_type { + MATCH_STRICT = 0, + MATCH_SUBSTR, + MATCH_REGEX, + MATCH_GTE, + MATCH_GT, + MATCH_LTE, + MATCH_LT, + MATCH_EQ, +}; + /*! \brief retval -1 if no such header, 0 if not found, 1 if found */ -static int header_match(struct smtp_msg_process *mproc, const char *header, const char *find, int strict) +static int header_match(struct smtp_msg_process *mproc, const char *header, const char *find, enum match_type matchtype) { int found = -1; size_t findlen = 0; @@ -111,7 +123,9 @@ static int header_match(struct smtp_msg_process *mproc, const char *header, cons start++; ltrim(start); bbs_strterm(start, '\r'); - if (strict) { + /*! \todo BUGBUG FIXME Technically, header values could cross line boundaries for multi-line headers, + * but the logic here wouldn't match if a string is split by CR LF. */ + if (matchtype == MATCH_STRICT) { /* Exact match, easy */ if (!findlen) { findlen = strlen(find); @@ -128,7 +142,14 @@ static int header_match(struct smtp_msg_process *mproc, const char *header, cons } else { break; } - } else { /* Things like "CONTAINS" can be done with LIKE... technically even EQUALS could be, too... */ + } else if (matchtype == MATCH_SUBSTR) { + /* Things like "CONTAINS" can be done with LIKE... (it's just a subset of it), technically even EQUALS could be, too... + * However, this is obviously more efficient. */ + found = strstr(start, find) ? 1 : 0; + if (found) { + break; + } + } else if (matchtype == MATCH_REGEX) { /* Use a regular expression for LIKE */ if (!regcompiled) { int errcode; @@ -144,6 +165,40 @@ static int header_match(struct smtp_msg_process *mproc, const char *header, cons if (found) { break; } + } else { /* numeric comparisons */ + double threshold, actual, diff; + /* We don't use numcmp, since we need to support floating point numbers */ + if (sscanf(find, "%4lf", &threshold) != 1) { + bbs_warning("Invalid numeric value: %s\n", find); + } else { + int count; + actual = 0; + count = sscanf(find, "%4lf", &actual); + bbs_debug(5, "Found: %d, Threshold: %f, Actual: %f (%s)\n", found, threshold, actual, find); + switch (matchtype) { + case MATCH_GTE: + found = count == 1 && actual >= (threshold - DBL_EPSILON); + break; + case MATCH_GT: + found = count == 1 && actual > (threshold - DBL_EPSILON); + break; + case MATCH_LTE: + found = count == 1 && actual <= (threshold + DBL_EPSILON); + break; + case MATCH_LT: + found = count == 1 && actual < (threshold + DBL_EPSILON); + break; + case MATCH_EQ: + diff = actual - threshold; + if (diff < 0) { + diff = -diff; + } + found = count == 1 && diff < DBL_EPSILON; /* Floating point equality comparison */ + break; + default: + bbs_soft_assert(0); /* Shouldn't be reached */ + } + } } } if (regcompiled) { @@ -156,6 +211,8 @@ static void __attribute__ ((nonnull (2, 3, 4))) str_match(const char *matchtype, { if (!strcasecmp(matchtype, "EQUALS")) { *match = !strcmp(a, expr); + } else if (!strcasecmp(matchtype, "CONTAINS")) { + *match = strstr(a, expr) ? 1 : 0; } else if (!strcasecmp(matchtype, "LIKE")) { regex_t regexbuf; int errcode; @@ -222,15 +279,39 @@ static int test_condition(struct smtp_msg_process *mproc, int lineno, int lastre matchtype = strsep(&s, " "); expr = s; if (!strcasecmp(matchtype, "EXISTS")) { - found = header_match(mproc, header, NULL, 1); + found = header_match(mproc, header, NULL, MATCH_STRICT); match = found >= 0; } else if (!strcasecmp(matchtype, "EQUALS")) { REQUIRE_ARG(expr); - found = header_match(mproc, header, expr, 1); + found = header_match(mproc, header, expr, MATCH_STRICT); + match = found == 1; + } else if (!strcasecmp(matchtype, "CONTAINS")) { + REQUIRE_ARG(expr); + found = header_match(mproc, header, expr, MATCH_SUBSTR); match = found == 1; } else if (!strcasecmp(matchtype, "LIKE")) { REQUIRE_ARG(expr); - found = header_match(mproc, header, expr, 0); + found = header_match(mproc, header, expr, MATCH_REGEX); + match = found == 1; + } else if (!strcmp(matchtype, ">=")) { + REQUIRE_ARG(expr); + found = header_match(mproc, header, expr, MATCH_GTE); + match = found == 1; + } else if (!strcmp(matchtype, ">")) { + REQUIRE_ARG(expr); + found = header_match(mproc, header, expr, MATCH_GT); + match = found == 1; + } else if (!strcmp(matchtype, "<=")) { + REQUIRE_ARG(expr); + found = header_match(mproc, header, expr, MATCH_LTE); + match = found == 1; + } else if (!strcmp(matchtype, "<")) { + REQUIRE_ARG(expr); + found = header_match(mproc, header, expr, MATCH_LT); + match = found == 1; + } else if (!strcmp(matchtype, "==")) { + REQUIRE_ARG(expr); + found = header_match(mproc, header, expr, MATCH_EQ); match = found == 1; } else { bbs_warning("Invalid HEADER command match type: %s\n", matchtype); @@ -240,7 +321,14 @@ static int test_condition(struct smtp_msg_process *mproc, int lineno, int lastre char *file = fullfile; REQUIRE_ARG(s); if (*s != '/') { - snprintf(fullfile, sizeof(fullfile), "%s/%s", usermaildir, s); + if (usermaildir) { + snprintf(fullfile, sizeof(fullfile), "%s/%s", usermaildir, s); + } else { + /* This is a system rule not associated with a particular mailbox, + * so we can't process this rule. */ + bbs_warning("Path '%s' is invalid in this context. Relative paths can only be used for mailbox-associated MailScript rules!\n", s); + return 0; /* Not a match */ + } } else { file = s; } @@ -273,7 +361,7 @@ static int do_action(struct smtp_msg_process *mproc, int lineno, char *s) char newdir[512]; REQUIRE_ARG(s); if (!STARTS_WITH(s, "imap:") && !STARTS_WITH(s, "imaps:")) { - /* Doesn't support INBOX */ + /* Doesn't support INBOX (and doesn't need to, that's the default) */ if (mproc->userid) { snprintf(newdir, sizeof(newdir), "%s/%d/.%s", mailbox_maildir(NULL), mproc->userid, s); } else { @@ -285,13 +373,20 @@ static int do_action(struct smtp_msg_process *mproc, int lineno, char *s) } } REPLACE(mproc->newdir, s); - } else if (!strcasecmp(next, "BOUNCE")) { + } else if (!strcasecmp(next, "BOUNCE") || !strcasecmp(next, "REJECT")) { mproc->bounce = 1; + if (!strcasecmp(next, "REJECT")) { + mproc->drop = 1; + } /* for BOUNCE, reject at protocol level, but don't implicitly drop it */ free_if(mproc->bouncemsg); + if (s && *s == '"') { + s++; /* Strip quotes */ + } if (!strlen_zero(s)) { mproc->bouncemsg = strdup(s); + bbs_strterm(mproc->bouncemsg, '"'); /* Strip any trailing quotes */ } - } else if (!strcasecmp(next, "DROP")) { + } else if (!strcasecmp(next, "DISCARD")) { mproc->drop = 1; } else if (!strcasecmp(next, "EXEC")) { int res; @@ -308,12 +403,12 @@ static int do_action(struct smtp_msg_process *mproc, int lineno, char *s) argc = bbs_argv_from_str(argv, ARRAY_LEN(argv), s); /* Parse string into argv */ if (argc < 1 || argc > (int) ARRAY_LEN(argv)) { bbs_warning("Invalid EXEC action\n"); - return -1; /* Rules may rely on a return code of 0 for success, so don't return 0 if we didn't do anything */ + return 1; /* Rules may rely on a return code of 0 for success, so don't return 0 if we didn't do anything */ } EXEC_PARAMS_INIT_HEADLESS(x); res = bbs_execvp(mproc->node, &x, argv[0], argv); /* Directly return the exit code */ return res; - } else if (!strcasecmp(next, "FORWARD")) { + } else if (!strcasecmp(next, "REDIRECT")) { REQUIRE_ARG(s); /* Don't allow forwarding to self, or that will create a loop (one that can't even be detected, since message is not modified) */ if (!stringlist_contains(mproc->forward, s)) { @@ -329,6 +424,13 @@ static int do_action(struct smtp_msg_process *mproc, int lineno, char *s) return 0; } +/*! + * \brief Run rules using a given MailScript rules file + * \param mproc + * \param rulesfile Full path to MailScript to execute + * \param usermaildir ull path to maildir of corresponding mailbox, if exists (could be NULL) + * \retval 0 to continue, -1 to exit rule processing + */ static int run_rules(struct smtp_msg_process *mproc, const char *rulesfile, const char *usermaildir) { int res = 0; @@ -344,7 +446,7 @@ static int run_rules(struct smtp_msg_process *mproc, const char *rulesfile, cons int if_count = 0; if (!bbs_file_exists(rulesfile)) { - bbs_debug(7, "File %s doesn't exist, no rules to evaluate\n", rulesfile); + bbs_debug(7, "MailScript %s doesn't exist\n", rulesfile); return 0; } @@ -427,11 +529,19 @@ static int run_rules(struct smtp_msg_process *mproc, const char *rulesfile, cons } else if (STARTS_WITH(s, "RETURN")) { break; } else if (STARTS_WITH(s, "EXIT")) { - res = -1; + if (mproc->iteration == FILTER_MAILBOX) { + bbs_warning("EXIT not allowed for user mailbox rules, doing RETURN instead\n"); + } else { + /* We only allow EXIT to abort global rules processing + * if this is a systemwide rule. Otherwise, that would + * allow a user to skip after.sieve and after.rules + * from being run, where some actions may be enforced system-wide. */ + res = 1; /* Return 1, not -1, since we don't want to abort the SMTP transaction, just rules processing */ + } break; } else { retval = do_action(mproc, lineno, s); - } + } } else if (STARTS_WITH(s, "IF ")) { int cond, negate = 0; s += STRLEN("IF "); @@ -462,33 +572,42 @@ static int run_rules(struct smtp_msg_process *mproc, const char *rulesfile, cons return res; } +static char before_rules[256]; +static char after_rules[256]; + static int mailscript(struct smtp_msg_process *mproc) { - int res; - char fullfile[265]; - char fullfile2[256]; - const char *usermaildir; + char filepath[256]; + const char *mboxmaildir; - snprintf(fullfile, sizeof(fullfile), "%s/.rules", mailbox_maildir(NULL)); + /* Calculate maildir path, if we have a mailbox */ if (mproc->userid) { - snprintf(fullfile2, sizeof(fullfile2), "%s/%d", mailbox_maildir(NULL), mproc->userid); - usermaildir = fullfile2; + snprintf(filepath, sizeof(filepath), "%s/%d", mailbox_maildir(NULL), mproc->userid); + mboxmaildir = filepath; + } else if (mproc->mbox) { + mboxmaildir = mailbox_maildir(mproc->mbox); } else { - usermaildir = mailbox_maildir(mproc->mbox); - } - res = run_rules(mproc, fullfile, usermaildir); - if (!res) { - snprintf(fullfile, sizeof(fullfile), "%s/.rules", usermaildir); - run_rules(mproc, fullfile, usermaildir); + mboxmaildir = NULL; } - if (mproc->fp) { - fclose(mproc->fp); + + if (mproc->iteration == FILTER_BEFORE_MAILBOX) { + return run_rules(mproc, before_rules, mboxmaildir); + } else if (mproc->iteration == FILTER_AFTER_MAILBOX) { + return run_rules(mproc, after_rules, mboxmaildir); + } else { /* FILTER_MAILBOX */ + char script[263]; + if (!mboxmaildir) { + return 0; /* Can't execute per-mailbox callback if there is no mailbox */ + } + snprintf(script, sizeof(script), "%s/.rules", mboxmaildir); + return run_rules(mproc, script, mboxmaildir); } - return 0; } static int load_module(void) { + snprintf(before_rules, sizeof(before_rules), "%s/before.rules", mailbox_maildir(NULL)); + snprintf(after_rules, sizeof(after_rules), "%s/after.rules", mailbox_maildir(NULL)); return smtp_register_processor(mailscript); } diff --git a/modules/mod_sieve.c b/modules/mod_sieve.c index 3d64e2f..439d996 100644 --- a/modules/mod_sieve.c +++ b/modules/mod_sieve.c @@ -189,6 +189,7 @@ static int my_reject(sieve2_context_t *s, void *varg) bbs_debug(3, "Action: REJECT, message: %s\n", msg); sieve->actiontaken = 1; sieve->mproc->bounce = 1; + sieve->mproc->drop = 1; REPLACE(sieve->mproc->bouncemsg, msg); return SIEVE2_OK; } @@ -510,28 +511,25 @@ sieve2_callback_t sieve_callbacks[] = { { 0 }, /* NULL doesn't work here */ }; -static int sieve(struct smtp_msg_process *mproc) +/*! + * \brief Execute a single Sieve script + * \param mproc + * \param scriptfile Full path to Sieve script to execute + * \retval 0 to continue, -1 to abort rules processing + */ +static int script_exec(struct smtp_msg_process *mproc, const char *scriptfile) { int res; - char fullfile2[256]; - const char *usermaildir; struct sieve_exec sieve; sieve2_context_t *sieve2_context = NULL; - memset(&sieve, 0, sizeof(sieve)); - - if (mproc->userid) { - snprintf(fullfile2, sizeof(fullfile2), "%s/%d", mailbox_maildir(NULL), mproc->userid); - usermaildir = fullfile2; - } else { - usermaildir = mailbox_maildir(mproc->mbox); + if (!bbs_file_exists(scriptfile)) { + bbs_debug(7, "Sieve script %s doesn't exist\n", scriptfile); + return 0; /* Script doesn't exist */ } - snprintf(sieve.scriptpath, sizeof(sieve.scriptpath), "%s/.sieve", usermaildir); - if (!bbs_file_exists(sieve.scriptpath)) { - bbs_debug(5, "No Sieve script %s\n", sieve.scriptpath); - return 0; - } + memset(&sieve, 0, sizeof(sieve)); + safe_strncpy(sieve.scriptpath, scriptfile, sizeof(sieve.scriptpath)); /* libsieve setup */ sieve.mproc = mproc; @@ -556,7 +554,6 @@ static int sieve(struct smtp_msg_process *mproc) bbs_error("sieve2_execute %d: %s\n", res, sieve2_errstr(res)); goto cleanup; } - bbs_debug(4, "Action %s\n", sieve.actiontaken ? "taken" : "not taken"); cleanup: @@ -575,6 +572,45 @@ static int sieve(struct smtp_msg_process *mproc) return 0; } +static char before_rules[256]; +static char after_rules[256]; + +static int sieve(struct smtp_msg_process *mproc) +{ + char filepath[256]; + const char *mboxmaildir; + + if (mproc->direction != SMTP_MSG_DIRECTION_IN) { + return 0; /* Currently, Sieve can only be used for filtering inbound mail. If support for Sieve extension for outbound mail is added, this could change. */ + } + + /* Calculate maildir path, if we have a mailbox */ + if (mproc->userid) { + snprintf(filepath, sizeof(filepath), "%s/%d", mailbox_maildir(NULL), mproc->userid); + mboxmaildir = filepath; + } else if (mproc->mbox) { + mboxmaildir = mailbox_maildir(mproc->mbox); + } else { + mboxmaildir = NULL; + } + + /* Unlike MailScript, we don't use mboxmaildir at all currently, + * apart from computing the mailbox's Sieve script (below) if non-NULL. */ + + if (mproc->iteration == FILTER_BEFORE_MAILBOX) { + return script_exec(mproc, before_rules); + } else if (mproc->iteration == FILTER_AFTER_MAILBOX) { + return script_exec(mproc, after_rules); + } else { /* FILTER_MAILBOX */ + char script[263]; + if (!mboxmaildir) { + return 0; /* Can't execute per-mailbox callback if there is no mailbox */ + } + snprintf(script, sizeof(script), "%s/.sieve", mboxmaildir); + return script_exec(mproc, script); + } +} + static char *get_capabilities(void) { char *caps; @@ -663,6 +699,8 @@ static int load_module(void) /* See libsieve's src/sv_include/sieve2.h */ bbs_warning("Expected SIEVE2_VALUE_LAST to be 27, but was %d?\n", SIEVE2_VALUE_LAST); } + snprintf(before_rules, sizeof(before_rules), "%s/before.sieve", mailbox_maildir(NULL)); + snprintf(after_rules, sizeof(after_rules), "%s/after.sieve", mailbox_maildir(NULL)); if (sieve_register_provider(script_validate, get_capabilities())) { return -1; } diff --git a/modules/mod_smtp_delivery_external.c b/modules/mod_smtp_delivery_external.c index 0bc8dd6..90a9730 100644 --- a/modules/mod_smtp_delivery_external.c +++ b/modules/mod_smtp_delivery_external.c @@ -1133,7 +1133,7 @@ static int process_queue_file(struct mailq_run *qrun, struct mailq_file *mqf) QUEUE_INCR_STAT(processed); static_routes = get_static_routes(mqf->domain); - bbs_debug(2, "Processing message %s (%s -> %s), via %s for '%s'\n", mqf->fullname, mqf->realfrom, mqf->realto, static_routes ? "static route(s)" : "MX lookup", mqf->domain); + bbs_debug(2, "Processing message %s (<%s> -> %s), via %s for '%s'\n", mqf->fullname, mqf->realfrom, mqf->realto, static_routes ? "static route(s)" : "MX lookup", mqf->domain); if (static_routes) { if (stringlist_is_empty(static_routes)) { /* In theory, should never happen */ @@ -1160,7 +1160,7 @@ static int process_queue_file(struct mailq_run *qrun, struct mailq_file *mqf) bbs_warning("Recipient domain %s does not have any MX or A records\n", mqf->domain); /* Just treat as undeliverable at this point and return to sender (if no MX records now, probably won't be any the next time we try) */ /* Send a delivery failure response, then delete the file. */ - bbs_warning("Delivery of message %s from %s to %s has failed permanently (no MX records)\n", mqf->fullname, mqf->realfrom, mqf->realto); + bbs_warning("Delivery of message %s from <%s> to %s has failed permanently (no MX records)\n", mqf->fullname, mqf->realfrom, mqf->realto); /* There isn't any SMTP level error at this point yet, we have to make our own error message for the bounce message */ snprintf(buf, sizeof(buf), "No MX record(s) located for hostname %s", mqf->domain); /* No status code */ smtp_tx_data_reset(&tx); @@ -1185,7 +1185,7 @@ static int process_queue_file(struct mailq_run *qrun, struct mailq_file *mqf) if (!res) { /* Successful delivery. */ bbs_debug(6, "Delivery successful after %d attempt%s, discarding queue file\n", mqf->newretries, ESS(mqf->newretries)); - bbs_smtp_log(4, NULL, "Delivery succeeded after queuing: %s -> %s (%s)\n", mqf->realfrom, mqf->realto, buf); + bbs_smtp_log(4, NULL, "Delivery succeeded after queuing: <%s> -> %s (%s)\n", mqf->realfrom, mqf->realto, buf); smtp_trigger_dsn(DELIVERY_DELIVERED, &tx, &mqf->created, mqf->realfrom, mqf->realto, buf, fileno(mqf->fp), mqf->metalen, mqf->size - mqf->metalen); fclose(mqf->fp); mqf->fp = NULL; /* For parallel task framework, since cleanup is always called */ @@ -1198,7 +1198,7 @@ static int process_queue_file(struct mailq_run *qrun, struct mailq_file *mqf) if (res == -2 || res > 0 || mqf->newretries >= (int) max_retries) { /* Send a delivery failure response, then delete the file. */ bbs_warning("Delivery of message %s from %s to %s has failed permanently after %d retries\n", mqf->fullname, mqf->realfrom, mqf->realto, mqf->newretries); - bbs_smtp_log(1, NULL, "Delivery failed permanently after queuing: %s -> %s (%s)\n", mqf->realfrom, mqf->realto, buf); + bbs_smtp_log(1, NULL, "Delivery failed permanently after queuing: <%s> -> %s (%s)\n", mqf->realfrom, mqf->realto, buf); /* To the dead letter office we go */ /* XXX buf will only contain the last line of the SMTP transaction, since it was using the readline buffer * Thus, if we got a multiline error, only the last line is currently included in the non-delivery report */ @@ -1209,7 +1209,7 @@ static int process_queue_file(struct mailq_run *qrun, struct mailq_file *mqf) QUEUE_INCR_STAT(failed); return 0; } else { - bbs_smtp_log(3, NULL, "Delivery delayed after queuing: %s -> %s (%s)\n", mqf->realfrom, mqf->realto, buf); + bbs_smtp_log(3, NULL, "Delivery delayed after queuing: <%s> -> %s (%s)\n", mqf->realfrom, mqf->realto, buf); mailq_file_punt(mqf); /* Try again later */ smtp_trigger_dsn(DELIVERY_DELAYED, &tx, &mqf->created, mqf->realfrom, mqf->realto, buf, fileno(mqf->fp), mqf->metalen, mqf->size - mqf->metalen); QUEUE_INCR_STAT(delayed); @@ -1663,17 +1663,33 @@ static void *smtp_async_send(void *varg) /*! \brief Accept delivery of a message to an external recipient, sending it now if possible and queuing it otherwise */ static int external_delivery(struct smtp_session *smtp, struct smtp_response *resp, const char *from, const char *recipient, const char *user, const char *domain, int fromlocal, int tolocal, int srcfd, size_t datalen, void **freedata) { + struct smtp_msg_process mproc; + struct smtp_response tmpresp; /* Dummy that gets thrown away, if needed */ #ifndef BUGGY_SEND_IMMEDIATE char buf[256] = ""; #endif - int res = -1; + int res; UNUSED(user); UNUSED(freedata); if (tolocal) { - return 0; + return 0; /* Not for us */ + } + + /* Even though it's not really an "outgoing" message, + * it makes more sense to run callbacks as such here. + * There is no mailbox corresponding to this filter execution, + * so this is purely for global before/after rules + * that may want to target non-mailbox mail. */ + res = smtp_run_delivery_callbacks(smtp, &mproc, NULL, &resp, SMTP_DIRECTION_OUT, recipient, datalen, freedata); + if (res) { + return res; + } + if (!resp) { + resp = &tmpresp; /* We already set the error, don't allow appendmsg to override it if we're not going to drop immediately */ } + res = -1; /* Reset to -1 before continuing */ if (smtp_is_exempt_relay(smtp)) { bbs_debug(2, "%s is explicitly authorized to relay mail from %s\n", smtp_sender_ip(smtp), smtp_from_domain(smtp)); @@ -1864,7 +1880,7 @@ static int relay(struct smtp_session *smtp, struct smtp_msg_process *mproc, int bbs_hostname(), smtp_protname(smtp), timestamp); bbs_debug(5, "Relaying message via %s:%d (user: %s)\n", url.host, url.port, S_IF(url.user)); - /* XXX smtp->recipients is "used up" by try_send, so this relies on the message being DROP'ed as there will be no recipients remaining afterwards + /* XXX smtp->recipients is "used up" by try_send, so this relies on the message being discarded as there will be no recipients remaining afterwards * Instead, we could duplicate the recipients list to avoid this restriction. */ /* XXX A cool optimization would be if the IMAP server supported BURL IMAP and we did a MOVETO, use BURL with the SMTP server */ res = try_send(smtp, &tx, url.host, url.port, STARTS_WITH(url.prot, "smtps"), 1, url.user, url.pass, url.user, NULL, recipients, prepend, (size_t) prependlen, srcfd, 0, datalen, buf, sizeof(buf)); diff --git a/modules/mod_smtp_delivery_local.c b/modules/mod_smtp_delivery_local.c index 0a5ccd3..b978621 100644 --- a/modules/mod_smtp_delivery_local.c +++ b/modules/mod_smtp_delivery_local.c @@ -121,6 +121,7 @@ static void notify_firstmsg(struct mailbox *mbox) /*! * \brief Save a message to a maildir folder * \param smtp SMTP session + * \param[out] resp Custom failure response to send * \param mbox Mailbox to which message is being appended * \param mproc * \param recipient Recipient address (incoming), NULL for saving copies of sent messages (outgoing) @@ -211,14 +212,14 @@ static int do_local_delivery(struct smtp_session *smtp, struct smtp_response *re { struct mailbox *mbox; struct smtp_msg_process mproc; - struct smtp_response tmpresp; - char recip_buf[256]; + struct smtp_response tmpresp; /* Dummy that gets thrown away, if needed */ + int res; UNUSED(from); UNUSED(fromlocal); if (!tolocal) { - return 0; + return 0; /* Not for us */ } mbox = mailbox_get_by_name(user, domain); @@ -241,45 +242,13 @@ static int do_local_delivery(struct smtp_session *smtp, struct smtp_response *re return -1; } - /* recipient includes <>, - * but the mail filtering engines don't want that, - * and just want to consume the address itself. - * XXX Can be revisited if the use of variables with and without <> is ever made consistent! */ - safe_strncpy(recip_buf, recipient, sizeof(recip_buf)); - bbs_strterm(recip_buf, '>'); - - /* SMTP callbacks for incoming messages */ - smtp_mproc_init(smtp, &mproc); - mproc.size = (int) datalen; - mproc.recipient = recip_buf + 1; /* Without <> */ - mproc.dir = SMTP_DIRECTION_IN; - mproc.direction = SMTP_MSG_DIRECTION_IN; - mproc.mbox = mbox; - mproc.userid = 0; - - if (smtp_message_quarantinable(smtp)) { - /* We set the override mailbox before running callbacks, - * because users should have the final say in being able - * to override moving messages to particular mailboxes. - * Moving quarantined messages to "Junk" is just the default. */ - bbs_debug(5, "Message should be quarantined, so initializing destination mailbox to 'Junk'\n"); - mproc.newdir = strdup("Junk"); - } - - if (smtp_run_callbacks(&mproc)) { - return -1; /* If returned nonzero, it's assumed it responded with an SMTP error code as appropriate. */ + res = smtp_run_delivery_callbacks(smtp, &mproc, mbox, &resp, SMTP_DIRECTION_IN, recipient, datalen, freedata); + if (res) { + return res; } - - if (mproc.bounce) { - const char *msg = S_OR(mproc.bouncemsg, "This message has been rejected by the recipient"); - /*! \todo We should allow the filtering engine to set the response code too */ - smtp_abort(resp, 554, 5.7.1, msg); /* XXX Best default SMTP code for this? */ - *freedata = mproc.bouncemsg; /* This is a bit awkward. We still need to use this after we return. Make it net_smtp's problem now. */ + if (!resp) { resp = &tmpresp; /* We already set the error, don't allow appendmsg to override it if we're not going to drop immediately */ } - if (mproc.drop) { - return mproc.bounce ? -1 : 1; /* Silently drop message */ - } return appendmsg(smtp, resp, mbox, &mproc, recipient, srcfd, datalen, NULL, 0) ? -1 : 1; } diff --git a/modules/mod_smtp_filter.c b/modules/mod_smtp_filter.c index 1c17d61..47e54f4 100644 --- a/modules/mod_smtp_filter.c +++ b/modules/mod_smtp_filter.c @@ -130,6 +130,11 @@ static int auth_filter_cb(struct smtp_filter_data *f) return 0; } + /* If we didn't do any checks, don't add the header at all. */ + if (!f->spf && !f->dkim && !f->arc && !f->dmarc) { + return 0; + } + /* Add Authentication-Results header with the results of various tests */ len = asprintf(&buf, "%s" "%s%s%s%s" "%s%s" "%s%s" "%s%s", bbs_hostname(), diff --git a/modules/mod_smtp_filter_arc.c b/modules/mod_smtp_filter_arc.c index c7915fd..b39cd8a 100644 --- a/modules/mod_smtp_filter_arc.c +++ b/modules/mod_smtp_filter_arc.c @@ -149,7 +149,9 @@ static int arc_filter_verify_cb(struct smtp_filter_data *f) } /* All right, we've now verified the message. */ +#ifdef DEBUG_ARC bbs_debug(3, "ARC chain: %s\n", arc_chain_status_str(msg)); +#endif REPLACE(f->arc, arc_chain_status_str(msg)); cleanup: diff --git a/modules/mod_smtp_filter_dkim.c b/modules/mod_smtp_filter_dkim.c index 04c799a..e0f6f95 100644 --- a/modules/mod_smtp_filter_dkim.c +++ b/modules/mod_smtp_filter_dkim.c @@ -219,26 +219,32 @@ static int dkim_verify_filter_cb(struct smtp_filter_data *f) bh = dkim_sig_getbh(sig); /* Body hash */ sigerror = dkim_sig_geterror(sig); +#ifdef DEBUG_DKIM +#define DKIM_DEBUG(level, fmt, ...) bbs_debug(level, fmt, ## __VA_ARGS__) +#else +#define DKIM_DEBUG(level, fmt, ...) +#endif + if (sigflags & DKIM_SIGFLAG_IGNORE) { bbs_warning("DKIM_SIGFLAG_IGNORE\n"); /* Shouldn't happen */ } if (sigflags & DKIM_SIGFLAG_PROCESSED) { - bbs_debug(5, "DKIM_SIGFLAG_PROCESSED\n"); + DKIM_DEBUG(5, "DKIM_SIGFLAG_PROCESSED\n"); } if (sigflags & DKIM_SIGFLAG_PASSED) { - bbs_debug(5, "DKIM_SIGFLAG_PASSED\n"); + DKIM_DEBUG(5, "DKIM_SIGFLAG_PASSED\n"); } if (sigflags & DKIM_SIGFLAG_TESTKEY) { - bbs_debug(5, "DKIM_SIGFLAG_TESTKEY\n"); + DKIM_DEBUG(5, "DKIM_SIGFLAG_TESTKEY\n"); } if (sigflags & DKIM_SIGFLAG_NOSUBDOMAIN) { - bbs_debug(5, "DKIM_SIGFLAG_NOSUBDOMAIN\n"); + DKIM_DEBUG(5, "DKIM_SIGFLAG_NOSUBDOMAIN\n"); } if (bh == DKIM_SIGBH_MATCH) { - bbs_debug(5, "DKIM_SIGBH_MATCH\n"); + DKIM_DEBUG(5, "DKIM_SIGBH_MATCH\n"); } else if (bh == DKIM_SIGBH_MISMATCH) { - bbs_debug(5, "DKIM_SIGBH_MISMATCH\n"); + DKIM_DEBUG(5, "DKIM_SIGBH_MISMATCH\n"); } else { bbs_warning("DKIM_SIGBH_UNTESTED\n"); /* Shouldn't happen */ } @@ -302,7 +308,7 @@ static int dkim_verify_filter_cb(struct smtp_filter_data *f) statp == DKIM_STAT_OK ? "\"" : "" ); - bbs_debug(5, "DKIM result: %s\n", dkimresult); + DKIM_DEBUG(5, "DKIM result: %s\n", dkimresult); REPLACE(f->dkim, dkimresult); cleanup: diff --git a/modules/mod_smtp_filter_dmarc.c b/modules/mod_smtp_filter_dmarc.c index d7e6e78..ccbce20 100644 --- a/modules/mod_smtp_filter_dmarc.c +++ b/modules/mod_smtp_filter_dmarc.c @@ -164,7 +164,7 @@ static int dmarc_filter_cb(struct smtp_filter_data *f) is_ipv6 = !bbs_hostname_is_ipv4(f->node->ip); /* If it's not IPv4, must be IPv6? */ domain = smtp_mail_from_domain(f->smtp); if (!domain) { - bbs_warning("Missing domain for received email?\n"); + /* Will be empty if MAIL FROM is empty */ return 0; } diff --git a/modules/mod_smtp_mailing_lists.c b/modules/mod_smtp_mailing_lists.c index bdd071b..1ab8561 100644 --- a/modules/mod_smtp_mailing_lists.c +++ b/modules/mod_smtp_mailing_lists.c @@ -525,9 +525,12 @@ static int archive_list_msg(const char *listname, int srcfd, size_t msglen) static int blast_exploder(struct smtp_session *smtp, struct smtp_response *resp, const char *from, const char *recipient, const char *user, const char *domain, int fromlocal, int tolocal, int srcfd, size_t datalen, void **freedata) { + struct smtp_msg_process mproc; + struct smtp_response tmpresp; /* Dummy that gets thrown away, if needed */ char name[256]; char *addr, *subaddr; struct mailing_list *l; + int res; UNUSED(recipient); UNUSED(freedata); @@ -536,6 +539,19 @@ static int blast_exploder(struct smtp_session *smtp, struct smtp_response *resp, return 0; } + /* Even though it's not really an "outgoing" message, + * it makes more sense to run callbacks as such here. + * There is no mailbox corresponding to this filter execution, + * so this is purely for global before/after rules + * that may want to target non-mailbox mail. */ + res = smtp_run_delivery_callbacks(smtp, &mproc, NULL, &resp, SMTP_DIRECTION_OUT, recipient, datalen, freedata); + if (res) { + return res; + } + if (!resp) { + resp = &tmpresp; /* We already set the error, don't allow appendmsg to override it if we're not going to drop immediately */ + } + safe_strncpy(name, user, sizeof(name)); subaddr = name; addr = strsep(&subaddr, "+"); @@ -568,7 +584,6 @@ static int blast_exploder(struct smtp_session *smtp, struct smtp_response *resp, if (strlen_zero(subaddr)) { /* It's the reflector address */ char tmpattach[256] = "/tmp/lbbsmailXXXXXX"; - int res; size_t msglen; FILE *fp = bbs_mkftemp(tmpattach, 0600); if (!fp) { diff --git a/modules/mod_smtp_recipient_monitor.c b/modules/mod_smtp_recipient_monitor.c index 3e07c48..13b86eb 100755 --- a/modules/mod_smtp_recipient_monitor.c +++ b/modules/mod_smtp_recipient_monitor.c @@ -78,6 +78,9 @@ static int processor(struct smtp_msg_process *mproc) if (mproc->dir != SMTP_DIRECTION_SUBMIT) { return 0; /* Only applies to user submissions */ } + if (mproc->iteration != FILTER_MAILBOX) { + return 0; /* Only execute for mailboxes, and not before/after as well */ + } if (!mproc->userid) { return 0; /* Only for user-level filters, not global */ } diff --git a/modules/mod_spamassassin.c b/modules/mod_spamassassin.c index 96eb316..7443043 100644 --- a/modules/mod_spamassassin.c +++ b/modules/mod_spamassassin.c @@ -40,6 +40,7 @@ static int spam_filter_cb(struct smtp_filter_data *f) char buf[1024]; struct readline_data rldata; struct bbs_exec_params x; + int headers_written = 0; /* The only thing that this module really does is * execute the SpamAssassin binary, passing it the email message on STDIN, @@ -70,6 +71,27 @@ static int spam_filter_cb(struct smtp_filter_data *f) PIPE_CLOSE(input); return -1; } + + /* See comment from smtp_run_filters, in net_smtp.c. + * TL;DR f->inputfd only gives us the original message, not headers + * appended by other filters that just ran, like SPF, DMARC, etc. + * Unlike most other filters, SpamAssassin needs those headers + * in order to do its job accurately, so we also explicitly + * read whatever is in the output file BEFORE the rest of the message. + * Conveniently, this actually ends up matching the order that the + * headers will be in when the final message is actually written to disk, + * so this faithfully reproduces the message up to this point in time, + * up to the last filter that executed just before this one. */ + if (f->outputfd != -1) { + long int outputbytes = lseek(f->outputfd, 0, SEEK_CUR); /* Get current position to figure out how many bytes have been written thus far */ + spliced = bbs_splice(f->outputfd, input[1], (size_t) outputbytes); /* bbs_splice leaves the file offset intact */ + if (spliced != (ssize_t) outputbytes) { + bbs_error("splice %d -> %d failed (%lu != %ld): %s\n", f->outputfd, input[1], spliced, f->size, strerror(errno)); + res = -1; + goto cleanup; + } + } + /* We cannot use bbs_copy_file because that will attempt to use copy_file_range, * which only works for regular files. * In this case, since the destination is a pipe, we can use splice(2) */ @@ -115,9 +137,17 @@ static int spam_filter_cb(struct smtp_filter_data *f) } /* Don't use isspace because we don't want to count newlines */ if (buf[0] != ' ' && buf[0] != '\t' && !STARTS_WITH(buf, "X-Spam-")) { - break; + if (headers_written) { + break; + } + continue; } smtp_filter_write(f, "%s\r\n", buf); + headers_written++; + } + + if (!headers_written) { + bbs_warning("No X-Spam headers prepended?\n"); } cleanup: diff --git a/modules/mod_webmail.c b/modules/mod_webmail.c index 109f71a..1592c95 100644 --- a/modules/mod_webmail.c +++ b/modules/mod_webmail.c @@ -198,8 +198,13 @@ static void __attribute__ ((format (gnu_printf, 7, 8))) log_mailimap(const char if (r) { /* Usually all the fields of r we print here are NULL, but we should have some tag and response info for debugging */ - __bbs_log(error ? LOG_ERROR : LOG_WARNING, 0, file, lineno, func, "%s [%d: %s]: %d, %s/%s/%s/%s/%s\n", - buf, code, mailimap_strerror(code), last_sent_tag, S_IF(client->imap_response), S_IF(r->rsp_alert), S_IF(r->rsp_parse), S_IF(r->rsp_atom), S_IF(r->rsp_value)); + if (r->rsp_alert || r->rsp_parse || r->rsp_atom || r->rsp_value) { + __bbs_log(error ? LOG_ERROR : LOG_WARNING, 0, file, lineno, func, "%s [%d: %s]: %d, %s/%s/%s/%s/%s\n", + buf, code, mailimap_strerror(code), last_sent_tag, S_IF(client->imap_response), S_IF(r->rsp_alert), S_IF(r->rsp_parse), S_IF(r->rsp_atom), S_IF(r->rsp_value)); + } else { + __bbs_log(error ? LOG_ERROR : LOG_WARNING, 0, file, lineno, func, "%s [%d: %s]: %d, %s\n", + buf, code, mailimap_strerror(code), last_sent_tag, S_IF(client->imap_response)); + } } else { __bbs_log(error ? LOG_ERROR : LOG_WARNING, 0, file, lineno, func, "%s [%d: %s]\n", buf, code, mailimap_strerror(code)); } diff --git a/nets/net_smtp.c b/nets/net_smtp.c index a25f5a6..283e2e1 100644 --- a/nets/net_smtp.c +++ b/nets/net_smtp.c @@ -121,7 +121,11 @@ void bbs_smtp_log(int level, struct smtp_session *smtp, const char *fmt, ...) strftime(datestr, sizeof(datestr), "%Y-%m-%d %T", &logdate); bbs_mutex_lock(&loglock); - fprintf(smtplogfp, "[%s.%03d] %p: ", datestr, (int) now.tv_usec / 1000, smtp); + if (smtp) { + fprintf(smtplogfp, "[%s.%03d] %p: ", datestr, (int) now.tv_usec / 1000, smtp); + } else { + fprintf(smtplogfp, "[%s.%03d] ", datestr, (int) now.tv_usec / 1000); + } va_start(ap, fmt); vfprintf(smtplogfp, fmt, ap); @@ -1520,7 +1524,25 @@ void smtp_run_filters(struct smtp_filter_data *fdata, enum smtp_direction dir) continue; } } - /* Filter applicable to scope */ + + /* Filter applicable to scope, execute it */ + + /*! \note SMTP filters will receive the message by reading it from f->inputfd, + * and then use smtp_filter_add_header or smtp_filter_write + * in order to prepend to the message headers (which is f->outputfile) + * + * The complication is that if we reuse the same output file for all filter callbacks, + * then headers added earlier on are not available to successive filters. + * For example, SpamAssassin needs SPF, DMARC, etc. headers, + * and the priorities of these filters ensure they run before SpamAssassin does. + * But if the headers they add are only in the output file, then SpamAssassin + * won't read them, just by reading from f->inputfd. + * + * While we could close the file and create a new combined file after each iteration, + * that would be somewhat inefficient, given that currently the only filter that requires + * this sort of thing is the SpamAssassin one. Therefore, that module has logic + * to also process what's been appended to the output file as well as the original input file. */ + bbs_debug(4, "Executing %s SMTP filter %s %p...\n", smtp_filter_direction_name(f->direction), smtp_filter_type_name(f->type), f); bbs_module_ref(f->mod, 2); if (f->type == SMTP_FILTER_PREPEND) { @@ -1618,17 +1640,103 @@ int smtp_run_callbacks(struct smtp_msg_process *mproc) int res = 0; struct smtp_processor *proc; +#define EXECUTE_FILTERS(iter)\ + mproc->iteration = iter; \ + RWLIST_TRAVERSE(&processors, proc, entry) { \ + bbs_module_ref(proc->mod, 3); \ + res |= proc->cb(mproc); \ + bbs_module_unref(proc->mod, 3); \ + if (res) { \ + bbs_debug(4, "Message processor returned %d\n", res); \ + goto done; /* Stop processing immediately if a processor returns nonzero */ \ + } \ + } \ + + /* We make 3 passes, so that the postmaster can enforce a hierarchy of filters, + * with some always executing before or after a mailbox's rules. */ RWLIST_RDLOCK(&processors); - RWLIST_TRAVERSE(&processors, proc, entry) { - bbs_module_ref(proc->mod, 3); - res |= proc->cb(mproc); - bbs_module_unref(proc->mod, 3); - if (res) { - break; /* Stop processing immediately if a processor returns nonzero */ - } - } + EXECUTE_FILTERS(FILTER_BEFORE_MAILBOX); + EXECUTE_FILTERS(FILTER_MAILBOX); + EXECUTE_FILTERS(FILTER_AFTER_MAILBOX); +#undef EXECUTE_FILTERS +done: RWLIST_UNLOCK(&processors); - return res; + if (mproc->fp) { + fclose(mproc->fp); + mproc->fp = NULL; + } + return res == -1 ? -1 : 0; /* If we aborted callbacks, 1 was returned, but we should return 0 since most callers just check for nonzero return */ +} + +int smtp_run_delivery_callbacks(struct smtp_session *smtp, struct smtp_msg_process *mproc, struct mailbox *mbox, struct smtp_response **restrict resp_ptr, enum smtp_direction dir, const char *recipient, size_t datalen, void **freedata) +{ + char recip_buf[256]; + struct smtp_response *resp = *resp_ptr; + + /* recipient includes <>, + * but the mail filtering engines don't want that, + * and just want to consume the address itself. + * XXX Can be revisited if the use of variables with and without <> is ever made consistent! */ + safe_strncpy(recip_buf, recipient, sizeof(recip_buf)); + bbs_strterm(recip_buf, '>'); + + /* The local delivery agent (mod_smtp_delivery_local) also runs message processing + * so that individual users' filter rules (Sieve or MailScript) will run. + * For external delivery, the destination mailbox is not local, so it does not make + * sense to run any individual user rules in that case... there is no user! + * However, in both cases, the postmaster may configure system-wide MailScript rules that + * apply to all messages, e.g. refuse acceptance of any messages + * with an X-Spam-Score of 10 or greater, etc. We should run those rules here. */ + smtp_mproc_init(smtp, mproc); + mproc->size = (int) datalen; + bbs_strterm(recip_buf, '>'); + mproc->recipient = recip_buf + 1; /* Without <> */ + /* This is a little bit ambiguous, honestly... + * the message was either incoming or a submission, initially, + * but for non-local delivery, it is really an outgoing at this point. + * This also allows differentiating from local delivery in the global + * MailScript rules: if direction is IN, then it will only apply to local mailboxes. + * If direction is OUT, then it will only apply to messages for non-local mailboxes. */ + mproc->dir = dir; + mproc->direction = dir == SMTP_DIRECTION_OUT ? SMTP_MSG_DIRECTION_OUT : SMTP_MSG_DIRECTION_IN; + /* We only want to run global/system-wide rules, not per-user rules, + * there may not even be an associated local mailbox. */ + mproc->mbox = mbox; + mproc->userid = 0; + + if (dir == SMTP_DIRECTION_IN && smtp_message_quarantinable(smtp)) { /* e.g. DMARC failure */ + /* We set the override mailbox before running callbacks, + * because users should have the final say in being able + * to override moving messages to particular mailboxes. + * Moving quarantined messages to "Junk" is just the default. */ + bbs_debug(5, "Message should be quarantined, so initializing destination mailbox to 'Junk'\n"); + mproc->newdir = strdup("Junk"); + } + + if (smtp_run_callbacks(mproc)) { + return -1; /* If returned nonzero, it's assumed it responded with an SMTP error code as appropriate. */ + } + + /* mod_sieve and mod_mailscript don't check for this and won't prevent it, but this won't work if it happens: */ + if (!mbox && mproc->newdir) { + /* This is a global before/after rule that does not correspond to any mailbox, + * for example a message we accepted that needs to be delivered to another server. + * Since this is something the administrator configured, it isn't inappropriate to warn here, + * since this is an actionable warning. */ + bbs_warning("Messages cannot be moved for non-mailbox destinations\n"); /* fileinto (Sieve) or MOVETO (MailScript), will get ignored */ + } + + if (mproc->bounce) { + const char *msg = S_OR(mproc->bouncemsg, "This message has been rejected by the recipient"); /* Use custom response if provided, default otherwise */ + /*! \todo We should allow the filtering engine to set the response code too (not just the message) */ + smtp_abort(resp, 554, 5.7.1, msg); /* XXX Best default SMTP code for this? */ + *freedata = mproc->bouncemsg; /* This is a bit awkward. We still need to use this after we return. Make it net_smtp's problem now. */ + *resp_ptr = NULL; /* We already set the error, don't let anything else set it afterwards */ + } + if (mproc->drop) { + return mproc->bounce ? -1 : 1; /* Silently drop message */ + } + return 0; } struct smtp_delivery_outcome { @@ -1897,7 +2005,7 @@ static int duplicate_loop_avoidance(struct smtp_session *smtp, char *recipient) { char *tmp = NULL; const char *normalized_recipient; - /* The MailScript FORWARD rule will result in recipients being added to + /* The MailScript REDIRECT rule will result in recipients being added to * the recipients list while we're in this loop. * However the same message is sent to the new target, since we forward the raw message, which means * we can't rely on counting Received headers to detect mail loops (for local users). @@ -1933,6 +2041,8 @@ static int duplicate_loop_avoidance(struct smtp_session *smtp, char *recipient) int smtp_message_quarantinable(struct smtp_session *smtp) { + /* In theory, a filter rule could exist to quarantine mail on DMARC failure, + * but this is builtin for ease of use. */ return smtp->tflags.quarantine; } @@ -1940,7 +2050,6 @@ int smtp_message_quarantinable(struct smtp_session *smtp) static int expand_and_deliver(struct smtp_session *smtp, const char *filename, size_t datalen) { char *recipient; - int res = 0; int srcfd; int total, succeeded = 0; struct smtp_filter_data filterdata; @@ -1985,6 +2094,7 @@ static int expand_and_deliver(struct smtp_session *smtp, const char *filename, s /* A filter has indicated that this message should be rejected. * XXX Currently, this only happens if a DMARC reject occured, so that is hardcoded here for now. */ close(srcfd); + bbs_smtp_log(2, smtp, "Message from <%s> rejected due to policy failure\n", smtp->from); smtp_reply(smtp, 550, 5.7.1, "Message rejected due to policy failure"); return 0; /* Return 0 to inhibit normal failure message, since we already responded */ } else if (filterdata.quarantine) { @@ -2068,17 +2178,20 @@ static int expand_and_deliver(struct smtp_session *smtp, const char *filename, s } if (mres == 1) { /* Delivery or queuing to this recipient succeeded */ - bbs_smtp_log(4, smtp, "Delivery succeeded or queued: %s -> %s\n", smtp->from, recipient); + bbs_smtp_log(4, smtp, "Delivery succeeded or queued: <%s> -> %s\n", smtp->from, recipient); succeeded++; - } else if (res < 0) { /* Includes if the message has no handler */ + } else if (mres < 0) { /* Includes if the message has no handler */ + char bouncemsg[512]; /* Process any error message before unlocking the list. * If there are multiple recipients, we cannot send an SMTP reply * just for one of the recipients (otherwise we might send multiple SMTP responses). * Instead, we have to send a bounce message. * If this is the only recipient, we can bounce at the SMTP level. */ - bbs_smtp_log(2, smtp, "Delivery failed: %s -> %s\n", smtp->from, recipient); + const char *replymsg = S_OR(resp.reply, "Message delivery failed"); + snprintf(bouncemsg, sizeof(bouncemsg), "%d%s%s %s", + resp.code ? resp.code : 451, resp.subcode ? " " : "", S_OR(resp.subcode, ""), replymsg); + bbs_smtp_log(2, smtp, "Delivery failed: <%s> -> %s: %s\n", smtp->from, recipient, bouncemsg); if (total > 1) { - char bouncemsg[512]; struct smtp_delivery_outcome *f; /* Since there's more than one recipient, we need to send a bounce * to the sender. This ensures there is only one SMTP reply at the @@ -2090,9 +2203,6 @@ static int expand_and_deliver(struct smtp_session *smtp, const char *filename, s /* Ideally, this path is avoided as much as possible. Invalid recipients are rejected at RCPT TO stage, * and ideally we should catch all other recipient errors (e.g. out of quota) there, rather than here, * so that the upstream MTA can handle errors for each recipient directly. */ - snprintf(bouncemsg, sizeof(bouncemsg), "%d%s%s %s", - resp.code ? resp.code : 451, resp.subcode ? " " : "", S_OR(resp.subcode, ""), - S_OR(resp.reply, "Message delivery failed")); f = smtp_delivery_outcome_new(recipient, NULL, NULL, resp.subcode, bouncemsg, "x-unix", "end of DATA", DELIVERY_FAILED, NULL); if (ALLOC_SUCCESS(f)) { bounces[numbounces++] = f; @@ -2120,10 +2230,14 @@ static int expand_and_deliver(struct smtp_session *smtp, const char *filename, s smtp_delivery_outcome_free(bounces, numbounces); if (succeeded) { /* If anything succeeded, reply with a 250 OK. We already send individual bounces for the failed recipients. */ + bbs_smtp_log(2, smtp, "Message from <%s> accepted for delivery to %d/%d recipient%s\n", smtp->from, succeeded, total, ESS(total)); smtp_reply(smtp, 250, 2.6.0, "Message accepted for delivery"); } else if (resp.code && !strlen_zero(resp.subcode) && !strlen_zero(resp.reply)) { /* All deliveries failed */ /* We could also send a bounce in this case, but even easier, just do it in the SMTP transaction */ + bbs_smtp_log(2, smtp, "Message from <%s> rejected in full by custom policy: %d %s %s\n", smtp->from, resp.code, resp.subcode, resp.reply); smtp_resp_reply(smtp, resp.code, resp.subcode, resp.reply); + } else { + bbs_soft_assert(0); /* Shouldn't be reachable... */ } RWLIST_UNLOCK(&handlers); /* Can't unlock while resp might still be used, and it's a RDLOCK, so okay */ @@ -2394,13 +2508,13 @@ static int do_deliver(struct smtp_session *smtp, const char *filename, size_t da return 0; /* If returned nonzero, it's assumed it responded with an SMTP error code as appropriate. */ } /* - * For incoming messages, we process MOVETO after DROP, since if we drop, we're probably discarding, + * For incoming messages, we process MOVETO after DISCARD, since if we drop, we're probably discarding, * but here we process it first. * By default, outgoing SMTP submissions aren't saved at all (persistently). * This accounts for the case where we want to save a local copy, * i.e. emulating IMAP's APPEND, but having the SMTP server do it * to the client doesn't have to upload the message twice. - * If we're relaying, we might RELAY and then DROP, so we should save the copy before we abort. + * If we're relaying, we might RELAY and then DISCARD, so we should save the copy before we abort. * * This is the same idea behind BURL IMAP by the way, but BURL IMAP requires both client and server supports, * and BURL support is virtually nonexistent amongst clients - only Trojita supports it. @@ -2524,13 +2638,17 @@ static int do_deliver(struct smtp_session *smtp, const char *filename, size_t da } close_if(srcfd); if (mproc.bounce) { - const char *msg = "This message has been rejected by the sender"; - msg = S_OR(mproc.bouncemsg, msg); - smtp_reply(smtp, 554, 5.7.1, "%s", msg); /* XXX Best default SMTP code for this? */ + smtp_reply(smtp, 554, 5.7.1, "%s", S_OR(mproc.bouncemsg, "This message has been rejected by the sender")); /* XXX Best default SMTP code for this? */ free_if(mproc.bouncemsg); + /* We don't return here, because technically, we allow a bounce message to be sent, + * without actually dropping the message at this point. + * In Sieve filtering, there is no distinction, REJECT will set bounce and drop to 1, + * as will the REJECT action in MailScript. However, the BOUNCE action on its own + * in MailScript can be used to reject the message, outwardly, but continue processing it, + * and even accept it. This flexibility is intended for advanced filter usage. */ } if (mproc.drop) { - /*! \todo BUGBUG For DIRECTION OUT, if we FORWARD, then DROP, we'll just drop here and forward won't happen (same for BOUNCE) */ + /*! \todo BUGBUG For DIRECTION OUT, if we REDIRECT, then DISCARD, we'll just drop here and forward won't happen (same for REJECT) */ bbs_debug(5, "Discarding message and ceasing all further processing\n"); return 0; /* Silently drop message. We MUST do this for RELAYed messages, since we must not allow those to be sent again afterwards. */ } @@ -2810,6 +2928,9 @@ static int handle_data(struct smtp_session *smtp, char *s, struct readline_data } bbs_debug(5, "Handling receipt of %lu-byte message\n", smtp->tflags.datalen); + bbs_smtp_log(5, smtp, "Received message: SIZE=%lu,IP=%s,MAILFROM=<%s>,recipients=%d,hopcount=%d,failcount=%d,8bit=%s,quarantine=%s\n", + smtp->tflags.datalen, smtp->node->ip, smtp->from, smtp->tflags.numrecipients, smtp->tflags.hopcount, smtp->failures, + BBS_YN(smtp->tflags.is8bit), BBS_YN(smtp->tflags.quarantine)); smtp->datafile = template; dres = do_deliver(smtp, template, smtp->tflags.datalen); diff --git a/tests/.rules b/tests/before.rules similarity index 84% rename from tests/.rules rename to tests/before.rules index 0f17c76..6d12af5 100644 --- a/tests/.rules +++ b/tests/before.rules @@ -25,8 +25,7 @@ ENDRULE RULE MATCH DIRECTION IN MATCH HEADER Subject EQUALS Test Subject 2 -ACTION BOUNCE -ACTION DROP +ACTION REJECT ACTION EXIT ENDRULE @@ -34,7 +33,7 @@ RULE MATCH DIRECTION IN MATCH HEADER Subject EQUALS Test Subject 3 ACTION BOUNCE This is a custom bounce message -ACTION DROP +ACTION DISCARD ACTION EXIT ENDRULE @@ -45,7 +44,7 @@ IF RETVAL == 1 TEST HEADER Subject EQUALS Not equal to this IF NOT RETVAL == 0 ACTION MOVETO Trash - ACTION DROP + ACTION DISCARD ENDIF ACTION NOOP These actions should execute ACTION MOVETO Junk @@ -53,7 +52,7 @@ ENDIF COMMENT ACTION NOOP This should not execute ACTION MOVETO Trash - ACTION DROP + ACTION DISCARD ENDCOMMENT ACTION NOOP This should be visible ACTION EXIT @@ -63,14 +62,14 @@ RULE MATCH DIRECTION IN MATCH HEADER SUBJECT EQUALS Test Subject 5 # Header names are case-insensitive MATCH HEADER Cc LIKE ^[A-Za-z0-9._%+-]+@example\.org$ # regex -ACTION DROP +ACTION DISCARD ACTION EXIT ENDRULE RULE MATCH DIRECTION IN MATCH HEADER X-Drop-Message EXISTS -ACTION DROP +ACTION DISCARD ACTION EXIT ENDRULE @@ -80,7 +79,7 @@ MATCH HEADER Subject EQUALS Test Subject 6 # false returns 1, true returns 0 ACTION EXEC false IF RETVAL == 1 - ACTION DROP + ACTION DISCARD ENDIF ENDRULE @@ -88,7 +87,7 @@ RULE MATCH DIRECTION IN MATCH HEADER Subject EQUALS Test Subject 7 MATCH MAILFROM EQUALS external@example.net -ACTION DROP +ACTION DISCARD ACTION EXIT ENDRULE @@ -96,15 +95,15 @@ RULE MATCH DIRECTION IN MATCH HEADER Subject EQUALS Test Subject 8 MATCH MAILFROM EQUALS external@example.net -ACTION FORWARD +ACTION REDIRECT ENDRULE RULE MATCH DIRECTION IN MATCH HEADER Subject EQUALS Test Subject 9 MATCH MAILFROM EQUALS external@example.net -ACTION FORWARD -ACTION FORWARD +ACTION REDIRECT +ACTION REDIRECT ENDRULE RULE @@ -112,7 +111,7 @@ MATCH DIRECTION IN MATCH HEADER Subject EQUALS Test Subject 10 ACTION EXEC echo "Temp file is ${MAILFILE}" IF RETVAL == 0 - ACTION DROP + ACTION DISCARD ENDIF ENDRULE @@ -120,5 +119,5 @@ RULE MATCH DIRECTION IN MATCH HEADER Subject EQUALS Test Subject 11 MATCH FILE .fake -ACTION DROP +ACTION DISCARD ENDRULE diff --git a/tests/test_mailscript.c b/tests/test_mailscript.c index 9df3c29..3a296f7 100644 --- a/tests/test_mailscript.c +++ b/tests/test_mailscript.c @@ -39,7 +39,7 @@ static int pre(void) system("rm -rf /tmp/test_lbbs/maildir"); /* Purge the contents of the directory, if it existed. */ mkdir(TEST_MAIL_DIR, 0700); /* Make directory if it doesn't exist already (of course it won't due to the previous step) */ - system("cp .rules " TEST_MAIL_DIR); + system("cp before.rules " TEST_MAIL_DIR); /* Global before MailScript */ return 0; } @@ -74,7 +74,7 @@ static int run(void) CLIENT_EXPECT_EVENTUALLY(clientfd, "220 "); - /* Test each of the rules in test/.rules, one by one */ + /* Test each of the rules in test/before.rules, one by one */ /* Should be moved to .Junk */ SWRITE(clientfd, "EHLO " TEST_EXTERNAL_DOMAIN ENDL); diff --git a/tests/test_sieve.c b/tests/test_sieve.c index c9664f2..b49acca 100644 --- a/tests/test_sieve.c +++ b/tests/test_sieve.c @@ -77,7 +77,7 @@ static int run(void) CLIENT_EXPECT_EVENTUALLY(clientfd, "220 "); - /* Test each of the rules in test/.sieve, one by one */ + /* Test each of the rules in .sieve, one by one */ /* Note: Everything below here is mostly the same as in test_mailscript * (at least at the time of this coding).