From 17e6d6f7fb9a68889aeeb30253f9d33ea2d0d00d Mon Sep 17 00:00:00 2001 From: InterLinked1 <24227567+InterLinked1@users.noreply.github.com> Date: Fri, 8 Nov 2024 18:21:31 -0500 Subject: [PATCH] modules: Minor protocol and module enhancements. * mod_http: Recognize Max-Forwards header. This is not currently used for anything, but could be in the future. * mod_http_proxy: Improve /0 CIDR range detection for warning. * mod_smtp_delivery_external: Include SMTP response in SMTP logs, so we can later see exactly why delivery of a message failed. * mod_smtp_delivery_external: Remove year from mailq printouts. Messages are never retried for more than a year, so including the year in the timestamps just takes up space unnecessarily. --- include/mod_http.h | 1 + modules/mod_http.c | 30 ++++++++++++++++++++++++++++ modules/mod_http_proxy.c | 2 +- modules/mod_smtp_delivery_external.c | 16 +++++++-------- nets/net_smtp.c | 1 + 5 files changed, 41 insertions(+), 9 deletions(-) diff --git a/include/mod_http.h b/include/mod_http.h index 028620f..2ae6731 100644 --- a/include/mod_http.h +++ b/include/mod_http.h @@ -117,6 +117,7 @@ struct http_request { unsigned int expect100:1; /*!< Expecting 100-continue */ unsigned int parsedbody:1; unsigned int absolute:1; /*!< Absolute host used in request */ + unsigned int noforward:1; /*!< Don't forward request further (i.e. Max-Forwards: 0) */ }; struct http_response { diff --git a/modules/mod_http.c b/modules/mod_http.c index e96558d..10940dd 100644 --- a/modules/mod_http.c +++ b/modules/mod_http.c @@ -1819,6 +1819,36 @@ static int http_handle_request(struct http_session *http, char *buf) return res; } + /* RFC 9110 7.6.2 + * For OPTIONS and TRACE requests, if a Max-Forwards header is provided, + * we MUST NOT forward the request if its value is 0, + * and MUST decrement it otherwise (or set it to the local max supported Max-Forwards) + * We MAY ignore Max-Forwards for other methods, but don't have to, either... + * Currently, we don't forward requests (outside of the CONNECT proxy support), + * but we handle this anyways. */ + if (http->req->method & (HTTP_METHOD_OPTIONS | HTTP_METHOD_TRACE)) { + const char *maxforwards = http_request_header(http, "Max-Forwards"); + if (!strlen_zero(maxforwards)) { + int maxfwds = atoi(maxforwards); + if (maxfwds) { + char newmaxforwards[16]; + snprintf(newmaxforwards, sizeof(newmaxforwards), "%d", --maxfwds); + /* BUGBUG This will only work if the case matches (e.g. request received + * was case-sensitively 'Max-Forwards'. If not, we will end up creating + * a new header with the canonical casing, and leave the old one intact... ouch. + * Leaving this unfixed for now, but it forwarding is ever added and this matters, + * we will need to be able to update headers case-insensitively, to be compatible + * with modifying existing headers as received. */ + http_set_header(http, "Max-Forwards", newmaxforwards); + } else { + /* Currently, there is nothing that checks this, + * but if OPTIONS and TRACE were to be implemented and in a way that supported forwarding, + * they would need to obey this. */ + http->req->noforward = 1; + } + } + } + /* Proxy requests really need to be handled before doing anything else, since they're fairly low level. * We don't want to read or process the body. * We don't care what the request is for, diff --git a/modules/mod_http_proxy.c b/modules/mod_http_proxy.c index bd807c9..4f26d50 100644 --- a/modules/mod_http_proxy.c +++ b/modules/mod_http_proxy.c @@ -48,7 +48,7 @@ static void add_proxy_client(const char *source, const char *domains) { struct http_proxy_client *c; - if (STARTS_WITH(source, "0.0.0.0")) { + if (STARTS_WITH(source, "0.0.0.0") || strstr(source, "/0")) { /* If someone wants to shoot him/herself in the foot, at least provide a warning */ bbs_notice("This server is configured as an open proxy and may be abused!\n"); } diff --git a/modules/mod_smtp_delivery_external.c b/modules/mod_smtp_delivery_external.c index 6d56feb..666925c 100644 --- a/modules/mod_smtp_delivery_external.c +++ b/modules/mod_smtp_delivery_external.c @@ -1180,7 +1180,7 @@ static int process_queue_file(struct mailq_run *qrun, struct mailq_file *mqf) if (!res) { /* Successful delivery. */ bbs_debug(6, "Delivery successful after %d attempt%s, discarding queue file\n", mqf->newretries, ESS(mqf->newretries)); - bbs_smtp_log(4, NULL, "Delivery succeeded after queuing: %s -> %s\n", mqf->realfrom, mqf->realto); + bbs_smtp_log(4, NULL, "Delivery succeeded after queuing: %s -> %s (%s)\n", mqf->realfrom, mqf->realto, buf); smtp_trigger_dsn(DELIVERY_DELIVERED, &tx, &mqf->created, mqf->realfrom, mqf->realto, buf, fileno(mqf->fp), mqf->metalen, mqf->size - mqf->metalen); fclose(mqf->fp); mqf->fp = NULL; /* For parallel task framework, since cleanup is always called */ @@ -1193,7 +1193,7 @@ static int process_queue_file(struct mailq_run *qrun, struct mailq_file *mqf) if (res == -2 || res > 0 || mqf->newretries >= (int) max_retries) { /* Send a delivery failure response, then delete the file. */ bbs_warning("Delivery of message %s from %s to %s has failed permanently after %d retries\n", mqf->fullname, mqf->realfrom, mqf->realto, mqf->newretries); - bbs_smtp_log(1, NULL, "Delivery failed permanently after queuing: %s -> %s\n", mqf->realfrom, mqf->realto); + bbs_smtp_log(1, NULL, "Delivery failed permanently after queuing: %s -> %s (%s)\n", mqf->realfrom, mqf->realto, buf); /* To the dead letter office we go */ /* XXX buf will only contain the last line of the SMTP transaction, since it was using the readline buffer * Thus, if we got a multiline error, only the last line is currently included in the non-delivery report */ @@ -1204,7 +1204,7 @@ static int process_queue_file(struct mailq_run *qrun, struct mailq_file *mqf) QUEUE_INCR_STAT(failed); return 0; } else { - bbs_smtp_log(3, NULL, "Delivery delayed after queuing: %s -> %s\n", mqf->realfrom, mqf->realto); + bbs_smtp_log(3, NULL, "Delivery delayed after queuing: %s -> %s (%s)\n", mqf->realfrom, mqf->realto, buf); mailq_file_punt(mqf); /* Try again later */ smtp_trigger_dsn(DELIVERY_DELAYED, &tx, &mqf->created, mqf->realfrom, mqf->realto, buf, fileno(mqf->fp), mqf->metalen, mqf->size - mqf->metalen); QUEUE_INCR_STAT(delayed); @@ -1517,8 +1517,8 @@ static int on_queue_file_cli_mailq(const char *dir_name, const char *filename, v return 0; } - strftime(arrival_date, sizeof(arrival_date), "%a, %d %b %Y %H:%M:%S", &mqf->created); - strftime(retry_date, sizeof(retry_date), "%a, %d %b %Y %H:%M:%S", &mqf->retried); + strftime(arrival_date, sizeof(arrival_date), "%a, %d %b %H:%M:%S", &mqf->created); + strftime(retry_date, sizeof(retry_date), "%a, %d %b %H:%M:%S", &mqf->retried); /* For user convenience, try to calculate when message delivery will be attempted next. */ next_retry_time = mqf->retriedtime + queue_retry_threshold(mqf->retries); /* Minimum time that it would get processed */ @@ -1546,11 +1546,11 @@ static int on_queue_file_cli_mailq(const char *dir_name, const char *filename, v } localtime_r(&next_retry_time, &est_retry); - strftime(next_retry_date, sizeof(next_retry_date), "%a, %d %b %Y %H:%M:%S", &est_retry); + strftime(next_retry_date, sizeof(next_retry_date), "%a, %d %b %H:%M:%S", &est_retry); /* Ensure the format is synchronized with the heading in cli_mailq */ /* Printing mqf->retries this way is already 1-indexed as well. */ - bbs_dprintf(qrun->clifd, "%7d %-25s %-25s %-25s %-20s %5ld %-35s %s\n", + bbs_dprintf(qrun->clifd, "%7d %-21s %-21s %-21s %-20s %5ld %-35s %s\n", mqf->retries, arrival_date, retry_date, next_retry_date, filename, (mqf->size + 1023) / 1024, /* Display size in KB, rounded up to the nearest KB */ mqf->realfrom, mqf->realto); @@ -1569,7 +1569,7 @@ static int cli_mailq(struct bbs_cli_args *a) qrun.host_ends_with = a->argv[1]; } - bbs_dprintf(a->fdout, "%7s %-25s %-25s %-25s %-20s %5s %-35s %s\n", "Retries", "Orig Date", "Last Retry", "Est. Next Retry", "Filename", "Size", "Sender", "Recipient"); + bbs_dprintf(a->fdout, "%7s %-21s %-21s %-21s %-20s %5s %-35s %s\n", "Retries", "Orig Date", "Last Retry", "Est. Next Retry", "Filename", "Size", "Sender", "Recipient"); run_queue(&qrun, on_queue_file_cli_mailq); bbs_dprintf(a->fdout, "%d message%s currently in mail queue\n", qrun.total, ESS(qrun.total)); mailq_run_cleanup(&qrun); diff --git a/nets/net_smtp.c b/nets/net_smtp.c index 6cce712..6164204 100644 --- a/nets/net_smtp.c +++ b/nets/net_smtp.c @@ -858,6 +858,7 @@ static int parse_mail_parameters(struct smtp_session *smtp, char *s) * In my experience, most mail that requires 8BITMIME is spam. * But it's probably not our place to reject such mail. */ if (!strcasecmp(d, "8BITMIME")) { + bbs_debug(3, "Sender indicated this is an 8-bit message... increasing spam score\n"); smtp->tflags.is8bit = 1; smtp->failures++; /* Penalize 8-bit messages. These are probably spam. */ } else if (!strcasecmp(d, "7BIT")) {