From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id D5872623DC for ; Thu, 20 Aug 2020 16:08:25 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C34892DEEA for ; Thu, 20 Aug 2020 16:08:25 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id A88202DEDA for ; Thu, 20 Aug 2020 16:08:23 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 32D2143398 for ; Thu, 20 Aug 2020 16:08:23 +0200 (CEST) To: Proxmox Backup Server development discussion , Stoiko Ivanov , Hannes Laimer References: <20200820092351.71864-1-h.laimer@proxmox.com> <20200820092351.71864-2-h.laimer@proxmox.com> <20200820143403.17201a47@rosa.proxmox.com> From: Thomas Lamprecht Message-ID: Date: Thu, 20 Aug 2020 16:08:19 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:80.0) Gecko/20100101 Thunderbird/80.0 MIME-Version: 1.0 In-Reply-To: <20200820143403.17201a47@rosa.proxmox.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.606 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -1.361 Looks like a legit reply (A) 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [mod.rs, proxmox.com, email.rs, ietf.org] Subject: Re: [pbs-devel] [PATCH proxmox 1/1] email: add small function to send multi-part emails using sendmail X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Aug 2020 14:08:25 -0000 On 20.08.20 14:34, Stoiko Ivanov wrote: > Thanks for the patch! > > some comments/suggestions on e-mail inline: > > On Thu, 20 Aug 2020 11:23:50 +0200 > Hannes Laimer wrote: > >> Signed-off-by: Hannes Laimer >> --- >> proxmox/src/tools/email.rs | 130 +++++++++++++++++++++++++++++++++++++ >> proxmox/src/tools/mod.rs | 1 + >> 2 files changed, 131 insertions(+) >> create mode 100644 proxmox/src/tools/email.rs >> >> diff --git a/proxmox/src/tools/email.rs b/proxmox/src/tools/email.rs >> new file mode 100644 >> index 0000000..b73a6d6 >> --- /dev/null >> +++ b/proxmox/src/tools/email.rs >> @@ -0,0 +1,130 @@ >> +//! Email related utilities. >> + >> +use std::process::{Command, Stdio}; >> +use anyhow::{bail, Error}; >> +use std::io::Write; >> +use crate::tools::time::time; >> + >> + >> +/// Sends multi-part mail with text and/or html to a list of recipients >> +/// >> +/// ``sendmail`` is used for sending the mail. >> +pub fn sendmail(mailto: Vec<&str>, >> + subject: &str, >> + text: Option<&str>, >> + html: Option<&str>, >> + mailfrom: Option<&str>, >> + author: Option<&str>) -> Result<(), Error> { >> + let mail_regex = regex::Regex::new(r"^[a-zA-Z\.0-9]+@[a-zA-Z\.0-9]+$").unwrap(); >> + >> + if mailto.is_empty() { >> + bail!("At least one recipient has to be specified!") >> + } >> + >> + for recipient in &mailto { >> + if !mail_regex.is_match(recipient) { >> + bail!("'{}' is not a valid email address", recipient) >> + } >> + } >> + >> + let mailfrom = mailfrom.unwrap_or("root"); >> + if !mailfrom.eq("root") && !mail_regex.is_match(mailfrom) { >> + bail!("'{}' is not a valid email address", mailfrom) >> + } >> + >> + let recipients = mailto.join(","); >> + let author = author.unwrap_or("Proxmox Backup Server"); >> + >> + let mut sendmail_process = match Command::new("/usr/sbin/sendmail") >> + .arg("-B") >> + .arg("8BITMIME") >> + .arg("-f") >> + .arg(mailfrom) >> + .arg("--") >> + .arg(&recipients) >> + .stdin(Stdio::piped()) >> + .spawn() { >> + Err(err) => bail!("could not spawn sendmail process: {}", err), >> + Ok(process) => process >> + }; >> + let mut body = String::new(); >> + let boundary = format!("----_=_NextPart_001_{}", time()?); >> + >> + body.push_str("Content-Type: multipart/alternative;\n"); >> + body.push_str(&format!("\tboundary=\"{}\"\n", boundary)); >> + body.push_str("MIME-Version: 1.0\n"); >> + body.push_str(&format!("FROM: {} <{}>\n", author, mailfrom)); >> + body.push_str(&format!("TO: {}\n", &recipients)); >> + body.push_str(&format!("SUBJECT: {}\n", subject)); > > iirc the header needs to be encoded if it contains non-ascii characters > (the content-type charset only relates to the body) - I assume that > postfix/exim would do the correct thing these days (and send the mail with > smtputf-8 extension (though I'm not sure what would happen if the > receiving server does not support that) > - would suggest to test sending such a mail with a non-ascii character > in the subject. > > writing the header names (TO FROM SUBJECT) in all-caps seems odd to my > eyes (but I guess this is pretty cosmetic) > > I would suggest adding a Date header (though postfix does that for locally > submitted e-mail) - else the mails are more likely to be considered spam. > > Finally - if you only have a text/plain part I would only send that part > (without the wrapping in multipart/alternative) (similarly for a > single text/html part without text/plain part). > > >> + body.push('\n'); >> + body.push_str("This is a multi-part message in MIME format.\n\n"); >> + body.push_str(&format!("--{}\n", boundary)); >> + if let Some(text) = text { >> + body.push_str("Content-Type: text/plain;\n"); >> + body.push_str("\tcharset=\"UTF8\"\n"); > While most MTA, scanners, MUAs won't care much - the charset should > probably be written as 'UTF-8' (see https://tools.ietf.org/html/rfc3629 > (section 8)) > You're right with your feedback, but I guess that most stems from the original Perl implementation[0] Hannes used as model. We (meaning one of you two ;P) should overhaul that one too then. [0]: https://git.proxmox.com/?p=pve-common.git;a=blob;f=src/PVE/Tools.pm;h=f9270d9b10340eec789346e81f4a926772ccabfd;hb=HEAD#l1439