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,