Hi,
Find below the first stab at a final patch making generated bounce
mails follow more closely the RFCs 3461, 3464 and 6522. The patch
includes the patch from my previous mail in this thread.
The patch is based on opensmtp 7.5.0rc1 (w/ the additional patch in
the rc1 announcement thread on this ml).
It does, in short:
- uses the correct content type(s) for DSNs, as specified in RFC3464 and
RFC6522
- adds xtext_decode() utility func (following the style/interface of the
base64_decode() utility func)
- extends the data structures of the bounce message and envelope, as
necesary, to write the required fields outlined by RFC3461 and RFC3464
into the machine readable sub mime part, namely, per-message field:
* Original-Envelope-Id: (required if ENVID= was used, see RFC3461)
and per recipiend fields:
* Original-Recipient: (required if ORCPT= was used, see RFC3461)
* Arrival-Date: (optional, but added it while at it...)
* Diagnostic-Code: (optional, but added it while at it...)
- adds a check whether the SMTP ENVID= param is valid xtext (RFC3461)
- minor: removes a space in already set Final-Recipient: field
(following more closely the example layout from RFC3464)
One thing I wasn't sure about what to do is *where* to handle the xtext
decoding of the ENVID= and ORCPT= SMTP params. Right now the SMTP
handling part just considers them "opaque text", as they are just handed
down the relays, unmodified. When copying those into a DSN, they must be
decoded, though, and the decoded result is considered being printable
US-ASCII. From RFC3461 4.2 about ORCPT:
`Due to limitations in the Delivery Status Notification format, the
value of the original recipient address prior to encoding as "xtext"
MUST consist entirely of printable (graphic and white space)
characters from the US-ASCII repertoire.`
A functional identical wording is used in 4.4 for the ENVID= param.
The patch below keeps the SMTP behaviour to just treat those params as
opaque strings, but checks if the decoded part is US-ASCII when
generating a bounce - and if it's not, doesn't write it to the
respective DSN field(s).
I think the SMTP handling might be the better place to check for that,
and refuse such SMTP params outright, maybe?
Thanks
--- ./usr.sbin/smtpd/bounce.c
+++ ./usr.sbin/smtpd/bounce.c
@@ -57,6 +57,9 @@
char *report;
uint8_t esc_class;
uint8_t esc_code;
+ char *errorline;
+ /* decoded xtext SMTP ORCPT param, len always <= encoded */
+ char orcpt[DSN_ORCPT_LEN+1];
};
struct bounce_message {
@@ -68,6 +71,9 @@
char *to;
time_t timeout;
TAILQ_HEAD(, bounce_envelope) envelopes;
+ time_t creation;
+ /* decoded xtext SMTP ENVID param, len always <= encoded */
+ char envid[DSN_ENVID_LEN+1];
};
struct bounce_session {
@@ -120,6 +126,18 @@
}
}
+static int
+count_printable_ascii_chars(const char *s)
+{
+ const char *p = s;
+ for (; *p; ++p) {
+ if (!isprint((unsigned char)*p))
+ break;
+ }
+ return p - s;
+}
+
+
void
bounce_add(uint64_t evpid)
{
@@ -127,6 +145,7 @@
struct envelope evp;
struct bounce_message key, *msg;
struct bounce_envelope *be;
+ int n;
bounce_init();
@@ -164,6 +183,14 @@
msg->msgid = key.msgid;
msg->bounce = key.bounce;
+ msg->creation = evp.creation;
+ n = xtext_decode(evp.dsn_envid, msg->envid, sizeof(msg->envid));
+ if (n == -1 || n != count_printable_ascii_chars(msg->envid)) {
+ msg->envid[0] = '\0';
+ log_debug("debug: bounce: ill-formed ENVID param (decoded
xtext not "
+ "printable ascii, not setting DSN Original-Envelope-Id:
header");
+ }
+
TAILQ_INIT(&msg->envelopes);
msg->smtpname = xstrdup(evp.smtpname);
@@ -187,11 +214,18 @@
be = xmalloc(sizeof *be);
be->id = evpid;
be->report = xstrdup(buf);
+ be->errorline = xstrdup(evp.errorline);
(void)strlcpy(be->dest.user, evp.dest.user, sizeof(be->dest.user));
(void)strlcpy(be->dest.domain, evp.dest.domain,
sizeof(be->dest.domain));
be->esc_class = evp.esc_class;
be->esc_code = evp.esc_code;
+ n = xtext_decode(evp.dsn_orcpt, be->orcpt, sizeof(be->orcpt));
+ if (n == -1 || n != count_printable_ascii_chars(be->orcpt)) {
+ be->orcpt[0] = '\0';
+ log_debug("debug: bounce: ill-formed ORCPT param (decoded xtext not
"
+ "printable ascii, not setting DSN Original-Recipient:
header");
+ }
TAILQ_INSERT_TAIL(&msg->envelopes, be, entry);
log_debug("debug: bounce: adding report %16"PRIx64": %s", be->id,
be->report);
@@ -404,6 +438,7 @@
return (1);
}
+
static int
bounce_next(struct bounce_session *s)
{
@@ -452,7 +487,8 @@
"To: %s\r\n"
"Date: %s\r\n"
"MIME-Version: 1.0\r\n"
- "Content-Type: multipart/mixed;"
+ "Content-Type: multipart/report;"
+ "report-type=delivery-status;"
"boundary=\"%16" PRIu64 "/%s\"\r\n"
"\r\n"
"This is a MIME-encapsulated message.\r\n"
@@ -510,21 +546,35 @@
"\r\n",
s->boundary, s->smtpname);
+ if(strlen(s->msg->envid) > 0)
+ io_xprintf(s->io,
+ "Original-Envelope-Id: %s\r\n",
+ s->msg->envid);
+
io_xprintf(s->io,
"Reporting-MTA: dns; %s\r\n"
+ "Arrival-Date: %s\r\n"
"\r\n",
- s->smtpname);
+ s->smtpname,
+ time_to_text(s->msg->creation));
TAILQ_FOREACH(evp, &s->msg->envelopes, entry) {
+ if(strlen(evp->orcpt) > 0)
+ io_xprintf(s->io,
+ "Original-Recipient: %s\r\n",
+ evp->orcpt);
+
io_xprintf(s->io,
- "Final-Recipient: rfc822; %s@%s\r\n"
+ "Final-Recipient: rfc822;%s@%s\r\n"
"Action: %s\r\n"
"Status: %s\r\n"
+ "Diagnostic-Code: %s\r\n"
"\r\n",
evp->dest.user,
evp->dest.domain,
action_str(&s->msg->bounce),
- esc_code(evp->esc_class, evp->esc_code));
+ esc_code(evp->esc_class, evp->esc_code),
+ evp->errorline);
}
log_trace(TRACE_BOUNCE, "bounce: %p: >>> [... %zu bytes ...]",
@@ -534,12 +584,21 @@
break;
case BOUNCE_DATA_MESSAGE:
- io_xprintf(s->io,
- "--%16" PRIu64 "/%s\r\n"
- "Content-Description: Message headers\r\n"
- "Content-Type: text/rfc822-headers\r\n"
- "\r\n",
- s->boundary, s->smtpname);
+ if(s->msg->bounce.type != B_FAILED || s->msg->bounce.dsn_ret !=
DSN_RETFULL) {
+ io_xprintf(s->io,
+ "--%16" PRIu64 "/%s\r\n"
+ "Content-Description: Message headers\r\n"
+ "Content-Type: text/rfc822-headers\r\n"
+ "\r\n",
+ s->boundary, s->smtpname);
+ } else {
+ io_xprintf(s->io,
+ "--%16" PRIu64 "/%s\r\n"
+ "Content-Description: Full message\r\n"
+ "Content-Type: message/rfc822\r\n"
+ "\r\n",
+ s->boundary, s->smtpname);
+ }
n = io_queued(s->io);
while (io_queued(s->io) < BOUNCE_HIWAT) {
@@ -629,6 +688,7 @@
}
TAILQ_REMOVE(&msg->envelopes, be, entry);
free(be->report);
+ free(be->errorline);
free(be);
n += 1;
}
--- ./usr.sbin/smtpd/smtpd.h
+++ ./usr.sbin/smtpd/smtpd.h
@@ -1757,6 +1757,7 @@
int base64_decode(char const *, unsigned char *, size_t);
int base64_encode_rfc3548(unsigned char const *, size_t,
char *, size_t);
+int xtext_decode(char const *, unsigned char *, size_t);
void xclosefrom(int);
void log_trace_verbose(int);
--- ./usr.sbin/smtpd/util.c
+++ ./usr.sbin/smtpd/util.c
@@ -838,6 +838,41 @@
return ret;
}
+/* guarantees output is null terminated, returns num data bytes (without
+ * trailing null) stored at dest, or -1 on error */
+int
+xtext_decode(char const *src, unsigned char *dest, size_t destsize)
+{
+ char s[3];
+ size_t i = 0;
+
+ s[2] = '\0';
+
+ if(!valid_xtext(src))
+ return 0;
+
+ while (*src != '\0') {
+ if (i >= destsize)
+ return (-1);
+
+ if (*src == '+') {
+ s[0] = (unsigned char)*++src;
+ s[1] = (unsigned char)*++src;
+ dest[i++] = (unsigned char)strtoul(s, NULL, 16);
+ }
+ else
+ dest[i++] = *src;
+
+ ++src;
+ }
+
+ if (i >= destsize)
+ return (-1);
+
+ dest[i] = '\0';
+ return (i);
+}
+
void
log_trace0(const char *emsg, ...)
{
--- ./usr.sbin/smtpd/smtp_session.c
+++ ./usr.sbin/smtpd/smtp_session.c
@@ -2395,6 +2395,12 @@
tx->evp.dsn_ret = DSN_RETFULL;
} else if (ADVERTISE_EXT_DSN(tx->session) && strncasecmp(opt,
"ENVID=", 6) == 0) {
opt += 6;
+ if (!valid_xtext(opt)) {
+ smtp_reply(tx->session,
+ "501 ENVID parameter syntax error");
+ smtp_tx_free(tx);
+ return;
+ }
if (strlcpy(tx->evp.dsn_envid, opt,
sizeof(tx->evp.dsn_envid))
>= sizeof(tx->evp.dsn_envid)) {
smtp_reply(tx->session,
