* [pmg-devel] [PATCH pmg-api 1/4] add helper for parsing SA channel.d files
2020-12-16 17:18 [pmg-devel] [PATCH pmg-api 0/4] add support for locally configured SA channels Stoiko Ivanov
@ 2020-12-16 17:18 ` Stoiko Ivanov
2020-12-30 13:07 ` Fabian Grünbichler
2020-12-16 17:18 ` [pmg-devel] [PATCH pmg-api 2/4] api: spamassassin: read local channels Stoiko Ivanov
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Stoiko Ivanov @ 2020-12-16 17:18 UTC (permalink / raw)
To: pmg-devel
RHEL/CentOS based SpamAssassin implementations ship an update script,
which reads shell snippets from
/etc/mail/spamassassin/channel.d/*.conf and uses the information there
to update SA rules from the configured channels [0].
Noticed the existence of this directory/mechanism while reading the
announcement of the updatechannel for the KAM ruleset [1].
Parsing the file as text, instead of sourcing it in a shell, since I
hope that the channel files distributed don't rely on running commands
to get the ruleset url and gpg key.
[0] https://src.fedoraproject.org/rpms/spamassassin/blob/master/f/sa-update.cronscript
[1] https://mcgrail.com/template/kam.cf_channel
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PMG/Utils.pm | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index d3fae9e..3f5b045 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -1442,5 +1442,32 @@ sub domain_regex {
return $regex;
}
+sub local_spamassassin_channels {
+
+ my $res = [];
+
+ my $local_channel_dir = '/etc/mail/spamassassin/channel.d/';
+ my $read_sa_channel = sub {
+ my ($filename) = @_;
+
+ my $channel_file = $local_channel_dir.$filename;
+ my $content = PVE::Tools::file_get_contents($channel_file);
+ my $channel = {
+ filename => $channel_file,
+ };
+
+ ($channel->{keyid}) = ($content =~ /^KEYID=([a-fA-F0-9]+)$/m);
+ die "no KEYID in $filename!\n" if !defined($channel->{keyid});
+ ($channel->{channelurl}) = ($content =~ /CHANNELURL=(.+)$/m);
+ die "no CHANNELURL in $filename!\n" if !defined($channel->{channelurl});
+ ($channel->{gpgkey}) = ($content =~ /(-----BEGIN PGP PUBLIC KEY BLOCK-----.+-----END PGP PUBLIC KEY BLOCK-----)/s);
+ die "no GPG public key in $filename!\n" if !defined($channel->{gpgkey});
+
+ push(@$res, $channel);
+ };
+
+ PVE::Tools::dir_glob_foreach($local_channel_dir, '.*\.conf', $read_sa_channel);
+ return $res;
+}
1;
--
2.20.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api 1/4] add helper for parsing SA channel.d files
2020-12-16 17:18 ` [pmg-devel] [PATCH pmg-api 1/4] add helper for parsing SA channel.d files Stoiko Ivanov
@ 2020-12-30 13:07 ` Fabian Grünbichler
0 siblings, 0 replies; 10+ messages in thread
From: Fabian Grünbichler @ 2020-12-30 13:07 UTC (permalink / raw)
To: pmg-devel, Stoiko Ivanov
On December 16, 2020 6:18 pm, Stoiko Ivanov wrote:
> RHEL/CentOS based SpamAssassin implementations ship an update script,
> which reads shell snippets from
> /etc/mail/spamassassin/channel.d/*.conf and uses the information there
> to update SA rules from the configured channels [0].
>
> Noticed the existence of this directory/mechanism while reading the
> announcement of the updatechannel for the KAM ruleset [1].
>
> Parsing the file as text, instead of sourcing it in a shell, since I
> hope that the channel files distributed don't rely on running commands
> to get the ruleset url and gpg key.
>
> [0] https://src.fedoraproject.org/rpms/spamassassin/blob/master/f/sa-update.cronscript
> [1] https://mcgrail.com/template/kam.cf_channel
>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> src/PMG/Utils.pm | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> index d3fae9e..3f5b045 100644
> --- a/src/PMG/Utils.pm
> +++ b/src/PMG/Utils.pm
> @@ -1442,5 +1442,32 @@ sub domain_regex {
> return $regex;
> }
>
> +sub local_spamassassin_channels {
> +
> + my $res = [];
> +
> + my $local_channel_dir = '/etc/mail/spamassassin/channel.d/';
> + my $read_sa_channel = sub {
> + my ($filename) = @_;
> +
> + my $channel_file = $local_channel_dir.$filename;
> + my $content = PVE::Tools::file_get_contents($channel_file);
> + my $channel = {
> + filename => $channel_file,
> + };
> +
> + ($channel->{keyid}) = ($content =~ /^KEYID=([a-fA-F0-9]+)$/m);
> + die "no KEYID in $filename!\n" if !defined($channel->{keyid});
> + ($channel->{channelurl}) = ($content =~ /CHANNELURL=(.+)$/m);
should maybe be anchored as well?
> + die "no CHANNELURL in $filename!\n" if !defined($channel->{channelurl});
> + ($channel->{gpgkey}) = ($content =~ /(-----BEGIN PGP PUBLIC KEY BLOCK-----.+-----END PGP PUBLIC KEY BLOCK-----)/s);
could also be anchored:
(^|\n)
(\n|$)
or something similar
> + die "no GPG public key in $filename!\n" if !defined($channel->{gpgkey});
> +
> + push(@$res, $channel);
> + };
> +
> + PVE::Tools::dir_glob_foreach($local_channel_dir, '.*\.conf', $read_sa_channel);
> + return $res;
> +}
>
> 1;
> --
> 2.20.1
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pmg-devel] [PATCH pmg-api 2/4] api: spamassassin: read local channels
2020-12-16 17:18 [pmg-devel] [PATCH pmg-api 0/4] add support for locally configured SA channels Stoiko Ivanov
2020-12-16 17:18 ` [pmg-devel] [PATCH pmg-api 1/4] add helper for parsing SA channel.d files Stoiko Ivanov
@ 2020-12-16 17:18 ` Stoiko Ivanov
2020-12-30 13:07 ` Fabian Grünbichler
2020-12-16 17:18 ` [pmg-devel] [PATCH pmg-api 3/4] api: spamassassin: update " Stoiko Ivanov
2020-12-16 17:18 ` [pmg-devel] [PATCH pmg-api 4/4] pmg-daily: run sa-update for " Stoiko Ivanov
3 siblings, 1 reply; 10+ messages in thread
From: Stoiko Ivanov @ 2020-12-16 17:18 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
the patch of src/PMG/API2/SpamAssassin mostly indents code and I found it
helpful to view it with `git diff -w`
src/PMG/API2/SpamAssassin.pm | 83 +++++++++++++++++++++---------------
1 file changed, 49 insertions(+), 34 deletions(-)
diff --git a/src/PMG/API2/SpamAssassin.pm b/src/PMG/API2/SpamAssassin.pm
index eab02d9..5f9c3a5 100644
--- a/src/PMG/API2/SpamAssassin.pm
+++ b/src/PMG/API2/SpamAssassin.pm
@@ -80,50 +80,65 @@ __PACKAGE__->register_method({
my ($param) = @_;
my $saversion = $Mail::SpamAssassin::VERSION;
- my $channelfile = "/var/lib/spamassassin/$saversion/updates_spamassassin_org.cf";
+ my $sa_update_dir = "/var/lib/spamassassin/$saversion/";
+
+ my $check_channel = sub {
+ my ($channel) = @_;
+
+ # taken from sa-update:
+ my $channel_file_base = $channel;
+ $channel_file_base =~ tr/A-Za-z0-9-/_/cs;
+ my $channelfile = "${sa_update_dir}${channel_file_base}.cf";
+
+ my $mtime = -1;
+ my $version = -1;
+ my $newversion = -1;
+
+ if (-f $channelfile) {
+ # stat metadata cf file
+ $mtime = (stat($channelfile))[9]; # 9 is mtime
+
+ # parse version from metadata cf file
+ my $metadata = PVE::Tools::file_read_firstline($channelfile);
+ if ($metadata =~ m/\s([0-9]+)$/) {
+ $version = $1;
+ } else {
+ warn "invalid metadata in '$channelfile'\n";
+ }
+ }
+ # call sa-update to see if updates are available
- my $mtime = -1;
- my $version = -1;
- my $newversion = -1;
+ my $cmd = "$SAUPDATE -v --checkonly --channel $channel";
+ PVE::Tools::run_command($cmd, noerr => 1, logfunc => sub {
+ my ($line) = @_;
- if (-f $channelfile) {
- # stat metadata cf file
- $mtime = (stat($channelfile))[9]; # 9 is mtime
+ if ($line =~ m/Update available for channel \S+: -?[0-9]+ -> ([0-9]+)/) {
+ $newversion = $1;
+ }
+ });
- # parse version from metadata cf file
- my $metadata = PVE::Tools::file_read_firstline($channelfile);
- if ($metadata =~ m/\s([0-9]+)$/) {
- $version = $1;
- } else {
- warn "invalid metadata in '$channelfile'\n";
- }
- }
- # call sa-update to see if updates are available
+ my $result = {
+ channel => $channel,
+ };
- my $cmd = "$SAUPDATE -v --checkonly";
- PVE::Tools::run_command($cmd, noerr => 1, logfunc => sub {
- my ($line) = @_;
+ $result->{version} = $version if $version > -1;
+ $result->{update_version} = $newversion if $newversion > -1;
+ $result->{last_updated} = $mtime if $mtime > -1;
- if ($line =~ m/Update available for channel \S+: -?[0-9]+ -> ([0-9]+)/) {
- $newversion = $1;
+ if ($newversion > $version) {
+ $result->{update_avail} = 1;
+ } else {
+ $result->{update_avail} = 0;
}
- });
-
- my $result = {
- channel => 'updates.spamassassin.org',
+ return $result;
};
- $result->{version} = $version if $version > -1;
- $result->{update_version} = $newversion if $newversion > -1;
- $result->{last_updated} = $mtime if $mtime > -1;
+ my @channels = ('updates.spamassassin.org');
- if ($newversion > $version) {
- $result->{update_avail} = 1;
- } else {
- $result->{update_avail} = 0;
- }
+ my $localchannels = PMG::Utils::local_spamassassin_channels();
+ push(@channels, map { $_->{channelurl} } @$localchannels);
- return [$result];
+ return [ map { $check_channel->($_) } @channels];
}});
__PACKAGE__->register_method({
--
2.20.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api 2/4] api: spamassassin: read local channels
2020-12-16 17:18 ` [pmg-devel] [PATCH pmg-api 2/4] api: spamassassin: read local channels Stoiko Ivanov
@ 2020-12-30 13:07 ` Fabian Grünbichler
0 siblings, 0 replies; 10+ messages in thread
From: Fabian Grünbichler @ 2020-12-30 13:07 UTC (permalink / raw)
To: pmg-devel, Stoiko Ivanov
On December 16, 2020 6:18 pm, Stoiko Ivanov wrote:
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> the patch of src/PMG/API2/SpamAssassin mostly indents code and I found it
> helpful to view it with `git diff -w`
>
> src/PMG/API2/SpamAssassin.pm | 83 +++++++++++++++++++++---------------
> 1 file changed, 49 insertions(+), 34 deletions(-)
>
> diff --git a/src/PMG/API2/SpamAssassin.pm b/src/PMG/API2/SpamAssassin.pm
> index eab02d9..5f9c3a5 100644
> --- a/src/PMG/API2/SpamAssassin.pm
> +++ b/src/PMG/API2/SpamAssassin.pm
> @@ -80,50 +80,65 @@ __PACKAGE__->register_method({
> my ($param) = @_;
>
> my $saversion = $Mail::SpamAssassin::VERSION;
> - my $channelfile = "/var/lib/spamassassin/$saversion/updates_spamassassin_org.cf";
> + my $sa_update_dir = "/var/lib/spamassassin/$saversion/";
> +
> + my $check_channel = sub {
> + my ($channel) = @_;
> +
> + # taken from sa-update:
> + my $channel_file_base = $channel;
> + $channel_file_base =~ tr/A-Za-z0-9-/_/cs;
this is syntax we don't use anywhere else, so maybe add a comment or
rewrite it using a regular replacement?
> + my $channelfile = "${sa_update_dir}${channel_file_base}.cf";
> +
> + my $mtime = -1;
> + my $version = -1;
> + my $newversion = -1;
> +
> + if (-f $channelfile) {
> + # stat metadata cf file
> + $mtime = (stat($channelfile))[9]; # 9 is mtime
> +
> + # parse version from metadata cf file
> + my $metadata = PVE::Tools::file_read_firstline($channelfile);
> + if ($metadata =~ m/\s([0-9]+)$/) {
> + $version = $1;
> + } else {
> + warn "invalid metadata in '$channelfile'\n";
> + }
> + }
> + # call sa-update to see if updates are available
>
> - my $mtime = -1;
> - my $version = -1;
> - my $newversion = -1;
> + my $cmd = "$SAUPDATE -v --checkonly --channel $channel";
> + PVE::Tools::run_command($cmd, noerr => 1, logfunc => sub {
> + my ($line) = @_;
>
> - if (-f $channelfile) {
> - # stat metadata cf file
> - $mtime = (stat($channelfile))[9]; # 9 is mtime
> + if ($line =~ m/Update available for channel \S+: -?[0-9]+ -> ([0-9]+)/) {
> + $newversion = $1;
> + }
> + });
>
> - # parse version from metadata cf file
> - my $metadata = PVE::Tools::file_read_firstline($channelfile);
> - if ($metadata =~ m/\s([0-9]+)$/) {
> - $version = $1;
> - } else {
> - warn "invalid metadata in '$channelfile'\n";
> - }
> - }
> - # call sa-update to see if updates are available
> + my $result = {
> + channel => $channel,
> + };
>
> - my $cmd = "$SAUPDATE -v --checkonly";
> - PVE::Tools::run_command($cmd, noerr => 1, logfunc => sub {
> - my ($line) = @_;
> + $result->{version} = $version if $version > -1;
> + $result->{update_version} = $newversion if $newversion > -1;
> + $result->{last_updated} = $mtime if $mtime > -1;
>
> - if ($line =~ m/Update available for channel \S+: -?[0-9]+ -> ([0-9]+)/) {
> - $newversion = $1;
> + if ($newversion > $version) {
> + $result->{update_avail} = 1;
> + } else {
> + $result->{update_avail} = 0;
> }
> - });
> -
> - my $result = {
> - channel => 'updates.spamassassin.org',
> + return $result;
> };
>
> - $result->{version} = $version if $version > -1;
> - $result->{update_version} = $newversion if $newversion > -1;
> - $result->{last_updated} = $mtime if $mtime > -1;
> + my @channels = ('updates.spamassassin.org');
>
> - if ($newversion > $version) {
> - $result->{update_avail} = 1;
> - } else {
> - $result->{update_avail} = 0;
> - }
> + my $localchannels = PMG::Utils::local_spamassassin_channels();
> + push(@channels, map { $_->{channelurl} } @$localchannels);
>
> - return [$result];
> + return [ map { $check_channel->($_) } @channels];
> }});
>
> __PACKAGE__->register_method({
> --
> 2.20.1
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pmg-devel] [PATCH pmg-api 3/4] api: spamassassin: update local channels
2020-12-16 17:18 [pmg-devel] [PATCH pmg-api 0/4] add support for locally configured SA channels Stoiko Ivanov
2020-12-16 17:18 ` [pmg-devel] [PATCH pmg-api 1/4] add helper for parsing SA channel.d files Stoiko Ivanov
2020-12-16 17:18 ` [pmg-devel] [PATCH pmg-api 2/4] api: spamassassin: read local channels Stoiko Ivanov
@ 2020-12-16 17:18 ` Stoiko Ivanov
2020-12-30 13:07 ` Fabian Grünbichler
2020-12-16 17:18 ` [pmg-devel] [PATCH pmg-api 4/4] pmg-daily: run sa-update for " Stoiko Ivanov
3 siblings, 1 reply; 10+ messages in thread
From: Stoiko Ivanov @ 2020-12-16 17:18 UTC (permalink / raw)
To: pmg-devel
This patch adds a helper to loop over all present Spamassassin
channels files in /etc/mail/spamassassin/channel.d and:
* import the included gpg key into sa-update's keyring
* run sa-update for each channel separately
the verbose argument of the helper is for reusing the code in
pmg-daily (where we only want to log errors and be less informative)
In order to only hardcode the path of sa-update once the definition
was moved to PMG::Utils.
The choice of invoking sa-update for each channel separately is based,
instead of providing multiple '--channel' and '--gpgkey' options to
a single command was made to prevent downloading signatures, which
were signed by a key not configured for the channel.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PMG/API2/SpamAssassin.pm | 6 +++---
src/PMG/Utils.pm | 29 +++++++++++++++++++++++++++++
2 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/src/PMG/API2/SpamAssassin.pm b/src/PMG/API2/SpamAssassin.pm
index 5f9c3a5..df46c64 100644
--- a/src/PMG/API2/SpamAssassin.pm
+++ b/src/PMG/API2/SpamAssassin.pm
@@ -11,15 +11,13 @@ use PVE::RESTHandler;
use PMG::RESTEnvironment;
use PVE::JSONSchema qw(get_standard_option);
-use PMG::Utils;
+use PMG::Utils qw($SAUPDATE);
use PMG::Config;
use Mail::SpamAssassin;
use base qw(PVE::RESTHandler);
-my $SAUPDATE = '/usr/bin/sa-update';
-
__PACKAGE__->register_method ({
name => 'index',
path => '',
@@ -174,6 +172,8 @@ __PACKAGE__->register_method({
my $cmd = "$SAUPDATE -v";
PVE::Tools::run_command($cmd, noerr => 1);
+
+ PMG::Utils::update_local_spamassassin_channels(1);
};
return $rpcenv->fork_worker('saupdate', undef, $authuser, $realcmd);
diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index 3f5b045..de74d4e 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -44,6 +44,7 @@ use base 'Exporter';
our @EXPORT_OK = qw(
postgres_admin_cmd
+$SAUPDATE
);
my $valid_pmg_realms = ['pam', 'pmg', 'quarantine'];
@@ -1442,6 +1443,8 @@ sub domain_regex {
return $regex;
}
+our $SAUPDATE = '/usr/bin/sa-update';
+
sub local_spamassassin_channels {
my $res = [];
@@ -1470,4 +1473,30 @@ sub local_spamassassin_channels {
return $res;
}
+sub update_local_spamassassin_channels {
+ my ($verbose) = @_;
+ # import all configured channel's gpg-keys to sa-update's keyring
+ my $importcmd = "$SAUPDATE";
+ $importcmd .= '-v' if $verbose;
+ my $localchannels = PMG::Utils::local_spamassassin_channels();
+ for my $channel (@$localchannels) {
+ $importcmd .= " --import $channel->{filename}";
+ }
+ print "Importing gpg files from locally available channel definitions\n" if $verbose;
+ PVE::Tools::run_command($importcmd, noerr => 1);
+
+ my $fresh_updates = 0;
+
+ for my $channel (@$localchannels) {
+ my $cmd = "$SAUPDATE -v --channel $channel->{channelurl} --gpgkey $channel->{keyid}";
+ print "Updating $channel->{channelurl}\n" if $verbose;
+ my $ret = PVE::Tools::run_command($cmd, noerr => 1);
+ die "updating $channel->{channelurl} failed - sa-update exited with $ret\n" if $ret >= 2;
+
+ $fresh_updates = 1 if $ret == 0;
+ }
+
+ return $fresh_updates
+}
+
1;
--
2.20.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api 3/4] api: spamassassin: update local channels
2020-12-16 17:18 ` [pmg-devel] [PATCH pmg-api 3/4] api: spamassassin: update " Stoiko Ivanov
@ 2020-12-30 13:07 ` Fabian Grünbichler
2020-12-30 16:11 ` Stoiko Ivanov
0 siblings, 1 reply; 10+ messages in thread
From: Fabian Grünbichler @ 2020-12-30 13:07 UTC (permalink / raw)
To: pmg-devel, Stoiko Ivanov
On December 16, 2020 6:18 pm, Stoiko Ivanov wrote:
> This patch adds a helper to loop over all present Spamassassin
> channels files in /etc/mail/spamassassin/channel.d and:
> * import the included gpg key into sa-update's keyring
> * run sa-update for each channel separately
>
> the verbose argument of the helper is for reusing the code in
> pmg-daily (where we only want to log errors and be less informative)
>
> In order to only hardcode the path of sa-update once the definition
> was moved to PMG::Utils.
>
> The choice of invoking sa-update for each channel separately is based,
> instead of providing multiple '--channel' and '--gpgkey' options to
> a single command was made to prevent downloading signatures, which
> were signed by a key not configured for the channel.
>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> src/PMG/API2/SpamAssassin.pm | 6 +++---
> src/PMG/Utils.pm | 29 +++++++++++++++++++++++++++++
> 2 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/src/PMG/API2/SpamAssassin.pm b/src/PMG/API2/SpamAssassin.pm
> index 5f9c3a5..df46c64 100644
> --- a/src/PMG/API2/SpamAssassin.pm
> +++ b/src/PMG/API2/SpamAssassin.pm
> @@ -11,15 +11,13 @@ use PVE::RESTHandler;
> use PMG::RESTEnvironment;
> use PVE::JSONSchema qw(get_standard_option);
>
> -use PMG::Utils;
> +use PMG::Utils qw($SAUPDATE);
> use PMG::Config;
>
> use Mail::SpamAssassin;
>
> use base qw(PVE::RESTHandler);
>
> -my $SAUPDATE = '/usr/bin/sa-update';
> -
> __PACKAGE__->register_method ({
> name => 'index',
> path => '',
> @@ -174,6 +172,8 @@ __PACKAGE__->register_method({
> my $cmd = "$SAUPDATE -v";
>
> PVE::Tools::run_command($cmd, noerr => 1);
> +
> + PMG::Utils::update_local_spamassassin_channels(1);
> };
>
> return $rpcenv->fork_worker('saupdate', undef, $authuser, $realcmd);
> diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> index 3f5b045..de74d4e 100644
> --- a/src/PMG/Utils.pm
> +++ b/src/PMG/Utils.pm
> @@ -44,6 +44,7 @@ use base 'Exporter';
>
> our @EXPORT_OK = qw(
> postgres_admin_cmd
> +$SAUPDATE
> );
>
> my $valid_pmg_realms = ['pam', 'pmg', 'quarantine'];
> @@ -1442,6 +1443,8 @@ sub domain_regex {
> return $regex;
> }
>
> +our $SAUPDATE = '/usr/bin/sa-update';
> +
> sub local_spamassassin_channels {
>
> my $res = [];
> @@ -1470,4 +1473,30 @@ sub local_spamassassin_channels {
> return $res;
> }
>
> +sub update_local_spamassassin_channels {
> + my ($verbose) = @_;
> + # import all configured channel's gpg-keys to sa-update's keyring
> + my $importcmd = "$SAUPDATE";
> + $importcmd .= '-v' if $verbose;
> + my $localchannels = PMG::Utils::local_spamassassin_channels();
> + for my $channel (@$localchannels) {
> + $importcmd .= " --import $channel->{filename}";
> + }
> + print "Importing gpg files from locally available channel definitions\n" if $verbose;
> + PVE::Tools::run_command($importcmd, noerr => 1);
why is $importcmd not an array/list? how does the import behave if one
of X channel files is bad/corrupt/malformed/...? wouldn't it make sense
to import + update each channel on their own?
is the noerr needed?
> + my $fresh_updates = 0;
> +
> + for my $channel (@$localchannels) {
> + my $cmd = "$SAUPDATE -v --channel $channel->{channelurl} --gpgkey $channel->{keyid}";
> + print "Updating $channel->{channelurl}\n" if $verbose;
> + my $ret = PVE::Tools::run_command($cmd, noerr => 1);
$cmd should also be a list..
> + die "updating $channel->{channelurl} failed - sa-update exited with $ret\n" if $ret >= 2;
> +
> + $fresh_updates = 1 if $ret == 0;
> + }
> +
> + return $fresh_updates
> +}
> +
> 1;
> --
> 2.20.1
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api 3/4] api: spamassassin: update local channels
2020-12-30 13:07 ` Fabian Grünbichler
@ 2020-12-30 16:11 ` Stoiko Ivanov
0 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2020-12-30 16:11 UTC (permalink / raw)
To: Fabian Grünbichler; +Cc: pmg-devel
On Wed, 30 Dec 2020 14:07:22 +0100
Fabian Grünbichler <f.gruenbichler@proxmox.com> wrote:
> On December 16, 2020 6:18 pm, Stoiko Ivanov wrote:
> > This patch adds a helper to loop over all present Spamassassin
> > channels files in /etc/mail/spamassassin/channel.d and:
> > * import the included gpg key into sa-update's keyring
> > * run sa-update for each channel separately
> >
> > the verbose argument of the helper is for reusing the code in
> > pmg-daily (where we only want to log errors and be less informative)
> >
> > In order to only hardcode the path of sa-update once the definition
> > was moved to PMG::Utils.
> >
> > The choice of invoking sa-update for each channel separately is based,
> > instead of providing multiple '--channel' and '--gpgkey' options to
> > a single command was made to prevent downloading signatures, which
> > were signed by a key not configured for the channel.
> >
> > Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> > ---
> > src/PMG/API2/SpamAssassin.pm | 6 +++---
> > src/PMG/Utils.pm | 29 +++++++++++++++++++++++++++++
> > 2 files changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/PMG/API2/SpamAssassin.pm b/src/PMG/API2/SpamAssassin.pm
> > index 5f9c3a5..df46c64 100644
> > --- a/src/PMG/API2/SpamAssassin.pm
> > +++ b/src/PMG/API2/SpamAssassin.pm
> > @@ -11,15 +11,13 @@ use PVE::RESTHandler;
> > use PMG::RESTEnvironment;
> > use PVE::JSONSchema qw(get_standard_option);
> >
> > -use PMG::Utils;
> > +use PMG::Utils qw($SAUPDATE);
> > use PMG::Config;
> >
> > use Mail::SpamAssassin;
> >
> > use base qw(PVE::RESTHandler);
> >
> > -my $SAUPDATE = '/usr/bin/sa-update';
> > -
> > __PACKAGE__->register_method ({
> > name => 'index',
> > path => '',
> > @@ -174,6 +172,8 @@ __PACKAGE__->register_method({
> > my $cmd = "$SAUPDATE -v";
> >
> > PVE::Tools::run_command($cmd, noerr => 1);
> > +
> > + PMG::Utils::update_local_spamassassin_channels(1);
> > };
> >
> > return $rpcenv->fork_worker('saupdate', undef, $authuser, $realcmd);
> > diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> > index 3f5b045..de74d4e 100644
> > --- a/src/PMG/Utils.pm
> > +++ b/src/PMG/Utils.pm
> > @@ -44,6 +44,7 @@ use base 'Exporter';
> >
> > our @EXPORT_OK = qw(
> > postgres_admin_cmd
> > +$SAUPDATE
> > );
> >
> > my $valid_pmg_realms = ['pam', 'pmg', 'quarantine'];
> > @@ -1442,6 +1443,8 @@ sub domain_regex {
> > return $regex;
> > }
> >
> > +our $SAUPDATE = '/usr/bin/sa-update';
> > +
> > sub local_spamassassin_channels {
> >
> > my $res = [];
> > @@ -1470,4 +1473,30 @@ sub local_spamassassin_channels {
> > return $res;
> > }
> >
> > +sub update_local_spamassassin_channels {
> > + my ($verbose) = @_;
> > + # import all configured channel's gpg-keys to sa-update's keyring
> > + my $importcmd = "$SAUPDATE";
> > + $importcmd .= '-v' if $verbose;
> > + my $localchannels = PMG::Utils::local_spamassassin_channels();
> > + for my $channel (@$localchannels) {
> > + $importcmd .= " --import $channel->{filename}";
> > + }
> > + print "Importing gpg files from locally available channel definitions\n" if $verbose;
> > + PVE::Tools::run_command($importcmd, noerr => 1);
>
> why is $importcmd not an array/list? how does the import behave if one
> of X channel files is bad/corrupt/malformed/...? wouldn't it make sense
> to import + update each channel on their own?
Great catch - Thanks!
While the man-page of sa-update(1p) explictly states that it is supported
to import multiple keys at once with multiple '--import' options - the
Getopt::Long usage in sa-update does not - it simply imports the last
provided on the command line.
if the file is malformed - sa-updates exits with 2 (and gpg's error output)
-> will change it to run sa-update --import for each channel separately
(and provide the arguments as list).
>
> is the noerr needed?
in this case it's wrong (I copied from the sa-update invocation below,
where it's needed) - sa-update exits quite directly after calling `gpg
--import` - with gpg's exit status.
>
> > + my $fresh_updates = 0;
> > +
> > + for my $channel (@$localchannels) {
> > + my $cmd = "$SAUPDATE -v --channel $channel->{channelurl} --gpgkey $channel->{keyid}";
> > + print "Updating $channel->{channelurl}\n" if $verbose;
> > + my $ret = PVE::Tools::run_command($cmd, noerr => 1);
>
> $cmd should also be a list..
>
> > + die "updating $channel->{channelurl} failed - sa-update exited with $ret\n" if $ret >= 2;
> > +
> > + $fresh_updates = 1 if $ret == 0;
> > + }
> > +
> > + return $fresh_updates
> > +}
> > +
> > 1;
> > --
> > 2.20.1
> >
> >
> >
> > _______________________________________________
> > pmg-devel mailing list
> > pmg-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
> >
> >
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pmg-devel] [PATCH pmg-api 4/4] pmg-daily: run sa-update for local channels
2020-12-16 17:18 [pmg-devel] [PATCH pmg-api 0/4] add support for locally configured SA channels Stoiko Ivanov
` (2 preceding siblings ...)
2020-12-16 17:18 ` [pmg-devel] [PATCH pmg-api 3/4] api: spamassassin: update " Stoiko Ivanov
@ 2020-12-16 17:18 ` Stoiko Ivanov
2020-12-30 13:07 ` Fabian Grünbichler
3 siblings, 1 reply; 10+ messages in thread
From: Stoiko Ivanov @ 2020-12-16 17:18 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/bin/pmg-daily | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/bin/pmg-daily b/src/bin/pmg-daily
index 32ccb95..f5b0650 100755
--- a/src/bin/pmg-daily
+++ b/src/bin/pmg-daily
@@ -73,12 +73,21 @@ if (my $http_proxy = $cfg->get('admin', 'http_proxy')) {
}
# update spamassassin rules
+my $restart_filter = 0;
if (system('sa-update') == 0) {
# if the exit code is 0, new updates were downloaded
# then restart the pmg-smtp-filter to load the new rules
- PMG::Utils::service_cmd('pmg-smtp-filter', 'restart');
+ $restart_filter = 1;
}
+eval {
+ $restart_filter ||= PMG::Utils::update_local_spamassassin_channels(0);
+};
+if (my $err = $@) {
+ syslog('err', $err);
+}
+
+PMG::Utils::service_cmd('pmg-smtp-filter', 'restart') if $restart_filter;
# run bayes database maintainance
system('sa-learn --force-expire >/dev/null 2>&1');
--
2.20.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api 4/4] pmg-daily: run sa-update for local channels
2020-12-16 17:18 ` [pmg-devel] [PATCH pmg-api 4/4] pmg-daily: run sa-update for " Stoiko Ivanov
@ 2020-12-30 13:07 ` Fabian Grünbichler
0 siblings, 0 replies; 10+ messages in thread
From: Fabian Grünbichler @ 2020-12-30 13:07 UTC (permalink / raw)
To: pmg-devel, Stoiko Ivanov
On December 16, 2020 6:18 pm, Stoiko Ivanov wrote:
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> src/bin/pmg-daily | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/src/bin/pmg-daily b/src/bin/pmg-daily
> index 32ccb95..f5b0650 100755
> --- a/src/bin/pmg-daily
> +++ b/src/bin/pmg-daily
> @@ -73,12 +73,21 @@ if (my $http_proxy = $cfg->get('admin', 'http_proxy')) {
> }
>
> # update spamassassin rules
> +my $restart_filter = 0;
> if (system('sa-update') == 0) {
> # if the exit code is 0, new updates were downloaded
> # then restart the pmg-smtp-filter to load the new rules
> - PMG::Utils::service_cmd('pmg-smtp-filter', 'restart');
> + $restart_filter = 1;
> }
>
> +eval {
> + $restart_filter ||= PMG::Utils::update_local_spamassassin_channels(0);
> +};
> +if (my $err = $@) {
> + syslog('err', $err);
> +}
could just be
syslog(..) if $@
> +
> +PMG::Utils::service_cmd('pmg-smtp-filter', 'restart') if $restart_filter;
> # run bayes database maintainance
> system('sa-learn --force-expire >/dev/null 2>&1');
>
> --
> 2.20.1
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread