[pmg-devel] [PATCH pmg-api/pmg-gui v2 0/6] DKIM sign mails generated by PMG itself.

[pmg-devel] [PATCH pmg-api/pmg-gui v2 0/6] DKIM sign mails generated by PMG itself.

[Stoiko Ivanov]

supersedes:
https://lore.proxmox.com/pmg-devel/20250225102656.413135-1-s.ivanov@proxmox.com/T/#u
https://lore.proxmox.com/pmg-devel/20250224232451.399803-1-s.ivanov@proxmox.com/T/#t

v1->v2:
* add pattern, and lenght restriction on from-header format (patch 1)
* use the default setting of 'Proxmox Mail Gateway <postmaster>' everywhere
  instead of special-casing
* change the dkim-signing to not die when receiving an empty or domain-less
  envelope-sender (I only tested the positive case yesterday, where signing
  source was the header) - keep in mind that the effect was a warning in
  the journal, which is now a info
* add the minimal GUI-patch to the series

I kept patch 1/1 seperately, mostly due to the change being used in all
3 following patches (and since the commit-message as to why the change
should not cause regression grew rather large)
I'm fine with all 3 patches being squashed together if this seems better
to anyone.

Thanks very much to Maximilano and Dominik for the feedback provided

pmg-api:
Stoiko Ivanov (6):
  config: add admin-mail-from key
  reports: use admin-mail-from as from header
  smtp-engine: use admin-mail-from as from header for bounces
  ruledb: use admin-mail-from where sensible
  dkim: signer: log info instead of die'ing when missing domain
  reinject_local_mail: sign mails with DKIM based on header

 src/PMG/Backup.pm        |  2 +-
 src/PMG/CLI/pmgreport.pm |  2 +-
 src/PMG/Config.pm        |  7 +++++++
 src/PMG/DKIMSign.pm      | 10 ++++++++--
 src/PMG/Quarantine.pm    |  1 +
 src/PMG/RuleDB/Notify.pm |  8 ++++----
 src/PMG/SMTP.pm          |  7 ++++---
 src/PMG/Utils.pm         | 15 +++++++++++++++
 src/bin/pmg-smtp-filter  |  1 +
 9 files changed, 42 insertions(+), 11 deletions(-)

pmg-gui:
Stoiko Ivanov (1):
  configuration: options: add admin-mail-from row.

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

-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api v2 1/6] config: add admin-mail-from key

[Stoiko Ivanov]

To be used as From header for mails generated by the PMG stack.
Currently this is hardcoded to postmaster in a few places, and
Proxmox Mail Gateway <postmaster> in a few other places..

While PMG generates mails as the mail-system and postmaster is a
good choice for such mails, users sometimes wish to customize the
e-mail shown e.g. in the backup-notification and the daily admin
report.

The set value will be used in the header only, following the similar
setting 'mailfrom' in the 'spamquar' setting.
I decided against reusing the existing setting, as it's specifically
tied to a category.

The format is restricted by regex to printable ascii-characters for
now[1], as being more permissive later (even allowing all unicode
printables) is easy. The length restriction to 998 characters ist due
to a few SMTP-RFCs [2].

Envelope-from addresses for mail generated by will remain empty or
'postmaster'.

This is a first step towards DKIM signing those mails.

Users will have to opt-in to it and use a email with domain-part to
get DKIM signing.

I decided against changing the current behavior of using 'postmaster'
without domain, as this makes postfix complete the address with
'@$myorigin.$mydomain'[0], and I've seen quite a few installations
which set those settings in their main.cf.in templates.

[0] https://www.postfix.org/postconf.5.html#internal_mail_filter_classes
[1] https://perldoc.perl.org/perlrecharclass
[2] https://www.rfc-editor.org/rfc/rfc5322#section-2.1.1

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Config.pm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/PMG/Config.pm b/src/PMG/Config.pm
index 3170241..eabf81b 100644
--- a/src/PMG/Config.pm
+++ b/src/PMG/Config.pm
@@ -140,6 +140,12 @@ EODESC
 	    enum => [qw(header envelope)],
 	    default => 'envelope',
 	},
+	'admin-mail-from' => {
+	    description => "Text for 'From' header in admin mails and bounces.",
+	    type => 'string',
+	    pattern => '^\p{PosixPrint}{1,998}$',
+	    default => 'Proxmox Mail Gateway <postmaster>',
+	},
     };
 }
 
@@ -159,6 +165,7 @@ sub options {
 	dkim_sign_all_mail => { optional => 1 },
 	dkim_selector => { optional => 1 },
 	'dkim-use-domain' => { optional => 1 },
+	'admin-mail-from' => { optional => 1 },
     };
 }
 
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api v2 2/6] reports: use admin-mail-from as from header

