Re: smtpd: relax ORCPT check again

2020-11-18 Thread gilles
November 18, 2020 10:17 PM, "Joerg Jung"  wrote:

> On Wed, Nov 18, 2020 at 08:57:48PM +, gil...@poolp.org wrote:
> 
>> November 18, 2020 9:54 PM, "Joerg Jung"  wrote:
>> 
>> Hi,
>> 
>> in my opinion revision 1.423 of smtp_session.c went a bit too far.
>> Enforcing that ORCPT has to have domain results in breakage in real
>> world usage. For example, sending from root user cron jobs mails
>> via Postfix aliased to an address handled by OpenSMTPD will fail.
>> 
>> RFC leaves some room for interpretation here, but interoperability
>> may be considered important as well. GitHub issue #1084 [1] has
>> some more elaborations and examples, but was opened for the same
>> reason.
>> 
>> Therefore, I propose the following diff below to slightly relax
>> the ORCPT check again by skipping the domain part check.
>> 
>> Comments, OK?
>> 
>> maybe just skip the check ONLY if domain part is empty ?
> 
> Like in the diff below and assuming that domain part is always
> nul terminated after text_to_mailaddr()?
> 

This reads better IMO yes


> OK?
> 
> Index: smtp_session.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
> retrieving revision 1.426
> diff -u -p -r1.426 smtp_session.c
> --- smtp_session.c 24 Apr 2020 11:34:07 - 1.426
> +++ smtp_session.c 18 Nov 2020 21:10:36 -
> @@ -2583,7 +2583,8 @@ smtp_tx_rcpt_to(struct smtp_tx *tx, cons
> 
> if (!text_to_mailaddr(>evp.dsn_orcpt, opt) ||
> !valid_localpart(tx->evp.dsn_orcpt.user) ||
> - !valid_domainpart(tx->evp.dsn_orcpt.domain)) {
> + (strlen(tx->evp.dsn_orcpt.domain) != 0 &&
> + !valid_domainpart(tx->evp.dsn_orcpt.domain))) {
> smtp_reply(tx->session,
> "553 ORCPT address syntax error");
> return;



Re: smtpd: relax ORCPT check again

2020-11-18 Thread Joerg Jung
On Wed, Nov 18, 2020 at 08:57:48PM +, gil...@poolp.org wrote:
> November 18, 2020 9:54 PM, "Joerg Jung"  wrote:
> 
> > Hi,
> > 
> > in my opinion revision 1.423 of smtp_session.c went a bit too far.
> > Enforcing that ORCPT has to have domain results in breakage in real
> > world usage. For example, sending from root user cron jobs mails 
> > via Postfix aliased to an address handled by OpenSMTPD will fail.
> > 
> > RFC leaves some room for interpretation here, but interoperability 
> > may be considered important as well. GitHub issue #1084 [1] has
> > some more elaborations and examples, but was opened for the same
> > reason.
> > 
> > Therefore, I propose the following diff below to slightly relax
> > the ORCPT check again by skipping the domain part check. 
> > 
> > Comments, OK?
> > 
> 
> maybe just skip the check ONLY if domain part is empty ?
> 

Like in the diff below and assuming that domain part is always
nul terminated after text_to_mailaddr()?

OK?


Index: smtp_session.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.426
diff -u -p -r1.426 smtp_session.c
--- smtp_session.c  24 Apr 2020 11:34:07 -  1.426
+++ smtp_session.c  18 Nov 2020 21:10:36 -
@@ -2583,7 +2583,8 @@ smtp_tx_rcpt_to(struct smtp_tx *tx, cons
 
if (!text_to_mailaddr(>evp.dsn_orcpt, opt) ||
!valid_localpart(tx->evp.dsn_orcpt.user) ||
-   !valid_domainpart(tx->evp.dsn_orcpt.domain)) {
+   (strlen(tx->evp.dsn_orcpt.domain) != 0 &&
+!valid_domainpart(tx->evp.dsn_orcpt.domain))) {
smtp_reply(tx->session,
"553 ORCPT address syntax error");
return;



smtpd: relax ORCPT check again

2020-11-18 Thread Joerg Jung
Hi,

in my opinion revision 1.423 of smtp_session.c went a bit too far.
Enforcing that ORCPT has to have domain results in breakage in real
world usage.  For example, sending from root user cron jobs mails 
via Postfix aliased to an address handled by OpenSMTPD will fail.

RFC leaves some room for interpretation here, but interoperability 
may be considered important as well.  GitHub issue #1084 [1] has
some more elaborations and examples, but was opened for the same
reason.

Therefore, I propose the following diff below to slightly relax
the ORCPT check again by skipping the domain part check. 

Comments, OK?

Thanks,
Regards,
Joerg

[1] https://github.com/OpenSMTPD/OpenSMTPD/issues/1084



Index: smtp_session.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.426
diff -u -p -r1.426 smtp_session.c
--- smtp_session.c  24 Apr 2020 11:34:07 -  1.426
+++ smtp_session.c  18 Nov 2020 20:24:28 -
@@ -2582,8 +2582,7 @@ smtp_tx_rcpt_to(struct smtp_tx *tx, cons
opt += 7;
 
if (!text_to_mailaddr(>evp.dsn_orcpt, opt) ||
-   !valid_localpart(tx->evp.dsn_orcpt.user) ||
-   !valid_domainpart(tx->evp.dsn_orcpt.domain)) {
+   !valid_localpart(tx->evp.dsn_orcpt.user)) {
smtp_reply(tx->session,
"553 ORCPT address syntax error");
return;