On Tue, Dec 04, 2018 at 01:10:13PM +0000, [email protected] wrote:
> Gilles Chehade <[email protected]> wrote:
> > On Tue, Dec 04, 2018 at 03:22:14AM +0000, [email protected] wrote:
> > > Gilles Chehade <[email protected]> wrote:
> > > > On Mon, Dec 03, 2018 at 06:11:29PM -0200, Cristiano Costa wrote:
> > > > > Hello everyone,
> > > > > 
> > > > > Recently, we have updated our external MTA to use OpenBSD(6.4) and 
> > > > > opensmptd.
> > > > > 
> > > > > After we updated, our mailman (that run on a Linux machine) started to
> > > > > send "bounce message w/no discernible addresses" messages to us (list
> > > > > admins). It seems that mailman was not able to parse bounce messages
> > > > > sent from opensmtpd. Mailman uses the bounces to remove users from our
> > > > > mailing lists automatically.
> > > > > 
> > > > > I have opened a ticket in the mailman bug tracker and the guys there
> > > > > will fix it. However, they also replied in my bug report:
> > > > > 
> > > > > > It turns out that this (the bounce format) is almost an RFC 3464 
> > > > > > compliant DSN and should be recognized by Mailman/Bouncers/DSN.py 
> > > > > > except for the fact that the 'action' in the Delivery Report is 
> > > > > > 'error'. This is non-compliant. RFC 3464, sec 2.3.3 defines action 
> > > > > > as:
> > > > > >
> > > > > >        action-value =
> > > > > >                "failed" / "delayed" / "delivered" / "relayed" / 
> > > > > > "expanded"
> > > > > >
> > > > > >    Thus, 'Action: error' in this DSN is non-compliant. It should be 
> > > > > > 'Action: failed'. You may wish to report this to the opensmtpd 
> > > > > > developers."
> > > > > 
> > > > > So maybe you would like to patch opensmtpd too.
> > > > > 
> > > > > The link to the bug report is 
> > > > > https://bugs.launchpad.net/mailman/+bug/1805137
> > > > > 
> > > > > Thanks,
> > > > > 
> > > > > Please keep the great work,
> > > > > 
> > > > 
> > > > yes, we'll definitely issue a fix for that.
> > > > 
> > > > thanks,
> > > > gilles
> > > 
> > > This diff uses the standard action-values "failed", "delivered".
> > > Could you give this a try, thanks.
> > > 
> > 
> > The diff reads ok though maybe you could take it one step further and
> > rename the type enums to match:
> > 
> > failed -> B_FAILED
> > delayed -> B_DELAYED
> > relayd/delivered -> B_DELIVERED
> 
> Sure, updated diff...
> 

better,

comments inlined


