From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id B58221FF0AA for ; Tue, 20 Oct 2026 09:56:20 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 61C6C21533; Tue, 20 Oct 2026 09:56:20 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 20 Oct 2026 09:56:07 +0200 Message-Id: Subject: Re: [PATCH proxmox] fix #7814: send-mail: normalize LF line endings to CRLF endings To: "Samuel Rufinatscha" , X-Mailer: aerc 0.20.0 References: <20260730140126.444424-1-s.sterz@proxmox.com> <8e9a15bd-c981-4e74-b751-3e7a5683f056@proxmox.com> In-Reply-To: <8e9a15bd-c981-4e74-b751-3e7a5683f056@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1792482967892 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.784 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: SFQVB2N2NF6JHKK4Q7P2FXCGRJRCUCHY X-Message-ID-Hash: SFQVB2N2NF6JHKK4Q7P2FXCGRJRCUCHY X-MailFrom: s.sterz@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Mon Oct 19, 2026 at 5:49 PM CEST, Samuel Rufinatscha wrote: > Hi Shannon, > > I ran some additional tests, which passed. One note/question regarding > the base64 path: > > Removing \r before base64 encoding changes an valid \r\n into \n after > decoding but if I understand correctly previously it would have > preserved \r\n (as of no normalization). > From the commit message I understand that removing CR before encoding > is intentional but is changing valid CRLF intentional here too? i deemed a reasonable trade-off as stripping only if no \n follows would have increased complexity quite a bit. note that i can't do the `replace("\n", "\r\n")` trick there as that would transform bare `\n` to CRLF. bare `\n` was extensively tested across multiple MUAs and free mail providers (gmail, outlook, etc.), though, so i wanted to err on the side of caution. since this was reported bug i wanted to give a solution that should work sooner than later. however, since this has been dragged out already, i can eventually get around to do another round of testing with just replace LF with CRLF too. if you are aware of an rfc or best practice on what to do in these cases, i'd appreciate any input, though. i can also provide a commit message that is a bit more telling when it comes to this case in particular. > Besides that I tested: > > - the bytes that are passed to send() and forward() using a sendmail > mock (wrote bytes to a file). verified that \n becomes \r\n, \r is > removed and \r\n is unchanged > - created a binary attachment containing all 256 byte values and > verified that the content is unchanged after base64 decoding > > On 7/30/26 4:01 PM, Shannon Sterz wrote: >> RFC5321 Section 2.3.8 requires that mails do not contain bare CR or LF >> bytes. this patch normalizes unix style LF line endings to CRLF line >> endings. it also removes bare CR bytes. >> >> when text would be base64 encoded anyway, CR bytes will be dropped, to >> keep the behavior for encoded and non-encoded text aligned. however, >> since this code was already extensively tested against multiple MUAs, >> encoded LF line endings are kept as-is. >> >> also adds tests to encode this behavior. >> >> Signed-off-by: Shannon Sterz >> --- >> >> Notes: >> v1: https://lore.proxmox.com/all/20260714084719.68076-1-s.sterz@pro= xmox.com/ >> >> changes since v1 (thanks @ Lukas Wagner): >> >> * fixed a typo (than -> then) >> * adjusted how the capacity for forwarded messages that need to be >> normalized is calculated. >> >> proxmox-sendmail/src/lib.rs | 244 ++++++++++++++++++++++++++++++++---- >> 1 file changed, 218 insertions(+), 26 deletions(-) >> >> diff --git a/proxmox-sendmail/src/lib.rs b/proxmox-sendmail/src/lib.rs >> index b3ff56e5..355ed12d 100644 >> --- a/proxmox-sendmail/src/lib.rs >> +++ b/proxmox-sendmail/src/lib.rs >> @@ -50,7 +50,10 @@ fn encode_base64_formatted>(raw: T) ->= String { >> } >> >> fn encode_filename_formatted(filename: &str) -> String { >> - let encoded =3D format!("UTF-8''{}", utf8_percent_encode(filename, = RFC5987SET)); >> + let encoded =3D format!( >> + "UTF-8''{}", >> + utf8_percent_encode(&filename.replace('\r', ""), RFC5987SET) >> + ); >> >> let format_prefix =3D |l| { >> if let Some(l) =3D l { >> @@ -63,6 +66,18 @@ fn encode_filename_formatted(filename: &str) -> Strin= g { >> format_encoded_text(&encoded, format_prefix, ";", 0, true) >> } >> >> +/// Strip bare CR bytes, then base64 encode. This is useful to make sur= e we treat encoded (header) >> +/// values the same as non-encoded one. Stripping CR bytes is necessary= according to RFC 5321 Section >> +/// Section 2.3.8 (https://datatracker.ietf.org/doc/html/rfc5321#sectio= n-2.3.8). >> +fn strip_cr_base64>(raw: T) -> String { >> + let normalized: Vec =3D raw >> + .as_ref() >> + .iter() >> + .filter_map(|b| (*b !=3D b'\r').then_some(*b)) >> + .collect(); >> + proxmox_base64::encode(&normalized) >> +} >> + >> /// Helper function used to format text to not exceed line limits impo= sed by standards such as RFC >> /// 2045 Section 6.8 that specifies that Base64 content transfer encod= ed text "must be represented >> /// in lines of no more than 76 characters each". It also allows takin= g into account any necessary >> @@ -166,11 +181,7 @@ impl Recipient { >> fn format_recipient(&self) -> String { >> if let Some(name) =3D &self.name { >> if !name.is_ascii() { >> - format!( >> - "=3D?utf-8?B?{}?=3D <{}>", >> - proxmox_base64::encode(name), >> - self.email >> - ) >> + format!("=3D?utf-8?B?{}?=3D <{}>", strip_cr_base64(name= ), self.email) >> } else { >> format!("{name} <{}>", self.email) >> } >> @@ -199,7 +210,7 @@ impl Attachment<'_> { >> if self.filename.is_ascii() { >> let _ =3D write!(attachment, "{}", &self.filename); >> } else { >> - let encoded =3D proxmox_base64::encode(&self.filename); >> + let encoded =3D strip_cr_base64(&self.filename); >> let prefix_fn =3D |line| { >> if Some(0) =3D=3D line { >> "=3D?utf-8?B?" >> @@ -451,7 +462,10 @@ impl<'a> Mail<'a> { >> >> /// Forwards an email message to a given list of recipients. >> /// >> - /// `message` must be compatible with ``sendmail`` (the message is = piped into stdin unmodified). >> + /// `message` must be compatible with ``sendmail``. The message is = piped into `stdin` >> + /// unmodified. One exception is stripping bare CR bytes and normal= izing LF line endings to >> + /// CRLF line endings in accordance with RFC 5321 Section 2.3.8 >> + /// (https://datatracker.ietf.org/doc/html/rfc5321#section-2.3.8). >> #[cfg(feature =3D "mail-forwarder")] >> pub fn forward( >> mailto: &[&str], >> @@ -485,11 +499,25 @@ impl<'a> Mail<'a> { >> .spawn() >> .with_context(|| "could not spawn sendmail process")?; >> >> + // this will always have enough capacity, since the worst case = is a message consisting only >> + // of "\n" and that would exactly double in size. messages are = also unlikely to be that big >> + // that the extra allocated space here really matters. so set t= he capacity to the upper >> + // bound to avoid more complex capacity calculations that would= take more time to run. >> + let mut normalized_message: Vec =3D Vec::with_capacity(mess= age.len() * 2); >> + >> + for byte in message { >> + match byte { >> + b'\r' =3D> continue, >> + b'\n' =3D> normalized_message.extend_from_slice(&[b'\r'= , b'\n']), >> + b =3D> normalized_message.push(*b), >> + } >> + } >> + >> sendmail_process >> .stdin >> .take() >> .unwrap() >> - .write_all(message) >> + .write_all(&normalized_message) >> .with_context(|| "couldn't write to sendmail stdin")?; >> >> sendmail_process >> @@ -520,7 +548,9 @@ impl<'a> Mail<'a> { >> write!(mail, "\n--{file_boundary}--")?; >> } >> >> - Ok(mail) >> + // normalize line endings from `\n` to `\r\n` and drop bare `\r= ` according to RFC 5321 >> + // 2.3.8: https://datatracker.ietf.org/doc/html/rfc5321#section= -2.3.8 >> + Ok(mail.replace('\r', "").replace('\n', "\r\n")) >> } >> >> fn format_header( >> @@ -561,7 +591,7 @@ impl<'a> Mail<'a> { >> writeln!( >> header, >> "Subject: =3D?utf-8?B?{}?=3D", >> - proxmox_base64::encode(&self.subject) >> + strip_cr_base64(&self.subject) >> )?; >> } else { >> writeln!(header, "Subject: {}", self.subject)?; >> @@ -571,7 +601,7 @@ impl<'a> Mail<'a> { >> writeln!( >> header, >> "From: =3D?utf-8?B?{}?=3D <{}>", >> - proxmox_base64::encode(&self.mail_author), >> + strip_cr_base64(&self.mail_author), >> self.mail_from >> )?; >> } else { >> @@ -606,17 +636,15 @@ impl<'a> Mail<'a> { >> header.push_str("Auto-Submitted: auto-generated;\n"); >> >> for (key, value) in &self.headers { >> - // Strip CR/LF so an interpolated key or value cannot injec= t extra header lines. >> - let key =3D key.replace(['\r', '\n'], ""); >> - let value =3D value.replace(['\r', '\n'], ""); >> + // Strip LF so an interpolated key or value cannot inject e= xtra header lines. Bare CR >> + // will be stripped for non-ASCII text before encoding and = for ASCII text globally >> + // before finishing formatting. >> + let key =3D key.replace('\n', ""); >> + let value =3D value.replace('\n', ""); >> if value.is_ascii() { >> writeln!(header, "{key}: {value}")?; >> } else { >> - writeln!( >> - header, >> - "{key}: =3D?utf-8?B?{}?=3D", >> - proxmox_base64::encode(&value) >> - )?; >> + writeln!(header, "{key}: =3D?utf-8?B?{}?=3D", strip_cr_= base64(&value))?; >> } >> } >> >> @@ -653,7 +681,7 @@ impl<'a> Mail<'a> { >> body.push_str(&self.body_txt); >> } else { >> body.push_str("Content-Transfer-Encoding: base64\n\n"); >> - body.push_str(&encode_base64_formatted(&self.body_txt)); >> + body.push_str(&encode_base64_formatted(self.body_txt.replac= e('\r', ""))); >> } >> >> if let Some(html) =3D &self.body_html { >> @@ -666,7 +694,7 @@ impl<'a> Mail<'a> { >> body.push_str(html); >> } else { >> body.push_str("Content-Transfer-Encoding: base64\n\n")= ; >> - body.push_str(&encode_base64_formatted(html)); >> + body.push_str(&encode_base64_formatted(html.replace('\r= ', ""))); >> } >> >> write!(body, "\n--{html_boundary}--")?; >> @@ -1140,10 +1168,10 @@ Content-Transfer-Encoding: 7bit >> ]; >> >> let mail =3D Mail::new( >> - "Sender N=C3=A4me", >> - "from@example.com", >> - "Subject Lin=C3=AB", >> - "Lorem Ipsum Dolor Sit\nAm=C3=ABt", >> + "Sender N=C3=A4\rme", >> + "from@example.\rcom", >> + "Subject Lin=C3=AB\r", >> + "Lorem Ipsum Dolor \rSit\nAm=C3=ABt", >> ) >> .with_recipient_and_name("Receiver N=C3=A4me", "receiver@examp= le.com") >> .with_attachment("deadbeef.bin", "application/octet-stream", &= bin) >> @@ -1344,4 +1372,168 @@ ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuv= wxyz >> >> assert_eq!(suffixed, out); >> } >> + >> + #[test] >> + fn line_endings_are_normalized_when_not_encoded() { >> + let bin: [u8; 63] =3D [ >> + 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,= 0xbe, 0xef, 0xde, 0xad, >> + 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,= 0xbe, 0xef, 0xde, 0xad, >> + 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,= 0xbe, 0xef, 0xde, 0xad, >> + 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,= 0xbe, 0xef, 0xde, 0xad, >> + 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0x0d, >> + ]; >> + >> + let mail =3D Mail::new( >> + "Se\rnder Name", >> + "fro\rm@example.com", >> + "Sub\rject Line", >> + "Lor\rem Ipsum Dolor Sit\nAmet", >> + ) >> + .with_recipient_and_name("R\receiver Name", "receive\rr@example= .com") >> + .with_attachment("deadb\reef.bin", "application/octet-stream", = &bin) >> + .with_html_alt("\n\t<= pre>\n\t\tLorem Ipsum Dolor Si\rt Amet\n\t\n") >> + .with_header("X-CR-HEADE\r", "value\r"); >> + >> + let body =3D mail.format_mail(1732806251).expect("could not for= mat mail"); >> + >> + assert!(body.is_ascii()); >> + >> + let expected =3D r#"Content-Type: multipart/mixed; >> + boundary=3D"----_=3D_NextPart_001_1732806251" >> +MIME-Version: 1.0 >> +Subject: Subject Line >> +From: Sender Name >> +To: Receiver Name >> +Date: Thu, 28 Nov 2024 16:04:11 +0100 >> +Auto-Submitted: auto-generated; >> +X-CR-HEADE: value >> + >> +This is a multi-part message in MIME format. >> + >> +------_=3D_NextPart_001_1732806251 >> +Content-Type: multipart/alternative; boundary=3D"----_=3D_NextPart_002_= 1732806251" >> +MIME-Version: 1.0 >> + >> +------_=3D_NextPart_002_1732806251 >> +Content-Type: text/plain; >> + charset=3D"UTF-8" >> +Content-Transfer-Encoding: 7bit >> + >> +Lorem Ipsum Dolor Sit >> +Amet >> +------_=3D_NextPart_002_1732806251 >> +Content-Type: text/html; >> + charset=3D"UTF-8" >> +Content-Transfer-Encoding: 7bit >> + >> + >> + 
>> +		Lorem Ipsum Dolor Sit Amet
>> +
>> + >> +------_=3D_NextPart_002_1732806251-- >> +------_=3D_NextPart_001_1732806251 >> +Content-Type: application/octet-stream; >> + name=3D"deadbeef.bin" >> +Content-Disposition: attachment; >> + filename*0*=3DUTF-8''deadbeef.bin >> +Content-Transfer-Encoding: base64 >> + >> +3q2+796tvu/erb7v3q3erb7v3q2+796tvu/erd6tvu/erb7v3q2+796t3q2+796tvu/erb7= v >> +3q2+796tvu8N >> +------_=3D_NextPart_001_1732806251--"#; >> + >> + // manually compare here to ensure we split specifically only o= n `\r\n` line endings >> + >> + let lines1 =3D body.split("\r\n"); >> + let lines2 =3D expected.split("\n"); >> + >> + for ((i, line1), line2) in lines1.enumerate().zip(lines2) { >> + if !(line1.starts_with("Date:") && line2.starts_with("Date:= ")) { >> + assert_eq!(line1, line2, "lines at {i} do not match"); >> + } >> + } >> + >> + assert_eq!(body.lines().count(), expected.lines().count()); >> + } >> + >> + #[test] >> + fn line_endings_are_normalized_when_encoded() { >> + let bin: [u8; 63] =3D [ >> + 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,= 0xbe, 0xef, 0xde, 0xad, >> + 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,= 0xbe, 0xef, 0xde, 0xad, >> + 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,= 0xbe, 0xef, 0xde, 0xad, >> + 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad,= 0xbe, 0xef, 0xde, 0xad, >> + 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0x0d, >> + ]; >> + >> + let mail =3D Mail::new( >> + "Se\rnder N=C3=A4me", >> + "fro\rm@example.com", >> + "Sub\rject Lin=C3=AB", >> + "Lor\rem Ipsum Dolor Sit\nAm=C3=ABt", >> + ) >> + .with_recipient_and_name("R\receiver N=C3=A4me", "receive\rr@ex= ample.com") >> + .with_attachment("=F0=9F=90=84=F0=9F=92=80.b\rin", "ima\rge/bmp= ", &bin) >> + .with_html_alt("\n\t<= pre>\n\t\tLorem Ipsum D=C3=B6lor Si\rt Amet\n\t\n") >> + .with_header("X-CR-HEADE\r", "val=C3=BCe\r"); >> + >> + let body =3D mail.format_mail(1732806251).expect("could not for= mat mail"); >> + >> + assert!(body.is_ascii()); >> + >> + let expected =3D r#"Content-Type: multipart/mixed; >> + boundary=3D"----_=3D_NextPart_001_1732806251" >> +MIME-Version: 1.0 >> +Subject: =3D?utf-8?B?U3ViamVjdCBMaW7Dqw=3D=3D?=3D >> +From: =3D?utf-8?B?U2VuZGVyIE7DpG1l?=3D >> +To: =3D?utf-8?B?UmVjZWl2ZXIgTsOkbWU=3D?=3D >> +Date: Thu, 28 Nov 2024 16:04:11 +0100 >> +Auto-Submitted: auto-generated; >> +X-CR-HEADE: =3D?utf-8?B?dmFsw7xl?=3D >> + >> +This is a multi-part message in MIME format. >> + >> +------_=3D_NextPart_001_1732806251 >> +Content-Type: multipart/alternative; boundary=3D"----_=3D_NextPart_002_= 1732806251" >> +MIME-Version: 1.0 >> + >> +------_=3D_NextPart_002_1732806251 >> +Content-Type: text/plain; >> + charset=3D"UTF-8" >> +Content-Transfer-Encoding: base64 >> + >> +TG9yZW0gSXBzdW0gRG9sb3IgU2l0CkFtw6t0 >> +------_=3D_NextPart_002_1732806251 >> +Content-Type: text/html; >> + charset=3D"UTF-8" >> +Content-Transfer-Encoding: base64 >> + >> +PGh0bWwgbGFuZz0iZGUtYXQiPjxoZWFkPjwvaGVhZD48Ym9keT4KCTxwcmU+CgkJTG9yZW0= g >> +SXBzdW0gRMO2bG9yIFNpdCBBbWV0Cgk8L3ByZT4KPC9ib2R5PjwvaHRtbD4=3D >> +------_=3D_NextPart_002_1732806251-- >> +------_=3D_NextPart_001_1732806251 >> +Content-Type: image/bmp; >> + name=3D"=3D?utf-8?B?8J+QhPCfkoAuYmlu?=3D" >> +Content-Disposition: attachment; >> + filename*0*=3DUTF-8''%F0%9F%90%84%F0%9F%92%80.bin >> +Content-Transfer-Encoding: base64 >> + >> +3q2+796tvu/erb7v3q3erb7v3q2+796tvu/erd6tvu/erb7v3q2+796t3q2+796tvu/erb7= v >> +3q2+796tvu8N >> +------_=3D_NextPart_001_1732806251--"#; >> + >> + // manually compare here to ensure we split specifically only o= n `\r\n` line endings >> + >> + let lines1 =3D body.split("\r\n"); >> + let lines2 =3D expected.split("\n"); >> + >> + for ((i, line1), line2) in lines1.enumerate().zip(lines2) { >> + if !(line1.starts_with("Date:") && line2.starts_with("Date:= ")) { >> + assert_eq!(line1, line2, "lines at {i} do not match"); >> + } >> + } >> + >> + assert_eq!(body.lines().count(), expected.lines().count()); >> + } >> }