mod_smtp_filter_dmarc: Add DMARC reporting support and integration.

* Facilitate aggregate DMARC reporting using the opendmarc scripts by
  logging DMARC results to a history file that can be consumed by the
  opendmarc scripts.
* Send immediate, individual failure reports (ruf) upon DMARC
  failure, if configured.
* Improve the general robustness of DMARC verification and reporting.

Related fixes and improvements:

* mod_smtp_filter: Fix Authentication-Results header being generated
  prior to DMARC verification.
* net_smtp: More clearly differentiate between MAIL FROM and From
  header address where needed.
* mail.c: Extend mailer API to allow submitting entire RFC822 messages.
* mail.c: Fix missing unlock.
* lock.c: Print lock error if one occurs.

README.rst

@@ -165,6 +165,10 @@ Config files go in :code:`/etc/lbbs` and are as follows:
 
 * :code:`mod_smtp_fetchmail.conf` - SMTP remote message queue management
 
+* :code:`mod_smtp_filter_dkim.conf` - DKIM signing
+
+* :code:`mod_smtp_filter_dmarc.conf` - DMARC verification and reporting
+
 * :code:`mod_smtp_mailing_lists.conf` - Mailing list configuration
 
 * :code:`modules.conf` - module loading settings (to disable a module, you do it here)

bbs/config.c

@@ -290,7 +290,8 @@ int bbs_config_free(struct bbs_config *c)
 	RWLIST_UNLOCK(&configs);
 
 	if (!cfg) {
-		bbs_error("Couldn't find config %s\n", c->name);
+		bbs_error("Couldn't find config\n"); /* c->name might not be valid here so don't print it? */
+		bbs_log_backtrace(); /* So we can see what module this was for */
 	} else {
 		config_free(cfg);
 	}

bbs/lock.c

