Re: smtpd: fix catch-all in virtual aliases

2020-04-28 Thread gilles
April 28, 2020 11:02 AM, "Joerg Jung" mailto:m...@umaxx.net?to=%22Joerg%20Jung%22%20)> wrote:
On 28. Apr 2020, at 10:10, gil...@poolp.org (mailto:gil...@poolp.org) wrote: 
 April 28, 2020 8:55 AM, "Joerg Jung" mailto:m...@umaxx.net)> 
wrote:
 Also this change might break existing valid setups (e.g. with mailing list
servers), but people will likely know how to cope with it. 
Do you have an example of an existing valid setup that is broken with this ?
No example, I just presumed (that’s why I wrote “might”). 
ok, because I can't think of a setup that could be broken with this as @ is the 
very last wildcard matching everything not matched earlier.

this diff fixes an actual reported bug and passed various tests mixing no 
wildcard, user wildcard, domain wildcard.

I think it should go in as it is a regression that probably got introduced a 
while back, the @ wildcard used to work for sure.


Re: smtpd: fix catch-all in virtual aliases

2020-04-28 Thread Todd C . Miller
On Sun, 26 Apr 2020 18:30:25 +0200, Eric Faurot wrote:

> When a catch-all entry (@) is used in a virtual alias table, it
> eventually (and mistakenly) catches everything that expands to a
> username. For example, with:
>
> f...@example.com  user
> @catchall
>
> "f...@example.com" expands to "user" as expected, but then "user"
> expands to "catchall" because it is interpreted as "user@" (empty
> domain).
>
> The catch-all fallback mechanism is really meant for full email
> addresses in virtual context, and should not happen for usernames.
> The following diff fixes it.

That makes sense.  OK millert@

I see that the catch-all behavior is documented in makemap(8) but
not aliases(5).  Does it make sense to document it there too?  That's
the first place I looked (the makemap manual was the 3rd place I
looked).

 - todd



Re: smtpd: fix catch-all in virtual aliases

2020-04-28 Thread Joerg Jung


> On 28. Apr 2020, at 10:10, gil...@poolp.org wrote:
> April 28, 2020 8:55 AM, "Joerg Jung" mailto:m...@umaxx.net>> 
> wrote:
> 
>> Also this change might break existing valid setups (e.g. with mailing list
>> servers), but people will likely know how to cope with it.
> 
> Do you have an example of an existing valid setup that is broken with this ?

No example, I just presumed (that’s why I wrote “might”).



Re: smtpd: fix catch-all in virtual aliases

2020-04-28 Thread gilles
April 28, 2020 8:55 AM, "Joerg Jung"  wrote:

>> On 26. Apr 2020, at 18:30, Eric Faurot  wrote:
>> 
>> When a catch-all entry (@) is used in a virtual alias table, it
>> eventually (and mistakenly) catches everything that expands to a
>> username. For example, with:
>> 
>> f...@example.com user
>> @ catchall
>> 
>> "f...@example.com" expands to "user" as expected, but then "user"
>> expands to "catchall" because it is interpreted as "user@" (empty
>> domain).
> 
> Which makes sense to me. If one doesn’t specify a domain after the ‘@‘,
> I would expect to really catch-all for all domains and all users.
> 

that's not how mailaddr work in OpenSMTPD, the semantic is this one:

user=> user@*
user@domain => user@domain
@domain => *@domain
@   => *

and this is not an aliases only thing, this is how table lookups are
performed for type K_MAILADDR.

This allows you to do stuff like follows:

root : root
gil...@poolp.org : gilles
e...@faurot.net  : eric
@poolp.org   : poolpcatchall
@faurot.net  : faurotcatchall
@: catchallusuer


>> The catch-all fallback mechanism is really meant for full email
>> addresses in virtual context, and should not happen for usernames.
>> The following diff fixes it.
> 
> Yes, I agree that catch-all only really meant to be used for single virtual
> domain context and not with primary domains.
> 
> But instead of allowing the syntax and ignoring the case in aliases.c
> as in your diff below, I would prefer to “fail" on parsing of the table and
> error logging that an empty domain after ‘@‘ is not a valid syntax, no?
> 
> Also this change might break existing valid setups (e.g. with mailing list
> servers), but people will likely know how to cope with it.
> 

Do you have an example of an existing valid setup that is broken with this ?



Re: smtpd: fix catch-all in virtual aliases

2020-04-28 Thread Joerg Jung


> On 26. Apr 2020, at 18:30, Eric Faurot  wrote:
> 
> When a catch-all entry (@) is used in a virtual alias table, it
> eventually (and mistakenly) catches everything that expands to a
> username. For example, with:
> 
>f...@example.com  user
>@catchall
> 
> "f...@example.com" expands to "user" as expected, but then "user"
> expands to "catchall" because it is interpreted as "user@" (empty
> domain).

Which makes sense to me. If one doesn’t specify a domain after the ‘@‘,
I would expect to really catch-all for all domains and all users. 

> The catch-all fallback mechanism is really meant for full email
> addresses in virtual context, and should not happen for usernames.
> The following diff fixes it.

Yes, I agree that catch-all only really meant to be used for single virtual
domain context and not with primary domains. 

But instead of allowing the syntax and ignoring the case in aliases.c
as in your diff below, I would prefer to “fail" on parsing of the table and 
error logging that an empty domain after ‘@‘ is not a valid syntax, no?

Also this change might break existing valid setups (e.g. with mailing list 
servers), but people will likely know how to cope with it. 

Regards,
Joerg

> Index: aliases.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/aliases.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 aliases.c
> --- aliases.c 28 Dec 2018 12:47:28 -  1.77
> +++ aliases.c 26 Apr 2020 16:04:51 -
> @@ -164,6 +164,10 @@ aliases_virtual_get(struct expand *expan
>   if (ret)
>   goto expand;
> 
> + /* Do not try catch-all entries if there is no domain */
> + if (domain[0] == '\0')
> + return 0;
> +
>   if (!bsnprintf(buf, sizeof(buf), "@%s", domain))
>   return 0;
>   /* Failed ? We lookup for catch all for virtual domain */
> 



smtpd: fix catch-all in virtual aliases

2020-04-26 Thread Eric Faurot
Hi.

When a catch-all entry (@) is used in a virtual alias table, it
eventually (and mistakenly) catches everything that expands to a
username. For example, with:

f...@example.com  user
@catchall

"f...@example.com" expands to "user" as expected, but then "user"
expands to "catchall" because it is interpreted as "user@" (empty
domain).

The catch-all fallback mechanism is really meant for full email
addresses in virtual context, and should not happen for usernames.
The following diff fixes it.


Eric.

Index: aliases.c
===
RCS file: /cvs/src/usr.sbin/smtpd/aliases.c,v
retrieving revision 1.77
diff -u -p -r1.77 aliases.c
--- aliases.c   28 Dec 2018 12:47:28 -  1.77
+++ aliases.c   26 Apr 2020 16:04:51 -
@@ -164,6 +164,10 @@ aliases_virtual_get(struct expand *expan
if (ret)
goto expand;
 
+   /* Do not try catch-all entries if there is no domain */
+   if (domain[0] == '\0')
+   return 0;
+
if (!bsnprintf(buf, sizeof(buf), "@%s", domain))
return 0;
/* Failed ? We lookup for catch all for virtual domain */