Index | Thread | Search

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

Download raw body.

Thread
  • Omar Polo:

    [patch] vacation(1): parse angle-addr in Return-Path

  • 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);
    -	}
     }
     
     /*
    
    
    
  • Omar Polo:

    [patch] vacation(1): parse angle-addr in Return-Path