@@ -177,7 +177,7 @@ int __bbs_mutex_lock(bbs_mutex_t *t, const char *filename, int lineno, const cha
 	res = pthread_mutex_lock(&t->mutex);
 #endif /* DETECT_DEADLOCKS */
 	if (unlikely(res)) {
-		lock_warning("Failed to obtain mutex %s\n", name);
+		lock_warning("Failed to obtain mutex %s: %s\n", name, strerror(res));
 	} else {
 		t->info.lastlocked = time(NULL);
 		if (unlikely(++t->info.owners != 1)) {
@@ -298,7 +298,7 @@ int __bbs_rwlock_rdlock(bbs_rwlock_t *t, const char *filename, int lineno, const
 
 	res = pthread_rwlock_rdlock(&t->lock);
 	if (unlikely(res)) {
-		lock_warning("Failed to obtain rdlock %s\n", name);
+		lock_warning("Failed to obtain rdlock %s: %s\n", name, strerror(res));
 	} else {
 		pthread_mutex_lock(&t->intlock);
 		t->info.lastlocked = time(NULL);
@@ -324,7 +324,7 @@ int __bbs_rwlock_wrlock(bbs_rwlock_t *t, const char *filename, int lineno, const
 
 	res = pthread_rwlock_wrlock(&t->lock);
 	if (unlikely(res)) {
-		lock_warning("Failed to obtain wrlock %s\n", name);
+		lock_warning("Failed to obtain wrlock %s: %s\n", name, strerror(res));
 	} else {
 		/* If wrlock succeeded, we don't need to lock the internal mutex, since there can't be any readers right now */
 		t->info.lastlocked = time(NULL);

bbs/mail.c

@@ -43,9 +43,11 @@
 #include "include/linkedlists.h"
 #include "include/startup.h"
 #include "include/reload.h"
+#include "include/stringlist.h"
 
 struct mailer {
-	int (*mailer)(MAILER_PARAMS);
+	int (*simple_mailer)(SIMPLE_MAILER_PARAMS);
+	int (*full_mailer)(FULL_MAILER_PARAMS);
 	struct bbs_module *module;
 	RWLIST_ENTRY(mailer) entry;
 	unsigned int priority;
@@ -237,13 +239,13 @@ int __attribute__ ((format (gnu_printf, 6, 7))) bbs_mail_fmt(int async, const ch
 	return res;
 }
 
-int __bbs_register_mailer(int (*mailer)(MAILER_PARAMS), void *mod, int priority)
+int __bbs_register_mailer(int (*simple_mailer)(SIMPLE_MAILER_PARAMS), int (*full_mailer)(FULL_MAILER_PARAMS), void *mod, int priority)
 {
 	struct mailer *m;
 
 	RWLIST_WRLOCK(&mailers);
 	RWLIST_TRAVERSE(&mailers, m, entry) {
-		if (m->mailer == mailer) {
+		if (m->simple_mailer == simple_mailer && m->full_mailer == full_mailer) {
 			break;
 		}
 	}
@@ -257,19 +259,29 @@ int __bbs_register_mailer(int (*mailer)(MAILER_PARAMS), void *mod, int priority)
 		RWLIST_UNLOCK(&mailers);
 		return -1;
 	}
-	m->mailer = mailer;
+	m->simple_mailer = simple_mailer;
+	m->full_mailer = full_mailer;
 	m->module = mod;
 	m->priority = (unsigned int) priority;
 	RWLIST_INSERT_SORTED(&mailers, m, entry, priority); /* Insert in order of priority */
 	RWLIST_UNLOCK(&mailers);
 	return 0;
 }
 
-int bbs_unregister_mailer(int (*mailer)(MAILER_PARAMS))
+int bbs_unregister_mailer(int (*simple_mailer)(SIMPLE_MAILER_PARAMS), int (*full_mailer)(FULL_MAILER_PARAMS))
 {
 	struct mailer *m;
 
-	m = RWLIST_WRLOCK_REMOVE_BY_FIELD(&mailers, mailer, mailer, entry);
+	RWLIST_WRLOCK(&mailers);
+	RWLIST_TRAVERSE_SAFE_BEGIN(&mailers, m, entry) {
+		if (m->simple_mailer == simple_mailer && m->full_mailer == full_mailer) {
+			RWLIST_REMOVE_CURRENT(entry);
+			break;
+		}
+	}
+	RWLIST_TRAVERSE_SAFE_END;
+	RWLIST_UNLOCK(&mailers);
+
 	if (!m) {
 		bbs_error("Failed to unregister mailer: not currently registered\n");
 		return -1;
@@ -307,8 +319,11 @@ int bbs_mail(int async, const char *to, const char *from, const char *replyto, c
 
 	/* Hand off the delivery of the message itself to the appropriate module */
 	RWLIST_TRAVERSE(&mailers, m, entry) {
+		if (!m->simple_mailer) {
+			continue;
+		}
 		bbs_module_ref(m->module, 1);
-		res = m->mailer(async, to, from, replyto, errorsto, subject, body);
+		res = m->simple_mailer(async, to, from, replyto, errorsto, subject, body);
 		bbs_module_unref(m->module, 1);
 		if (res >= 0) {
 			break;
@@ -318,3 +333,158 @@ int bbs_mail(int async, const char *to, const char *from, const char *replyto, c
 
 	return res;
 }
+
+static void push_recipients(struct stringlist *restrict slist, int *restrict count, char *restrict s)
+{
+	char *recip, *recips = s;
+
+	if (strlen_zero(recips)) {
+		return;
+	}
+
+	bbs_term_line(recips);
+	while ((recip = strsep(&recips, ","))) {
+		char buf[256];
+		if (strlen_zero(recip)) {
+			continue;
+		}
+		trim(recip);
+		if (strlen_zero(recip)) {
+			continue;
+		}
+		snprintf(buf, sizeof(buf), "<%s>", recip); /* Recipients need to be surrounded by <> */
+		bbs_debug(6, "Adding recipient '%s'\n", buf);
+		stringlist_push(slist, buf);
+		(*count)++;
+	}
+}
+
+int bbs_mail_message(const char *tmpfile, const char *mailfrom, struct stringlist *recipients)
+{
+	struct mailer *m;
+	char mailfrombuf[256];
+	char tmpfile2[256];
+	int res = -1;
+	struct stringlist reciplist;
+
+	if (!mailfrom) {
+		mailfrom = ""; /* Empty MAIL FROM address */
+	} else {
+		const char *tmpaddr = strchr(mailfrom, '<');
+		/* This is just for the MAIL FROM, so just the address, no name */
+		if (tmpaddr) {
+			/* Shouldn't have <>, but if it does, transparently remove them */
+			bbs_strncpy_until(mailfrombuf, tmpaddr + 1, sizeof(mailfrombuf), '>');
+			mailfrom = mailfrombuf;
+		}
+	}
+
+	/* Extract recipients from message if needed */
+	if (!recipients) {
+		int rewrite = 0;
+		int in_header = 0;
+		int recipient_count = 0;
+		char line[1002];
+		/* Parse message for recipients to add.
+		 * Check To, Cc, and Bcc headers. */
+		FILE *fp = fopen(tmpfile, "r");
+		if (!fp) {
+			bbs_error("fopen(%s) failed: %s\n", tmpfile, strerror(errno));
+			return -1;
+		}
+		/* Process each line until end of headers */
+		stringlist_init(&reciplist);
+		while ((fgets(line, sizeof(line), fp))) {
+			if (strlen(line) <= 2) {
+				break;
+			}
+			if (STARTS_WITH(line, "To:")) {
+				push_recipients(&reciplist, &recipient_count, line + STRLEN("To:"));
+				in_header = 1;
+			} else if (STARTS_WITH(line, "Cc:")) {
+				push_recipients(&reciplist, &recipient_count, line + STRLEN("Cc:"));
+				in_header = 1;
+			} else if (STARTS_WITH(line, "Bcc:")) {
+				push_recipients(&reciplist, &recipient_count, line + STRLEN("Bcc:"));
+				in_header = 1;
+				rewrite = 1;
+			}
+			if (line[0] == ' ') {
+				/* Continue previous header */
+				if (in_header) { /* In header we care about */
+					push_recipients(&reciplist, &recipient_count, line + 1);
+				}
+			} else {
+				in_header = 0;
+			}
+		}
+		bbs_debug(4, "Parsed %d recipient%s from message\n", recipient_count, ESS(recipient_count));
+		/* If there are any Bcc headers, we need to remove those recipients,
+		 * and regenerate the message. */
+		if (rewrite) {
+			int inheaders = 1;
+			FILE *fp2;
+			strcpy(tmpfile2, "/tmp/smtpbccXXXXXX");
+			fp2 = bbs_mkftemp(tmpfile2, MAIL_FILE_MODE);
+			if (!fp2) {
+				stringlist_empty_destroy(&reciplist);
+				return -1;
+			}
+			rewind(fp);
+			bbs_debug(2, "Rewriting message since it contains a Bcc header\n");
+			while ((fgets(line, sizeof(line), fp))) {
+				if (inheaders) {
+					if (!strncmp(line, "\r\n", 2)) {
+						inheaders = 0;
+					} else if (STARTS_WITH(line, "Bcc:")) {
+						in_header = 1;
+						continue; /* Skip this line */
+					} else if (line[0] == ' ') {
+						if (in_header) {
+							/* Skip if this is a multiline Bcc header */
+							continue;
+						}
+					} else {
+						in_header = 0;
+					}
+				}
+				/* Copy line */
+				fwrite(line, 1, strlen(line), fp2);
+			}
+			fclose(fp2);
+			fclose(fp);
+			/* Swap the files */
+			bbs_delete_file(tmpfile);
+			tmpfile = tmpfile2;
+		} else {
+			fclose(fp);
+		}
+	}
+
+	/* XXX smtp_inject consumes the stringlist and assumes responsibility for destroying it.
+	 * The code here is in theory set up to try another mailer if the first one fails.
+	 * However, it seems possible that the stringlist could have been modified prior to failure,
+	 * meaning a retry using another mailer wouldn't get the full list.
+	 * In practice, this is not currently an issue, since there are only two mailers,
+	 * and only in net_smtp do we accept a stringlist of recipients.
+	 * However, in the future, especially if that changes, it might be worth duplicating
+	 * the stringlist here prior to each attempt, just to be safe.
+	 * If we did that, we'd also want to destroy the stringlist after the list iteration.
+	 */
+
+	RWLIST_RDLOCK(&mailers);
+	RWLIST_TRAVERSE(&mailers, m, entry) {
+		if (!m->full_mailer) {
+			continue;
+		}
+		bbs_module_ref(m->module, 2);
+		res = m->full_mailer(tmpfile, mailfrom, recipients ? recipients : &reciplist);
+		bbs_module_unref(m->module, 2);
+		if (res >= 0) {
+			break;
+		}
+	}
+	RWLIST_UNLOCK(&mailers);
+
+	return res;
+}

bbs/node.c

@@ -825,7 +825,7 @@ int bbs_interrupt_node(unsigned int nodenum)
 void __bbs_node_interrupt_ack(struct bbs_node *node, const char *file, int line, const char *func)
 {
 	bbs_assert(node->thread == pthread_self());
-	bbs_debug(2, "Node %u acknowledged interrupt at %s:%d %s()\n", node->id, file, line, func);
+	__bbs_log(LOG_DEBUG, 2, file, line, func, "Node %u acknowledged interrupt\n", node->id);
 	node->interruptack = 1;
 }
 

configs/mod_smtp_filter_dmarc.conf

@@ -0,0 +1,47 @@
+; mod_smtp_filter_dmarc.conf
+
+; DMARC enforcement
+[enforcement]
+reject=yes ; Reject messages that fail DMARC if sending domain's policy says to. (Can set to 'no' for debugging)
+quarantine=yes ; Quarantine messages that fail DMARC if sending domain's policy says to. (Can set to 'no' for debugging)
+
+; Note: if you have specified addresses for the rua/ruf parameters of your DMARC records that belong to a different domain,
+; you will need to ensure you have authorized cross-domain delivery of DMARC reports for delivery to succeed.
+; See: https://dmarc.org/2015/08/receiving-dmarc-reports-outside-your-domain/
+;
+; You may find it useful to your own testing to ensure that policy enforcement is as desired.
+; It is best to do this between servers under your control to avoid penalizing a server that "shouldn't" be sending messages.
+; A recommended tool for this is "swaks", which you can easily run on another server, e.g.:
+; $ cd /tmp && wget https://raw.githubusercontent.com/jetmore/swaks/develop/swaks && chmod +x swaks
+; $ ./swaks --from [email protected] --helo example.com --server smtp.example.net --to [email protected]
+;
+; Note for the privacy conscious: DMARC reports can expose information about your email infrastructure that you may not have
+; intended to be public. For example, suppose Alice sends Bob an email. Normally, Bob's email provider (B), will send aggregate DMARC
+; reports to Alice's email server (A). However, suppose Bob forwards his email to another mail provider, C, but doesn't want
+; people to know email sent to B will get forwarded to C's servers. However, C, if configured to send DMARC reports, will send
+; DMARC reports to A (likely showing an SPF fail, but a DKIM pass, assuming B did not modify the message before forwarding it).
+; If A has access to the DMARC reports (say she operates the mail server) and she doesn't know anyone else using mail server C,
+; then she can deduce the emails sent to B were forwarded there.
+; This is not a security vulnerability; it is by design; however, the privacy-conscious may wish to NOT enable DMARC reporting
+; on this server, if you do not want people to know you are forwarding mail here.
+
+; DMARC reporting configuration
+[reporting]
+;reportfailures=yes ; Report DMARC failures to the sending domain (ruf). Messages will be sent from dmarc-noreply@<SMTP hostname>
+                    ; Also known as "forensic" reports. Default is no.
+                    ; Most mail servers no longer send these reports, so you may not want to enable this without a good reason, see:
+                    ; https://dmarcian.com/where-are-the-forensicfailure-reports/ and https://dmarcian.com/where-are-the-forensicfailure-reports/
+;[email protected] ; Bcc the specified address on all failure reports (ruf) that are sent. No default.
+                                      ; This can be useful if you are troubleshooting delivery issues and want to receive copies of any DMARC failure reports.
+
+; History file used for aggregate (rua) DMARC reporting.
+; LBBS does not handle aggregate DMARC reporting itself; the provided Perl reporting scripts
+; in the OpenDMARC project's source tree should be used.
+; This file has the same functionality as the log file denoted by the HistoryFile setting
+; in the native OpenDMARC filter. Using OpenDMARC provided scripts, this file can be periodically
+; imported into a database and further scripts can thence generate the actual DMARC reports.
+; More details here: https://github.com/trusteddomainproject/OpenDMARC/blob/master/opendmarc/opendmarc.conf.sample#L203
+; Default is 'none' (no log file is created by default).
+
+; This is the default path hardcoded into opendmarc-importstats. If you change this, make sure to update that script as well.
+;historyfile=/var/tmp/dmarc.dat