consolidated chunks of code duplicated 4x into log_and_deny and log_and_pass

in apr_*_handler subs, return DECLINED when connection is not available to read 
(like during tests)

added 23 tests

deprecate action argument
        'action log' did nothing, better logging controls available with 
loglevel
        'action deny'  ->  reject 1
        'action denysoft' =>  reject 1 reject_type temp

POD
use head2 for config options (instead of over, item, back)
added loglevel section
updated for replacement of action with reject options
---
 plugins/check_earlytalker        |  193 ++++++++++++++++++++------------------
 t/plugin_tests/check_earlytalker |  147 +++++++++++++++++++++++++++++
 2 files changed, 249 insertions(+), 91 deletions(-)
 create mode 100644 t/plugin_tests/check_earlytalker

diff --git a/plugins/check_earlytalker b/plugins/check_earlytalker
index 6c4eedf..06a218c 100644
--- a/plugins/check_earlytalker
+++ b/plugins/check_earlytalker
@@ -1,4 +1,5 @@
 #!perl -w
+
 =head1 NAME
 
 check_earlytalker - Check that the client doesn't talk before we send the SMTP 
banner
@@ -16,9 +17,7 @@ on all mail/rcpt commands in the transaction.
 
 =head1 CONFIGURATION
 
-=over 4
-
-=item wait [integer]
+=head2 wait [integer]
 
 The number of seconds to delay the initial greeting to see if the connecting
 host speaks first.  The default is 1.  Do not select a value that is too high,
@@ -27,32 +26,40 @@ greeting timeouts -- these are known to range as low as 30 
seconds, and may
 in some cases be configured lower by mailserver admins.  Network transit time
 must also be allowed for.
 
-=item action [string: deny, denysoft, log]
+=head2 reject <boolean>
+
+Do we reject/deny connections to early talkers?
+
+  check_earlytalker reject [ 0 | 1 ]
+
+Default: I<reject 1>
 
-What to do when matching an early-talker -- the options are I<deny>,
-I<denysoft> or I<log>.
+=head2 reject_type [ temp | perm ]
 
-If I<log> is specified, the connection will be allowed to proceed as normal,
-and only a warning will be logged.
+What type of rejection to send. A temporary rejection tells the remote server 
to try again later. A permanent error tells it to give up permanently.
 
-The default is I<denysoft>.
+Default I<reject_type temp>.
 
-=item defer-reject [boolean]
+=head2 defer-reject [boolean]
 
 When an early-talker is detected, if this option is set to a true value, the
 SMTP greeting will be issued as usual, but all RCPT/MAIL commands will be
-issued a deny or denysoft (depending on the value of I<action>).  The default
+issued a deny or denysoft (depending on the value of I<reject_type>). The 
default
 is to react at the SMTP greeting stage by issuing the apropriate response code
 and terminating the SMTP connection.
 
-=item check-at [ CONNECT | DATA ]
+  check_earlytalker defer-reject [ 0 | 1 ]
+
+=head2 check-at [ CONNECT | DATA ]
 
 Specifies when to check for early talkers. You can specify this option
 multiple times to check more than once.
 
 The default is I<check-at CONNECT> only.
 
-=back
+=head2 loglevel
+
+Adjust the quantity of logging for this plugin. See docs/logging.pod
 
 =cut
 
