From: Omar Polo <op@omarpolo.com>
Subject: Re: [patch] vacation(1): parse angle-addr in Return-Path
To: "Sven M. Hallberg" <pesco@khjk.org>
Cc: tech@openbsd.org
Date: Thu, 22 Aug 2024 23:43:39 +0200

Hello,

On 2024/08/22 09:30:37 +0200, Sven M. Hallberg <pesco@khjk.org> wrote:
> Sven M. Hallberg on Mon, Jul 22 2024:
> > The vacation program seems to parse the Return-Path header
> > incorrectly.
> 
> One-month ping.

sorry for the delay :/

Makes sense to me to strip the <> from the Return-Path.

> There is another easy to fix issue in that vacation will exit(1) if it
> can't find a Return-Path header or mbox From line. This can (and does)
> happen, however, since Return-Path is in fact an optional header. In
> such a case, we should just exit with success and do nothing. Otherwise
> our MTA will think we couldn't "deliver" its message.
> 
> Updated diff below.
> -p

Also makes sense to me.  RFC 3834 explicitly says that

: -  Responders MUST NOT generate any response for which the
:    destination of that response would be a null address (e.g., an
:    address for which SMTP MAIL FROM or Return-Path is <>), since the
:    response would not be delivered to a useful destination.
      
and in general seems little useful to spam syslog on mails without a
From or Return-Path.

> [...]
> @@ -306,9 +310,13 @@ findme:			for (cur = names; !tome && cur
>  	if (!tome)
>  		exit(0);
>  	if (!*from) {
> -		syslog(LOG_NOTICE,
> -		    "no initial \"From\" or \"Return-Path\"line.");
> -		exit(1);
> +		/*
> +		 * No initial "From " line or "Return-Path:" header.
> +		 *
> +		 * NB: Return-Path is optional. If it is not present, we
> +		 * shall not respond, cf. RFC 3834.
> +		 */
> +		exit(0);
>  	}
>  }
>  

I'd just fold the "!tome" check together wiht the from one.

I intend to commit the following diff in a few days provided nobody
objects it.


diff /usr/src
commit - b9fb712364e92aea382d3457b4c3aa8604c9811d
path + /usr/src
blob - 7105f567c9236250ab3a51972afcfe27cd744120
file + usr.bin/vacation/vacation.c
--- usr.bin/vacation/vacation.c
+++ usr.bin/vacation/vacation.c
@@ -246,6 +246,10 @@ readheaders(void)
 				break;
 			for (p = buf + 12; isspace((unsigned char)*p); ++p)
 				;
+			if (*p == '<') {
+				++p;
+				p[strcspn(p, ">")] = '\0';
+			}
 			if (strlcpy(from, p, sizeof(from)) >= sizeof(from)) {
 				syslog(LOG_NOTICE,
 				    "Return-Path %s exceeds limits", p);
@@ -303,13 +307,8 @@ readheaders(void)
 findme:			for (cur = names; !tome && cur; cur = cur->next)
 				tome += nsearch(cur->name, buf);
 		}
-	if (!tome)
+	if (!tome || !*from)
 		exit(0);
-	if (!*from) {
-		syslog(LOG_NOTICE,
-		    "no initial \"From\" or \"Return-Path\"line.");
-		exit(1);
-	}
 }
 
 /*