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

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

[Stoiko Ivanov]

Since it's now (since 8.1) possible to sign mails based on their From
header we can sign most of the mails PMG generates itself:
* spam- and admin-reports
* notifications from the rulesystem
* backup notifications
* quarantine-link mails
* a specific bounce we generate ourselves.

This series partially addresses:
* https://bugzilla.proxmox.com/show_bug.cgi?id=4658
* https://bugzilla.proxmox.com/show_bug.cgi?id=2971
and fixes
* https://bugzilla.proxmox.com/show_bug.cgi?id=3423

I tested it on my local system, but another set of eyes would be very much
appreciated

Stoiko Ivanov (7):
  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: degrade missing domain in from header to info
  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        |  6 ++++++
 src/PMG/DKIMSign.pm      |  5 ++++-
 src/PMG/Quarantine.pm    |  1 +
 src/PMG/RuleDB/Notify.pm | 11 ++++++++---
 src/PMG/SMTP.pm          |  7 ++++---
 src/PMG/Utils.pm         | 22 ++++++++++++++++++++++
 src/bin/pmg-smtp-filter  |  1 +
 9 files changed, 48 insertions(+), 9 deletions(-)

-- 
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 1/7] config: add admin-mail-from key

[Stoiko Ivanov]

To be used as From header address for mails generated by the PMG
stack. Currently this is hardcoded to postmaster in a few 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.

PMG will as keep sending most mails with an empty envelope-from or
with '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

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

diff --git a/src/PMG/Config.pm b/src/PMG/Config.pm
index 3170241..c583cc6 100644
--- a/src/PMG/Config.pm
+++ b/src/PMG/Config.pm
@@ -140,6 +140,11 @@ EODESC
 	    enum => [qw(header envelope)],
 	    default => 'envelope',
 	},
+	'admin-mail-from' => {
+	    description => "Text for 'From' header in admin mails and bounces.",
+	    type => 'string',
+	    default => 'postmaster',
+	},
     };
 }
 
@@ -159,6 +164,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

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

[Dominik Csapak]

high level:

wouldn't it make sense to squash this patch with the ones where it's actually used?

IMHO it only makes sense to split such things when the changes are rather
large, but that's not the case here.

On 2/25/25 00:24, Stoiko Ivanov wrote:
> To be used as From header address for mails generated by the PMG
> stack. Currently this is hardcoded to postmaster in a few 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.
> 
> PMG will as keep sending most mails with an empty envelope-from or
> with '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
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>   src/PMG/Config.pm | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/src/PMG/Config.pm b/src/PMG/Config.pm
> index 3170241..c583cc6 100644
> --- a/src/PMG/Config.pm
> +++ b/src/PMG/Config.pm
> @@ -140,6 +140,11 @@ EODESC
>   	    enum => [qw(header envelope)],
>   	    default => 'envelope',
>   	},
> +	'admin-mail-from' => {
> +	    description => "Text for 'From' header in admin mails and bounces.",
> +	    type => 'string',

should we try to restrict this a bit?

> +	    default => 'postmaster',> +	},
>       };
>   }
>   
> @@ -159,6 +164,7 @@ sub options {
>   	dkim_sign_all_mail => { optional => 1 },
>   	dkim_selector => { optional => 1 },
>   	'dkim-use-domain' => { optional => 1 },
> +	'admin-mail-from' => { optional => 1 },
>       };
>   }
>   



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

[pmg-devel] [PATCH pmg-api 2/7] 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..99b9a57 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', 1) // "Proxmox Mail Gateway <postmaster>";
     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..39a2309 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', 1) // "Proxmox Mail Gateway <postmaster>";
 	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

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

[Dominik Csapak]

On 2/25/25 00:24, Stoiko Ivanov wrote:
> 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..99b9a57 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', 1) // "Proxmox Mail Gateway <postmaster>";

we use 'Proxmox Mail Gateway <postmaster>' here as default but '<postmaster>' / 'postmaster' 
elsewhere. But the user can only configure it one time with the same
value set for all.

Couldn't we also set 'Proxmox Mail Gateway <postmaster>' as default and use that everywhere?
Then we could use the default from the config instead of setting the default manually everywhere?

>       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..39a2309 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', 1) // "Proxmox Mail Gateway <postmaster>";
>   	PMG::Utils::finalize_report($tt, 'pmgreport', $vars, $mailfrom, $email, $param->{debug});
>   
>   	return undef;



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

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

[Stoiko Ivanov]

On Tue, 25 Feb 2025 11:32:49 +0100
Dominik Csapak <d.csapak@proxmox.com> wrote:

> On 2/25/25 00:24, Stoiko Ivanov wrote:
> > 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..99b9a57 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', 1) // "Proxmox Mail Gateway <postmaster>";  
> 
> we use 'Proxmox Mail Gateway <postmaster>' here as default but '<postmaster>' / 'postmaster' 
> elsewhere. But the user can only configure it one time with the same
> value set for all.
> 
> Couldn't we also set 'Proxmox Mail Gateway <postmaster>' as default and use that everywhere?
> Then we could use the default from the config instead of setting the default manually everywhere?

