Re: PATCH: mis-configured luser_relay becomes black hole

[J. Thomsen]

On Tue, 6 Mar 2018 17:28:00 -0500 (EST),Wietse Venema  
wrote:

> I was able to fix this on the train. If luser_relay
>specifies a non-existent local address, then the luser_relay feature
>becomes a black hole. This was introduced three months after the
>first public release, on 19990302. 

Very good.
This is a beautiful example on the fact that in this business even 19 years of 
intensive testing
does not find all bugs :)

Thanks,
Jørgen

Re: PATCH: mis-configured luser_relay becomes black hole

[Wietse Venema]

Wietse Venema:
> Wietse Venema:
> > Wietse Venema:
> > > However, when luser_relay handes mail for a non-existent recipient,
> > > and it is configured with a non-existent local user, then the local
> > > delivery agent will go through the same code path twice with the
> > > same address, in which case the duplicate suppressor will silently
> > > terminate the recursion like it does for all other deliveries.
> > > 
> > > This may or may not be easy to fix. For now, don't configure
> > > luser_relay with a non-existent address, just like postmaster, if
> > > you don't want errors to go unreported.
> > 
> > Easy enough: I was able to fix this on the train. If luser_relay
> > specifies a non-existent local address, then the luser_relay feature
> > becomes a black hole. This was introduced three months after the
> > first public release, on 19990302. This patch should apply to every
> > supported Postfix release.
> 
> The patch needs to be updated for the case that luser_relay specifies
> a bare address (no @domain).  I'll revisit this in the next day or so.

Done.

Wietse

diff -ur /var/tmp/postfix-3.4-20180222/src/local/unknown.c src/local/unknown.c
--- /var/tmp/postfix-3.4-20180222/src/local/unknown.c   2015-01-11 
15:30:20.0 -0500
+++ src/local/unknown.c 2018-03-06 19:29:36.168526066 -0500
@@ -73,11 +73,14 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Application-specific. */
 
 #include "local.h"
 
+#define STREQ(x,y) (strcasecmp((x),(y)) == 0)
+
 /* deliver_unknown - delivery for unknown recipients */
 
 int deliver_unknown(LOCAL_STATE state, USER_ATTR usr_attr)
@@ -85,6 +88,7 @@
 const char *myname = "deliver_unknown";
 int status;
 VSTRING *expand_luser;
+VSTRING *canon_luser;
 static MAPS *transp_maps;
 const char *map_transport;
 
@@ -139,8 +143,20 @@
 if (*var_luser_relay) {
state.msg_attr.unmatched = 0;
expand_luser = vstring_alloc(100);
+   canon_luser = vstring_alloc(100);
local_expand(expand_luser, var_luser_relay, , _attr, (void *) 
0);
-   status = deliver_resolve_addr(state, usr_attr, STR(expand_luser));
+   /* In case luser_relay specifies a domain-less address. */
+   canon_addr_external(canon_luser, vstring_str(expand_luser));
+   /* Assumes that the address resolver won't change the address. */
+   if (STREQ(vstring_str(canon_luser), state.msg_attr.rcpt.address)) {
+   dsb_simple(state.msg_attr.why, "5.1.1",
+  "unknown user: \"%s\"", state.msg_attr.user);
+   status = bounce_append(BOUNCE_FLAGS(state.request),
+  BOUNCE_ATTR(state.msg_attr));
+   } else {
+   status = deliver_resolve_addr(state, usr_attr, STR(expand_luser));
+   }
+   vstring_free(canon_luser);
vstring_free(expand_luser);
return (status);
 }
@@ -149,8 +165,6 @@
  * If no alias was found for a required reserved name, toss the message
  * into the bit bucket, and issue a warning instead.
  */
