relax ORCPT syntax validation

We expected the ORCPT parameter to be a valid rfc822 address.  This is
wrong on multiple levels:

 - any other IANA-registered "addr-type" can be used
 - the parameter may be encoded and we didn't decode it prior validation
 - RFC3461 explicitly states that "[..] the address associated with the
   ORCPT keyword is NOT constrained to conform to the syntax rules for
   that 'addr-type'".

Instead, just validate the xtext and preserve the ORCPT value as-is.

Issue originally reported by Tim Kuijsten, Tassilo Philipp and others.

ok millert@

diff --git a/usr.sbin/smtpd/envelope.c b/usr.sbin/smtpd/envelope.c
index c9611be..b433096 100644 (file)
--- a/usr.sbin/smtpd/envelope.c
+++ b/usr.sbin/smtpd/envelope.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: envelope.c,v 1.51 2023/02/06 18:35:52 semarie Exp $   */
+/*     $OpenBSD: envelope.c,v 1.52 2024/01/03 08:11:15 op Exp $        */
 
 /*
  * Copyright (c) 2013 Eric Faurot <eric@openbsd.org>
@@ -443,7 +443,8 @@ ascii_load_field(const char *field, struct envelope *ep, char *buf)
                return ascii_load_uint8(&ep->dsn_notify, buf);
 
        if (strcasecmp("dsn-orcpt", field) == 0)
-               return ascii_load_mailaddr(&ep->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(&ep->dsn_ret, buf);
@@ -703,11 +704,8 @@ ascii_dump_field(const char *field, const struct envelope *ep,
        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(&ep->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);

diff --git a/usr.sbin/smtpd/mta.c b/usr.sbin/smtpd/mta.c
index f0bb42c..fe43322 100644 (file)
--- a/usr.sbin/smtpd/mta.c
+++ b/usr.sbin/smtpd/mta.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: mta.c,v 1.246 2023/11/08 08:46:35 op Exp $    */
+/*     $OpenBSD: mta.c,v 1.247 2024/01/03 08:11:15 op Exp $    */
 
 /*
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@openbsd.org>
@@ -809,11 +809,8 @@ mta_handle_envelope(struct envelope *evp, const char *smarthost)
        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, evp->dsn_envid,
            sizeof e->dsn_envid);
        e->dsn_notify = evp->dsn_notify;

diff --git a/usr.sbin/smtpd/mta_session.c b/usr.sbin/smtpd/mta_session.c
index 160b41d..5bdb56c 100644 (file)
--- a/usr.sbin/smtpd/mta_session.c
+++ b/usr.sbin/smtpd/mta_session.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: mta_session.c,v 1.149 2023/05/31 16:51:46 op Exp $    */
+/*     $OpenBSD: mta_session.c,v 1.150 2024/01/03 08:11:15 op Exp $    */
 
 /*
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@openbsd.org>
@@ -809,7 +809,7 @@ again:
                            e->dest,
                            e->dsn_notify ? " NOTIFY=" : "",
                            e->dsn_notify ? dsn_strnotify(e->dsn_notify) : "",
-                           e->dsn_orcpt ? " ORCPT=rfc822;" : "",
+                           e->dsn_orcpt ? " ORCPT=" : "",
                            e->dsn_orcpt ? e->dsn_orcpt : "");
                } else
                        mta_send(s, "RCPT TO:<%s>", e->dest);

diff --git a/usr.sbin/smtpd/smtp_session.c b/usr.sbin/smtpd/smtp_session.c
index f0ede54..bc048cf 100644 (file)
--- a/usr.sbin/smtpd/smtp_session.c
+++ b/usr.sbin/smtpd/smtp_session.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: smtp_session.c,v 1.438 2023/12/23 10:29:05 op Exp $   */
+/*     $OpenBSD: smtp_session.c,v 1.439 2024/01/03 08:11:15 op Exp $   */
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
@@ -2471,16 +2471,15 @@ smtp_tx_rcpt_to(struct smtp_tx *tx, const char *line)
                                    " combined with other options");
                                return;
                        }
