all lists on 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 v3 0/5] improve local mail injection and add smtputf8 support
Date: Fri, 17 Mar 2023 19:44:49 +0100	[thread overview]
Message-ID: <20230317184456.26465-1-s.ivanov@proxmox.com> (raw)

Thanks to Dominik's feedback and testing this series so well we found one
issue:
* reinject_local_mail does encode the envelope-addresses (if they are not
purely ascii) - but the quarantine delivery passes the data already utf-8
encoded to it (since it's read this way from the database)

This would have been an issue previously as well - but since the
quarantine delivery never got smtputf8 support it was not noticed.

We discussed the rest of the feedback off-list - to summarize it:
* reinject_local_mail already does take the smtputf8 setting from pmg.conf
  into consideration (pmg-api patch 2/5)

changes v2 -> v3:
* patch 5/5 for pmg-api was added to properly decode database results
  before quarantine delivery or adding addresses to the user welcomelist
  or blocklists
* other patches are left as they were

original cover-letter for v2:
This series is the improved second part of:
https://lists.proxmox.com/pipermail/pmg-devel/2023-January/002257.html

and incorporates a few other improvments, which came up during implementation.

Changes from v1:
1 dropped the already applied and released stop-gap fix
2 added 'reinject_local_mail` helper to scan and check the mail, instead
  of checking with mail_needs_smtputf8 at all call-sites  (Thanks to Thomas
  for the suggestion!)
3 while add it went ahead and added smtputf8 as new option to the mail section
  of pmg.conf (reason for the docs and gui patch)
4 during testing noted the duplication of reinject_mail in
  deliver_quarantined_mail (with the latter not receiving any of the
  improvments for the former) leading to the refactoring in api-patch 3 and 4

point 4 is independent of the rest, but review should be easier after looking
through the other patches.

the docs-patch is needed (as build-depends), due to the new pmg.conf option

original cover-letter for v1:
This series addresses an issue some of our users reported in our
community forum with pmg-api_7.2-3:
* the decision if smtputf8 is needed is based on the envelope-addresses
  and the headers (if they contain non-ascii characters we use SMTPUTF8
* this breaks environments where SMTPUTF8 is disabled (mostly because
  some downstream servers do not support this), but mails still contain
  non-ascii data (while this is against the relevant rfc, which say that
  header-data must contain only ascii characters (and should be encoded
  with MIME-words otherwise), it is seemingly quite common in the wild)

one testmail I got from a user from the forum had the From header
correctly encoded, but added an X-DFrom header with the unencoded from.

The first patch simply drops the header-inspection and should enable
most of the reporters to receive mail again.

The second patch tries to address the smtputf8 issue a bit differently
than what we currently do
- For mails received via SMTP it simply sets SMTPUTF8 if the original
  postfix processes sent pmg-smtp-filter the mail with the flag, and
  does not set it otherwise
- For locally generated mail it detects if its needed by checking the
  envelope-addresses and the headers for non-ascii characters.

This should follow postfix own functioning quite closely:
https://www.postfix.org/SMTPUTF8_README.html

processing by pmg-smtp-filter should not change the need for the flag,
since we don't rewrite envelope-addresses, and modify filed does
mime-encode the resulting header.

Sending as two patches, since the first one would be good to get out
soon (as it's affecting a few setups), while the second one might
benefit from a bit more testing (I did some tests, which all looked
good, but might have overlooked some cases)

pmg-api:
Stoiko Ivanov (5):
  smtputf8: keep smtputf8 from incoming postfix, detect for local mail
  config: make smtputf8 configurable through the API
  reinject mail: improve error logging
  quarantine: use reinject_local_mail to deliver quarantined mail
  api: quarantine: decode addresses before delivery/userlisting

 src/PMG/API2/Quarantine.pm | 12 ++++---
 src/PMG/Config.pm          |  7 +++++
 src/PMG/Quarantine.pm      | 64 +++++++++-----------------------------
 src/PMG/RuleDB/Notify.pm   |  2 +-
 src/PMG/SMTP.pm            |  3 +-
 src/PMG/Utils.pm           | 53 +++++++++++++++++++++----------
 src/templates/main.cf.in   |  4 +++
 7 files changed, 73 insertions(+), 72 deletions(-)

pmg-docs:
Stoiko Ivanov (1):
  doc-generator: add new option smtputf8

 gen-pmg.conf.5-opts.pl | 1 +
 1 file changed, 1 insertion(+)

pmg-gui:
Stoiko Ivanov (1):
  mail proxy options: add smtputf8 checkbox

 js/MailProxyOptions.js | 3 +++
 1 file changed, 3 insertions(+)

-- 
2.30.2





             reply	other threads:[~2023-03-17 18:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-17 18:44 Stoiko Ivanov [this message]
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 1/5] smtputf8: keep smtputf8 from incoming postfix, detect for local mail Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 2/5] config: make smtputf8 configurable through the API Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 3/5] reinject mail: improve error logging Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 4/5] quarantine: use reinject_local_mail to deliver quarantined mail Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 5/5] api: quarantine: decode addresses before delivery/userlisting Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-gui v3 1/1] mail proxy options: add smtputf8 checkbox Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-docs v3 1/1] doc-generator: add new option smtputf8 Stoiko Ivanov
2023-03-23 11:18 ` [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support Dominik Csapak
2023-03-27 10:33 ` [pmg-devel] applied-series: " Thomas Lamprecht

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=20230317184456.26465-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal