Skip to content

Commit

Permalink
modules: Minor protocol and module enhancements.
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
InterLinked1 committed Nov 8, 2024
1 parent acfdcda commit 17e6d6f
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 9 deletions.
1 change: 1 addition & 0 deletions include/mod_http.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
30 changes: 30 additions & 0 deletions modules/mod_http.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion modules/mod_http_proxy.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
16 changes: 8 additions & 8 deletions modules/mod_smtp_delivery_external.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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 */
Expand All @@ -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);
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions nets/net_smtp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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")) {
Expand Down

0 comments on commit 17e6d6f

Please sign in to comment.