public inbox for pmg-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: pmg-devel@lists.proxmox.com
Subject: [pmg-devel] [PATCH pmg-api v2] utils: fix mailflow if smtputf8 is disabled
Date: Wed, 21 Dec 2022 15:53:43 +0100	[thread overview]
Message-ID: <20221221145343.80373-1-s.ivanov@proxmox.com> (raw)

with the recent addition of smtputf8 support for the rulesystem setups
explicitly disabling smtputf8 in postfix got broken.

This is mostly noticeable for the spamreports (the receivers are taken
from the database and potentially decoded from utf-8, which sets the
'is_utf8' flag, and then tries to use the smtputf8 extension when
reinjecting the mail, which fails (since smtputf8 is disabled)

Instead of checking for the internal flag, we check for occurence of
characters which are not ascii printable (everything excluding
controlcharacters - '[\x20-\x7E]') in the envelope-addresses and
headers (there also for [\r\n\t], due to searching all headers and
folding). - see
https://perldoc.perl.org/perlunifaq#What-is-%22the-UTF8-flag%22?  and
https://perldoc.perl.org/perlrecharclass#POSIX-Character-Classes

The only diversion from the requirements in the smptutf8 rfc
https://www.rfc-editor.org/rfc/rfc6531
is that we do not check the headers of all parts of a multipart
message (think suggested filename for an attachment), but I assume
that this should not be an issue in mail-transit

the addresses now always get encoded as UTF-8, as this is robust for
aascii-only addresses.

reported in our community forum:
https://forum.proxmox.com/threads/.119387/

issue is reproducible by setting
`smtputf8_enable = no` in postfix main.cf
and sending a spamreport using `pmgqm`

regular mailflow should not be affected in those setups (as no utf-8
addresses would come into the system)

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
v1->v2:
* as suggested by Dominik (huge thanks for the thorough review and the
  suggestions!) the (top-level) mail headers are also scanned for non-ascii
  printable characters (and \n\r\t, since those occur in headers as strings)
* put the test in a sub of its own
* addresses are now always encoded as utf-8 (since for ascii only addresses
  this should be identity

 src/PMG/Utils.pm | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index 10193f6..825b8d9 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -221,6 +221,24 @@ sub subst_values_for_header {
     return $res;
 }
 
+sub mail_needs_smtputf8 {
+    my ($entity, $sender, $targets) = @_;
+
+    return 1 if ($sender =~ /[^\p{PosixPrint}]/);
+
+    foreach my $target (@$targets) {
+	if ($target =~ /[^\p{PosixPrint}]/) {
+	    return 1;
+	}
+    }
+
+    if ($entity->head()->as_string() =~ /([^\p{PosixPrint}\n\r\t])/) {
+	return 1;
+    }
+
+    return 0;
+}
+
 sub reinject_mail {
     my ($entity, $sender, $targets, $xforward, $me, $params) = @_;
 
@@ -245,23 +263,9 @@ sub reinject_mail {
 	    }
 	}
 
-	my $has_utf8_targets = 0;
-	foreach my $target (@$targets) {
-	    if (utf8::is_utf8($target)) {
-		$has_utf8_targets = 1;
-		last;
-	    }
-	}
-
 	my $mail_opts = " BODY=8BITMIME";
-	my $sender_addr;
-	if (utf8::is_utf8($sender)) {
-	    $sender_addr = encode('UTF-8', $smtp->_addr($sender));
-	    $mail_opts .= " SMTPUTF8";
-	} else {
-	    $sender_addr = $smtp->_addr($sender);
-	    $mail_opts .= " SMTPUTF8" if $has_utf8_targets;
-	}
+	$mail_opts .= " SMTPUTF8" if mail_needs_smtputf8($entity, $sender, $targets);
+	my $sender_addr = encode('UTF-8', $smtp->_addr($sender));
 
 	if (defined($params->{mail})) {
 	    my $mailparams = $params->{mail};
@@ -284,12 +288,8 @@ sub reinject_mail {
 		    $rcpt_opts .= " $p=$rcptparams->{$p}";
 		}
 	    }
+	    $rcpt_addr = encode('UTF-8', $smtp->_addr($target));
 
-	    if (utf8::is_utf8($target)) {
-		$rcpt_addr = encode('UTF-8', $smtp->_addr($target));
-	    } else {
-		$rcpt_addr = $smtp->_addr($target);
-	    }
 	    if (!$smtp->_RCPT("TO:" . $rcpt_addr . $rcpt_opts)) {
 		syslog ('err', "smtp error - got: %s %s", $smtp->code, scalar($smtp->message));
 		die "smtp to: ERROR";
-- 
2.30.2





             reply	other threads:[~2022-12-21 14:54 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-21 14:53 Stoiko Ivanov [this message]
2022-12-23 12:41 ` [pmg-devel] applied: " Dominik Csapak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221221145343.80373-1-s.ivanov@proxmox.com \
    --to=s.ivanov@proxmox.com \
    --cc=pmg-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal