net_smtp: Update message processor interface.

A common thing for each message processor to do is check the direction,
scope, and filter iteration of a processor pass (among other things)
to determine if it wants to handle it. Since these are common checks,
pull these out from individual modules into net_smtp, and have the
processor callback define when it should run using bitmasks for each
of these properties. This is similar to the filter interface, which
already allows bitmasks to be passed along.

Also fix a recent bug where delivery callbacks in mod_smtp_mailing_lists
were always being run, even if the recipient wasn't a mailing list.

Additionally, a lot of noisy debug output has been disabled by default.

include/net_smtp.h

@@ -74,8 +74,8 @@ enum smtp_filter_type {
 };
 
 enum smtp_filter_scope {
-	SMTP_SCOPE_INDIVIDUAL = 0, 	/* Run individually for each recipient of a message */
-	SMTP_SCOPE_COMBINED, 		/* Run once for all recipients of a message */
+	SMTP_SCOPE_INDIVIDUAL = (1 << 0),	/* Run individually for each recipient of a message */
+	SMTP_SCOPE_COMBINED = (1 << 1),		/* Run once for all recipients of a message */
 };
 
 enum smtp_direction {
@@ -84,6 +84,8 @@ enum smtp_direction {
 	SMTP_DIRECTION_OUT = (1 << 2),		/*!< Outgoing mail to another MTA */
 };
 
+#define SMTP_DIRECTION_ALL (SMTP_DIRECTION_SUBMIT | SMTP_DIRECTION_IN | SMTP_DIRECTION_OUT)
+
 /*!
  * \note There are two different "filtering" APIs available,
  * based on the smtp_filter_data (filters) and smtp_msg_process (message processors) structures.
@@ -251,11 +253,13 @@ int smtp_message_quarantinable(struct smtp_session *smtp);
 #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 */
+	FILTER_BEFORE_MAILBOX = (1 << 0),	/*!< Execute before the mailbox filters */
+	FILTER_MAILBOX = (1 << 1),			/*!< Mailbox filter execution */
+	FILTER_AFTER_MAILBOX = (1 << 2), 	/*!< Execute after the mailbox filters */
 };
 