-#define STREQ(x,y) (strcasecmp(x,y) == 0)
-
 if (STREQ(state.msg_attr.user, MAIL_ADDR_MAIL_DAEMON)
|| STREQ(state.msg_attr.user, MAIL_ADDR_POSTMASTER)) {
msg_warn("required alias not found: %s", state.msg_attr.user);

Re: PATCH: mis-configured luser_relay becomes black hole

[Wietse Venema]

Wietse Venema:
> Wietse Venema:
> > However, when luser_relay handes mail for a non-existent recipient,
> > and it is configured with a non-existent local user, then the local
> > delivery agent will go through the same code path twice with the
> > same address, in which case the duplicate suppressor will silently
> > terminate the recursion like it does for all other deliveries.
> > 
> > This may or may not be easy to fix. For now, don't configure
> > luser_relay with a non-existent address, just like postmaster, if
> > you don't want errors to go unreported.
> 
> Easy enough: I was able to fix this on the train. If luser_relay
> specifies a non-existent local address, then the luser_relay feature
> becomes a black hole. This was introduced three months after the
> first public release, on 19990302. This patch should apply to every
> supported Postfix release.

The patch needs to be updated for the case that luser_relay specifies
a bare address (no @domain).  I'll revisit this in the next day or so.

Wietse

> diff '--exclude=man' '--exclude=html' '--exclude=README_FILES' 
> '--exclude=INSTALL' '--exclude=.indent.pro' '--exclude=Makefile.in' -r -ur 
> /var/tmp/postfix-3.4-20180222/src/local/unknown.c src/local/unknown.c
> --- /var/tmp/postfix-3.4-20180222/src/local/unknown.c 2015-01-11 
> 15:30:20.0 -0500
> +++ src/local/unknown.c   2018-03-06 17:06:17.477496641 -0500
> @@ -78,6 +78,8 @@
>  
>  #include "local.h"
>  
> +#define STREQ(x,y) (strcasecmp((x),(y)) == 0)
> +
>  /* deliver_unknown - delivery for unknown recipients */
>  
>  int deliver_unknown(LOCAL_STATE state, USER_ATTR usr_attr)
> @@ -140,6 +142,13 @@
>   state.msg_attr.unmatched = 0;
>   expand_luser = vstring_alloc(100);
>   local_expand(expand_luser, var_luser_relay, , _attr, (void *) 
> 0);
> + /* Assumes that the address resolver will not change the address. */
> + if (STREQ(vstring_str(expand_luser), state.msg_attr.rcpt.address)) {
> + dsb_simple(state.msg_attr.why, "5.1.1",
> +"unknown user: \"%s\"", state.msg_attr.user);
> + return (bounce_append(BOUNCE_FLAGS(state.request),
> +   BOUNCE_ATTR(state.msg_attr)));
> + }
>   status = deliver_resolve_addr(state, usr_attr, STR(expand_luser));
>   vstring_free(expand_luser);
>   return (status);
> @@ -149,8 +158,6 @@
>   * If no alias was found for a required reserved name, toss the message
>   * into the bit bucket, and issue a warning instead.
>   */
> -#define STREQ(x,y) (strcasecmp(x,y) == 0)
> -
>  if (STREQ(state.msg_attr.user, MAIL_ADDR_MAIL_DAEMON)
>   || STREQ(state.msg_attr.user, MAIL_ADDR_POSTMASTER)) {
>   msg_warn("required alias not found: %s", state.msg_attr.user);
> 

PATCH: mis-configured luser_relay becomes black hole

[Wietse Venema]

Wietse Venema:
> However, when luser_relay handes mail for a non-existent recipient,
> and it is configured with a non-existent local user, then the local
> delivery agent will go through the same code path twice with the
> same address, in which case the duplicate suppressor will silently
> terminate the recursion like it does for all other deliveries.
> 
> This may or may not be easy to fix. For now, don't configure
> luser_relay with a non-existent address, just like postmaster, if
> you don't want errors to go unreported.

Easy enough: I was able to fix this on the train. If luser_relay
specifies a non-existent local address, then the luser_relay feature
becomes a black hole. This was introduced three months after the
first public release, on 19990302. This patch should apply to every
supported Postfix release.

Wietse


diff '--exclude=man' '--exclude=html' '--exclude=README_FILES' 
'--exclude=INSTALL' '--exclude=.indent.pro' '--exclude=Makefile.in' -r -ur 
/var/tmp/postfix-3.4-20180222/src/local/unknown.c src/local/unknown.c
--- /var/tmp/postfix-3.4-20180222/src/local/unknown.c   2015-01-11 
15:30:20.0 -0500
+++ src/local/unknown.c 2018-03-06 17:06:17.477496641 -0500
@@ -78,6 +78,8 @@
 
 #include "local.h"
 
+#define STREQ(x,y) (strcasecmp((x),(y)) == 0)
+
 /* deliver_unknown - delivery for unknown recipients */
 
 int deliver_unknown(LOCAL_STATE state, USER_ATTR usr_attr)
@@ -140,6 +142,13 @@
state.msg_attr.unmatched = 0;
expand_luser = vstring_alloc(100);
local_expand(expand_luser, var_luser_relay, , _attr, (void *) 
0);
+   /* Assumes that the address resolver will not change the address. */
+   if (STREQ(vstring_str(expand_luser), state.msg_attr.rcpt.address)) {
+   dsb_simple(state.msg_attr.why, "5.1.1",
+  "unknown user: \"%s\"", state.msg_attr.user);
+   return (bounce_append(BOUNCE_FLAGS(state.request),
+ BOUNCE_ATTR(state.msg_attr)));
+   }
status = deliver_resolve_addr(state, usr_attr, STR(expand_luser));
vstring_free(expand_luser);
return (status);
@@ -149,8 +158,6 @@
  * If no alias was found for a required reserved name, toss the message
  * into the bit bucket, and issue a warning instead.
  */
-#define STREQ(x,y) (strcasecmp(x,y) == 0)
-
 if (STREQ(state.msg_attr.user, MAIL_ADDR_MAIL_DAEMON)
|| STREQ(state.msg_attr.user, MAIL_ADDR_POSTMASTER)) {
msg_warn("required alias not found: %s", state.msg_attr.user);