[Stoiko Ivanov]

this sets the admin-mail-from header information for all local emails
generated through the templateing system, which are not already using
the 'mailfrom' setting from the 'spamquar' config-section.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Backup.pm        | 2 +-
 src/PMG/CLI/pmgreport.pm | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PMG/Backup.pm b/src/PMG/Backup.pm
index c820d6b..1a8f282 100644
--- a/src/PMG/Backup.pm
+++ b/src/PMG/Backup.pm
@@ -417,7 +417,7 @@ sub send_backup_notification {
 
     my $tt = PMG::Config::get_template_toolkit();
 
-    my $mailfrom = "Proxmox Mail Gateway <postmaster>";
+    my $mailfrom = $cfg->get('admin', 'admin-mail-from');
     PMG::Utils::finalize_report($tt, 'backup-notification', $vars, $mailfrom, $email);
 
 }
diff --git a/src/PMG/CLI/pmgreport.pm b/src/PMG/CLI/pmgreport.pm
index 0d7fcc8..8bad77a 100644
--- a/src/PMG/CLI/pmgreport.pm
+++ b/src/PMG/CLI/pmgreport.pm
@@ -358,7 +358,7 @@ __PACKAGE__->register_method ({
 	    return undef;
 	}
 
-	my $mailfrom = "Proxmox Mail Gateway <postmaster>";
+	my $mailfrom = $cfg->get('admin', 'admin-mail-from');
 	PMG::Utils::finalize_report($tt, 'pmgreport', $vars, $mailfrom, $email, $param->{debug});
 
 	return undef;
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api v2 3/6] smtp-engine: use admin-mail-from as from header for bounces

[Stoiko Ivanov]

generate_ndr is currently only used to generate a bounce-mail if the
following occur:
* email is blocked only for part of the receivers
* before-queue-filtering is active - in the after-queue case postfix
  generates the bounces for us.

