spamassassin: add explicit default reject_type

consolidate the two data_post methods into one (more linear, simpler)
more informative log message
add new headers to top of headers (not bottom (consistent MTA behavior))
This commit is contained in:
Matt Simerson 2012-06-22 18:29:28 -04:00
parent a08745acc4
commit 1d67c849cf
2 changed files with 70 additions and 54 deletions

View File

@ -141,6 +141,7 @@ use warnings;
use Qpsmtpd::Constants;
use Qpsmtpd::DSN;
use Socket qw(:DEFAULT :crlf);
use IO::Handle;
@ -155,12 +156,14 @@ sub register {
if ( ! defined $self->{_args}{reject} && defined $self->{_args}{reject_threshold} ) {
$self->{_args}{reject} = $self->{_args}{reject_threshold};
};
if ( ! defined $self->{_args}{reject_type} ) {
$self->{_args}{reject_type} = 'perm';
};
$self->register_hook('data_post', 'check_spam_reject');
$self->register_hook('data_post', 'check_spam_munge_subject');
$self->register_hook('data_post', 'data_post_handler');
}
sub hook_data_post {
sub data_post_handler {
my ($self, $transaction) = @_;
return (DECLINED) if $self->is_immune();
@ -180,7 +183,8 @@ sub hook_data_post {
my $headers = $self->parse_spamd_response( $SPAMD ) or return (DECLINED);
$self->insert_spam_headers( $transaction, $headers, $username );
return (DECLINED);
$self->munge_subject( $transaction );
return $self->reject( $transaction );
};
sub select_spamd_username {
@ -361,52 +365,62 @@ sub print_to_spamd {
$self->log(LOGDEBUG, "check_spam: finished sending to spamd");
};
sub check_spam_reject {
sub reject {
my ($self, $transaction) = @_;
my $sa_results = $self->get_spam_results($transaction) or do {
$self->log(LOGNOTICE, "skip: no spamassassin results");
$self->log(LOGNOTICE, "skip, no results");
return DECLINED;
};
my $score = $sa_results->{score} or do {
$self->log(LOGERROR, "skip: error getting spamassassin score");
$self->log(LOGERROR, "skip, error getting score");
return DECLINED;
};
my $ham_or_spam = $sa_results->{is_spam} eq 'Yes' ? 'Spam' : 'Ham';
my $status = "$ham_or_spam, $score";
my $learn;
if ( $sa_results->{autolearn} ) {
$learn = "learn=". $sa_results->{autolearn};
};
my $reject = $self->{_args}{reject} or do {
$self->log(LOGERROR, "skip: reject not set ($ham_or_spam, $score)");
$self->log(LOGERROR, "skip, reject disabled ($status, $learn)");
return DECLINED;
};
if ( $score < $reject ) {
$self->log(LOGINFO, "pass, $ham_or_spam, $score < $reject");
return DECLINED;
};
if ( $ham_or_spam eq 'Spam' ) {
$self->log(LOGINFO, "fail, $status < $reject, $learn");
return DECLINED;
}
else {
$self->log(LOGINFO, "pass, $status < $reject, $learn");
return DECLINED;
}
}
$self->connection->notes('karma', $self->connection->notes('karma') - 1);
# default of media_unsupported is DENY, so just change the message
$self->log(LOGINFO, "deny, $ham_or_spam, $score > $reject");
return Qpsmtpd::DSN->media_unsupported("spam score exceeded threshold");
$self->log(LOGINFO, "deny, $status, > $reject, $learn");
return ($self->get_reject_type(), "spam score exceeded threshold");
}
sub check_spam_munge_subject {
sub munge_subject {
my ($self, $transaction) = @_;
my $sa = $self->get_spam_results($transaction) or return;
my $qp_num = $self->{_args}{munge_subject_threshold};
my $sa = $self->get_spam_results($transaction) or return DECLINED;
my $required = $sa->{required} || $qp_num or do {
$self->log(LOGDEBUG, "skipping munge, no user or qpsmtpd pref set");
return DECLINED;
return;
};
return DECLINED unless $sa->{score} > $required;
return unless $sa->{score} > $required;
my $subject_prefix = $self->qp->config('subject_prefix') || '*** SPAM ***';
my $subject = $transaction->header->get('Subject') || '';
$transaction->header->replace('Subject', "$subject_prefix $subject");
return DECLINED;
}
sub get_spam_results {
@ -465,7 +479,7 @@ sub _cleanup_spam_header {
$old_header_name = ($old_header_name =~ s/^X-//) ? "X-Old-$old_header_name" : "Old-$old_header_name";
for my $header ( $transaction->header->get($header_name) ) {
$transaction->header->add($old_header_name, $header) if $action eq 'rename';
$transaction->header->add($old_header_name, $header, 0) if $action eq 'rename';
$transaction->header->delete($header_name);
}
}

View File

@ -25,9 +25,9 @@ sub register_tests {
$self->register_test('test_connect_to_spamd', 4);
$self->register_test('test_parse_spam_header', 10);
$self->register_test('test_get_spam_results', 19);
$self->register_test('test_check_spam_munge_subject', 4);
$self->register_test('test_check_spam_reject', 2);
$self->register_test('test_get_spam_results', 20);
$self->register_test('test_munge_subject', 4);
$self->register_test('test_reject', 2);
}
sub test_connect_to_spamd {
@ -43,38 +43,38 @@ sub test_connect_to_spamd {
$self->{_args}{spamd_socket} = '/var/run/spamd/spamd.socket';
my $SPAMD = $self->connect_to_spamd();
if ( $SPAMD ) {
ok( $SPAMD, "connect_to_spamd, socket");
ok( $SPAMD, "socket");
$self->print_to_spamd( $SPAMD, $message, $length, $username );
shutdown($SPAMD, 1); # close our side of the socket (tell spamd we're done)
my $headers = $self->parse_spamd_response( $SPAMD );
#warn Data::Dumper::Dumper($headers);
ok( $headers, "connect_to_spamd, socket response\n");
ok( $headers, "socket response\n");
}
else {
ok( 1 == 1, "connect_to_spamd, socket connect FAILED");
ok( 1 == 1, "connect_to_spamd, socket response FAILED");
ok( 1 == 1, "socket connect FAILED");
ok( 1 == 1, "socket response FAILED");
};
# Try a TCP/IP connection
$self->{_args}{spamd_socket} = '127.0.0.1:783';
$SPAMD = $self->connect_to_spamd();
if ( $SPAMD ) {
ok( $SPAMD, "connect_to_spamd, tcp/ip");
ok( $SPAMD, "tcp/ip");
#warn Data::Dumper::Dumper($SPAMD);
$self->print_to_spamd( $SPAMD, $message, $length, $username );
shutdown($SPAMD, 1); # close our side of the socket (tell spamd we're done)
my $headers = $self->parse_spamd_response( $SPAMD );
#warn Data::Dumper::Dumper($headers);
ok( $headers, "connect_to_spamd, tcp/ip response\n");
ok( $headers, "tcp/ip response\n");
}
else {
ok( 1 == 1, "connect_to_spamd, tcp/ip connect FAILED");
ok( 1 == 1, "connect_to_spamd, tcp/ip response FAILED");
ok( 1 == 1, "tcp/ip connect FAILED");
ok( 1 == 1, "tcp/ip response FAILED");
};
};
sub test_check_spam_reject {
sub test_reject {
my $self = shift;
my $transaction = $self->qp->transaction;
@ -83,17 +83,17 @@ sub test_check_spam_reject {
# message scored a 10, should pass
$self->{_args}{reject} = 12;
$transaction->notes('spamassassin', { is_spam => 'Yes', score => 10 } );
my $r = $self->check_spam_reject($transaction);
cmp_ok( DECLINED, '==', $r, "check_spam_reject, $r");
my $r = $self->reject($transaction);
cmp_ok( DECLINED, '==', $r, "r: $r");
# message scored a 15, should fail
$self->{_args}{reject} = 12;
$transaction->notes('spamassassin', { is_spam => 'Yes', score => 15 } );
($r) = $self->check_spam_reject($transaction);
cmp_ok( DENY, '==', $r, "check_spam_reject, $r");
($r) = $self->reject($transaction);
cmp_ok( DENY, '==', $r, "r: $r");
};
sub test_check_spam_munge_subject {
sub test_munge_subject {
my $self = shift;
my $transaction = $self->qp->transaction;
@ -103,31 +103,31 @@ sub test_check_spam_munge_subject {
$self->{_args}{munge_subject_threshold} = 5;
$transaction->notes('spamassassin', { score => 6 } );
$transaction->header->add('Subject', $subject);
$self->check_spam_munge_subject($transaction);
$self->munge_subject($transaction);
my $r = $transaction->header->get('Subject'); chomp $r;
cmp_ok($r, 'eq', "*** SPAM *** $subject", "check_spam_munge_subject +");
cmp_ok($r, 'eq', "*** SPAM *** $subject", "+");
$transaction->header->delete('Subject'); # cleanup
$self->{_args}{munge_subject_threshold} = 5;
$transaction->notes('spamassassin', { score => 3 } );
$transaction->header->add('Subject', $subject);
$self->check_spam_munge_subject($transaction);
$self->munge_subject($transaction);
$r = $transaction->header->get('Subject'); chomp $r;
cmp_ok($r, 'eq', $subject, "check_spam_munge_subject -");
cmp_ok($r, 'eq', $subject, "-");
$transaction->header->delete('Subject'); # cleanup
$transaction->notes('spamassassin', { score => 3, required => 4 } );
$transaction->header->add('Subject', $subject);
$self->check_spam_munge_subject($transaction);
$self->munge_subject($transaction);
$r = $transaction->header->get('Subject'); chomp $r;
cmp_ok($r, 'eq', $subject, "check_spam_munge_subject -");
cmp_ok($r, 'eq', $subject, "-");
$transaction->header->delete('Subject'); # cleanup
$transaction->notes('spamassassin', { score => 5, required => 4 } );
$transaction->header->add('Subject', $subject);
$self->check_spam_munge_subject($transaction);
$self->munge_subject($transaction);
$r = $transaction->header->get('Subject'); chomp $r;
cmp_ok($r, 'eq', "*** SPAM *** $subject", "check_spam_munge_subject +");
cmp_ok($r, 'eq', "*** SPAM *** $subject", "+");
};
sub test_get_spam_results {
@ -145,15 +145,17 @@ sub test_get_spam_results {
$r_ref->{hits} = delete $r_ref->{score}; # SA v2 compat
};
my $r2 = _reassemble_header($r_ref);
cmp_ok( $h, 'eq', $r2, "get_spam_results ($h)" );
cmp_ok( $h, 'eq', $r2, $h );
# this time it should be cached
$r_ref = $self->get_spam_results($transaction);
next if $h =~ /hits=/; # caching is broken for SA v2 headers
if ( $h =~ /hits=/ ) {
ok( 1 );
next;
}; # caching is broken for SA v2 headers
$r2 = _reassemble_header($r_ref);
cmp_ok( $h, 'eq', $r2, "get_spam_results ($h)" );
cmp_ok( $h, 'eq', $r2, $h );
};
};
sub test_parse_spam_header {
@ -161,11 +163,11 @@ sub test_parse_spam_header {
foreach my $h ( @sample_headers ) {
my $r_ref = $self->parse_spam_header($h);
if ( $h =~ /hits=/ ) {
if ( $h =~ /hits=/ ) {
$r_ref->{hits} = delete $r_ref->{score}; # SA v2 compat
};
my $r2 = _reassemble_header($r_ref);
cmp_ok( $h, 'eq', $r2, "parse_spam_header ($h)" );
cmp_ok( $h, 'eq', $r2, $h );
};
};
@ -181,7 +183,7 @@ sub test_message {
return <<'EO_MESSAGE'
To: Fictitious User <fict@example.com>
From: No Such <such@example.com>
Subject: jose can you see, by the dawns early light?
Subject: jose can you see, by the dawns early light?
What so proudly we.
EO_MESSAGE