* [pmg-devel] [PATCH pmg-api 1/1] fix #2795: add support for DSN
@ 2021-11-22 19:50 Stoiko Ivanov
2021-11-24 9:33 ` Dominik Csapak
0 siblings, 1 reply; 2+ messages in thread
From: Stoiko Ivanov @ 2021-11-22 19:50 UTC (permalink / raw)
To: pmg-devel
Delivery status notifications (DSN) are defined in RFC 3464 [0].
They introduce additional key-value parameters for the MAIL and RCPT
commands.
The default config we ship offers DSN on the internal port (but not on
the external port) - which works fine in after-queue filtering mode,
but not in before-queue filtering mode.
This patchset adds support for collecting the relevant parameters and
passing them on to the postfix instance [1], which actually sends out the
mail.
additionally postfix does syntax-checking before passing the mail to
the proxy (before queue filtering).
Since the handling is done by postfix we don't need to generate the
DSN for any of the cases.
tested with various combinations of the -V, -N and -R parameters to
sendmail (e.g.):
```
/usr/sbin/sendmail -N success,delay,failure \
-V '<xxxxxxxx@test.proxmox.com>'\
-R hdrs test@test.domain.example
```
some tests with invalid combinations were also done with netcat.
[0] https://tools.ietf.org/html/rfc3464
[1] http://www.postfix.org/DSN_README.html
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PMG/RuleDB/Accept.pm | 2 +-
src/PMG/RuleDB/BCC.pm | 6 +++++-
src/PMG/SMTP.pm | 46 +++++++++++++++++++++++++++++++++-------
src/PMG/Utils.pm | 19 ++++++++++++++---
src/bin/pmg-smtp-filter | 1 +
5 files changed, 61 insertions(+), 13 deletions(-)
diff --git a/src/PMG/RuleDB/Accept.pm b/src/PMG/RuleDB/Accept.pm
index 0bcf250..cd67ea2 100644
--- a/src/PMG/RuleDB/Accept.pm
+++ b/src/PMG/RuleDB/Accept.pm
@@ -122,7 +122,7 @@ sub execute {
} else {
my ($qid, $code, $mess) = PMG::Utils::reinject_mail(
$entity, $msginfo->{sender}, $tg,
- $msginfo->{xforward}, $msginfo->{fqdn});
+ $msginfo->{xforward}, $msginfo->{fqdn}, $msginfo->{param});
if ($qid) {
foreach (@$tg) {
syslog('info', "%s: accept mail to <%s> (%s) (rule: %s)", $queue->{logid}, encode('UTF-8', $_), $qid, $rulename);
diff --git a/src/PMG/RuleDB/BCC.pm b/src/PMG/RuleDB/BCC.pm
index a8db3f5..d364690 100644
--- a/src/PMG/RuleDB/BCC.pm
+++ b/src/PMG/RuleDB/BCC.pm
@@ -156,9 +156,13 @@ sub execute {
$entity->print ($fh);
print $fh "bcc end\n";
} else {
+ my $param = {};
+ for my $bcc (@bcc_targets) {
+ $param->{rcpt}->{$bcc}->{notify} = "never";
+ }
my $qid = PMG::Utils::reinject_mail(
$entity, $msginfo->{sender}, \@bcc_targets,
- $msginfo->{xforward}, $msginfo->{fqdn}, 1);
+ $msginfo->{xforward}, $msginfo->{fqdn}, $param);
foreach (@bcc_targets) {
if ($qid) {
syslog('info', "%s: bcc to <%s> (rule: %s, %s)", $queue->{logid}, $_, $rulename, $qid);
diff --git a/src/PMG/SMTP.pm b/src/PMG/SMTP.pm
index 68c833e..c7aba01 100644
--- a/src/PMG/SMTP.pm
+++ b/src/PMG/SMTP.pm
@@ -38,6 +38,7 @@ sub reset {
delete $self->{smtputf8};
delete $self->{xforward};
delete $self->{status};
+ delete $self->{param};
}
sub abort {
@@ -100,25 +101,54 @@ sub loop {
$self->reply ("250 2.5.0 OK");
next;
} elsif ($cmd eq 'mail') {
- if ($args =~ m/^from:\s*<([^\s\>]*)>([^>]*)$/i) {
+ if ($args =~ m/^from:\s*<([^\s\>]*?)>(.*)$/i) {
delete $self->{to};
- my ($from, $opts) = ($1, $2);
- if ($opts =~ m/\sSMTPUTF8/) {
- $self->{smtputf8} = 1;
- $from = decode('UTF-8', $from);
+ my ($from, $opts) = ($1, lc($2));
+
+ my $err = 0;
+ for my $opt (split(' ', $opts)) {
+ if ($opt =~ /(ret|envid)=([^ =]+)/ ) {
+ $self->{param}->{mail}->{$1} = $2;
+ } elsif ($opt =~ m/smtputf8/) {
+ $self->{smtputf8} = 1;
+ $from = decode('UTF-8', $from);
+ } elsif ($opt =~ m/(?:size=(:?\d+)|body=8bitmime)/) {
+ #skip
+ } else {
+ $err = 1;
+ last;
+ }
}
$self->{from} = $from;
- $self->reply ('250 2.5.0 OK');
+ if ($err) {
+ $self->reply ("501 5.5.2 Syntax: MAIL FROM: unknown parameter");
+ } else {
+ $self->reply ('250 2.5.0 OK');
+ }
next;
} else {
$self->reply ("501 5.5.2 Syntax: MAIL FROM: <address>");
next;
}
} elsif ($cmd eq 'rcpt') {
- if ($args =~ m/^to:\s*<([^\s\>]+)>[^>]*$/i) {
+ if ($args =~ m/^to:\s*<([^\s\>]+?)>(.*)$/i) {
my $to = $self->{smtputf8} ? decode('UTF-8', $1) : $1;
+ my $opts = lc($2);
push @{$self->{to}} , $to;
- $self->reply ('250 2.5.0 OK');
+ my $err = 0;
+ for my $opt (split(' ', $opts)) {
+ if ($opt =~ /(notify|orcpt)=([^ =]+)/i ) {
+ $self->{param}->{rcpt}->{$to}->{$1} = $2;
+ } else {
+ $err = 1;
+ last;
+ }
+ }
+ if ($err) {
+ $self->reply ("501 5.5.2 Syntax: RCPT TO: unknown parameter");
+ } else {
+ $self->reply ('250 2.5.0 OK');
+ }
next;
} else {
$self->reply ("501 5.5.2 Syntax: RCPT TO: <address>");
diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index 92c3a7a..4eebfa5 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -203,7 +203,7 @@ sub subst_values {
}
sub reinject_mail {
- my ($entity, $sender, $targets, $xforward, $me, $nodsn) = @_;
+ my ($entity, $sender, $targets, $xforward, $me, $params) = @_;
my $smtp;
my $resid;
@@ -244,15 +244,28 @@ sub reinject_mail {
$mail_opts .= " SMTPUTF8" if $has_utf8_targets;
}
+ if (defined($params->{mail})) {
+ my $mailparams = $params->{mail};
+ for my $p (keys %$mailparams) {
+ $mail_opts .= " $p=$mailparams->{$p}";
+ }
+ }
+
if (!$smtp->_MAIL("FROM:" . $sender_addr . $mail_opts)) {
syslog('err', "smtp error - got: %s %s", $smtp->code, scalar ($smtp->message));
die "smtp from: ERROR";
}
- my $rcpt_opts = $nodsn ? " NOTIFY=NEVER" : "";
-
foreach my $target (@$targets) {
my $rcpt_addr;
+ my $rcpt_opts = '';
+ if (defined($params->{rcpt}->{$target})) {
+ my $rcptparams = $params->{rcpt}->{$target};
+ for my $p (keys %$rcptparams) {
+ $rcpt_opts .= " $p=$rcptparams->{$p}";
+ }
+ }
+
if (utf8::is_utf8($target)) {
$rcpt_addr = encode('UTF-8', $smtp->_addr($target));
} else {
diff --git a/src/bin/pmg-smtp-filter b/src/bin/pmg-smtp-filter
index b070c8e..45eb125 100755
--- a/src/bin/pmg-smtp-filter
+++ b/src/bin/pmg-smtp-filter
@@ -640,6 +640,7 @@ sub handle_smtp {
$msginfo->{sender} = $smtp->{from};
$msginfo->{xforward} = $smtp->{xforward};
$msginfo->{targets} = $smtp->{to};
+ $msginfo->{param} = $smtp->{param};
my $dkim_sign = $msginfo->{trusted} && $pmg_cfg->get('admin', 'dkim_sign');
if ($dkim_sign) {
--
2.30.2
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [pmg-devel] [PATCH pmg-api 1/1] fix #2795: add support for DSN
2021-11-22 19:50 [pmg-devel] [PATCH pmg-api 1/1] fix #2795: add support for DSN Stoiko Ivanov
@ 2021-11-24 9:33 ` Dominik Csapak
0 siblings, 0 replies; 2+ messages in thread
From: Dominik Csapak @ 2021-11-24 9:33 UTC (permalink / raw)
To: Stoiko Ivanov, pmg-devel
works generally as advertised, i have some (minor) remarks
though (comments inline)
On 11/22/21 20:50, Stoiko Ivanov wrote:
> Delivery status notifications (DSN) are defined in RFC 3464 [0].
> They introduce additional key-value parameters for the MAIL and RCPT
> commands.
>
> The default config we ship offers DSN on the internal port (but not on
> the external port) - which works fine in after-queue filtering mode,
> but not in before-queue filtering mode.
>
> This patchset adds support for collecting the relevant parameters and
> passing them on to the postfix instance [1], which actually sends out the
> mail.
>
> additionally postfix does syntax-checking before passing the mail to
> the proxy (before queue filtering).
>
> Since the handling is done by postfix we don't need to generate the
> DSN for any of the cases.
>
> tested with various combinations of the -V, -N and -R parameters to
> sendmail (e.g.):
> ```
> /usr/sbin/sendmail -N success,delay,failure \
> -V '<xxxxxxxx@test.proxmox.com>'\
> -R hdrs test@test.domain.example
> ```
>
> some tests with invalid combinations were also done with netcat.
>
> [0] https://tools.ietf.org/html/rfc3464
> [1] http://www.postfix.org/DSN_README.html
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> src/PMG/RuleDB/Accept.pm | 2 +-
> src/PMG/RuleDB/BCC.pm | 6 +++++-
> src/PMG/SMTP.pm | 46 +++++++++++++++++++++++++++++++++-------
> src/PMG/Utils.pm | 19 ++++++++++++++---
> src/bin/pmg-smtp-filter | 1 +
> 5 files changed, 61 insertions(+), 13 deletions(-)
>
> diff --git a/src/PMG/RuleDB/Accept.pm b/src/PMG/RuleDB/Accept.pm
> index 0bcf250..cd67ea2 100644
> --- a/src/PMG/RuleDB/Accept.pm
> +++ b/src/PMG/RuleDB/Accept.pm
> @@ -122,7 +122,7 @@ sub execute {
> } else {
> my ($qid, $code, $mess) = PMG::Utils::reinject_mail(
> $entity, $msginfo->{sender}, $tg,
> - $msginfo->{xforward}, $msginfo->{fqdn});
> + $msginfo->{xforward}, $msginfo->{fqdn}, $msginfo->{param});
> if ($qid) {
> foreach (@$tg) {
> syslog('info', "%s: accept mail to <%s> (%s) (rule: %s)", $queue->{logid}, encode('UTF-8', $_), $qid, $rulename);
> diff --git a/src/PMG/RuleDB/BCC.pm b/src/PMG/RuleDB/BCC.pm
> index a8db3f5..d364690 100644
> --- a/src/PMG/RuleDB/BCC.pm
> +++ b/src/PMG/RuleDB/BCC.pm
> @@ -156,9 +156,13 @@ sub execute {
> $entity->print ($fh);
> print $fh "bcc end\n";
> } else {
> + my $param = {};
> + for my $bcc (@bcc_targets) {
> + $param->{rcpt}->{$bcc}->{notify} = "never";
> + }
> my $qid = PMG::Utils::reinject_mail(
> $entity, $msginfo->{sender}, \@bcc_targets,
> - $msginfo->{xforward}, $msginfo->{fqdn}, 1);
> + $msginfo->{xforward}, $msginfo->{fqdn}, $param);
> foreach (@bcc_targets) {
> if ($qid) {
> syslog('info', "%s: bcc to <%s> (rule: %s, %s)", $queue->{logid}, $_, $rulename, $qid);
> diff --git a/src/PMG/SMTP.pm b/src/PMG/SMTP.pm
> index 68c833e..c7aba01 100644
> --- a/src/PMG/SMTP.pm
> +++ b/src/PMG/SMTP.pm
> @@ -38,6 +38,7 @@ sub reset {
> delete $self->{smtputf8};
> delete $self->{xforward};
> delete $self->{status};
> + delete $self->{param};
> }
>
> sub abort {
> @@ -100,25 +101,54 @@ sub loop {
> $self->reply ("250 2.5.0 OK");
> next;
> } elsif ($cmd eq 'mail') {
> - if ($args =~ m/^from:\s*<([^\s\>]*)>([^>]*)$/i) {
> + if ($args =~ m/^from:\s*<([^\s\>]*?)>(.*)$/i) {
this change looks like it's unrelated on the first glance
i understand this is because options can contain '>'
(i mean local parts of mails can too, but this'll break other
parts already...)
a small notice in the commit message (or a comment) would be good though
could also be it's own patch (smthg like 'fix smtp option parsing' ?)
> delete $self->{to};
> - my ($from, $opts) = ($1, $2);
> - if ($opts =~ m/\sSMTPUTF8/) {
> - $self->{smtputf8} = 1;
> - $from = decode('UTF-8', $from);
> + my ($from, $opts) = ($1, lc($2));
> +
> + my $err = 0;
> + for my $opt (split(' ', $opts)) {
> + if ($opt =~ /(ret|envid)=([^ =]+)/ ) {
> + $self->{param}->{mail}->{$1} = $2;
> + } elsif ($opt =~ m/smtputf8/) {
> + $self->{smtputf8} = 1;
> + $from = decode('UTF-8', $from);
> + } elsif ($opt =~ m/(?:size=(:?\d+)|body=8bitmime)/) {
> + #skip
> + } else {
> + $err = 1;
> + last;
> + }
as talked off-list, i am unsure if rejecting 'unknown' parameters is
the best way. before, we simply ignore them, so if a mail-server already
sent invalid/unknown paramters it will now be blocked which is not
that good imho. i think we should still ignore/drop them or pass them
along.
> }
> $self->{from} = $from;
> - $self->reply ('250 2.5.0 OK');
> + if ($err) {
> + $self->reply ("501 5.5.2 Syntax: MAIL FROM: unknown parameter");
> + } else {
> + $self->reply ('250 2.5.0 OK');
> + }
> next;
> } else {
> $self->reply ("501 5.5.2 Syntax: MAIL FROM: <address>");
> next;
> }
> } elsif ($cmd eq 'rcpt') {
> - if ($args =~ m/^to:\s*<([^\s\>]+)>[^>]*$/i) {
> + if ($args =~ m/^to:\s*<([^\s\>]+?)>(.*)$/i) {
> my $to = $self->{smtputf8} ? decode('UTF-8', $1) : $1;
> + my $opts = lc($2);
> push @{$self->{to}} , $to;
> - $self->reply ('250 2.5.0 OK');
> + my $err = 0;
> + for my $opt (split(' ', $opts)) {
> + if ($opt =~ /(notify|orcpt)=([^ =]+)/i ) {
> + $self->{param}->{rcpt}->{$to}->{$1} = $2;
> + } else {
> + $err = 1;
> + last;
> + }
> + }
> + if ($err) {
> + $self->reply ("501 5.5.2 Syntax: RCPT TO: unknown parameter");
> + } else {
> + $self->reply ('250 2.5.0 OK');
> + }
> next;
> } else {
> $self->reply ("501 5.5.2 Syntax: RCPT TO: <address>");
> diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> index 92c3a7a..4eebfa5 100644
> --- a/src/PMG/Utils.pm
> +++ b/src/PMG/Utils.pm
> @@ -203,7 +203,7 @@ sub subst_values {
> }
>
> sub reinject_mail {
> - my ($entity, $sender, $targets, $xforward, $me, $nodsn) = @_;
> + my ($entity, $sender, $targets, $xforward, $me, $params) = @_;
>
> my $smtp;
> my $resid;
> @@ -244,15 +244,28 @@ sub reinject_mail {
> $mail_opts .= " SMTPUTF8" if $has_utf8_targets;
> }
>
> + if (defined($params->{mail})) {
> + my $mailparams = $params->{mail};
> + for my $p (keys %$mailparams) {
> + $mail_opts .= " $p=$mailparams->{$p}";
> + }
> + }
> + > if (!$smtp->_MAIL("FROM:" . $sender_addr . $mail_opts)) {
> syslog('err', "smtp error - got: %s %s", $smtp->code, scalar ($smtp->message));
> die "smtp from: ERROR";
> }
>
> - my $rcpt_opts = $nodsn ? " NOTIFY=NEVER" : "";
> -
> foreach my $target (@$targets) {
> my $rcpt_addr;
> + my $rcpt_opts = '';
> + if (defined($params->{rcpt}->{$target})) {
> + my $rcptparams = $params->{rcpt}->{$target};
> + for my $p (keys %$rcptparams) {
> + $rcpt_opts .= " $p=$rcptparams->{$p}";
> + }
> + }
> +
> if (utf8::is_utf8($target)) {
> $rcpt_addr = encode('UTF-8', $smtp->_addr($target));
> } else {
> diff --git a/src/bin/pmg-smtp-filter b/src/bin/pmg-smtp-filter
> index b070c8e..45eb125 100755
> --- a/src/bin/pmg-smtp-filter
> +++ b/src/bin/pmg-smtp-filter
> @@ -640,6 +640,7 @@ sub handle_smtp {
> $msginfo->{sender} = $smtp->{from};
> $msginfo->{xforward} = $smtp->{xforward};
> $msginfo->{targets} = $smtp->{to};
> + $msginfo->{param} = $smtp->{param};
>
> my $dkim_sign = $msginfo->{trusted} && $pmg_cfg->get('admin', 'dkim_sign');
> if ($dkim_sign) {
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-11-24 9:33 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-22 19:50 [pmg-devel] [PATCH pmg-api 1/1] fix #2795: add support for DSN Stoiko Ivanov
2021-11-24 9:33 ` Dominik Csapak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox