Re: [PATCH] relax 553 ORCPT address syntax error (was Re: EMails to "ORCPT=rfc822;u...@example.co
Thanks for looking into this! Also thanks for taking this a step further
and removing unneeded complexity, given the train of thought to consider
this param an opaque string.
I tried the patch in a real world scenario, having rolled it out to
multiple machines and relays, ran manual tests with this parameter, and
I didn't spot any issues, so far.
On Mon, Dec 11, 2023 at 08:56:08PM +0100, Omar Polo wrote:
[moving to tech@]
This seems to be a long-standing issue in opensmtpd. There seem to be
(at least) one fairly popular application that uses a, upon a first
look, odd ORCPT parameter:
On Fri, Oct 28, 2022 at 04:16:36PM +0200, Tim Kuijsten wrote:
I have refined and more thoroughly tested a previous patch that
relaxes the ORCPT address check.
Over the years mail has been rejected by senders that use RCPT
TO commands like:
RCPT TO:
ORCPT=rfc822;groupwise-i...@example.com:0:0 or RCPT
TO:
ORCPT=rfc822;groupwise-i...@example.com:0:0
NOTIFY=SUCCESS,FAILURE
In the above the domain part of the ORCPT address resolves to
example.com:0:0 which is rejected by smtpd with the message:
smtpd[20797]: 1a3a396cd4c57d05 smtp failed-command command="RCPT
TO:
ORCPT=rfc822;groupwise-i...@example.com:0:0
NOTIFY=SUCCESS,FAILURE" result="553 ORCPT address syntax error"
[...]
Quoting RFC 3461, the ORCPT parameter is defined as (¶ 4.2)
orcpt-parameter = "ORCPT=" original-recipient-address
original-recipient-address = addr-type ";" xtext
addr-type = atom
The "addr-type" portion MUST be an IANA-registered electronic mail
address-type (as defined in [3]), while the "xtext" portion contains
an encoded representation of the original recipient address using the
rules in section 5 of this document.
[...]
The "addr-type" portion of the original-recipient-address is used to
indicate the "type" of the address which appears in the ORCPT
parameter value. However, the address associated with the ORCPT
keyword is NOT constrained to conform to the syntax rules for that
"addr-type".
I'd like to emphasize the last sentence, which is the one I missed
upon the first couple of look at this RFC: even if addr-type is
rfc822, the xtext part (once decoded) is not constrained to conform to
the syntax rules of rfc822.
That is, this groupwise thingy is actually confirming to the rfc when
using a param of "rfc822;groupwise-i...@example.com:0:0".
Honesty, I fail to understand the meaning of the addr-type if any
gargabe is allowed.
The "once decoded" part is important since the RFC allows for ANY
character to be encoded, so rfc822;+45+78+62+6d+70+6c+65+40...
(i.e. example@...) is still a valid ORCPT param.
Tassilo' diff (which is a slightly modified version of the one
originally sent in the bug report by Tim) is available here
https://www.mail-archive.com/misc@opensmtpd.org/msg06054.html
and it relaxes the checks but IMHO it doesn't address the underlying
issue: we expect a valid rfc822 address where it's not mandatory.
I think we should just keep the ORCPT address as on opaque string
(modulo some validation) without trying to parse it.
I still have to test the diff below in a real-world scenario, but I'm
sending it out so other can hopefully give it a spin.
(and I couldn't resist splitting a long line in the process.)
Thanks,
Omar Polo
diff /home/op/w/smtpd
commit - 45a7eac758c7b1e9c4f16ab2dfcb25672b49aad9
path + /home/op/w/smtpd
blob - c9611beb48feab47602b766061a7546c375160a8
file + envelope.c
--- envelope.c
+++ envelope.c
@@ -443,7 +443,8 @@ ascii_load_field(const char *field, struct envelope *e
return ascii_load_uint8(>dsn_notify, buf);
if (strcasecmp("dsn-orcpt", field) == 0)
- return ascii_load_mailaddr(>dsn_orcpt, buf);
+ return ascii_load_string(ep->dsn_orcpt, buf,
+ sizeof(ep->dsn_orcpt));
if (strcasecmp("dsn-ret", field) == 0)
return ascii_load_dsn_ret(>dsn_ret, buf);
@@ -703,11 +704,8 @@ ascii_dump_field(const char *field, const struct envel
if (strcasecmp(field, "dsn-ret") == 0)
return ascii_dump_dsn_ret(ep->dsn_ret, buf, len);
- if (strcasecmp(field, "dsn-orcpt") == 0) {
- if (ep->dsn_orcpt.user[0] && ep->dsn_orcpt.domain[0])
- return ascii_dump_mailaddr(>dsn_orcpt, buf, len);
- return 1;
- }
+ if (strcasecmp(field, "dsn-orcpt") == 0)
+ return ascii_dump_string(ep->dsn_orcpt, buf, len);
if (strcasecmp(field, "dsn-envid") == 0)
return ascii_dump_string(ep->dsn_envid, buf, len);
blob - f0bb42c53ffb8f98012d542209bb55ecd1ae
file + mta.c
--- mta.c
+++ mta.c
@@ -809,11 +809,8 @@ mta_handle_envelope(struct envelope *evp, const char *
if (strcmp(buf, e->dest))
e->rcpt = xstrdup(buf);
e->task = task;
- if (evp->dsn_orcpt.user[0] && evp->dsn_orcpt.domain[0]) {
- (void)snprintf(buf, sizeof buf,
Re: [PATCH] relax 553 ORCPT address syntax error (was Re: EMails to "ORCPT=rfc822;u...@example.co
[moving to tech@]
This seems to be a long-standing issue in opensmtpd. There seem to be
(at least) one fairly popular application that uses a, upon a first
look, odd ORCPT parameter:
> >>> On Fri, Oct 28, 2022 at 04:16:36PM +0200, Tim Kuijsten wrote:
> I have refined and more thoroughly tested a previous patch that
> relaxes the ORCPT address check.
>
> Over the years mail has been rejected by senders that use RCPT
> TO commands like:
> RCPT TO:
> ORCPT=rfc822;groupwise-i...@example.com:0:0 or RCPT
> TO:
> ORCPT=rfc822;groupwise-i...@example.com:0:0
> NOTIFY=SUCCESS,FAILURE
>
> In the above the domain part of the ORCPT address resolves to
> example.com:0:0 which is rejected by smtpd with the message:
> smtpd[20797]: 1a3a396cd4c57d05 smtp failed-command command="RCPT
> TO:
> ORCPT=rfc822;groupwise-i...@example.com:0:0
> NOTIFY=SUCCESS,FAILURE" result="553 ORCPT address syntax error"
>
> [...]
Quoting RFC 3461, the ORCPT parameter is defined as (¶ 4.2)
orcpt-parameter = "ORCPT=" original-recipient-address
original-recipient-address = addr-type ";" xtext
addr-type = atom
The "addr-type" portion MUST be an IANA-registered electronic mail
address-type (as defined in [3]), while the "xtext" portion contains
an encoded representation of the original recipient address using the
rules in section 5 of this document.
[...]
The "addr-type" portion of the original-recipient-address is used to
indicate the "type" of the address which appears in the ORCPT
parameter value. However, the address associated with the ORCPT
keyword is NOT constrained to conform to the syntax rules for that
"addr-type".
I'd like to emphasize the last sentence, which is the one I missed
upon the first couple of look at this RFC: even if addr-type is
rfc822, the xtext part (once decoded) is not constrained to conform to
the syntax rules of rfc822.
That is, this groupwise thingy is actually confirming to the rfc when
using a param of "rfc822;groupwise-i...@example.com:0:0".
Honesty, I fail to understand the meaning of the addr-type if any
gargabe is allowed.
The "once decoded" part is important since the RFC allows for ANY
character to be encoded, so rfc822;+45+78+62+6d+70+6c+65+40...
(i.e. example@...) is still a valid ORCPT param.
Tassilo' diff (which is a slightly modified version of the one
originally sent in the bug report by Tim) is available here
https://www.mail-archive.com/misc@opensmtpd.org/msg06054.html
and it relaxes the checks but IMHO it doesn't address the underlying
issue: we expect a valid rfc822 address where it's not mandatory.
I think we should just keep the ORCPT address as on opaque string
(modulo some validation) without trying to parse it.
I still have to test the diff below in a real-world scenario, but I'm
sending it out so other can hopefully give it a spin.
(and I couldn't resist splitting a long line in the process.)
Thanks,
Omar Polo
diff /home/op/w/smtpd
commit - 45a7eac758c7b1e9c4f16ab2dfcb25672b49aad9
path + /home/op/w/smtpd
blob - c9611beb48feab47602b766061a7546c375160a8
file + envelope.c
--- envelope.c
+++ envelope.c
@@ -443,7 +443,8 @@ ascii_load_field(const char *field, struct envelope *e
return ascii_load_uint8(>dsn_notify, buf);
if (strcasecmp("dsn-orcpt", field) == 0)
- return ascii_load_mailaddr(>dsn_orcpt, buf);
+ return ascii_load_string(ep->dsn_orcpt, buf,
+ sizeof(ep->dsn_orcpt));
if (strcasecmp("dsn-ret", field) == 0)
return ascii_load_dsn_ret(>dsn_ret, buf);
@@ -703,11 +704,8 @@ ascii_dump_field(const char *field, const struct envel
if (strcasecmp(field, "dsn-ret") == 0)
return ascii_dump_dsn_ret(ep->dsn_ret, buf, len);
- if (strcasecmp(field, "dsn-orcpt") == 0) {
- if (ep->dsn_orcpt.user[0] && ep->dsn_orcpt.domain[0])
- return ascii_dump_mailaddr(>dsn_orcpt, buf, len);
- return 1;
- }
+ if (strcasecmp(field, "dsn-orcpt") == 0)
+ return ascii_dump_string(ep->dsn_orcpt, buf, len);
if (strcasecmp(field, "dsn-envid") == 0)
return ascii_dump_string(ep->dsn_envid, buf, len);
blob - f0bb42c53ffb8f98012d542209bb55ecd1ae
file + mta.c
--- mta.c
+++ mta.c
@@ -809,11 +809,8 @@ mta_handle_envelope(struct envelope *evp, const char *
if (strcmp(buf, e->dest))
e->rcpt = xstrdup(buf);
e->task = task;
- if (evp->dsn_orcpt.user[0] && evp->dsn_orcpt.domain[0]) {
- (void)snprintf(buf, sizeof buf, "%s@%s",
- evp->dsn_orcpt.user, evp->dsn_orcpt.domain);
- e->dsn_orcpt = xstrdup(buf);
- }
+ if (evp->dsn_orcpt[0] != '\0')
+ e->dsn_orcpt = xstrdup(evp->dsn_orcpt);
(void)strlcpy(e->dsn_envid,
