[pmg-devel] [PATCH pmg-api 2/3] add SACustom Package and API Calls for custom SpamAssassin scores

Stoiko Ivanov s.ivanov at proxmox.com
Wed Nov 13 15:42:24 CET 2019


On Fri,  8 Nov 2019 14:29:30 +0100
Dominik Csapak <d.csapak at proxmox.com> wrote:

> this uses our INotify interface to parse and write a custom sa config
> in /etc/mail/spamassassin/pmg-scores.cf with a shadow file in
> /var/cache/pmg-scores.cf (to track the diff)
> 
> add also api calls to create a new/delete/edit/revert/apply those custom
> rules
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  src/Makefile             |   2 +
>  src/PMG/API2/Config.pm   |   6 +
>  src/PMG/API2/SACustom.pm | 334 +++++++++++++++++++++++++++++++++++++++
>  src/PMG/SACustom.pm      |  89 +++++++++++
>  4 files changed, 431 insertions(+)
>  create mode 100644 src/PMG/API2/SACustom.pm
>  create mode 100644 src/PMG/SACustom.pm
> 
> diff --git a/src/Makefile b/src/Makefile
> index 89658db..e3bee39 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -77,6 +77,7 @@ LIBSOURCES =				\
>  	PMG/DKIMSign.pm			\
>  	PMG/Quarantine.pm		\
>  	PMG/Report.pm			\
> +	PMG/SACustom.pm			\
>  	PMG/RuleDB/Group.pm		\
>  	PMG/RuleDB/Rule.pm		\
>  	PMG/RuleDB/Object.pm		\
> @@ -130,6 +131,7 @@ LIBSOURCES =				\
>  	PMG/API2/Cluster.pm		\
>  	PMG/API2/ClamAV.pm		\
>  	PMG/API2/SpamAssassin.pm	\
> +	PMG/API2/SACustom.pm		\
>  	PMG/API2/Statistics.pm		\
>  	PMG/API2/MailTracker.pm		\
>  	PMG/API2/Backup.pm		\
> diff --git a/src/PMG/API2/Config.pm b/src/PMG/API2/Config.pm
> index 43653e4..1b3743e 100644
> --- a/src/PMG/API2/Config.pm
> +++ b/src/PMG/API2/Config.pm
> @@ -24,6 +24,7 @@ use PMG::API2::MimeTypes;
>  use PMG::API2::Fetchmail;
>  use PMG::API2::DestinationTLSPolicy;
>  use PMG::API2::DKIMSign;
> +use PMG::API2::SACustom;
>  
>  use base qw(PVE::RESTHandler);
>  
> @@ -87,6 +88,11 @@ __PACKAGE__->register_method({
>      path => 'dkim',
>  });
>  
> +__PACKAGE__->register_method({
> +    subclass => "PMG::API2::SACustom",
> +    path => 'customscores',
> +});
> +
>  __PACKAGE__->register_method ({
>      name => 'index', 
>      path => '',
> diff --git a/src/PMG/API2/SACustom.pm b/src/PMG/API2/SACustom.pm
> new file mode 100644
> index 0000000..f9ad67a
> --- /dev/null
> +++ b/src/PMG/API2/SACustom.pm
> @@ -0,0 +1,334 @@
> +package PMG::API2::SACustom;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::SafeSyslog;
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::RESTHandler;
> +use PVE::INotify;
> +use PVE::Tools qw(extract_param);
> +use PVE::Exception qw(raise_param_exc);
> +
> +use PMG::RESTEnvironment;
> +use PMG::Utils;
> +use PMG::SACustom;
> +
> +use base qw(PVE::RESTHandler);
> +
> +my $score_properties = {
> +    name => {
> +	type => 'string',
> +	description => "The name of the rule.",
> +	pattern => '[a-zA-Z\_\-\.0-9]+',
> +    },
> +    score => {
> +	type => 'number',
> +	description => "The score the rule should be valued at.",
> +    },
> +    comment => {
> +	type => 'string',
> +	description => 'The Comment.',
> +	optional => 1,
> +    },
> +};
> +
> +sub json_config_properties {
> +    my ($prop, $optional) = @_;
> +
> +    foreach my $opt (keys %$score_properties) {
> +	$prop->{$opt} = $score_properties->{$opt};
> +	if ($optional->{$opt}) {
> +	    $prop->{$opt}->{optional} = 1;
> +	}
> +    }
> +
> +    return $prop;
> +}
> +
Here $prop contains copies of the hashref in $score_properties, and the
update of the 'optional' key persists after the first invocation.

The following worked in my test for fixing this (although the readability
might be debatable):
```
map { $prop->{$_} = { %{$score_properties->{$_}}}  } keys %$score_properties;
map { $prop->{$_}->{optional} = 1 } keys %$optional;
```



> +__PACKAGE__->register_method({
> +    name => 'list_scores',
> +    path => '',
> +    method => 'GET',
> +    description => "List custom scores.",
> +    #    protected => 1,
> +    permissions => { check => [ 'admin', 'audit' ] },
> +    proxyto => 'master',
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => { },
> +    },
> +    returns => {
> +	type => 'array',
> +	items => {
> +	    type => 'object',
> +	    properties => json_config_properties({
> +		digest => get_standard_option('pve-config-digest'),
> +	    },
> +	    {
> +		# mark all properties optional, so that we can have
> +		# one entry with only digest, and all others without digest
> +		name => 1,
> +		score => 1,
> +		comment => 1,
> +	    }),
> +	},
> +	links => [ { rel => 'child', href => "{name}" } ],
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $restenv = PMG::RESTEnvironment->get();
> +
> +	my $tmp = PVE::INotify::read_file('pmg-scores.cf', 1);
> +
> +	my $changes = $tmp->{changes};
> +	$restenv->set_result_attrib('changes', $changes) if $changes;
> +
> +	my $res = [];
> +
> +	for my $rule (sort keys %{$tmp->{data}}) {
> +	    push @$res, $tmp->{data}->{$rule};
> +	}
> +
> +	my $digest = PMG::SACustom::calc_digest($tmp->{data});
> +
> +	push @$res, {
> +	    digest => $digest,
> +	};
> +
> +	return $res;
> +    }});
> +
> +__PACKAGE__->register_method({
> +    name => 'apply_score_changes',
> +    path => '',
> +    method => 'PUT',
> +    protected => 1,
> +    description => "Apply custom score changes.",
> +    proxyto => 'master',
> +    permissions => { check => [ 'admin' ] },
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    'restart-daemon' => {
> +		type => 'boolean',
> +		description => 'If set, also restarts pmg-smtp-filter. '.
> +			       'This is necessary for the changes to work.',
> +		default => 0,
> +		optional => 1,
> +	    },
> +	    digest => get_standard_option('pve-config-digest'),
> +	},
> +    },
> +    returns => { type => "string" },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $restenv = PMG::RESTEnvironment->get();
> +
> +	my $user = $restenv->get_user();
> +
> +	my $config = PVE::INotify::read_file('pmg-scores.cf');
> +
> +	my $digest = PMG::SACustom::calc_digest($config);
> +	PVE::Tools::assert_if_modified($digest, $param->{digest})
> +	    if $param->{digest};
> +
> +	my $realcmd = sub {
> +	    my $upid = shift;
> +
> +	    PMG::SACustom::apply_changes();
> +	    if ($param->{'restart-daemon'}) {
> +		syslog('info', "re-starting service pmg-smtp-filter: $upid\n");
> +		PMG::Utils::service_cmd('pmg-smtp-filter', 'restart');
> +	    }
> +	};
> +
> +	return $restenv->fork_worker('applycustomscores', undef, $user, $realcmd);
> +    }});
> +
> +__PACKAGE__->register_method({
> +    name => 'revert_score_changes',
> +    path => '',
> +    method => 'DELETE',
> +    protected => 1,
> +    description => "Revert custom score changes.",
> +    proxyto => 'master',
> +    permissions => { check => [ 'admin' ] },
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => { },
> +    },
> +    returns => { type => "null" },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	unlink PMG::SACustom::get_shadow_path();
> +
> +	return undef;
> +    }});
> +
> +
> +__PACKAGE__->register_method({
> +    name => 'create_score',
> +    path => '',
> +    method => 'POST',
> +    description => "Create custom SpamAssassin score",
> +    protected => 1,
> +    proxyto => 'master',
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => json_config_properties({
> +	    digest => get_standard_option('pve-config-digest'),
> +	}),
> +    },
> +    returns => { type => 'null' },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $name = extract_param($param, 'name');
> +	my $score = extract_param($param, 'score');
> +	my $comment = extract_param($param, 'comment');
> +
> +	my $code = sub {
> +	    my $config = PVE::INotify::read_file('pmg-scores.cf');
> +
> +	    my $digest = PMG::SACustom::calc_digest($config);
> +	    PVE::Tools::assert_if_modified($digest, $param->{digest})
> +		if $param->{digest};
> +
> +	    $config->{$name} = {
> +		name => $name,
> +		score => $score,
> +		comment => $comment,
> +	    };
> +
> +	    PVE::INotify::write_file('pmg-scores.cf', $config);
> +	};
> +
> +	PVE::Tools::lock_file("/var/lock/pmg-scores.cf.lck", 10, $code);
> +	die $@ if $@;
> +
> +	return undef;
> +    }});
> +
> +__PACKAGE__->register_method({
> +    name => 'get_score',
> +    path => '{name}',
> +    method => 'GET',
> +    description => "Get custom SpamAssassin score",
> +    protected => 1,
> +    proxyto => 'master',
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    name => {
> +		type => 'string',
> +		description => "The name of the rule.",
> +		pattern => '[a-zA-Z\_\-\.0-9]+',
> +	    },
> +	},
> +    },
> +    returns => {
> +	type => 'object',
> +	properties => json_config_properties(),
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $name = extract_param($param, 'name');
> +	my $config = PVE::INotify::read_file('pmg-scores.cf');
> +
> +	raise_param_exc({ name => "$name not found" })
> +	    if !$config->{$name};
> +
> +	return $config->{$name};
> +    }});
> +
> +__PACKAGE__->register_method({
> +    name => 'edit_score',
> +    path => '{name}',
> +    method => 'PUT',
> +    description => "Edit custom SpamAssassin score",
> +    protected => 1,
> +    proxyto => 'master',
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => json_config_properties({
> +	    digest => get_standard_option('pve-config-digest'),
> +	}),
> +    },
> +    returns => { type => 'null' },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $name = extract_param($param, 'name');
> +	my $score = extract_param($param, 'score');
> +	my $comment = extract_param($param, 'comment');
> +
> +	my $code = sub {
> +	    my $config = PVE::INotify::read_file('pmg-scores.cf');
> +
> +	    my $digest = PMG::SACustom::calc_digest($config);
> +	    PVE::Tools::assert_if_modified($digest, $param->{digest})
> +		if $param->{digest};
> +
> +	    $config->{$name} = {
> +		name => $name,
> +		score => $score,
> +		comment => $comment,
> +	    };
> +
> +	    PVE::INotify::write_file('pmg-scores.cf', $config);
> +	};
> +
> +	PVE::Tools::lock_file("/var/lock/pmg-scores.cf.lck", 10, $code);
> +	die $@ if $@;
> +
> +	return undef;
> +    }});
> +
> +__PACKAGE__->register_method({
> +    name => 'delete_score',
> +    path => '{name}',
> +    method => 'DELETE',
> +    description => "Edit custom SpamAssassin score",
> +    protected => 1,
> +    proxyto => 'master',
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    name => {
> +		type => 'string',
> +		description => "The name of the rule.",
> +		pattern => '[a-zA-Z\_\-\.0-9]+',
> +	    },
> +	    digest => get_standard_option('pve-config-digest'),
> +	},
> +    },
> +    returns => { type => 'null' },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $name = extract_param($param, 'name');
> +
> +	my $code = sub {
> +	    my $config = PVE::INotify::read_file('pmg-scores.cf');
> +
> +	    my $digest = PMG::SACustom::calc_digest($config);
> +	    PVE::Tools::assert_if_modified($digest, $param->{digest})
> +		if $param->{digest};
> +
> +	    delete $config->{$name};
> +
> +	    PVE::INotify::write_file('pmg-scores.cf', $config);
> +	};
> +
> +	PVE::Tools::lock_file("/var/lock/pmg-scores.cf.lck", 10, $code);
> +	die $@ if $@;
> +
> +	return undef;
> +    }});
> +
> +1;
> diff --git a/src/PMG/SACustom.pm b/src/PMG/SACustom.pm
> new file mode 100644
> index 0000000..b7a8cc9
> --- /dev/null
> +++ b/src/PMG/SACustom.pm
> @@ -0,0 +1,89 @@
> +package PMG::SACustom;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::INotify;
> +use Digest::SHA;
> +
> +my $shadow_path = "/var/cache/pmg-scores.cf";
> +my $conf_path = "/etc/mail/spamassassin/pmg-scores.cf";
> +
> +sub get_shadow_path {
> +    return $shadow_path;
> +}
> +
> +sub apply_changes {
> +    rename($shadow_path, $conf_path) if -f $shadow_path;
> +}
> +
> +sub calc_digest {
> +    my ($data) = @_;
> +
> +    my $raw = '';
> +
> +    foreach my $rule (sort keys %$data) {
> +	my $score = $data->{$rule}->{score};
> +	my $comment = $data->{$rule}->{comment} // "";
> +	$raw .= "$rule$score$comment";
> +    }
> +
> +    my $digest = Digest::SHA::sha1_hex($raw);
> +    return $digest;
> +}
> +
> +PVE::INotify::register_file('pmg-scores.cf', $conf_path,
> +			    \&read_pmg_cf,
> +			    \&write_pmg_cf,
> +			    undef,
> +			    always_call_parser => 1,
> +			    shadow => $shadow_path,
> +			    );
> +
> +sub read_pmg_cf {
> +    my ($filename, $fh) = @_;
> +
> +    my $scores = {};
> +
> +    my $comment = '';
> +    if (defined($fh)) {
> +	while (defined(my $line = <$fh>)) {
> +	    chomp $line;
> +	    next if $line =~ m/^\s*$/;
> +	    if ($line =~ m/^# ?(.*)\s*$/) {
> +		$comment = $1;
> +		next;
> +	    }
> +	    if ($line =~ m/^score\s+(\S+)\s+(\S+)\s*$/) {
> +		my $rule = $1;
> +		my $score = $2;
> +		$scores->{$rule} = {
> +		    name => $rule,
> +		    score => $score,
> +		    comment => $comment,
> +		};
> +		$comment = '';
> +	    } else {
> +		warn "parse error in '$filename': $line\n";
> +		$comment = '';
> +	    }
> +	}
> +    }
> +
> +    return $scores;
> +}
> +
> +sub write_pmg_cf {
> +    my ($filename, $fh, $scores) = @_;
> +
> +    foreach my $rule (sort keys %$scores) {
> +	my $comment = $scores->{$rule}->{comment};
> +	my $score = sprintf("%.3f", $scores->{$rule}->{score});
> +	PVE::Tools::safe_print($filename, $fh, "# $comment\n")
> +	    if defined($comment) && $comment !~ m/^\s*$/;
> +
> +	PVE::Tools::safe_print($filename, $fh, "score $rule $score\n");

hm, while we have that pattern a few times in our codebase (afair the last
one proposed by me) we could get away with only one call to safe_print:
```
my $line = '';
$line .= "# $comment\n" if defined($comment) && $comment !~ m/^\s*$/;
$line .= "score $rule $score\n"
PVE::Tools::safe_print($filename, $fh, $line);
```
(or even write the whole data to a string and use file_set_contents)

not really relevant - just caught my eye - happy with the solution
as it is as well!

> +    }
> +}
> +
> +1;




More information about the pmg-devel mailing list