net_smtp: Improve loop avoidance handling and flexibility.

* Allow user to set a custom hop limit smaller than the RFC-defined limit.
* Improve logging for messages and suspected loop avoidance.
* Tarpit messages with high hop counts.
* Don't dispatch a LOGOUT event upon node disconnect if the session was
  not authenticated.

configs/net_smtp.conf

@@ -37,6 +37,8 @@ maxretries=10  ; Number of times to attempt to deliver a message. If exceeded, m
                ; It is recommended that this be at least 10, to retry delivery for at least a few days before returning to sender.
 maxage=86400   ; Maximum age of a queued email that will be retried, before being returned.
 maxsize=300000 ; Maximum size of an email message, in bytes. Messages larger than this will be rejected. Default is 300,000 (appx. 300 KB)
+maxhops=20 ; Maximum number of hops (indicated by count of Received headers). Default is 100, per RFC 5321, but you will likely want to set this lower
+           ; to end mail loops earlier rather than later (though not so low that legitimate mail is rejected). Maximum value is 100.
 requirefromhelomatch=yes ; Require the MAIL FROM domain to match the domain advertised by the sending server in HELO/EHLO.
                          ; This may cause some mail to get rejected.
 						 ; In particular, if you are proxiying email to the BBS via another MTA (e.g. postfix) on the same server,

modules/mod_smtp_delivery_external.c

@@ -1823,11 +1823,11 @@ static int external_delivery(struct smtp_session *smtp, struct smtp_response *re
 				if (bbs_pthread_create_detached(&sendthread, NULL, smtp_async_send, filenamedup)) {
 					free(filenamedup);
 				} else {
-					bbs_debug(4, "Successfully queued message for immediate delivery: <%s> -> %s\n", from, recipient);
+					bbs_debug(4, "Successfully queued %lu-byte message for immediate delivery: <%s> -> %s\n", datalen, from, recipient);
 				}
 			}
 		} else {
-			bbs_debug(4, "Successfully queued message for delayed delivery: <%s> -> %s\n", from, recipient);
+			bbs_debug(4, "Successfully queued %lu-byte message for delayed delivery: <%s> -> %s\n", datalen, from, recipient);
 		}
 	}
 	return 1;

nets/net_smtp.c

@@ -70,8 +70,10 @@
 #define MAX_LOCAL_RECIPIENTS 100
 #define MAX_EXTERNAL_RECIPIENTS 10
 
-/* RFC 5321 6.3 */
-#define MAX_HOPS 100
+/* Loop avoidance */
+#define MAX_HOPS 100 /* RFC 5321 6.3 */
+#define HOP_COUNT_MAX_NORMAL_LEVEL 5 /* If more than 5 loops, tarpit message processing to slow down possible loops */
+#define HOP_COUNT_WARN_LEVEL 15 /* Very unlikely non-malformed/routed/looped emails would see more than a dozen hops */
 
 static int smtp_port = DEFAULT_SMTP_PORT;
 static int smtps_port = DEFAULT_SMTPS_PORT;
@@ -96,6 +98,9 @@ static bbs_mutex_t loglock = BBS_MUTEX_INITIALIZER;
 /*! \brief Max message size, in bytes */
 static unsigned int max_message_size = 300000;
 
+/*! \brief Maximum number of hops, per local policy */
+static unsigned int max_hops = MAX_HOPS;
+
 #define _smtp_reply(smtp, fmt, ...) \
 	bbs_debug(6, "%p <= " fmt, smtp, ## __VA_ARGS__); \
 	bbs_auto_fd_writef(smtp->node, smtp->node ? smtp->node->wfd : -1, fmt, ## __VA_ARGS__); \
@@ -1360,7 +1365,7 @@ int __smtp_filter_write(struct smtp_filter_data *f, const char *file, int line,
 		return -1;
 	}
 
-	__bbs_log(LOG_DEBUG, 6, file, line, func, "Prepending: %s\n", buf);
+	__bbs_log(LOG_DEBUG, 6, file, line, func, "Prepending(%d): %s", len, buf); /* Already ends in CR LF */
 	if (bbs_str_contains_bare_lf(buf)) {
 		bbs_warning("Appended data that contains bare LFs! Message is not RFC-compliant!\n");
 	}
@@ -2696,9 +2701,23 @@ static int handle_data(struct smtp_session *smtp, char *s, struct readline_data
 					smtp_reply(smtp, 451, 4.3.0, "Message not received successfully, try again");
 				}
 				break;
-			} else if (smtp->tflags.hopcount >= MAX_HOPS) {
-				smtp_reply(smtp, 554, 5.6.0, "Message exceeded %d hops, this may indicate a mail loop", MAX_HOPS);
-				break;
+			} else if (smtp->tflags.hopcount > 1) {
+				if (smtp->tflags.hopcount >= HOP_COUNT_WARN_LEVEL) {
+					bbs_warning("Current SMTP hop count is %d\n", smtp->tflags.hopcount);
+				} else {
+					bbs_debug(3, "Current SMTP hop count is %d\n", smtp->tflags.hopcount);
+				}
+				if (smtp->tflags.hopcount >= HOP_COUNT_MAX_NORMAL_LEVEL && smtp->node && !strcmp(smtp->node->ip, "127.0.0.1")) {
+					/* The greater the hop count, the more we slow the message down.
+					 * We only do this for non-localhost, mainly to avoid holding up the test suite. */
+					if (bbs_node_safe_sleep(smtp->node, 200 * smtp->tflags.hopcount)) {
+						return -1;
+					}
+				}
+				if (smtp->tflags.hopcount >= (int) max_hops) {
+					smtp_reply(smtp, 554, 5.6.0, "Message exceeded %u hops, this may indicate a mail loop", max_hops);
+					break;
+				}
 			}
 
 			bbs_debug(5, "Handling receipt of %lu-byte message\n", smtp->tflags.datalen);
@@ -2951,7 +2970,11 @@ static void smtp_handler(struct bbs_node *node, int msa, int secure)
 	stringlist_init(&smtp.sentrecipients);
 
 	handle_client(&smtp);
-	mailbox_dispatch_event_basic(EVENT_LOGOUT, node, NULL, NULL);
+
+	/* If this was not an authenticated SMTP session, don't generate a logout event. */
+	if (bbs_user_is_registered(node->user)) {
+		mailbox_dispatch_event_basic(EVENT_LOGOUT, node, NULL, NULL);
+	}
 
 	smtp_destroy(&smtp);
 }
@@ -2986,6 +3009,15 @@ static int load_config(void)
 
 	bbs_config_val_set_true(cfg, "general", "relayin", &accept_relay_in);
 	bbs_config_val_set_uint(cfg, "general", "maxsize", &max_message_size);
+	if (!bbs_config_val_set_uint(cfg, "general", "maxhops", &max_hops)) {
+		if (max_hops > MAX_HOPS) {
+			bbs_warning("Maximum possible value for setting 'maxhops' is %d\n", MAX_HOPS);
+			max_hops = MAX_HOPS;
+		} else if (max_hops < 1) {
+			bbs_warning("Minimum possible value for setting 'maxhops' is %d\n", 1);
+			max_hops = 1;
+		}
+	}
 	bbs_config_val_set_true(cfg, "general", "requirefromhelomatch", &requirefromhelomatch);
 	bbs_config_val_set_true(cfg, "general", "validatespf", &validatespf);
 	bbs_config_val_set_true(cfg, "general", "addreceivedmsa", &add_received_msa);