+#define FILTER_ALL_PASSES (FILTER_BEFORE_MAILBOX | FILTER_MAILBOX | FILTER_AFTER_MAILBOX)
+
 struct smtp_msg_process {
 	/* Inputs */
 	struct smtp_session *smtp;	/*!< SMTP session. Not originally included, so try to avoid using this! */
@@ -284,6 +288,13 @@ struct smtp_msg_process {
 	char *relayroute;			/*!< Relay route */
 };
 
+struct smtp_message_processor {
+	int (*callback)(struct smtp_msg_process *mproc); /*!< Callback function to execute to process the message */
+	enum smtp_direction dir; /*!< Direction(s) for which to execute callback */
+	enum smtp_filter_scope scope; /*!< Scope(s) for which to execute callback */
+	enum msg_process_iteration iteration; /*!< Pass(es) for which to execute callback (3 passes are done for each processor) */
+};
+
 /*! \brief Initialize an smtp_msg_process structure for use */
 void smtp_mproc_init(struct smtp_session *smtp, struct smtp_msg_process *mproc);
 
@@ -293,17 +304,19 @@ void smtp_mproc_init(struct smtp_session *smtp, struct smtp_msg_process *mproc);
  */
 #define smtp_register_processor(cb) __smtp_register_processor(cb, BBS_MODULE_SELF)
 
-int __smtp_register_processor(int (*cb)(struct smtp_msg_process *mproc), void *mod);
+int __smtp_register_processor(struct smtp_message_processor *proc, void *mod);
 
 /*! \brief Unregister an SMTP processor previously registered with smtp_register_processor */
-int smtp_unregister_processor(int (*cb)(struct smtp_msg_process *mproc));
+int smtp_unregister_processor(struct smtp_message_processor *proc);
 
 /*!
  * \brief Run SMTP callbacks for a message (only called by net_smtp)
  * \param mproc
  * \retval 0 to continue (some or all callbacks were executed and none returned -1), -1 to abort transaction immediately (because a callback returned -1)
  */
-int smtp_run_callbacks(struct smtp_msg_process *mproc, enum smtp_filter_scope scope);
+#define smtp_run_callbacks(mproc, scope) __smtp_run_callbacks(mproc, scope, __FILE__, __LINE__, __func__)
+
+int __smtp_run_callbacks(struct smtp_msg_process *mproc, enum smtp_filter_scope scope, const char *file, int line, const char *func);
 
 struct smtp_response {
 	/* Response */
@@ -325,8 +338,10 @@ struct smtp_response {
  * \param freedata
  * \retval 0 to continue, -1 if message should be aborted and a failure response generated, 1 if message is being silently dropped
  */
-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, enum smtp_filter_scope scope, const char *recipient, size_t datalen, void **freedata);
+#define smtp_run_delivery_callbacks(smtp, mproc, mbox, resp, dir, scope, recipient, datalen, freedata) __smtp_run_delivery_callbacks(smtp, mproc, mbox, resp, dir, scope, recipient, datalen, freedata, __FILE__, __LINE__, __func__)
+
+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, enum smtp_filter_scope scope, const char *recipient, size_t datalen, void **freedata, const char *file, int line, const char *func);
 
 #define smtp_abort(r, c, sub, msg) \
 	r->code = c; \

modules/mod_mailscript.c

@@ -240,7 +240,10 @@ static int test_condition(struct smtp_msg_process *mproc, int lineno, int lastre
 
 	REQUIRE_ARG(s);/* Empty match implicitly matches anything anyways */
 
+#ifdef EXTRA_DEBUG
 	bbs_debug(7, "Evaluating condition: %s\n", s);
+#endif
+
 	next = strsep(&s, " ");
 	REQUIRE_ARG(s);
 	if (!strcasecmp(next, "NOT")) {
@@ -636,9 +639,12 @@ static int run_rules(struct smtp_msg_process *mproc, const char *rulesfile, cons
 		} else {
 			bbs_warning("Invalid command: %s\n", s);
 		}
+
 		if (!was_skip && skip_rule) { /* Rule statement just evaluated as false */
+#ifdef EXTRA_DEBUG
 			/* We butchered the rule statement with strsep so can't print it out again */
 			bbs_debug(5, "Skipping rule, condition false\n");
+#endif
 		}
 	}
 
@@ -654,15 +660,6 @@ static int mailscript(struct smtp_msg_process *mproc)
 	char filepath[256];
 	const char *mboxmaildir;
 
-	if (mproc->scope != SMTP_SCOPE_INDIVIDUAL) {
-		/* Filters are only run for individual delivery.
-		 * Even global rules should use SMTP_SCOPE_INDIVIDUAL,
-		 * since they could manipulate the mailbox in some way,
-		 * and we don't have a single mailbox if processing
-		 * a message that will get delivered to multiple recipients. */
-		return 0;
-	}
-
 	/* Calculate maildir path, if we have a mailbox */
 	if (mproc->userid) {
 		snprintf(filepath, sizeof(filepath), "%s/%d", mailbox_maildir(NULL), mproc->userid);
@@ -701,16 +698,28 @@ static int mailscript(struct smtp_msg_process *mproc)
 	}
 }
 
+struct smtp_message_processor proc = {
+	.callback = mailscript,
+	.dir = SMTP_DIRECTION_ALL,
+	/* Filters are only run for individual delivery.
+	 * Even global rules should use SMTP_SCOPE_INDIVIDUAL,
+	 * since they could manipulate the mailbox in some way,
+	 * and we don't have a single mailbox if processing
+	 * a message that will get delivered to multiple recipients. */
+	.scope = SMTP_SCOPE_INDIVIDUAL,
+	.iteration = FILTER_ALL_PASSES, /* We handle all passes, with more granular logic in the callback */
+};
+
 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);
