Alright, find attached a first patch, fixing up some content-type
headers, as outlined by RFC3464 and RFC6522 - in detail:
The patch's first hunk follows RFC3464, which specifies that a DSN
should have a top-level type of "multipart/report" with a parameter
"report-type=delivery-status"; the actual format is described in
RFC6522, and is specified having two or three sub mime parts, a human
readable message, a machine parsable part with content-type
"message/delivery-status" and an optional 3rd part of either
"text/rfc822-headers" or "message/rfc822".
The latter is part of the second hunk of the patch, b/c
"text/rfc822-headers" was used in any case, even if the full message was
returned in the DSN.
Also, the end of the second hunk removes a check, which I think is
unintuitive to begin with, but unsure. Basically, it only returned the
headers for NOTIFY=SUCCESS DSNs when RET=HDRS was also set (explicitly
or implicitly).
Here's the section from RFC3461:
When the value of the RET parameter is FULL, the full
message SHOULD be returned for any DSN which conveys notification of
delivery failure. (However, if the length of the message is greater
than some implementation-specified length, the MTA MAY return only
the headers even if the RET parameter specified FULL.) If a DSN
contains no notifications of delivery failure, the MTA SHOULD return
only the headers.
The original code does what the last line implies, but only when
RET=HDRS is set, meaning it ignores it for anything but NOTIFY=SUCCESS,
and returns always the full message.
I'm not sure what would be best to do here, all in all the RFC says
'SHOULD'... either way, the check there should reflect the same logic
used in the if...else... block of the second hunk, which is what the
patch shoots for. I chose to keep it simple and simply do what RET= asks
for. So, if the original logic is kept, the second hunk's if...else...
block would need an additional "s->msg->bounce.type == B_DELIVERED"
check.
I tested the patch with different combinations of the RET= and NOTIFY=
parameters, and so far it seems to work fine.
PS: I'm working on related other patches, at the moment, which I'll mail
when they are done, namely adding a "Original-Envelope-ID" header in the
DSN when the ENVID= param was present, as well as "Original-Recipient"
for any ORCPT= param. Those are specified in RFC3461 section 6.3., but
they are a bit more involved as the params need to be decoded and
recoded, and I just haven't found the time, yet.
Thanks!
On Wed, Mar 13, 2024 at 12:04:24PM +0100, Tassilo Philipp wrote:
Ok, will get busy... I also noticed two other issues, one related to
ENVID= (should be generated for DSNs), and some data in the DSN's
message/delivery-status parts, that must be generated according to eh
RFC, when ENVID= or ORCPT= are present, which are not, currently. Will
look at those, as well. Might take a few days, though... will keep you
posted.
Cheers
On Wed, Mar 13, 2024 at 11:00:51AM +0000, gil...@poolp.org wrote:
March 13, 2024 10:31 AM, "Tassilo Philipp" <tphil...@potion-studios.com> wrote:
Hello,
I noticed that DSNs generated by OpenSMTPd use "Content-Type:
multipart/mixed", instead of "Content-Type: multipart/report", as
defined by RFC3461 (and described in RFC3464 and RFC3462). I
wonder if there's a reason for that?
I haven only done a cursory review of the actual multiparts
themselves, but they seem to (mostly) follow the mentioned RFCs
with for example "Content-Type: text/rfc822-headers". However, if
RET=FULL, the content type should be IMHO only "message/rfc822"
and not "text/rfc822-headers", the latter seems to be currently
used for both, RET=HDRS and RET=FULL. Need to read up on it more
to get a clearer picture, though, I just started digging into it.
Either way... I think the RFCs should be followed, want me to prepare a patch?
Yes please, patch and RFC reference so this can be checked too
Thanks,
--- ./usr.sbin/smtpd/bounce.c
+++ ./usr.sbin/smtpd/bounce.c
@@ -452,7 +452,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"
@@ -534,19 +535,27 @@
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.dsn_ret == DSN_RETHDRS) {
+ 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) {
if ((len = getline(&line, &sz, s->msgfp)) == -1)
break;
if (len == 1 && line[0] == '\n' && /* end of headers */
- s->msg->bounce.type == B_DELIVERED &&
s->msg->bounce.dsn_ret == DSN_RETHDRS) {
free(line);
fclose(s->msgfp);