-               } else if (ADVERTISE_EXT_DSN(tx->session) && strncasecmp(opt, "ORCPT=", 6) == 0) {
-                       opt += 6;
+               } else if (ADVERTISE_EXT_DSN(tx->session) &&
+                   strncasecmp(opt, "ORCPT=", 6) == 0) {
+                       size_t len = sizeof(tx->evp.dsn_orcpt);
 
-                       if (strncasecmp(opt, "rfc822;", 7) == 0)
-                               opt += 7;
+                       opt += 6;
 
-                       if (!text_to_mailaddr(&tx->evp.dsn_orcpt, opt) ||
-                           !valid_localpart(tx->evp.dsn_orcpt.user) ||
-                           (strlen(tx->evp.dsn_orcpt.domain) != 0 &&
-                            !valid_domainpart(tx->evp.dsn_orcpt.domain))) {
+                       if ((p = strchr(opt, ';')) == NULL ||
+                           !valid_xtext(p + 1) ||
+                           strlcpy(opt, tx->evp.dsn_orcpt, len) >= len) {
                                smtp_reply(tx->session,
                                    "553 ORCPT address syntax error");
                                return;

diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h
index 25090c3..5e1b16b 100644 (file)
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: smtpd.h,v 1.679 2023/11/08 08:46:35 op Exp $  */
+/*     $OpenBSD: smtpd.h,v 1.680 2024/01/03 08:11:15 op Exp $  */
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
@@ -467,6 +467,7 @@ struct maddrmap {
 #define DSN_NEVER   0x08
 
 #define        DSN_ENVID_LEN   100
+#define        DSN_ORCPT_LEN   500
 
 #define        SMTPD_ENVELOPE_VERSION          3
 struct envelope {
@@ -507,7 +508,7 @@ struct envelope {
        time_t                          nexttry;
        time_t                          lastbounce;
 
-       struct mailaddr                 dsn_orcpt;
+       char                            dsn_orcpt[DSN_ORCPT_LEN+1];
        char                            dsn_envid[DSN_ENVID_LEN+1];
        uint8_t                         dsn_notify;
        enum dsn_ret                    dsn_ret;
@@ -1703,6 +1704,7 @@ int valid_localpart(const char *);
 int valid_domainpart(const char *);
 int valid_domainname(const char *);
 int valid_smtp_response(const char *);
+int valid_xtext(const char *);
 int secure_file(int, char *, char *, uid_t, int);
 int  lowercase(char *, const char *, size_t);
 void xlowercase(char *, const char *, size_t);

diff --git a/usr.sbin/smtpd/util.c b/usr.sbin/smtpd/util.c
index feb663c..ba9029e 100644 (file)
--- a/usr.sbin/smtpd/util.c
+++ b/usr.sbin/smtpd/util.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: util.c,v 1.154 2021/06/14 17:58:16 eric Exp $ */
+/*     $OpenBSD: util.c,v 1.155 2024/01/03 08:11:15 op Exp $   */
 
 /*
  * Copyright (c) 2000,2001 Markus Friedl.  All rights reserved.
@@ -530,6 +530,30 @@ valid_smtp_response(const char *s)
        return 1;
 }
 
+int
+valid_xtext(const char *s)
+{
+       for (; *s != '\0'; ++s) {
+               if (*s < '!' || *s > '~' || *s == '=')
+                       return 0;
+
+               if (*s != '+')
+                       continue;
+
+               s++;
+               if (!isdigit((unsigned char)*s) &&
+                   !(*s >= 'A' && *s <= 'F'))
+                       return 0;
+
+               s++;
+               if (!isdigit((unsigned char)*s) &&
+                   !(*s >= 'A' && *s <= 'F'))
+                       return 0;
+       }
+
+       return 1;
+}
+
 int
 secure_file(int fd, char *path, char *userdir, uid_t uid, int mayread)
 {