From ef26c61b6d648d63b1b7a9e7e354889679c4bec0 Mon Sep 17 00:00:00 2001 From: Jared Johnson Date: Wed, 26 Nov 2014 16:28:52 -0600 Subject: [PATCH] Use Qpsmtpd::DB in karma plugin And clean up a few things in Qpsmtpd::DB --- lib/Qpsmtpd/DB/File/DBM.pm | 9 ++++-- lib/Qpsmtpd/Plugin.pm | 1 + plugins/greylisting | 10 +++--- plugins/karma | 66 ++++++++++++++++---------------------- 4 files changed, 39 insertions(+), 47 deletions(-) diff --git a/lib/Qpsmtpd/DB/File/DBM.pm b/lib/Qpsmtpd/DB/File/DBM.pm index afdd120..e5942c7 100644 --- a/lib/Qpsmtpd/DB/File/DBM.pm +++ b/lib/Qpsmtpd/DB/File/DBM.pm @@ -15,8 +15,12 @@ sub file_extension { sub lock { my ( $self ) = @_; - $self->nfs_locking ? $self->nfs_file_lock : $self->file_lock; - $self->tie_dbm; + if ( $self->nfs_locking ) { + $self->nfs_file_lock or return; + } else { + $self->file_lock or return; + } + return $self->tie_dbm; } sub file_lock { @@ -71,6 +75,7 @@ sub tie_dbm { return; }; $self->{tied} = \%db; + return 1; } sub nfs_locking { diff --git a/lib/Qpsmtpd/Plugin.pm b/lib/Qpsmtpd/Plugin.pm index e595f2a..b51b6cf 100644 --- a/lib/Qpsmtpd/Plugin.pm +++ b/lib/Qpsmtpd/Plugin.pm @@ -4,6 +4,7 @@ use warnings; use lib 'lib'; use parent 'Qpsmtpd::Base'; +use Qpsmtpd::DB; use Qpsmtpd::Constants; # more or less in the order they will fire diff --git a/plugins/greylisting b/plugins/greylisting index dca61f9..b00bc03 100644 --- a/plugins/greylisting +++ b/plugins/greylisting @@ -175,13 +175,9 @@ use warnings; use Net::IP; use Qpsmtpd::Constants; -use Qpsmtpd::DB; my $VERSION = '0.12'; -use AnyDBM_File; -use Fcntl qw(:DEFAULT :flock LOCK_EX LOCK_NB); - my $DENYMSG = "This mail is temporarily denied"; my %PERMITTED_ARGS = map { $_ => 1 } qw(per_recipient remote_ip sender recipient black_timeout grey_timeout white_timeout deny_late db_dir @@ -360,6 +356,7 @@ sub greylist { my $fmt = "%s:%d:%d:%d"; + $self->db->lock or return DECLINED; my $value = $self->db->get($key); if ( ! $value ) { # new IP or entry timed out - record new @@ -406,6 +403,7 @@ sub greylist { sub cleanup_and_return { my ($self, $return_val) = @_; + $self->db->unlock; return $return_val if defined $return_val; # explicit override return DECLINED if defined $self->{_args}{reject} && !$self->{_args}{reject}; @@ -435,7 +433,7 @@ sub get_greylist_key { sub convert_db { my $self = shift; - $self->db->lock; + $self->db->lock or return DECLINED; my $count = $self->db->size; my $converted = 0; @@ -457,7 +455,7 @@ sub convert_db { sub prune_db { my $self = shift; - $self->db->lock; + $self->db->lock or return DECLINED; my $count = $self->db->size; my $pruned = 0; diff --git a/plugins/karma b/plugins/karma index 5fadbb9..e629f96 100644 --- a/plugins/karma +++ b/plugins/karma @@ -231,9 +231,6 @@ use warnings; use Qpsmtpd::Constants; -BEGIN { @AnyDBM_File::ISA = qw(DB_File GDBM_File NDBM_File) } -use AnyDBM_File; -use Fcntl qw(:DEFAULT :flock LOCK_EX LOCK_NB); use Net::IP; sub register { @@ -264,23 +261,22 @@ sub hook_pre_connection { my $remote_ip = $args{remote_ip}; - my $db = $self->get_dbm_location(); - my $lock = $self->get_dbm_lock($db) or return DECLINED; - my $tied = $self->get_dbm_tie($db, $lock) or return DECLINED; + $self->db->lock or return DECLINED; my $key = $self->get_karma_key($remote_ip) or do { $self->log(LOGINFO, "skip, unable to get DB key"); return DECLINED; }; - if (!$tied->{$key}) { + my $value = $self->db->get($key); + if ( ! $value ) { $self->log(LOGDEBUG, "pass, no record"); - return $self->cleanup_and_return($tied, $lock); + return $self->cleanup_and_return(); } my ($penalty_start_ts, $naughty, $nice, $connects) = - $self->parse_db_record($tied->{$key}); + $self->parse_db_record($value); $self->calc_karma($naughty, $nice); - return $self->cleanup_and_return($tied, $lock); + return $self->cleanup_and_return(); } sub connect_handler { @@ -290,37 +286,36 @@ sub connect_handler { return DECLINED if $self->is_immune(); - my $db = $self->get_dbm_location(); - my $lock = $self->get_dbm_lock($db) or return DECLINED; - my $tied = $self->get_dbm_tie($db, $lock) or return DECLINED; + $self->db->lock or return DECLINED; my $key = $self->get_karma_key() or do { $self->log(LOGINFO, "skip, unable to get DB key"); return DECLINED; }; - if (!$tied->{$key}) { + my $value = $self->db->get($key); + if ( ! $value) { $self->log(LOGINFO, "pass, no record"); - return $self->cleanup_and_return($tied, $lock); + return $self->cleanup_and_return(); } my ($penalty_start_ts, $naughty, $nice, $connects) = - $self->parse_db_record($tied->{$key}); + $self->parse_db_record($value); my $summary = "$naughty naughty, $nice nice, $connects connects"; my $karma = $self->calc_karma($naughty, $nice); if (!$penalty_start_ts) { $self->log(LOGINFO, "pass, no penalty ($summary)"); - return $self->cleanup_and_return($tied, $lock); + return $self->cleanup_and_return(); } my $days_old = (time - $penalty_start_ts) / 86400; if ($days_old >= $self->{_args}{penalty_days}) { $self->log(LOGINFO, "pass, penalty expired ($summary)"); - return $self->cleanup_and_return($tied, $lock); + return $self->cleanup_and_return(); } - $tied->{$key} = join(':', $penalty_start_ts, $naughty, $nice, ++$connects); - $self->cleanup_and_return($tied, $lock); + $self->db->set( $key, join(':', $penalty_start_ts, $naughty, $nice, ++$connects) ); + $self->cleanup_and_return(); my $left = sprintf "%.2f", $self->{_args}{penalty_days} - $days_old; my $mess = "You were naughty. You cannot connect for $left more days."; @@ -414,13 +409,11 @@ sub disconnect_handler { return DECLINED; }; - my $db = $self->get_dbm_location(); - my $lock = $self->get_dbm_lock($db) or return DECLINED; - my $tied = $self->get_dbm_tie($db, $lock) or return DECLINED; + $self->db->lock or return DECLINED; my $key = $self->get_karma_key(); my ($penalty_start_ts, $naughty, $nice, $connects) = - $self->parse_db_record($tied->{$key}); + $self->parse_db_record( $self->db->get($key) ); my $history = ($nice || 0) - $naughty; my $log_mess = ''; @@ -450,8 +443,8 @@ sub disconnect_handler { } $self->log(LOGINFO, $log_mess . ", (msg: $karma, his: $history)"); - $tied->{$key} = join(':', $penalty_start_ts, $naughty, $nice, ++$connects); - return $self->cleanup_and_return($tied, $lock); + $self->db->set( $key, join(':', $penalty_start_ts, $naughty, $nice, ++$connects) ); + return $self->cleanup_and_return(); } sub illegal_envelope_format { @@ -489,10 +482,9 @@ sub calc_karma { } sub cleanup_and_return { - my ($self, $tied, $lock, $return_val) = @_; + my ( $self, $return_val ) = @_; - untie $tied; - close $lock; + $self->db->unlock; return $return_val if defined $return_val; # explicit override return DECLINED; } @@ -588,22 +580,18 @@ sub get_dbm_lock_nfs { sub prune_db { my $self = shift; - my $db = $self->get_dbm_location(); - my $lock = $self->get_dbm_lock($db) or return DECLINED; - my $tied = $self->get_dbm_tie($db, $lock) or return DECLINED; - my $count = keys %$tied; + $self->db->lock or return DECLINED; + my $count = $self->db->size; my $pruned = 0; - foreach my $key (keys %$tied) { - my $ts = $tied->{$key}; + foreach my $key ( $self->db->get_keys ) { + my $ts = $self->db->get($key); my $days_old = (time - $ts) / 86400; next if $days_old < $self->{_args}{penalty_days} * 2; - delete $tied->{$key}; + $self->delete($key); $pruned++; } - untie $tied; - close $lock; $self->log(LOGINFO, "pruned $pruned of $count DB entries"); - return $self->cleanup_and_return($tied, $lock, DECLINED); + return $self->cleanup_and_return(DECLINED); }