@@ -67,7 +74,7 @@ sub register {
 
   if (@args % 2) {
         $self->log(LOGERROR, "Unrecognized/mismatched arguments");
-        return undef;
+        return;
   }
   my %check_at;
   for (0..$#args) {
@@ -82,11 +89,17 @@ sub register {
   }
   $self->{_args} = {
         'wait' => 1,
-        'action' => 'denysoft',
-        'defer-reject' => 0,
         @args,
         'check-at' => \%check_at,
   };
+# backwards compat with old 'action' argument
+  if ( defined $self->{_args}{action} && ! defined $self->{_args}{reject} ) {
+       $self->{_args}{reject} = $self->{_args}{action} =~ /^deny/i ? 1 : 0;
+  };
+  if ( defined $self->{_args}{'defer-reject'} && ! defined 
$self->{_args}{reject_type} ) {
+       $self->{_args}{reject_type} = $self->{_args}{action} == 'denysoft' ? 
'temp' : 'perm';
+  };
+# /end compat
   if ( $qp->{conn} && $qp->{conn}->isa('Apache2::Connection')) {
       require APR::Const;
       APR::Const->import(qw(POLLIN SUCCESS));
@@ -98,117 +111,115 @@ sub register {
       $self->register_hook('data', 'data_handler');
   }
   $self->register_hook('mail', 'mail_handler')
-    if $self->{_args}->{'defer-reject'};
-  1;
+    if $self->{_args}{'defer-reject'};
+  $self->{_args}{reject} = 1 if ! defined $self->{_args}{reject};
 }
 
 sub apr_connect_handler {
     my ($self, $transaction) = @_;
 
     return DECLINED unless $self->{_args}{'check-at'}{CONNECT};
-    return DECLINED if ($self->qp->connection->notes('whitelisthost'));
-    my $ip = $self->qp->connection->remote_ip;
+    return DECLINED if $self->qp->connection->notes('whitelisthost');
 
-    my $c = $self->qp->{conn};
-    my $socket = $c->client_socket;
-    my $timeout = $self->{_args}->{'wait'} * 1_000_000;
+    my $c = $self->qp->{conn} or return DECLINED;
+    my $socket = $c->client_socket or return DECLINED;
+    my $timeout = $self->{_args}{'wait'} * 1_000_000;
 
     my $rc = $socket->poll($c->pool, $timeout, APR::Const::POLLIN());
     if ($rc == APR::Const::SUCCESS()) {
-        $self->log(LOGNOTICE, "remote host started talking before we said 
hello [$ip]");
-        if ($self->{_args}->{'defer-reject'}) {
+        if ($self->{_args}{'defer-reject'}) {
             $self->qp->connection->notes('earlytalker', 1);
-        }
-        else {
-            my $msg = 'Connecting host started transmitting before SMTP 
greeting';
-            return (DENY,$msg) if $self->{_args}->{'action'} eq 'deny';
-            return (DENYSOFT,$msg) if $self->{_args}->{'action'} eq 'denysoft';
-        }
-    }
-    else {
-        $self->log(LOGINFO, "pass: remote host said nothing spontaneous");
-    }
+            return DECLINED;
+        };
+        return $self->log_and_deny();
+    };
+    return $self->log_and_pass();
 }
 
 sub apr_data_handler {
     my ($self, $transaction) = @_;
 
     return DECLINED unless $self->{_args}{'check-at'}{DATA};
-    return DECLINED if ($self->qp->connection->notes('whitelisthost'));
-    my $ip = $self->qp->connection->remote_ip;
+    return DECLINED if $self->qp->connection->notes('whitelisthost');
 
-    my $c = $self->qp->{conn};
-    my $socket = $c->client_socket;
-    my $timeout = $self->{_args}->{'wait'} * 1_000_000;
+    my $c = $self->qp->{conn} or return DECLINED;
+    my $socket = $c->client_socket or return DECLINED;
+    my $timeout = $self->{_args}{'wait'} * 1_000_000;
 
     my $rc = $socket->poll($c->pool, $timeout, APR::Const::POLLIN());
     if ($rc == APR::Const::SUCCESS()) {
-        $self->log(LOGNOTICE, "remote host started talking before we said 
hello [$ip]");
-        my $msg = 'Connecting host started transmitting before SMTP greeting';
-        return (DENY,$msg) if $self->{_args}->{'action'} eq 'deny';
-        return (DENYSOFT,$msg) if $self->{_args}->{'action'} eq 'denysoft';
-    }
-    else {
-        $self->log(LOGINFO, "pass: remote host said nothing spontaneous");
-    }
+        return $self->log_and_deny();
+    };
+    return $self->log_and_pass();
 }
 
 sub connect_handler {
-  my ($self, $transaction) = @_;
-  my $in = new IO::Select;
-  my $ip = $self->qp->connection->remote_ip;
-
-  return DECLINED unless $self->{_args}{'check-at'}{CONNECT};
-  return DECLINED
-      if ($self->qp->connection->notes('whitelisthost'));
-
-  $in->add(\*STDIN) || return DECLINED;
-  if ($in->can_read($self->{_args}->{'wait'})) {
-    $self->log(LOGNOTICE, "remote host started talking before we said hello 
[$ip]");
-    if ($self->{_args}->{'defer-reject'}) {
-        $self->qp->connection->notes('earlytalker', 1);
-    } else {
-      my $msg = 'Connecting host started transmitting before SMTP greeting';
-      return (DENY,$msg) if $self->{_args}->{'action'} eq 'deny';
-      return (DENYSOFT,$msg) if $self->{_args}->{'action'} eq 'denysoft';
-    }
-  } else {
-    $self->log(LOGINFO, 'pass: remote host said nothing spontaneous');
-  }
-  return DECLINED;
+    my ($self, $transaction) = @_;
+    my $in = new IO::Select;
+
+    return DECLINED unless $self->{_args}{'check-at'}{CONNECT};
+    return DECLINED if $self->qp->connection->notes('whitelisthost');
+
+    $in->add(\*STDIN) or return DECLINED;
+    if (! $in->can_read($self->{_args}{'wait'})) {
+        return $self->log_and_pass();
+    };
+
+    if ( ! $self->{_args}{'defer-reject'}) {
+        return $self->log_and_deny();
+    };
+
+    $self->qp->connection->notes('earlytalker', 1);
+    return DECLINED;
 }
 
 sub data_handler {
-  my ($self, $transaction) = @_;
-  my $in = new IO::Select;
-  my $ip = $self->qp->connection->remote_ip;
+    my ($self, $transaction) = @_;
+    my $in = new IO::Select;
+
+    return DECLINED unless $self->{_args}{'check-at'}{DATA};
+    return DECLINED if $self->qp->connection->notes('whitelisthost');
+
+    $in->add(\*STDIN) or return DECLINED;
+    if ( ! $in->can_read($self->{_args}{'wait'})) {
+        return $self->log_and_pass();
+    };
 
-  return DECLINED unless $self->{_args}{'check-at'}{DATA};
-  return DECLINED
-      if ($self->qp->connection->notes('whitelisthost'));
+    return $self->log_and_deny();
+};
 
-  $in->add(\*STDIN) || return DECLINED;
-  if ($in->can_read($self->{_args}->{'wait'})) {
-    $self->log(LOGNOTICE, "remote host started talking before we said hello 
[$ip]");
+sub log_and_pass {
+    my $self = shift;
+    my $ip = $self->qp->connection->remote_ip || 'remote host';
+    $self->log(LOGINFO, "pass: $ip said nothing spontaneous");
+    return DECLINED;
+}
+
+sub log_and_deny {
+    my $self = shift;
+
+    my $ip = $self->qp->connection->remote_ip || 'remote host';
     my $msg = 'Connecting host started transmitting before SMTP greeting';
-    return (DENY,$msg) if $self->{_args}->{'action'} eq 'deny';
-    return (DENYSOFT,$msg) if $self->{_args}->{'action'} eq 'denysoft';
-  }
-  else {
-    $self->log(LOGINFO, 'pass: remote host said nothing spontaneous');
-  }
-  return DECLINED;
+
+    $self->qp->connection->notes('earlytalker', 1);
+    $self->log(LOGNOTICE, "fail: $ip started talking before we said hello");
+
+    return ( $self->get_reject_type(), $msg ) if $self->{_args}{reject};
+    return DECLINED;
 }
 
 sub mail_handler {
   my ($self, $transaction) = @_;
-  my $msg = 'Connecting host started transmitting before SMTP greeting';
 
   return DECLINED unless $self->qp->connection->notes('earlytalker');
-  return (DENY,$msg) if $self->{_args}->{'action'} eq 'deny';
-  return (DENYSOFT,$msg) if $self->{_args}->{'action'} eq 'denysoft';
-  return DECLINED;
+  return $self->log_and_deny();
 }
 
-1;
+sub get_reject_type {
+    my $self = shift;
+    my $deny = $self->{_args}{reject_type} or return DENY;
 
+    return $deny eq 'temp'       ? DENYSOFT
+         : $deny eq 'disconnect' ? DENY_DISCONNECT
+         : DENY;
+};
diff --git a/t/plugin_tests/check_earlytalker b/t/plugin_tests/check_earlytalker
new file mode 100644
index 0000000..570aebd
--- /dev/null
+++ b/t/plugin_tests/check_earlytalker
@@ -0,0 +1,147 @@
+#!perl -w
+
+use strict;
+use warnings;
+
+use Qpsmtpd::Constants;
+
+sub register_tests {
+    my $self = shift;
+
+    $self->register_test('test_reject_type', 3);
+    $self->register_test('test_log_and_pass', 1);
+    $self->register_test('test_log_and_deny', 3);
+    $self->register_test('test_data_handler', 3);
+    $self->register_test('test_connect_handler', 3);
+    $self->register_test('test_apr_data_handler', 3);
+    $self->register_test('test_apr_connect_handler', 3);
+    $self->register_test('test_mail_handler', 4);
+}
+
+sub test_apr_connect_handler {
+    my $self = shift;
+
+    $self->{_args}{'check-at'} = undef;
+    my ($code, $mess) = $self->apr_connect_handler();
+    cmp_ok( $code, '==', DECLINED, "no check-at set");
+
+    $self->{_args}{'check-at'}{'DATA'} = 1;
+    $self->qp->connection->notes('whitelisthost', 1);
+    ($code, $mess) = $self->apr_connect_handler();
+    cmp_ok( $code, '==', DECLINED, "whitelisted host");
+
+    $self->qp->connection->notes('whitelisthost', 0);
+    ($code, $mess) = $self->apr_connect_handler();
+    cmp_ok( $code, '==', DECLINED, "not sure");
+};
+
+sub test_apr_data_handler {
+    my $self = shift;
+
+    $self->{_args}{'check-at'} = undef;
+    my ($code, $mess) = $self->apr_data_handler();
+    cmp_ok( $code, '==', DECLINED, "no check-at set");
+
+    $self->{_args}{'check-at'}{'DATA'} = 1;
+    $self->qp->connection->notes('whitelisthost', 1);
+    ($code, $mess) = $self->apr_data_handler();
+    cmp_ok( $code, '==', DECLINED, "whitelisted host");
+
+    $self->qp->connection->notes('whitelisthost', 0);
+    ($code, $mess) = $self->apr_data_handler();
+    cmp_ok( $code, '==', DECLINED, "not sure");
+};
+
+sub test_connect_handler {
+    my $self = shift;
+
+    $self->{_args}{'check-at'} = undef;
+    my ($code, $mess) = $self->connect_handler();
+    cmp_ok( $code, '==', DECLINED, "no check-at set");
+
+    $self->{_args}{'check-at'}{'CONNECT'} = 1;
+    $self->qp->connection->notes('whitelisthost', 1);
+    ($code, $mess) = $self->connect_handler();
+    cmp_ok( $code, '==', DECLINED, "whitelisted host");
+
+    $self->qp->connection->notes('whitelisthost', 0);
+    ($code, $mess) = $self->connect_handler();
+    cmp_ok( $code, '==', DECLINED, "not sure");
+};
+
+sub test_data_handler {
+    my $self = shift;
+
+    $self->{_args}{'check-at'} = undef;
+    my ($code, $mess) = $self->data_handler();
+    cmp_ok( $code, '==', DECLINED, "no check-at set");
+
+    $self->{_args}{'check-at'}{'DATA'} = 1;
+    $self->qp->connection->notes('whitelisthost', 1);
+    ($code, $mess) = $self->data_handler();
+    cmp_ok( $code, '==', DECLINED, "whitelisted host");
+
+    $self->qp->connection->notes('whitelisthost', 0);
+    ($code, $mess) = $self->data_handler();
+    cmp_ok( $code, '==', DECLINED, "not sure");
+};
+
+sub test_log_and_pass {
+    my $self = shift;
+
+    my ($code, $mess) = $self->log_and_pass();
+    cmp_ok( $code, '==', DECLINED, "default");
+};
+
+sub test_log_and_deny {
+    my $self = shift;
+
+    $self->{_args}{reject_type} = undef;
+
+    my ($code, $mess) = $self->log_and_deny();
+    cmp_ok( $code, '==', DENY, "default");
+
+    $self->{_args}{reject_type} = 'temp';
+    ($code, $mess) = $self->log_and_deny();
+    cmp_ok( $code, '==', DENYSOFT, "bad, temp");
+
+    $self->{_args}{reject_type} = 'disconnect';
+    ($code, $mess) = $self->log_and_deny();
+    cmp_ok( $code, '==', DENY_DISCONNECT, "bad, disconnect");
+};
+
+sub test_mail_handler {
+    my $self = shift;
+
+    $self->{_args}{reject_type} = undef;
+    $self->qp->connection->notes('earlytalker', 0);
+
+    my ($code, $mess) = $self->mail_handler();
+    cmp_ok( $code, '==', DECLINED, "good");
+
+    $self->qp->connection->notes('earlytalker', 1);
+    ($code, $mess) = $self->mail_handler();
+    cmp_ok( $code, '==', DENY, "bad");
+
+    $self->{_args}{reject_type} = 'temp';
+    ($code, $mess) = $self->mail_handler();
+    cmp_ok( $code, '==', DENYSOFT, "bad, temp");
+
+    $self->{_args}{reject_type} = 'disconnect';
+    ($code, $mess) = $self->mail_handler();
+    cmp_ok( $code, '==', DENY_DISCONNECT, "bad, disconnect");
+};
+
+sub test_reject_type {
+    my $self = shift;
+
+    $self->{_args}{reject_type} = undef;
+    cmp_ok( $self->get_reject_type(), '==', DENY, "default");
+
+    $self->{_args}{reject_type} = 'temp';
+    cmp_ok( $self->get_reject_type(), '==', DENYSOFT, "defer");
+
+    $self->{_args}{reject_type} = 'disconnect';
+    cmp_ok( $self->get_reject_type(), '==', DENY_DISCONNECT, "disconnect");
+};
+
-- 
1.7.9.6

Reply via email to