dnsbl, POD tweaks, DENY type tests

consolidated POD at top of file
added example options to reject_type POD head
added an example loglevel entry

consolidated DENY[SOFT|DISCONNECT] logic into get_reject_type
added tests for get_reject_type

plugins/dnsbl

@@ -9,6 +9,115 @@ dnsbl - handle DNS BlackList lookups
 Plugin that checks the IP address of the incoming connection against
 a configurable set of RBL services.
 
+=head1 USAGE
+
+Add the following line to the config/plugins file:
+
+  dnsbl [ reject_type disconnect ] [loglevel -1]
+
+=head2 reject_type [ temp | perm ]
+
+To immediately drop the connection (since some blacklisted servers attempt
+multiple sends per session), set I<reject_type disconnect>. In most cases,
+an IP address that is listed should not be given the opportunity to begin a
+new transaction, since even the most volatile blacklists will return the same
+answer for a short period of time (the minimum DNS cache period).
+
+Default: perm
+
+=head2 loglevel
+
+Adjust the quantity of logging for this plugin. See docs/logging.pod
+
+  dnsbl [loglevel -1]
+
+=head1 CONFIG FILES
+
+This plugin uses the following configuration files. All are optional. Not
+specifying dnsbl_zones is like not using the plugin at all.
+
+=head2 dnsbl_zones
+
+Normal ip based dns blocking lists ("RBLs") which contain TXT records are
+specified simply as:
+
+  relays.ordb.org
+  spamsources.fabel.dk
+
+To configure RBL services which do not contain TXT records in the DNS,
+but only A records (e.g. the RBL+ at http://www.mail-abuse.org), specify your
+own error message to return in the SMTP conversation after a colon e.g.
+
+  rbl-plus.mail-abuse.org:You are listed at - http://http://www.mail-abuse.org/cgi-bin/lookup?%IP%
+
+The string %IP% will be replaced with the IP address of incoming connection.
+Thus a fully specified file could be:
+
+  sbl-xbl.spamhaus.org
+  list.dsbl.org
+  rbl-plus.mail-abuse.ja.net:Listed by rbl-plus.mail-abuse.ja.net - see <URL:http://www.mail-abuse.org/cgi-bin/lookup?%IP%>
+  relays.ordb.org
+
+=head2 dnsbl_allow
+
+List of allowed ip addresses that bypass RBL checking. Format is one entry per line,
+with either a full IP address or a truncated IP address with a period at the end.
+For example:
+
+  192.168.1.1
+  172.16.33.
+
+NB the environment variable RBLSMTPD is considered before this file is
+referenced. See below.
+
+=head2 dnsbl_rejectmsg
+
+A textual message that is sent to the sender on an RBL failure. The TXT record
+from the RBL list is also sent, but this file can be used to indicate what
+action the sender should take.
+
+For example:
+
+   If you think you have been blocked in error, then please forward
+   this entire error message to your ISP so that they can fix their problems.
+   The next line often contains a URL that can be visited for more information.
+
+=head1 Environment Variables
+
+=head2 RBLSMTPD
+
+The environment variable RBLSMTPD is supported and mimics the behaviour of
+Dan Bernstein's rblsmtpd. The exception to this is the '-' char at the
+start of RBLSMTPD which is used to force a hard error in Dan's rblsmtpd.
+NB I don't really see the benefit
+of using a soft error for a site in an RBL list. This just complicates
+things as it takes 7 days (or whatever default period) before a user
+gets an error email back. In the meantime they are complaining that their
+emails are being "lost" :(
+
+=over 4
+
+=item RBLSMTPD is set and non-empty
+
+The contents are used as the SMTP conversation error.
+Use this for forcibly blocking sites you don't like
+
+=item RBLSMTPD is set, but empty
+
+In this case no RBL checks are made.
+This can be used for local addresses.
+
+=item RBLSMTPD is not set
+
+All RBL checks will be made.
+This is the setting for remote sites that you want to check against RBL.
+
+=back
+
+=head1 Revisions
+
+See: https://github.com/smtpd/qpsmtpd/commits/master/plugins/dnsbl
+
 =cut
 
 sub register {
@@ -20,9 +129,6 @@ sub register {
     else {
         $self->{_args} = { @_ };
     };
-
-    my $rej = $self->{_args}{reject_type};
-    $self->{_dnsbl}{DENY} = (defined $rej && $rej =~ /^disconnect$/i) ? DENY_DISCONNECT : DENY;
 }
 
 sub hook_connect {
@@ -207,7 +313,7 @@ sub hook_rcpt {
     $result =~ s/%IP%/$remote_ip/g;
     my $msg =  $self->qp->config('dnsbl_rejectmsg');
     $self->log(LOGINFO, "fail: $msg");
-    return ($self->{_dnsbl}->{DENY}, join(' ', $msg, $result));
+    return ( $self->get_reject_type(), join(' ', $msg, $result));
   }
 
     my $note = $self->process_sockets or return DECLINED;
@@ -222,7 +328,7 @@ sub hook_rcpt {
     }
 
     $self->log(LOGINFO, 'fail');
-    return ($self->{_dnsbl}->{DENY}, $note);
+    return ( $self->get_reject_type(), $note);
 }
 
 sub hook_disconnect {
@@ -233,114 +339,11 @@ sub hook_disconnect {
   return DECLINED;
 }
 
-=head1 USAGE
+sub get_reject_type {
+    my $self = shift;
 
-Add the following line to the config/plugins file:
+    return   $self->{_args}{reject_type} eq 'temp'       ? DENYSOFT
+           : $self->{_args}{reject_type} eq 'disconnect' ? DENY_DISCONNECT
+           : DENY;
+};
 
-  dnsbl [ reject_type disconnect ] [loglevel -1]
-
-=head2 reject_type
-
-To immediately drop the connection (since some blacklisted servers attempt
-multiple sends per session), set the optional argument I<reject_type> to
-"disconnect" on the config/plugin entry. In most cases, an
-IP address that is listed should not be given the opportunity to begin
-a new transaction, since even the most volatile blacklists will return
-the same answer for a short period of time (the minimum DNS cache period).
-
-=head2 loglevel
-
-Adjust the quantity of logging for this plugin. See docs/logging.pod
-
-=head1 CONFIG FILES
-
-=over 4
-
-This plugin uses the following configuration files. All are optional. Not
-specifying dnsbl_zones is like not using the plugin at all.
-
-=item dnsbl_zones
-
-Normal ip based dns blocking lists ("RBLs") which contain TXT records are
-specified simply as:
-
-  relays.ordb.org
-  spamsources.fabel.dk
-
-To configure RBL services which do not contain TXT records in the DNS,
-but only A records (e.g. the RBL+ at http://www.mail-abuse.org), specify your
-own error message to return in the SMTP conversation after a colon e.g.
-
-  rbl-plus.mail-abuse.org:You are listed at - http://http://www.mail-abuse.org/cgi-bin/lookup?%IP%
-
-The string %IP% will be replaced with the IP address of incoming connection.
-Thus a fully specified file could be:
-
-  sbl-xbl.spamhaus.org
-  list.dsbl.org
-  rbl-plus.mail-abuse.ja.net:Listed by rbl-plus.mail-abuse.ja.net - see <URL:http://www.mail-abuse.org/cgi-bin/lookup?%IP%>
-  relays.ordb.org
-
-=item dnsbl_allow
-
-List of allowed ip addresses that bypass RBL checking. Format is one entry per line,
-with either a full IP address or a truncated IP address with a period at the end.
-For example:
-
-  192.168.1.1
-  172.16.33.
-
-NB the environment variable RBLSMTPD is considered before this file is
-referenced. See below.
-
-=item dnsbl_rejectmsg
-
-A textual message that is sent to the sender on an RBL failure. The TXT record
-from the RBL list is also sent, but this file can be used to indicate what
-action the sender should take.
-
-For example:
-
-   If you think you have been blocked in error, then please forward
-   this entire error message to your ISP so that they can fix their problems.
-   The next line often contains a URL that can be visited for more information.
-
-=back
-
-=head1 Environment Variables
-
-=head2 RBLSMTPD
-
-The environment variable RBLSMTPD is supported and mimics the behaviour of
-Dan Bernstein's rblsmtpd. The exception to this is the '-' char at the
-start of RBLSMTPD which is used to force a hard error in Dan's rblsmtpd.
-NB I don't really see the benefit
-of using a soft error for a site in an RBL list. This just complicates
-things as it takes 7 days (or whatever default period) before a user
-gets an error email back. In the meantime they are complaining that their
-emails are being "lost" :(
-
-=over 4
-
-=item RBLSMTPD is set and non-empty
-
-The contents are used as the SMTP conversation error.
-Use this for forcibly blocking sites you don't like
-
-=item RBLSMTPD is set, but empty
-
-In this case no RBL checks are made.
-This can be used for local addresses.
-
-=item RBLSMTPD is not set
-
-All RBL checks will be made.
-This is the setting for remote sites that you want to check against RBL.
-
-=back
-
-=head1 Revisions
-
-See: http://cvs.perl.org/viewcvs/qpsmtpd/plugins/dnsbl
-
-=cut

t/plugin_tests/dnsbl

@@ -13,6 +13,7 @@ sub register_tests {
     $self->register_test('test_ip_whitelisted', 3);
     $self->register_test('test_is_set_rblsmtpd', 4);
     $self->register_test('test_hook_disconnect', 1);
+    $self->register_test('test_reject_type', 3);
 }
 
 sub test_ip_whitelisted {
@@ -21,13 +22,13 @@ sub test_ip_whitelisted {
     $self->qp->connection->remote_ip('10.1.1.1');
 
     $self->qp->connection->relay_client(1);
-    ok( $self->ip_whitelisted('10.1.1.1'), "ip_whitelisted, +");
+    ok( $self->ip_whitelisted('10.1.1.1'), "yes, +");
 
     $self->qp->connection->relay_client(0);
-    ok( ! $self->ip_whitelisted('10.1.1.1'), "ip_whitelisted, -");
+    ok( ! $self->ip_whitelisted('10.1.1.1'), "no, -");
 
     $self->qp->connection->notes('whitelisthost', 'hello honey!');
-    ok( $self->ip_whitelisted('10.1.1.1'), "ip_whitelisted, +");
+    ok( $self->ip_whitelisted('10.1.1.1'), "yes, +");
     $self->qp->connection->notes('whitelisthost', undef);
 };
 
@@ -35,29 +36,30 @@ sub test_is_set_rblsmtpd {
     my $self = shift;
 
     $self->qp->connection->remote_ip('10.1.1.1');
-    ok( ! defined $self->is_set_rblsmtpd('10.1.1.1'), "is_set_rblsmtpd, undef");
+    ok( ! defined $self->is_set_rblsmtpd('10.1.1.1'), "undef");
 
     $ENV{RBLSMTPD} = "Yes we can!";
-    cmp_ok( 'Yes we can!','eq',$self->is_set_rblsmtpd('10.1.1.1'), "is_set_rblsmtpd, value");
+    cmp_ok( 'Yes we can!','eq',$self->is_set_rblsmtpd('10.1.1.1'), "value");
 
     $ENV{RBLSMTPD} = "Oh yeah?";
-    cmp_ok( 'Oh yeah?','eq',$self->is_set_rblsmtpd('10.1.1.1'), "is_set_rblsmtpd, value");
+    cmp_ok( 'Oh yeah?','eq',$self->is_set_rblsmtpd('10.1.1.1'), "value");
 
     $ENV{RBLSMTPD} = '';
-    cmp_ok( 1,'==',$self->is_set_rblsmtpd('10.1.1.1'), "is_set_rblsmtpd, empty");
+    cmp_ok( 1,'==',$self->is_set_rblsmtpd('10.1.1.1'), "empty");
 };
 
 sub test_hook_connect {
     my $self = shift;
 
     my $connection = $self->qp->connection;
+    $connection->relay_client(0);        # other tests may leave it enabled
     $connection->remote_ip('127.0.0.2'); # standard dnsbl test value
 
     cmp_ok( DECLINED, '==', $self->hook_connect($self->qp->transaction),
-        "hook_connect +");
+        "connect +");
 
-    ok($connection->notes('dnsbl_sockets'), "hook_connect, sockets");
+    ok($connection->notes('dnsbl_sockets'), "sockets +");
-    ok($connection->notes('dnsbl_domains'), "hook_connect, domains");
+    ok($connection->notes('dnsbl_domains'), "domains +");
 }
 
 sub test_hook_rcpt {
@@ -75,3 +77,15 @@ sub test_hook_disconnect {
         "hook_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");
+};