With this patch the header of the bounce message will contain
'Proxmox Mail Gateway <postmaster>' (or the setting of
admin-mail-from) instead of 'postmaster', but this should not affect
deliverability of these mails.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/SMTP.pm | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/PMG/SMTP.pm b/src/PMG/SMTP.pm
index b7bc5d3..15c8464 100644
--- a/src/PMG/SMTP.pm
+++ b/src/PMG/SMTP.pm
@@ -185,7 +185,8 @@ sub loop {
 			$self->reply ("250 2.5.0 OK ($queueid)");
 			if ($cfg->get('mail', 'ndr_on_block')) {
 			    my $dnsinfo = $cfg->get_host_dns_info();
-			    generate_ndr($self->{from}, [ @reject_rec ], $dnsinfo->{fqdn}, $queueid) if scalar(@reject_rec);
+			    my $from_header = $cfg->get('admin', 'admin-mail-from');
+			    generate_ndr($self->{from}, [ @reject_rec ], $dnsinfo->{fqdn}, $queueid, $from_header) if scalar(@reject_rec);
 			}
 		    } else {
 			$self->reply ("451 4.4.0 detected undelivered mail ($queueid)");
@@ -265,7 +266,7 @@ sub save_data {
 }
 
 sub generate_ndr {
-    my ($sender, $receivers, $hostname, $queueid) = @_;
+    my ($sender, $receivers, $hostname, $queueid, $from_header) = @_;
 
     my $ndr_text = <<EOF
 This is the mail system at host $hostname.
@@ -284,7 +285,7 @@ EOF
     my $ndr = MIME::Entity->build(
 	Type => 'multipart/report; report-type=delivery-status;',
 	To => $sender,
-	From => 'postmaster',
+	From => $from_header,
 	Subject => 'Undelivered Mail');
 
     $ndr->attach(
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api v2 4/6] ruledb: use admin-mail-from where sensible

[Stoiko Ivanov]

use the new setting as From header for notifications, but keep
the envelope-sender fixed as 'postmaster'.

add a comment that we do not want to sign a mail released from the
quarantine, as it remains the only use of 'postmaster' without this
fallback, but as above - only for the envelope-sender.

the admin-mail-from setting has to be carried through pmg-smtp-filter
in the msginfo variable as was done for the dkim settings as well.

This changes the From header for Notifications sent from the
rulesystem from: 'postmaster' to 'Proxmox Mail Gateway <postmaster>'
(or the setting of admin-mail-from).

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Quarantine.pm    | 1 +
 src/PMG/RuleDB/Notify.pm | 8 ++++----
 src/bin/pmg-smtp-filter  | 1 +
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/PMG/Quarantine.pm b/src/PMG/Quarantine.pm
index 77f64a0..4f756d5 100644
--- a/src/PMG/Quarantine.pm
+++ b/src/PMG/Quarantine.pm
@@ -116,6 +116,7 @@ sub deliver_quarantined_mail {
     $entity->head->delete('Delivered-To');
     $entity->head->delete('Return-Path');
 
+    # mail delivered from quarantine will not be DKIM signed as locally originating mails
     my $sender = 'postmaster'; # notify postmaster if something fails
 
     eval {
diff --git a/src/PMG/RuleDB/Notify.pm b/src/PMG/RuleDB/Notify.pm
index c024840..95d2ffa 100644
--- a/src/PMG/RuleDB/Notify.pm
+++ b/src/PMG/RuleDB/Notify.pm
@@ -206,8 +206,6 @@ sub execute {
 
     my $original;
 
-    my $from = 'postmaster';
-
     my $rulename = encode('UTF-8', $vars->{RULE} // 'unknown');
 
     my $body = PMG::Utils::subst_values($self->{body}, $vars);
@@ -224,10 +222,12 @@ sub execute {
     $to =~ s/[;,]/ /g;
     $to =~ s/\s+/,/g;
 
+    my $from_header = $msginfo->{admin_mail_from};
+
     my $top = MIME::Entity->build(
 	Encoding    => 'quoted-printable',
 	Charset => 'UTF-8',
-	From    => $from,
+	From    => $from_header,
 	To      => $to,
 	Subject => encode_mimewords(encode('UTF-8', $subject), "Charset" => "UTF-8"),
 	Data => encode('UTF-8', $body));
@@ -257,7 +257,7 @@ sub execute {
     } else {
 	my @targets = split(/\s*,\s*/, $to);
 	my $qid = PMG::Utils::reinject_local_mail(
-	    $top, $from, \@targets, undef, $msginfo->{fqdn});
+	    $top, 'postmaster', \@targets, undef, $msginfo->{fqdn});
 	foreach (@targets) {
 	    my $target = encode('UTF-8', $_);
 	    if ($qid) {
diff --git a/src/bin/pmg-smtp-filter b/src/bin/pmg-smtp-filter
index 60737ea..c36c8fb 100755
--- a/src/bin/pmg-smtp-filter
+++ b/src/bin/pmg-smtp-filter
@@ -646,6 +646,7 @@ sub handle_smtp {
 	    $msginfo->{dkim}->{sign_all} = $pmg_cfg->get('admin', 'dkim_sign_all_mail');
 	    $msginfo->{dkim}->{selector} = $pmg_cfg->get('admin', 'dkim_selector');
 	}
+	$msginfo->{admin_mail_from} = $pmg_cfg->get('admin', 'admin-mail-from');
 
 	$msginfo->{hostname} = PVE::INotify::nodename();
 	my $resolv = PVE::INotify::read_file('resolvconf');
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api v2 5/6] dkim: signer: log info instead of die'ing when missing domain

[Stoiko Ivanov]

for mail locally generated by PMG the signing sources
(envelope-sender, From header) can contain only a local-part
(postmaster) or even be empty (envelope-sender).

While such mail cannot be sensibly signed, it should be treated as if
the domain is not listed in DKIM-domains - by an log message on 'info'
level instead of a `die`.
the sub with the changed behavior is only used in this module, and
sign_entity as external entry-point is only called in eval context,
resulting in a log message on level 'warn'.

so effectively this change should only reduce log-levels for DKIM
failures in these cases from 'warning' to 'info'

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/DKIMSign.pm | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/PMG/DKIMSign.pm b/src/PMG/DKIMSign.pm
index 6f309c8..72f4809 100644
--- a/src/PMG/DKIMSign.pm
+++ b/src/PMG/DKIMSign.pm
@@ -59,9 +59,16 @@ sub signing_domain {
     my $input_domain;
     if ($use_domain eq 'header') {
 	$input_domain = parse_headers_for_signing($entity);
+	if (!defined($input_domain)) {
+	    syslog('info', "DKIM signing: no domain found in the headers from '$sender_email'");
+	    return 0;
+	}
     } else {
 	my @parts = split('@', $sender_email);
-	die "no domain in sender e-mail\n" if scalar(@parts) < 2;
+	if (scalar(@parts) < 2) {
+	    syslog('info', "DKIM signing: no domain found in '$sender_email'");
+	    return 0;
+	}
 	$input_domain = $parts[-1];
     }
 
@@ -107,7 +114,6 @@ sub parse_headers_for_signing {
 	$domain = $addresses[0]->host();
     }
 
-    die "there is no sender in the header\n" if !defined($domain);
     return $domain;
 }
 
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api v2 6/6] reinject_local_mail: sign mails with DKIM based on header

[Stoiko Ivanov]

as most mails PMG generates locally has an empty envelope-sender,
signing only makes sense when the from-header domain is used as
signing domain.

This fixes #3423, and partially addresses #2971 and #4658 (bounces
generated by postfix directly are not passed through our stack, and
should not be processed in general -  see
https://www.postfix.org/postconf.5.html#internal_mail_filter_classes).

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Utils.pm | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index b2a75fb..f7ee1c6 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -247,6 +247,21 @@ sub reinject_local_mail {
 	$params->{mail}->{smtputf8} = $needs_smtputf8;
     }
 
+    my $dkim_sign = $cfg->get('admin', 'dkim_sign');
+    if ($dkim_sign) {
+	my $dkim = {};
+	$dkim->{sign} = $dkim_sign;
+	$dkim->{use_domain} = $cfg->get('admin', 'dkim-use-domain');
+	$dkim->{sign_all} = $cfg->get('admin', 'dkim_sign_all_mail');
+	$dkim->{selector} = $cfg->get('admin', 'dkim_selector');
+	eval {
+	    $entity = PMG::DKIMSign::sign_entity($entity, $dkim, $sender);
+	};
+	if ($@) {
+	    syslog('warning', "Could not DKIM-Sign local mail: $@");
+	}
+    }
+
     return reinject_mail($entity, $sender, $targets, $xforward, $me, $params);
 }
 
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-gui v2 1/1] configuration: options: add admin-mail-from row.

[Stoiko Ivanov]

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 js/SystemOptions.js | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/js/SystemOptions.js b/js/SystemOptions.js
index 7a22e1a..3c8d960 100644
--- a/js/SystemOptions.js
+++ b/js/SystemOptions.js
@@ -75,6 +75,9 @@ Ext.define('PMG.SystemOptions', {
 	me.add_text_row('email', gettext("Administrator EMail"),
 			{ deleteEmpty: true, defaultValue: Proxmox.Utils.noneText });
 
+	me.add_text_row('admin-mail-from', gettext("'From:' for Admin Mail"),
+			{ deleteEmpty: true, defaultValue: Proxmox.Utils.noneText });
+
 	me.add_proxy_row('http_proxy', gettext("HTTP proxy"));
 
 	me.callParent();
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] applied-series: [PATCH pmg-api/pmg-gui v2 0/6] DKIM sign mails generated by PMG itself.

[Thomas Lamprecht]

Am 25.02.25 um 16:01 schrieb Stoiko Ivanov:
> supersedes:
> https://lore.proxmox.com/pmg-devel/20250225102656.413135-1-s.ivanov@proxmox.com/T/#u
> https://lore.proxmox.com/pmg-devel/20250224232451.399803-1-s.ivanov@proxmox.com/T/#t
> 
> v1->v2:
> * add pattern, and lenght restriction on from-header format (patch 1)
> * use the default setting of 'Proxmox Mail Gateway <postmaster>' everywhere
>   instead of special-casing
> * change the dkim-signing to not die when receiving an empty or domain-less
>   envelope-sender (I only tested the positive case yesterday, where signing
>   source was the header) - keep in mind that the effect was a warning in
>   the journal, which is now a info
> * add the minimal GUI-patch to the series
> 
> I kept patch 1/1 seperately, mostly due to the change being used in all
> 3 following patches (and since the commit-message as to why the change
> should not cause regression grew rather large)
> I'm fine with all 3 patches being squashed together if this seems better
> to anyone.
> 
> Thanks very much to Maximilano and Dominik for the feedback provided
> 
> pmg-api:
> Stoiko Ivanov (6):
>   config: add admin-mail-from key
>   reports: use admin-mail-from as from header
>   smtp-engine: use admin-mail-from as from header for bounces
>   ruledb: use admin-mail-from where sensible
>   dkim: signer: log info instead of die'ing when missing domain
>   reinject_local_mail: sign mails with DKIM based on header
> 
>  src/PMG/Backup.pm        |  2 +-
>  src/PMG/CLI/pmgreport.pm |  2 +-
>  src/PMG/Config.pm        |  7 +++++++
>  src/PMG/DKIMSign.pm      | 10 ++++++++--
>  src/PMG/Quarantine.pm    |  1 +
>  src/PMG/RuleDB/Notify.pm |  8 ++++----
>  src/PMG/SMTP.pm          |  7 ++++---
>  src/PMG/Utils.pm         | 15 +++++++++++++++
>  src/bin/pmg-smtp-filter  |  1 +
>  9 files changed, 42 insertions(+), 11 deletions(-)
> 
> pmg-gui:
> Stoiko Ivanov (1):
>   configuration: options: add admin-mail-from row.
> 
>  js/SystemOptions.js | 3 +++
>  1 file changed, 3 insertions(+)
> 


applied, thanks!

maybe adding some short hint/pointer in the docs might be nice here?


_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel