Re: DSN message format: shouldn't this use multipart/report (as of RFC3464)

2024-03-19 Thread Tassilo Philipp
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 +, gil...@poolp.org wrote:

March 13, 2024 10:31 AM, "Tassilo Philipp"  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"
+			

Re: DSN message format: shouldn't this use multipart/report (as of RFC3464)

2024-03-13 Thread Tassilo Philipp
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 +, gil...@poolp.org wrote:

March 13, 2024 10:31 AM, "Tassilo Philipp"  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,




Re: DSN message format: shouldn't this use multipart/report (as of RFC3464)

2024-03-13 Thread gilles
March 13, 2024 10:31 AM, "Tassilo Philipp"  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,