From 5440ffe36ddf153b60ab19facf7b05e4fa399221 Mon Sep 17 00:00:00 2001 From: InterLinked1 <24227567+InterLinked1@users.noreply.github.com> Date: Mon, 16 Dec 2024 20:46:56 -0500 Subject: [PATCH] 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 | 2 ++ modules/mod_smtp_delivery_external.c | 4 +-- nets/net_smtp.c | 46 +++++++++++++++++++++++----- 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/configs/net_smtp.conf b/configs/net_smtp.conf index d22637d..50e0566 100644 --- a/configs/net_smtp.conf +++ b/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, diff --git a/modules/mod_smtp_delivery_external.c b/modules/mod_smtp_delivery_external.c index 666925c..76328c0 100644 --- a/modules/mod_smtp_delivery_external.c +++ b/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; diff --git a/nets/net_smtp.c b/nets/net_smtp.c index 902dc91..518393a 100644 --- a/nets/net_smtp.c +++ b/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);