Looking through the changes in the series - the places where we use
'postmaster' (w/o "Proxmox Mail Gateway ") in the from-header are:
* Notification mails. 
* generate_ndr in SMTP.pm (before-queue-filtering blocking mails only for
  part of the receivers and generate_ndr_on_block set)

I think this change in behavior is acceptable (Notifications  - and would
rewrite it that way (should make things a bit shorter too)

Thanks for the tip!


> 
> >       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..39a2309 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', 1) // "Proxmox Mail Gateway <postmaster>";
> >   	PMG::Utils::finalize_report($tt, 'pmgreport', $vars, $mailfrom, $email, $param->{debug});
> >   
> >   	return undef;  
> 
> 



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

[pmg-devel] [PATCH pmg-api 3/7] 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.

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..f387fcb 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', 1) // '<postmaster>';
+			    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

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

[Maximiliano Sandoval]

Stoiko Ivanov <s.ivanov@proxmox.com> writes:

> 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.
>
> 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..f387fcb 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', 1) // '<postmaster>';
> +			    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) = @_;

This function would imo benefit from either having a NULL check for
`$from_header` or by defining here the fallback value.


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

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

[Stoiko Ivanov]

On Tue, 25 Feb 2025 10:53:29 +0100
Maximiliano Sandoval <m.sandoval@proxmox.com> wrote:

> Stoiko Ivanov <s.ivanov@proxmox.com> writes:
> 
> > 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.
> >
> > 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..f387fcb 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', 1) // '<postmaster>';
> > +			    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) = @_;  
> 
> This function would imo benefit from either having a NULL check for
> `$from_header` or by defining here the fallback value.
Thanks - good catch - as generate_ndr currently only used once (in the
lines above) - I think `die .. if !defined($from_header)` should not hurt

will add this in a v2 (but would wait for some more feedback)



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

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

[Dominik Csapak]

On 2/25/25 00:24, Stoiko Ivanov wrote:
> 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.
> 
> 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..f387fcb 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', 1) // '<postmaster>';

doesn't this now generate a different default? '<postmaster>' vs 'postmaster' ?

also wouldn't it be better to generate the default below where we actually use it instead of here?
then Maximilianos undef check concerns would be gone too.

> +			    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(



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

[pmg-devel] [PATCH pmg-api 4/7] 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.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Quarantine.pm    | 1 +
 src/PMG/RuleDB/Notify.pm | 6 +++---
 src/bin/pmg-smtp-filter  | 1 +
 3 files changed, 5 insertions(+), 3 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..1d31985 100644
--- a/src/PMG/RuleDB/Notify.pm
+++ b/src/PMG/RuleDB/Notify.pm
@@ -206,7 +206,7 @@ sub execute {
 
     my $original;
 
-    my $from = 'postmaster';
+    my $from_header = $msginfo->{admin_mail_from} // 'postmaster';
 
     my $rulename = encode('UTF-8', $vars->{RULE} // 'unknown');
 
@@ -227,7 +227,7 @@ sub execute {
     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..88690dd 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', 1);
 
 	$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 5/7] dkim: signer: degrade missing domain in from header to info

[Stoiko Ivanov]

for mail locally generated by PMG the From header can contain only a
local-part (postmaster).

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 potential for regression
should not be too high.

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

diff --git a/src/PMG/DKIMSign.pm b/src/PMG/DKIMSign.pm
index 6f309c8..be8f1d0 100644
--- a/src/PMG/DKIMSign.pm
+++ b/src/PMG/DKIMSign.pm
@@ -59,6 +59,10 @@ 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;
@@ -107,7 +111,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 6/7] 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 | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index b2a75fb..3303bac 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -247,6 +247,24 @@ 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, set mail address with domain as "
+		    ."'admin-mail-from': $@",
+	    );
+	}
+    }
+
     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

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

[Maximiliano Sandoval]

Stoiko Ivanov <s.ivanov@proxmox.com> writes:

> 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 | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> index b2a75fb..3303bac 100644
> --- a/src/PMG/Utils.pm
> +++ b/src/PMG/Utils.pm
> @@ -247,6 +247,24 @@ 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, set mail address with domain as "
> +		    ."'admin-mail-from': $@",

> set mail address with domain as 'admin-mail-from': $@"

nit: Perhaps this could be phrased differently, I am personally having
trouble understanding how the part after the `,` should be read.

> +	    );
> +	}
> +    }
> +
>      return reinject_mail($entity, $sender, $targets, $xforward, $me, $params);
>  }



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

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

[Dominik Csapak]

shouldn't this patch check if '$sender' is empty or 'postmaster'/'<postmaster>' before trying
to sign?

otherwise we now will always run into an error if 'dkim-use-domain' isn't set?
(since we'll split on @ on the sender and die for e.g. 'postmaster')
or am I missing something here?


On 2/25/25 00:24, Stoiko Ivanov wrote:
> 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 | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> index b2a75fb..3303bac 100644
> --- a/src/PMG/Utils.pm
> +++ b/src/PMG/Utils.pm
> @@ -247,6 +247,24 @@ 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, set mail address with domain as "
> +		    ."'admin-mail-from': $@",
> +	    );
> +	}
> +    }
> +
>       return reinject_mail($entity, $sender, $targets, $xforward, $me, $params);
>   }
>   



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