Sorry, some unnecessary whitespace change was part of the attached
patch. Find attached a cleaned up version of the patch (functionally the
same).
On Thu, Dec 07, 2023 at 01:34:42PM +0100, Tassilo Philipp wrote:
After a direct exchange with Omar Polo about the ORCPT patch, find
attached a revised version of it. The changes compared to Tim's last
patch are, just fyi:
1) valid_xtext() has been modified to avoid potential problems with
sign extensions when using the ctype is*() functions. I also narrowed
down the checks for hexchar letters to only allow ABCDEF (previously
it just checked if it's an uppercase letter). A superflous string
length check on hexchars was removed, b/c this is implicitly handled
when checking the characters ('\0' fails the isdigit() check and
related). While at it I reformatted a bit to be less verbose.
2) I reintroduced the check whether the entire ORCPT parameter exceeds
the rfc3461 specified 500 char limit that Tim had in his previous
patch.
3) I relaxed the check whether the ORCPT param is a valid mail
address, which was done by text_to_mailaddr(). The reason is that it
simply doesn't apply to the ORCPT parameter, as it's xtext encoded (so
you could for example write '@' - which that function tries to find -
as '+40'). It might also be a different address type altogether (e.g.
X.400 as given as example by the rfc), etc.. so that
text_to_mailaddr() logic might simply not apply to that param. So
basically what's left is using that function only to see whether it's
an uncommon ORCPT param (and printing that to the log), or not.
In theory, the xtext would have to be decoded, first, to be pendantic
about this, which I skipped b/c it won't make a difference in this
case for deliverability... not sure if this all should be simplified
even more to just check for xtext, and that's it, not writing any
"uncommon ORCPT param" to the log at all. In any case we leave the
ORCPT param untouched. Thoughts?
4) The patch is now rebased against the latest version of opensmtpd.
Patch is attached
Cheers
On Sat, Nov 18, 2023 at 11:54:33AM +0100, Tassilo Philipp wrote:
Sorry for another bump of this patch: can it be merged?
I know the groupwise example in this thread is rare and doesn't
affect a lot of smtpd users, but without it, it's unfortunately a
bit of a blocker for some people. We personally apply this patch to
our opensmtpd builds, but for others this might simply be a blocker
to use opensmtpd, as they cannot control e.g. what a client uses to
send them mail.
Such mails get rejected by any MTA on a route, no matter if the
rejecting server is just a helper relay. The weird ORCP format used
by groupwise is adhering to the RFC xtext spec, after all, and is
valid.
If this cannot/won't be merged, please share the reasons for why not.
Thank you
On Thu, Jul 20, 2023 at 09:58:16AM +0200, Tassilo Philipp wrote:
Sorry to shamelessly "bump" this, but any way to get this
integrated into upstream, eventually?
We used the original patch from Frank Scholl and then this
improved one in production now for like a year, now, and didn't
experience issues. In our case it is specifically needed for a
client that uses GroupWise[0] internally to send mails (which
seems to always generate mails with an "xtext" ORCPT param).
Thanks!
[0] https://www.microfocus.com/products/groupwise/
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:<[email protected]>
ORCPT=rfc822;[email protected]:0:0 or RCPT
TO:<[email protected]>
ORCPT=rfc822;[email protected]: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:<[email protected]>
ORCPT=rfc822;[email protected]:0:0
NOTIFY=SUCCESS,FAILURE" result="553 ORCPT address syntax error"
I've studied RFC 3461 section 4 and 4.2 but it's not entirely
clear to me if the above ORCPT command is valid or not. The
encoding adheres to the spec, which says it must be valid xtext.
With this patch smtpd accepts any ORCPT that is valid xtext as
defined in the RFC (and logs on informational message when it
consists of an invalid user or domain name).
Cheers,
Tim
---
usr.sbin/smtpd/smtp_session.c | 22 ++++++++++++++++++----
usr.sbin/smtpd/smtpd.h | 1 +
usr.sbin/smtpd/util.c | 32
++++++++++++++++++++++++++++++++ 3 files changed, 51
insertions(+), 4 deletions(-)
diff --git a/usr.sbin/smtpd/smtp_session.c
b/usr.sbin/smtpd/smtp_session.c index 72e13e8fd8d..c0c29d4a695
100644
--- a/usr.sbin/smtpd/smtp_session.c
+++ b/usr.sbin/smtpd/smtp_session.c
@@ -2415,6 +2415,7 @@ smtp_tx_create_message(struct smtp_tx *tx)
static void
smtp_tx_rcpt_to(struct smtp_tx *tx, const char *line)
{
+ struct mailaddr orcptaddr;
char *opt, *p;
char *copy;
char tmp[SMTP_LINE_MAX]; @@ -2469,10 +2470,23 @@
smtp_tx_rcpt_to(struct smtp_tx *tx, const char *line)
if (strncasecmp(opt, "rfc822;", 7) == 0)
opt += 7;
- 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 (!text_to_mailaddr(&orcptaddr, opt)) {
+ smtp_reply(tx->session,
+ "553 ORCPT address syntax
error"); + return;
+ }
+
+ if (valid_localpart(orcptaddr.user) &&
+ (strlen(orcptaddr.domain) != 0 &&
+
valid_domainpart(orcptaddr.domain))) { +
tx->evp.dsn_orcpt = orcptaddr; + } else if
(valid_xtext(opt)) {
+ log_info("%016"PRIx64" smtp "
+ "uncommon ORCPT: \"%s\",
u:\"%s\", d:\"%s\"",
+ tx->session->id,
+ opt, orcptaddr.user,
orcptaddr.domain); +
tx->evp.dsn_orcpt = orcptaddr;
+ } else {
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 125a6a5dfbe..c59706885e2 100644
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1702,6 +1702,7 @@ int mailaddr_match(const struct mailaddr
*, const struct mailaddr *);
int valid_localpart(const char *);
int valid_domainpart(const char *);
int valid_domainname(const char *);
+int valid_xtext(const char *s);
int valid_smtp_response(const char *);
int secure_file(int, char *, char *, uid_t, int);
int lowercase(char *, const char *, size_t);
diff --git a/usr.sbin/smtpd/util.c b/usr.sbin/smtpd/util.c
index feb663cc61e..0c4d0015fa4 100644
--- a/usr.sbin/smtpd/util.c
+++ b/usr.sbin/smtpd/util.c
@@ -515,6 +515,38 @@ valid_domainname(const char *str)
return 1; }
+int
+valid_xtext(const char *s)
+{
+ while (*s != '\0') {
+ if (*s == '=')
+ return 0;
+
+ if (*s < '\x21' || *s > '\x7e')
+ return 0;
+
+ if (*s == '+') {
+ /* expect hexchar "+XX" RFC 3461 4. */
+ if (strnlen(s, 3) != 3)
+ return 0;
+
+ s++;
+
+ if (!isdigit(*s) && !isupper(*s))
+ return 0;
+
+ s++;
+
+ if (!isdigit(*s) && !isupper(*s))
+ return 0;
+ }
+
+ s++;
+ }
+
+ return 1;
+}
+
int
valid_smtp_response(const char *s)
{
--
2.37.3
--- ./usr.sbin/smtpd/util.c.orig 2023-09-16 20:08:34.000000000 +0200
+++ ./usr.sbin/smtpd/util.c 2023-12-07 12:37:07.892905000 +0100
@@ -522,6 +522,27 @@
}
int
+valid_xtext(const char *s)
+{
+ while (*s != '\0') {
+ if (*s == '=' || *s < '\x21' || *s > '\x7e')
+ return 0;
+
+ if (*s == '+') { /* expect hexchar "+XX" RFC 3461 4. */
+ int i;
+ for(i = 0; i < 2; ++i) {
+ s++;
+ if (!isdigit((unsigned char)*s) && !(*s >= 'A' && *s <= 'F'))
+ return 0;
+ }
+ }
+ s++;
+ }
+
+ return 1;
+}
+
+int
valid_smtp_response(const char *s)
{
if (strlen(s) < 5)
--- ./usr.sbin/smtpd/smtpd.h.orig 2023-09-16 20:08:34.000000000 +0200
+++ ./usr.sbin/smtpd/smtpd.h 2023-12-07 12:37:07.893695000 +0100
@@ -1724,6 +1724,7 @@
int valid_localpart(const char *);
int valid_domainpart(const char *);
int valid_domainname(const char *);
+int valid_xtext(const char *s);
int valid_smtp_response(const char *);
int secure_file(int, char *, char *, uid_t, int);
int lowercase(char *, const char *, size_t);
--- ./usr.sbin/smtpd/smtp_session.c.orig 2023-09-16 20:08:34.000000000 +0200
+++ ./usr.sbin/smtpd/smtp_session.c 2023-12-07 13:01:05.791195000 +0100
@@ -50,6 +50,7 @@
#define SMTP_LINE_MAX 65535
#define DATA_HIWAT 65535
#define APPEND_DOMAIN_BUFFER_SIZE SMTP_LINE_MAX
+#define ORCPT_MAX 500 /* RFC 3461 4.2 */
enum smtp_state {
STATE_NEW = 0,
@@ -2434,6 +2435,7 @@
static void
smtp_tx_rcpt_to(struct smtp_tx *tx, const char *line)
{
+ struct mailaddr orcptaddr;
char *opt, *p;
char *copy;
char tmp[SMTP_LINE_MAX];
@@ -2483,17 +2485,31 @@
return;
}
} else if (ADVERTISE_EXT_DSN(tx->session) && strncasecmp(opt, "ORCPT=", 6) == 0) {
+ if (strnlen(opt, ORCPT_MAX + 1) > ORCPT_MAX) {
+ smtp_reply(tx->session,
+ "553 ORCPT parameter too long");
+ return;
+ }
+
opt += 6;
if (strncasecmp(opt, "rfc822;", 7) == 0)
opt += 7;
- 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 (text_to_mailaddr(&orcptaddr, opt) &&
+ valid_localpart(orcptaddr.user) &&
+ (strlen(orcptaddr.domain) != 0 &&
+ valid_domainpart(orcptaddr.domain))) {
+ tx->evp.dsn_orcpt = orcptaddr;
+ } else if (valid_xtext(opt)) {
+ log_info("%016"PRIx64" smtp "
+ "uncommon ORCPT: \"%s\", u:\"%s\",
d:\"%s\"",
+ tx->session->id,
+ opt, orcptaddr.user, orcptaddr.domain);
+ tx->evp.dsn_orcpt = orcptaddr;
+ } else {
smtp_reply(tx->session,
- "553 ORCPT address syntax error");
+ "553 ORCPT address syntax error");
return;
}
} else {
--- ./usr.sbin/smtpd/util.c.orig 2023-09-16 20:08:34.000000000 +0200
+++ ./usr.sbin/smtpd/util.c 2023-12-07 12:37:07.892905000 +0100
@@ -522,6 +522,27 @@
}
int
+valid_xtext(const char *s)
+{
+ while (*s != '\0') {
+ if (*s == '=' || *s < '\x21' || *s > '\x7e')
+ return 0;
+
+ if (*s == '+') { /* expect hexchar "+XX" RFC 3461 4. */
+ int i;
+ for(i = 0; i < 2; ++i) {
+ s++;
+ if (!isdigit((unsigned char)*s) && !(*s >= 'A'
&& *s <= 'F'))
+ return 0;
+ }
+ }
+ s++;
+ }
+
+ return 1;
+}
+
+int
valid_smtp_response(const char *s)
{
if (strlen(s) < 5)
--- ./usr.sbin/smtpd/smtpd.h.orig 2023-09-16 20:08:34.000000000 +0200
+++ ./usr.sbin/smtpd/smtpd.h 2023-12-07 12:37:07.893695000 +0100
@@ -1724,6 +1724,7 @@
int valid_localpart(const char *);
int valid_domainpart(const char *);
int valid_domainname(const char *);
+int valid_xtext(const char *s);
int valid_smtp_response(const char *);
int secure_file(int, char *, char *, uid_t, int);
int lowercase(char *, const char *, size_t);
--- ./usr.sbin/smtpd/smtp_session.c.orig 2023-09-16 20:08:34.000000000
+0200
+++ ./usr.sbin/smtpd/smtp_session.c 2023-12-07 13:50:50.887265000 +0100
@@ -50,6 +50,7 @@
#define SMTP_LINE_MAX 65535
#define DATA_HIWAT 65535
#define APPEND_DOMAIN_BUFFER_SIZE SMTP_LINE_MAX
+#define ORCPT_MAX 500 /* RFC 3461 4.2 */
enum smtp_state {
STATE_NEW = 0,
@@ -2434,6 +2435,7 @@
static void
smtp_tx_rcpt_to(struct smtp_tx *tx, const char *line)
{
+ struct mailaddr orcptaddr;
char *opt, *p;
char *copy;
char tmp[SMTP_LINE_MAX];
@@ -2483,15 +2485,29 @@
return;
}
} else if (ADVERTISE_EXT_DSN(tx->session) && strncasecmp(opt,
"ORCPT=", 6) == 0) {
+ if (strnlen(opt, ORCPT_MAX + 1) > ORCPT_MAX) {
+ smtp_reply(tx->session,
+ "553 ORCPT parameter too long");
+ return;
+ }
+
opt += 6;
if (strncasecmp(opt, "rfc822;", 7) == 0)
opt += 7;
- 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 (text_to_mailaddr(&orcptaddr, opt) &&
+ valid_localpart(orcptaddr.user) &&
+ (strlen(orcptaddr.domain) != 0 &&
+ valid_domainpart(orcptaddr.domain))) {
+ tx->evp.dsn_orcpt = orcptaddr;
+ } else if (valid_xtext(opt)) {
+ log_info("%016"PRIx64" smtp "
+ "uncommon ORCPT: \"%s\", u:\"%s\",
d:\"%s\"",
+ tx->session->id,
+ opt, orcptaddr.user, orcptaddr.domain);
+ tx->evp.dsn_orcpt = orcptaddr;
+ } else {
smtp_reply(tx->session,
"553 ORCPT address syntax error");
return;