> Index: bounce.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/bounce.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 bounce.c
> --- bounce.c  31 May 2018 21:06:12 -0000      1.79
> +++ bounce.c  4 Dec 2018 13:05:14 -0000
> @@ -150,13 +150,13 @@ bounce_add(uint64_t evpid)
>  
>       switch (evp.esc_class) {
>       case ESC_STATUS_OK:
> -             key.bounce.type = B_DSN;
> +             key.bounce.type = B_DELIVERED;
>               break;
>       case ESC_STATUS_TEMPFAIL:
> -             key.bounce.type = B_WARNING;
> +             key.bounce.type = B_DELAYED;
>               break;
>       default:
> -             key.bounce.type = B_ERROR;
> +             key.bounce.type = B_FAILED;
>       }
>  
>       key.bounce.dsn_ret = evp.dsn_ret;
> @@ -478,14 +478,14 @@ bounce_next(struct bounce_session *s)
>                   s->boundary, s->smtpname);
>  
>               switch (s->msg->bounce.type) {
> -             case B_ERROR:
> +             case B_FAILED:
>                       io_xprint(s->io, notice_error);
>                       break;
> -             case B_WARNING:
> +             case B_DELAYED:
>                       io_xprintf(s->io, notice_warning,
>                           bounce_duration(s->msg->bounce.delay));
>                       break;
> -             case B_DSN:
> +             case B_DELIVERED:
>                       io_xprint(s->io, s->msg->bounce.mta_without_dsn ?
>                           notice_relay : notice_success);
>                       break;
> @@ -498,7 +498,7 @@ bounce_next(struct bounce_session *s)
>               }
>               io_xprint(s->io, "\n");
>  
> -             if (s->msg->bounce.type == B_WARNING)
> +             if (s->msg->bounce.type == B_DELAYED)
>                       io_xprintf(s->io, notice_warning2,
>                           bounce_duration(s->msg->bounce.ttl));
>  
> @@ -549,7 +549,7 @@ bounce_next(struct bounce_session *s)
>                       if ((len = getline(&line, &sz, s->msgfp)) == -1)
>                               break;
>                       if (len == 1 && line[0] == '\n' && /* end of headers */
> -                         s->msg->bounce.type == B_DSN &&
> +                         s->msg->bounce.type == B_DELIVERED &&
>                           s->msg->bounce.dsn_ret ==  DSN_RETHDRS) {
>                               free(line);
>                               fclose(s->msgfp);
> @@ -795,15 +795,15 @@ static const char *
>  action_str(const struct delivery_bounce *b)
>  {
>       switch (b->type) {
> -     case B_ERROR:
> -             return ("error");
> -     case B_WARNING:
> +     case B_FAILED:
> +             return ("failed");
> +     case B_DELAYED:
>               return ("delayed");
> -     case B_DSN:
> +     case B_DELIVERED:
>               if (b->mta_without_dsn)
>                       return ("relayed");
>  
> -             return ("success");
> +             return ("delivered");
>       default:
>               log_warn("warn: bounce: unknown bounce_type");
>               return ("");
> Index: envelope.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/envelope.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 envelope.c
> --- envelope.c        29 May 2018 19:48:19 -0000      1.39
> +++ envelope.c        4 Dec 2018 13:05:14 -0000
> @@ -345,11 +345,11 @@ static int
>  ascii_load_bounce_type(enum bounce_type *dest, char *buf)
>  {
>       if (strcasecmp(buf, "error") == 0)
> -             *dest = B_ERROR;
> +             *dest = B_FAILED;
>       else if (strcasecmp(buf, "warn") == 0)
> -             *dest = B_WARNING;
> +             *dest = B_DELAYED;
>       else if (strcasecmp(buf, "dsn") == 0)
> -             *dest = B_DSN;
> +             *dest = B_DELIVERED;


the strcasecmp are not correct here


>       else
>               return 0;
>       return 1;
> @@ -574,13 +574,13 @@ ascii_dump_bounce_type(enum bounce_type 
>       char *p = NULL;
>  
>       switch (type) {
> -     case B_ERROR:
> +     case B_FAILED:
>               p = "error";
>               break;
> -     case B_WARNING:
> +     case B_DELAYED:
>               p = "warn";
>               break;
> -     case B_DSN:
> +     case B_DELIVERED:
>               p = "dsn";
>               break;

the dumped strings are not correct here


>       default:
> @@ -612,13 +612,13 @@ ascii_dump_field(const char *field, cons
>               return ascii_dump_string(ep->dispatcher, buf, len);
>  
>       if (strcasecmp(field, "bounce-delay") == 0) {
> -             if (ep->agent.bounce.type != B_WARNING)
> +             if (ep->agent.bounce.type != B_DELAYED)
>                       return (1);
>               return ascii_dump_time(ep->agent.bounce.delay, buf, len);
>       }
>  
>       if (strcasecmp(field, "bounce-ttl") == 0) {
> -             if (ep->agent.bounce.type != B_WARNING)
> +             if (ep->agent.bounce.type != B_DELAYED)
>                       return (1);
>               return ascii_dump_time(ep->agent.bounce.ttl, buf, len);
>       }
> Index: queue.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/queue.c,v
> retrieving revision 1.187
> diff -u -p -r1.187 queue.c
> --- queue.c   31 May 2018 21:06:12 -0000      1.187
> +++ queue.c   4 Dec 2018 13:05:14 -0000
> @@ -210,7 +210,7 @@ queue_imsg(struct mproc *p, struct imsg 
>               if (queue_envelope_load(evpid, &evp) == 0)
>                       return;
>  
> -             bounce.type = B_ERROR;
> +             bounce.type = B_FAILED;
>               envelope_set_errormsg(&evp, "Envelope expired");
>               envelope_set_esc_class(&evp, ESC_STATUS_TEMPFAIL);
>               envelope_set_esc_code(&evp, ESC_DELIVERY_TIME_EXPIRED);
> @@ -342,7 +342,7 @@ queue_imsg(struct mproc *p, struct imsg 
>                       return;
>               }
>               if (evp.dsn_notify & DSN_SUCCESS) {
> -                     bounce.type = B_DSN;
> +                     bounce.type = B_DELIVERED;
>                       bounce.dsn_ret = evp.dsn_ret;
>                       envelope_set_esc_class(&evp, ESC_STATUS_OK);
>                       if (imsg->hdr.type == IMSG_MDA_DELIVERY_OK)
> @@ -400,7 +400,7 @@ queue_imsg(struct mproc *p, struct imsg 
>                       m_close(p_scheduler);
>                       return;
>               }
> -             bounce.type = B_ERROR;
> +             bounce.type = B_FAILED;
>               envelope_set_errormsg(&evp, "%s", reason);
>               envelope_set_esc_class(&evp, ESC_STATUS_PERMFAIL);
>               envelope_set_esc_code(&evp, code);
> @@ -427,7 +427,7 @@ queue_imsg(struct mproc *p, struct imsg 
>               envelope_set_errormsg(&evp, "%s", "Loop detected");
>               envelope_set_esc_class(&evp, ESC_STATUS_TEMPFAIL);
>               envelope_set_esc_code(&evp, ESC_ROUTING_LOOP_DETECTED);
> -             bounce.type = B_ERROR;
> +             bounce.type = B_FAILED;
>               queue_bounce(&evp, &bounce);
>               queue_envelope_delete(evp.id);
>               m_create(p_scheduler, IMSG_QUEUE_DELIVERY_LOOP, 0, 0, -1);
> Index: scheduler.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/scheduler.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 scheduler.c
> --- scheduler.c       31 May 2018 21:06:12 -0000      1.58
> +++ scheduler.c       4 Dec 2018 13:04:53 -0000
> @@ -214,7 +214,7 @@ scheduler_imsg(struct mproc *p, struct i
>                           si.lastbounce < timestamp) {
>                               req.evpid = evp.id;
>                               req.timestamp = timestamp;
> -                             req.bounce.type = B_WARNING;
> +                             req.bounce.type = B_DELAYED;
>                               req.bounce.delay = env->sc_bounce_warn[i];
>                               req.bounce.ttl = si.ttl;
>                               m_compose(p, IMSG_SCHED_ENVELOPE_BOUNCE, 0, 0, 
> -1,
> Index: smtpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
> retrieving revision 1.575
> diff -u -p -r1.575 smtpd.h
> --- smtpd.h   30 Nov 2018 15:33:40 -0000      1.575
> +++ smtpd.h   4 Dec 2018 13:05:14 -0000
> @@ -374,9 +374,9 @@ struct table_backend {
>  
>  
>  enum bounce_type {
> -     B_ERROR,
> -     B_WARNING,
> -     B_DSN
> +     B_FAILED,
> +     B_DELAYED,
> +     B_DELIVERED
>  };
>  
>  enum dsn_ret {
> 
> --
> You received this mail because you are subscribed to [email protected]
> To unsubscribe, send a mail to: [email protected]
> 

-- 
Gilles Chehade                                                 @poolpOrg

https://www.poolp.org                 tip me: https://paypal.me/poolpOrg

-- 
You received this mail because you are subscribed to [email protected]
To unsubscribe, send a mail to: [email protected]

Reply via email to