nets, modules: Fix SMTP queuing and other email bugs.

* net_smtp: Fix delivery status notifications with messages for
  other outcomes.
* mod_smtp_delivery_external: Fix regression initializing all
  static route lists to empty.
* mod_smtp_delivery_external: Count messages "mailq" queue stats.
* mod_webmail: Add assertion for libetpan null dereference.
* mod_webmail: Prevent infinite IDLE loop if idle_stop fails.
* mod_smtp_filter_dmarc: Don't verify DMARC on messages from trusted upstreams.

include/net_smtp.h

@@ -150,6 +150,9 @@ int smtp_should_validate_spf(struct smtp_session *smtp);
 /*! \brief Whether DKIM validation should be performed */
 int smtp_should_validate_dkim(struct smtp_session *smtp);
 
+/*! \brief Whether DMARC verification should be performed */
+int smtp_should_verify_dmarc(struct smtp_session *smtp);
+
 /*! \brief Whether this is a message submission */
 int smtp_is_message_submission(struct smtp_session *smtp);
 

modules/mod_smtp_delivery_external.c

@@ -100,8 +100,8 @@ static int add_static_relay(const char *hostname, const char *route)
 	}
 	strcpy(s->data, hostname); /* Safe */
 	s->hostname = s->data;
-	stringlist_push_list(&s->routes, route);
 	stringlist_init(&s->routes);
+	stringlist_push_list(&s->routes, route);
 	RWLIST_INSERT_TAIL(&static_relays, s, entry);
 	return 0;
 }
