From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 170FE1FF09C for ; Mon, 19 Oct 2026 17:49:43 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id D20FB2151B; Mon, 19 Oct 2026 17:49:40 +0200 (CEST) Message-ID: <8e9a15bd-c981-4e74-b751-3e7a5683f056@proxmox.com> Date: Mon, 19 Oct 2026 17:49:31 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox] fix #7814: send-mail: normalize LF line endings to CRLF endings To: Shannon Sterz , pbs-devel@lists.proxmox.com References: <20260730140126.444424-1-s.sterz@proxmox.com> Content-Language: en-US From: Samuel Rufinatscha In-Reply-To: <20260730140126.444424-1-s.sterz@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.211 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: X2WAODE3BOA6QO7CV3J3UM5OAB3IIUUR X-Message-ID-Hash: X2WAODE3BOA6QO7CV3J3UM5OAB3IIUUR X-MailFrom: s.rufinatscha@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: 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? 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@proxmox.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 = format!("UTF-8''{}", utf8_percent_encode(filename, RFC5987SET)); > + let encoded = format!( > + "UTF-8''{}", > + utf8_percent_encode(&filename.replace('\r', ""), RFC5987SET) > + ); > > let format_prefix = |l| { > if let Some(l) = l { > @@ -63,6 +66,18 @@ fn encode_filename_formatted(filename: &str) -> String { > format_encoded_text(&encoded, format_prefix, ";", 0, true) > } > > +/// Strip bare CR bytes, then base64 encode. This is useful to make sure 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#section-2.3.8). > +fn strip_cr_base64>(raw: T) -> String { > + let normalized: Vec = raw > + .as_ref() > + .iter() > + .filter_map(|b| (*b != b'\r').then_some(*b)) > + .collect(); > + proxmox_base64::encode(&normalized) > +} > + > /// Helper function used to format text to not exceed line limits imposed by standards such as RFC > /// 2045 Section 6.8 that specifies that Base64 content transfer encoded text "must be represented > /// in lines of no more than 76 characters each". It also allows taking into account any necessary > @@ -166,11 +181,7 @@ impl Recipient { > fn format_recipient(&self) -> String { > if let Some(name) = &self.name { > if !name.is_ascii() { > - format!( > - "=?utf-8?B?{}?= <{}>", > - proxmox_base64::encode(name), > - self.email > - ) > + format!("=?utf-8?B?{}?= <{}>", strip_cr_base64(name), self.email) > } else { > format!("{name} <{}>", self.email) > } > @@ -199,7 +210,7 @@ impl Attachment<'_> { > if self.filename.is_ascii() { > let _ = write!(attachment, "{}", &self.filename); > } else { > - let encoded = proxmox_base64::encode(&self.filename); > + let encoded = strip_cr_base64(&self.filename); > let prefix_fn = |line| { > if Some(0) == line { > "=?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 normalizing 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 = "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 the capacity to the upper > + // bound to avoid more complex capacity calculations that would take more time to run. > + let mut normalized_message: Vec = Vec::with_capacity(message.len() * 2); > + > + for byte in message { > + match byte { > + b'\r' => continue, > + b'\n' => normalized_message.extend_from_slice(&[b'\r', b'\n']), > + b => 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: =?utf-8?B?{}?=", > - 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: =?utf-8?B?{}?= <{}>", > - 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 inject extra header lines. > - let key = key.replace(['\r', '\n'], ""); > - let value = value.replace(['\r', '\n'], ""); > + // Strip LF so an interpolated key or value cannot inject extra header lines. Bare CR > + // will be stripped for non-ASCII text before encoding and for ASCII text globally > + // before finishing formatting. > + let key = key.replace('\n', ""); > + let value = value.replace('\n', ""); > if value.is_ascii() { > writeln!(header, "{key}: {value}")?; > } else { > - writeln!( > - header, > - "{key}: =?utf-8?B?{}?=", > - proxmox_base64::encode(&value) > - )?; > + writeln!(header, "{key}: =?utf-8?B?{}?=", 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.replace('\r', ""))); > } > > if let Some(html) = &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 = Mail::new( > - "Sender Näme", > - "from@example.com", > - "Subject Linë", > - "Lorem Ipsum Dolor Sit\nAmët", > + "Sender Nä\rme", > + "from@example.\rcom", > + "Subject Linë\r", > + "Lorem Ipsum Dolor \rSit\nAmët", > ) > .with_recipient_and_name("Receiver Näme", "receiver@example.com") > .with_attachment("deadbeef.bin", "application/octet-stream", &bin) > @@ -1344,4 +1372,168 @@ ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz > > assert_eq!(suffixed, out); > } > + > + #[test] > + fn line_endings_are_normalized_when_not_encoded() { > + let bin: [u8; 63] = [ > + 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 = 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
\n\t\tLorem Ipsum Dolor Si\rt Amet\n\t
\n") > + .with_header("X-CR-HEADE\r", "value\r"); > + > + let body = mail.format_mail(1732806251).expect("could not format mail"); > + > + assert!(body.is_ascii()); > + > + let expected = r#"Content-Type: multipart/mixed; > + boundary="----_=_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. > + > +------_=_NextPart_001_1732806251 > +Content-Type: multipart/alternative; boundary="----_=_NextPart_002_1732806251" > +MIME-Version: 1.0 > + > +------_=_NextPart_002_1732806251 > +Content-Type: text/plain; > + charset="UTF-8" > +Content-Transfer-Encoding: 7bit > + > +Lorem Ipsum Dolor Sit > +Amet > +------_=_NextPart_002_1732806251 > +Content-Type: text/html; > + charset="UTF-8" > +Content-Transfer-Encoding: 7bit > + > + > + 
> +		Lorem Ipsum Dolor Sit Amet
> +
> + > +------_=_NextPart_002_1732806251-- > +------_=_NextPart_001_1732806251 > +Content-Type: application/octet-stream; > + name="deadbeef.bin" > +Content-Disposition: attachment; > + filename*0*=UTF-8''deadbeef.bin > +Content-Transfer-Encoding: base64 > + > +3q2+796tvu/erb7v3q3erb7v3q2+796tvu/erd6tvu/erb7v3q2+796t3q2+796tvu/erb7v > +3q2+796tvu8N > +------_=_NextPart_001_1732806251--"#; > + > + // manually compare here to ensure we split specifically only on `\r\n` line endings > + > + let lines1 = body.split("\r\n"); > + let lines2 = 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] = [ > + 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 = Mail::new( > + "Se\rnder Näme", > + "fro\rm@example.com", > + "Sub\rject Linë", > + "Lor\rem Ipsum Dolor Sit\nAmët", > + ) > + .with_recipient_and_name("R\receiver Näme", "receive\rr@example.com") > + .with_attachment("🐄💀.b\rin", "ima\rge/bmp", &bin) > + .with_html_alt("\n\t
\n\t\tLorem Ipsum Dölor Si\rt Amet\n\t
\n") > + .with_header("X-CR-HEADE\r", "valüe\r"); > + > + let body = mail.format_mail(1732806251).expect("could not format mail"); > + > + assert!(body.is_ascii()); > + > + let expected = r#"Content-Type: multipart/mixed; > + boundary="----_=_NextPart_001_1732806251" > +MIME-Version: 1.0 > +Subject: =?utf-8?B?U3ViamVjdCBMaW7Dqw==?= > +From: =?utf-8?B?U2VuZGVyIE7DpG1l?= > +To: =?utf-8?B?UmVjZWl2ZXIgTsOkbWU=?= > +Date: Thu, 28 Nov 2024 16:04:11 +0100 > +Auto-Submitted: auto-generated; > +X-CR-HEADE: =?utf-8?B?dmFsw7xl?= > + > +This is a multi-part message in MIME format. > + > +------_=_NextPart_001_1732806251 > +Content-Type: multipart/alternative; boundary="----_=_NextPart_002_1732806251" > +MIME-Version: 1.0 > + > +------_=_NextPart_002_1732806251 > +Content-Type: text/plain; > + charset="UTF-8" > +Content-Transfer-Encoding: base64 > + > +TG9yZW0gSXBzdW0gRG9sb3IgU2l0CkFtw6t0 > +------_=_NextPart_002_1732806251 > +Content-Type: text/html; > + charset="UTF-8" > +Content-Transfer-Encoding: base64 > + > +PGh0bWwgbGFuZz0iZGUtYXQiPjxoZWFkPjwvaGVhZD48Ym9keT4KCTxwcmU+CgkJTG9yZW0g > +SXBzdW0gRMO2bG9yIFNpdCBBbWV0Cgk8L3ByZT4KPC9ib2R5PjwvaHRtbD4= > +------_=_NextPart_002_1732806251-- > +------_=_NextPart_001_1732806251 > +Content-Type: image/bmp; > + name="=?utf-8?B?8J+QhPCfkoAuYmlu?=" > +Content-Disposition: attachment; > + filename*0*=UTF-8''%F0%9F%90%84%F0%9F%92%80.bin > +Content-Transfer-Encoding: base64 > + > +3q2+796tvu/erb7v3q3erb7v3q2+796tvu/erd6tvu/erb7v3q2+796t3q2+796tvu/erb7v > +3q2+796tvu8N > +------_=_NextPart_001_1732806251--"#; > + > + // manually compare here to ensure we split specifically only on `\r\n` line endings > + > + let lines1 = body.split("\r\n"); > + let lines2 = 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()); > + } > }