+	return smtp_register_processor(&proc);
 }
 
 static int unload_module(void)
 {
-	return smtp_unregister_processor(mailscript);
+	return smtp_unregister_processor(&proc);
 }
 
 BBS_MODULE_INFO_DEPENDENT("SMTP MailScript Engine", "net_smtp.so");

modules/mod_sieve.c

@@ -581,14 +581,6 @@ static int sieve(struct smtp_msg_process *mproc)
 	char filepath[256];
 	const char *mboxmaildir;
 
-	if (mproc->scope != SMTP_SCOPE_INDIVIDUAL) {
-		return 0; /* Filters are only run for individual delivery */
-	}
-
-	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);
@@ -707,6 +699,13 @@ static int script_validate(const char *filename, struct mailbox *mbox, char **er
 }
 #pragma GCC diagnostic pop /* -Wdiscarded-qualifiers */
 
+struct smtp_message_processor proc = {
+	.callback = sieve,
+	.dir = SMTP_DIRECTION_IN, /* Currently, Sieve can only be used for filtering inbound mail. If support for Sieve extension for outbound mail is added, this could change. */
+	.scope = SMTP_SCOPE_INDIVIDUAL, /* Filters are only run for individual delivery */
+	.iteration = FILTER_ALL_PASSES, /* We handle all passes, with more granular logic in the callback */
+};
+
 static int load_module(void)
 {
 	if (SIEVE2_VALUE_LAST != 27) {
@@ -718,13 +717,13 @@ static int load_module(void)
 	if (sieve_register_provider(script_validate, get_capabilities())) {
 		return -1;
 	}
-	return smtp_register_processor(sieve);
+	return smtp_register_processor(&proc);
 }
 
 static int unload_module(void)
 {
 	sieve_unregister_provider(script_validate);
-	return smtp_unregister_processor(sieve);
+	return smtp_unregister_processor(&proc);
 }
 
 BBS_MODULE_INFO_DEPENDENT("RFC5228 Sieve Filtering", "net_smtp.so");

modules/mod_smtp_greylisting.c

@@ -314,22 +314,13 @@ static int processor(struct smtp_msg_process *mproc)
 	time_t now;
 	int spamscore;
 
-	if (mproc->scope != SMTP_SCOPE_COMBINED) {
-		return 0;
-	} else if (mproc->dir != SMTP_DIRECTION_IN) {
-		return 0; /* Only applies to incoming mail */
-	} else if (smtp_is_exempt_relay(mproc->smtp)) {
+	if (smtp_is_exempt_relay(mproc->smtp)) {
 		/* We don't greylist outbound mail, only inbound mail from the Internet.
 		 * Technically, this check is a subset of bbs_ip_is_private_ipv4 below,
 		 * which ignores all private IPs. However, this is a flag check,
 		 * as opposed to parsing the IP address, so in a potentially common case,
 		 * this is much quicker to check now. */
 		return 0;
-	} else if (mproc->iteration != FILTER_BEFORE_MAILBOX) {
-		/* We want to do this on the first pass, before a user's mailbox rules even run.
-		 * Greylist typically takes precedence over most other handling.
-		 * This is still after DATA and after SpamAssassin has run, so we're good. */
-		return 0;
 	} else if (!mproc->smtp || !smtp_node(mproc->smtp)) {
 		bbs_warning("Not an interactive session?\n");
 		return 0;
@@ -473,12 +464,22 @@ static int load_config(void)
 	return 0;
 }
 
+struct smtp_message_processor proc = {
+	.callback = processor,
+	.dir = SMTP_DIRECTION_IN, /* Only applies to incoming mail */
+	.scope = SMTP_SCOPE_COMBINED, /* This is for the message as a whole, not instances of its delivery */
+	/* We want to do this on the first pass, before a user's mailbox rules even run.
+	 * Greylist typically takes precedence over most other handling.
+	 * This is still after DATA and after SpamAssassin has run, so we're good. */
+	.iteration = FILTER_BEFORE_MAILBOX,
+};
+
 static int load_module(void)
 {
 	if (load_config()) {
 		return -1;
 	}
-	if (smtp_register_processor(processor)) {
+	if (smtp_register_processor(&proc)) {
 		return -1;
 	}
 	bbs_cli_register_multiple(cli_commands_greylisting);
@@ -489,7 +490,7 @@ static int unload_module(void)
 {
 	int res;
 	bbs_cli_unregister_multiple(cli_commands_greylisting);
-	res = smtp_unregister_processor(processor);
+	res = smtp_unregister_processor(&proc);
 	RWLIST_WRLOCK_REMOVE_ALL(&greylisted_messages, entry, free);
 	RWLIST_WRLOCK_REMOVE_ALL(&safe_senders, entry, free);
 	return res;

modules/mod_smtp_mailing_lists.c

@@ -539,6 +539,15 @@ static int blast_exploder(struct smtp_session *smtp, struct smtp_response *resp,
 		return 0;
 	}
 
+	safe_strncpy(name, user, sizeof(name));
+	subaddr = name;
+	addr = strsep(&subaddr, "+");
+
+	l = find_list(addr, domain);
+	if (!l) {
+		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,
@@ -556,15 +565,6 @@ static int blast_exploder(struct smtp_session *smtp, struct smtp_response *resp,
 		resp = &tmpresp;
 	}
 
-	safe_strncpy(name, user, sizeof(name));
-	subaddr = name;
-	addr = strsep(&subaddr, "+");
-
-	l = find_list(addr, domain);
-	if (!l) {
-		return 0;
-	}
-
 	/* First, validate what permissions the sending user has for this list */
 	if (!sender_authorized(l, smtp, from)) {
 		bbs_auth("Unauthorized attempt to post to list %s by %s (%s) (fromlocal: %d)\n",

modules/mod_smtp_recipient_monitor.c

@@ -77,18 +77,9 @@ static int processor(struct smtp_msg_process *mproc)
 	struct deferred_attempt *attempt;
 	const char *fromheader;
 
-	if (mproc->dir != SMTP_DIRECTION_SUBMIT) {
-		return 0; /* Only applies to user submissions */
-	}
-	if (mproc->scope != SMTP_SCOPE_COMBINED) {
-		return 0; /* We don't need to do this per-recipient, once for all is good, hence combined */
-	}
 	if (!mproc->userid) {
 		return 0; /* Only for user-level filters, not global */
 	}
-	if (mproc->iteration != FILTER_BEFORE_MAILBOX) {
-		return 0; /* Do on pre-mailbox pass, to nip it in the bud as early as possible. */
-	}
 	if (!mproc->recipients) {
 		bbs_warning("Recipient list not available?\n");
 		return 0;
@@ -248,14 +239,21 @@ static int processor(struct smtp_msg_process *mproc)
 	return -1;
 }
 
+struct smtp_message_processor proc = {
+	.callback = processor,
+	.dir = SMTP_DIRECTION_SUBMIT, /* Only applies to user submissions */
+	.scope = SMTP_SCOPE_COMBINED, /* This is for the message as a whole, not instances of its delivery */
+	.iteration = FILTER_BEFORE_MAILBOX, /* Do on pre-mailbox pass, to nip it in the bud as early as possible. */
+};
+
 static int load_module(void)
 {
-	return smtp_register_processor(processor);
+	return smtp_register_processor(&proc);
 }
 
 static int unload_module(void)
 {
-	int res = smtp_unregister_processor(processor);
+	int res = smtp_unregister_processor(&proc);
 	RWLIST_WRLOCK_REMOVE_ALL(&deferred_attempts, entry, free);
 	return res;
 }