@@ -905,6 +905,7 @@ static int try_static_delivery(struct smtp_session *smtp, struct smtp_tx_data *t
 	const char *route;
 	struct stringitem *i = NULL;
 	int res = -1; /* Make condition true to start */
+
 	/* Static routes override doing an MX lookup for this domain.
 	 * We have one or more hostnames (with an optionally specified port) to try. */
 	while (res < 0 && (route = stringlist_next(static_routes, &i))) {
@@ -1057,7 +1058,15 @@ static int process_queue_file(struct mailq_run *qrun, struct mailq_file *mqf)
 	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);
 	if (static_routes) {
-		res = try_static_delivery(NULL, &tx, static_routes, mqf->realfrom, mqf->realto, fileno(mqf->fp), (off_t) mqf->metalen, mqf->size - mqf->metalen, buf, sizeof(buf));
+		if (stringlist_is_empty(static_routes)) {
+			/* In theory, should never happen */
+			bbs_error("No static routes available for delivery to %s?\n", mqf->domain);
+			fclose(mqf->fp);
+			mqf->fp = NULL; /* For parallel task framework, since cleanup is always called */
+			return 0;
+		} else {
+			res = try_static_delivery(NULL, &tx, static_routes, mqf->realfrom, mqf->realto, fileno(mqf->fp), (off_t) mqf->metalen, mqf->size - mqf->metalen, buf, sizeof(buf));
+		}
 	} else {
 		struct stringlist mxservers;
 		stringlist_init(&mxservers);
@@ -1432,6 +1441,9 @@ static int on_queue_file_cli_mailq(const char *dir_name, const char *filename, v
 		return 0;
 	}
 
+	/* Count messages in queue */
+	qrun->total++;
+
 	if (skip_qfile(qrun, mqf)) {
 		fclose(mqf->fp);
 		reset_accessed_time(mqf);

modules/mod_smtp_filter_dmarc.c

@@ -157,7 +157,7 @@ static int dmarc_filter_cb(struct smtp_filter_data *f)
 	char mctx_jobid[48];
 	time_t now;
 
-	if (smtp_is_exempt_relay(f->smtp)) {
+	if (smtp_is_exempt_relay(f->smtp) || !smtp_should_verify_dmarc(f->smtp)) {
 		return 0;
 	}
 

modules/mod_webmail.c

@@ -3082,6 +3082,14 @@ static int idle_start(struct ws_session *ws, struct imap_client *client)
 		int res;
 		bbs_debug(5, "Starting IDLE...\n");
 		webmail_log(7, client, "=> IDLE START\n");
+		/* I've seen repeated crashes at idle.c:80 in mailimap_idle in libetpan,
+		 * which suggests a NULL dereference, so check that here.
+		 * If the assert fails, libetpan would have crashed anyways. */
+		if (!client->mailbox) {
+			/* The assertion below will likely trigger as well, if this happens */
+			bbs_error("Attempt to IDLE without an active mailbox?\n");
+		}
+		bbs_assert_exists(client->imap->imap_selection_info);
 		res = mailimap_idle(client->imap);
 		if (res != MAILIMAP_NO_ERROR) {
 			bbs_warning("Failed to start IDLE: %s\n", maildriver_strerror(res));
@@ -3238,12 +3246,10 @@ static int on_poll_activity(struct ws_session *ws, void *data)
 			return -1;
 		}
 		bbs_debug(5, "Not currently idling, ignoring...\n");
-		idle_stop(ws, client);
-		return idle_start(ws, client);
+		return idle_stop(ws, client) || idle_start(ws, client);
 	} else if (strlen_zero(client->mailbox)) {
 		bbs_error("Client mailbox not set?\n");
-		idle_stop(ws, client);
-		return idle_start(ws, client);
+		return idle_stop(ws, client) || idle_start(ws, client);
 	}
 
 	/* IDLE activity! */
@@ -3280,6 +3286,7 @@ static int on_poll_activity(struct ws_session *ws, void *data)
 	if (res) {
 		if (idle_stop(ws, client)) {
 			bbs_warning("Failed to stop IDLE, terminating webmail session\n");
+			return -1;
 		}
 	}
 

nets/net_imap/imap_client_status.c

@@ -189,7 +189,7 @@ static int __parse_status_item(const char *full, const char *keyword, size_t key
 	return 0;
 }
 
-static int status_size_fetch_incremental(struct imap_client *client, const char *tag, size_t *mb_size, const char *old, const char *new)
+static int status_size_fetch_incremental(struct imap_client *client, const char *remotename, const char *tag, size_t *mb_size, const char *old, const char *new)
 {
 	ssize_t res;
 	size_t taglen;
@@ -263,7 +263,7 @@ static int status_size_fetch_incremental(struct imap_client *client, const char
 	}
 
 	/* We can do an incremental fetch! */
-	bbs_debug(7, "No messages expunged since last time, %d added (MESSAGES %d -> %d, UIDNEXT %d -> %d)\n", added, oldmessages, newmessages, oldnext, newnext);
+	bbs_debug(7, "No messages expunged in %s since last time, %d added (MESSAGES %d -> %d, UIDNEXT %d -> %d)\n", remotename, added, oldmessages, newmessages, oldnext, newnext);
 
 	/* imap->tag gets reused multiple times for different commands here...
 	 * something we SHOULD not do but servers are supposed to (MUST) tolerate. */
@@ -279,7 +279,7 @@ static int status_size_fetch_incremental(struct imap_client *client, const char
 			return -1;
 		}
 		if (!strncmp(buf, tag, taglen)) {
-			bbs_debug(3, "End of FETCH response: %s\n", buf);
+			bbs_debug(3, "End of FETCH response for '%s': %s\n", remotename, buf);
 			break;
 		}
 		/* Should get a response like this: * 48 FETCH (RFC822.SIZE 548) */
@@ -303,7 +303,7 @@ static int status_size_fetch_incremental(struct imap_client *client, const char
 	}
 
 	if (received != added) {
-		bbs_warning("Expected to get %d message sizes, but got %d? (=> %lu B)\n", added, received, *mb_size);
+		bbs_warning("Expected to get %d message sizes for '%s', but got %d? (=> %lu B)\n", added, remotename, received, *mb_size);
 		return -1;
 	}
 
@@ -396,7 +396,7 @@ static int append_size_item(struct imap_client *client, const char *remotename,
 		 */
 
 		/* Can we calculate the size incrementally? */
-		if (force || status_size_fetch_incremental(client, tag, &mb_size, buf, remote_status_resp)) { /* Add to what we already had */
+		if (force || status_size_fetch_incremental(client, remotename, tag, &mb_size, buf, remote_status_resp)) { /* Add to what we already had */
 			/* If not, resort to FETCH 1:* fallback as last resort */
 			mb_size = 0;
 			if (status_size_fetch_all(client, tag, &mb_size)) {

nets/net_smtp.c

@@ -494,13 +494,13 @@ static int smtp_ip_mismatch(const char *actual, const char *hostname)
 	return 0;
 }
 
-static int is_trusted_relay(const char *hostname)
+static int is_trusted_relay(const char *ip)
 {
 	const char *s;
 	struct stringitem *i = NULL;
 
 	while ((s = stringlist_next(&trusted_relays, &i))) {
-		if (bbs_ip_match_ipv4(hostname, s)) {
+		if (bbs_ip_match_ipv4(ip, s)) {
 			return 1;
 		}
 	}
@@ -536,7 +536,7 @@ static inline int is_benign_ip_mismatch(const char *helohost, const char *srcip)
 		return 0;
 	}
 
-	return smtp_relay_authorized(srcip, helohost);
+	return smtp_relay_authorized(srcip, helohost) || is_trusted_relay(srcip);
 }
 
 static int handle_helo(struct smtp_session *smtp, char *s, int ehlo)
@@ -1250,6 +1250,11 @@ int smtp_should_validate_dkim(struct smtp_session *smtp)
 	return smtp->tflags.dkimsig && !smtp->tflags.relay && (!smtp->node || !is_trusted_relay(smtp->node->ip));
 }
 
+int smtp_should_verify_dmarc(struct smtp_session *smtp)
+{
+	return !is_trusted_relay(smtp->node->ip);
+}
+
 int smtp_is_message_submission(struct smtp_session *smtp)
 {
 	return smtp->msa;
@@ -1611,7 +1616,7 @@ static int any_failures(struct smtp_delivery_outcome **f, int n)
 {
 	int i;
 	for (i = 0; i < n; i++) {
-		if (f[i]->action != DELIVERY_DELIVERED) {
+		if (f[i]->action == DELIVERY_FAILED) {
 			return 1;
 		}
 	}
@@ -1683,6 +1688,19 @@ int smtp_dsn(const char *sendinghost, struct tm *arrival, const char *sender, in
 			"be delivered to one or more recipients. It's attached below.\r\n\r\n");
 		fprintf(fp, "For further assistance, please send mail to postmaster.\r\n\r\n");
 		fprintf(fp, "If you do so, please include this problem report. You can delete your own text from the attached returned message.\r\n\r\n");
+	} else if (n == 1) {
+		switch (f[0]->action) {
+			case DELIVERY_DELAYED:
+				fprintf(fp, "Your message has been delayed. Delivery may succeed in the future, and we will send a final nondelivery notice if we are unable to deliver the message successfully.\r\n\r\n");
+				break;
+			case DELIVERY_DELIVERED:
+				fprintf(fp, "Your message has been delivered. A copy has been included for your reference.\r\n\r\n");
+				break;
+			case DELIVERY_RELAYED:
+			case DELIVERY_EXPANDED:
+				break;
+			case DELIVERY_FAILED: __builtin_unreachable(); /* any_failures() would be true for this case */
+		}
 	}
 	fprintf(fp, "Please, do not reply to this message.\r\n\r\n\r\n");