moved triplicated init_resolver into Plugin.pm

lib/Qpsmtpd/Plugin.pm

@@ -3,6 +3,8 @@ package Qpsmtpd::Plugin;
 use strict;
 use warnings;
 
+use Net::DNS;
+
 use Qpsmtpd::Constants;
 
 # more or less in the order they will fire
@@ -263,6 +265,17 @@ sub store_deferred_reject {
     return (DECLINED);
 };
 
+sub init_resolver {
+    my $self = shift;
+    return $self->{_resolver} if $self->{_resolver};
+    $self->log( LOGDEBUG, "initializing Net::DNS::Resolver");
+    $self->{_resolver} = Net::DNS::Resolver->new(dnsrch => 0);
+    my $timeout = $self->{_args}{dns_timeout} || 5;
+    $self->{_resolver}->tcp_timeout($timeout);
+    $self->{_resolver}->udp_timeout($timeout);
+    return $self->{_resolver};
+};
+
 sub is_immune {
     my $self = shift;
 

plugins/fcrdns

@@ -100,8 +100,6 @@ use warnings;
 
 use Qpsmtpd::Constants;
 
-use Net::DNS;
-
 sub register {
     my ($self, $qp) = (shift, shift);
     $self->{_args} = { @_ };
@@ -113,7 +111,7 @@ sub register {
         $self->{_args}{reject} = 0;
     };
 
-    $self->init_resolver();
+    $self->init_resolver() or return;
 
     $self->register_hook('connect',   'connect_handler');
     $self->register_hook('data_post', 'data_post_handler');
@@ -144,17 +142,6 @@ sub data_post_handler {
     return (DECLINED);
 };
 
-sub init_resolver {
-    my $self = shift;
-    return $self->{_resolver} if $self->{_resolver};
-    $self->log( LOGDEBUG, "initializing Net::DNS::Resolver");
-    $self->{_resolver} = Net::DNS::Resolver->new(dnsrch => 0);
-    my $timeout = $self->{_args}{timeout} || 5;
-    $self->{_resolver}->tcp_timeout($timeout);
-    $self->{_resolver}->udp_timeout($timeout);
-    return $self->{_resolver};
-};
-
 sub invalid_localhost {
     my ( $self ) = @_;
     return 1 if lc $self->qp->connection->remote_host ne 'localhost';

plugins/helo

@@ -109,7 +109,7 @@ Default: lenient
 
 =head3 lenient
 
-Reject failures of the following tests: is_in_badhelo, invalid_localhost,
+Runs the following tests: is_in_badhelo, invalid_localhost,
 is_forged_literal, and is_plain_ip.
 
 This setting is lenient enough not to cause problems for your Windows users.
@@ -121,11 +121,11 @@ IPs.
 
 Per RFC 2821, the HELO hostname is the FQDN of the sending server or an
 address literal. When I<policy rfc> is selected, all the lenient checks and
-the following are enforced: is_not_fqdn, no_forward_dns, and no_reverse_dns.
+the following are tested: is_not_fqdn, no_forward_dns, and no_reverse_dns.
 
 If you have Windows users that send mail via your server, do not choose
-I<policy rfc> without setting I<reject naughty> and using the B<naughty>
+I<policy rfc> without setting I<reject> to 0 or naughty.
-plugin. Windows PCs often send unqualified HELO names and will have trouble
+Windows PCs often send unqualified HELO names and will have trouble
 sending mail. The B<naughty> plugin defers the rejection, giving the user
 the opportunity to authenticate and bypass the rejection.
 
@@ -138,7 +138,7 @@ I have yet to see an address literal being used by a hammy sender. But I am
 not certain that blocking them all is prudent.
 
 It is recommended that I<policy strict> be used with <reject 0> and that you
-monitor your logs for false positives before enabling rejection.
+examine your logs for false positives.
 
 =head2 badhelo
 
@@ -223,21 +223,19 @@ use warnings;
 
 use Qpsmtpd::Constants;
 
-use Net::DNS;
-
 sub register {
     my ($self, $qp) = (shift, shift);
     $self->{_args} = { @_ };
+
     $self->{_args}{reject_type} = 'disconnect';
     $self->{_args}{policy} ||= 'lenient';
-    $self->{_args}{timeout} ||= 5;
+    $self->{_args}{dns_timeout} ||= $self->{_args}{timeout} || 5;
 
     if ( ! defined $self->{_args}{reject} ) {
         $self->{_args}{reject} = 1;
     };
-
     $self->populate_tests();
-    $self->init_resolver();
+    $self->init_resolver() or return;
 
     $self->register_hook('helo', 'helo_handler');
     $self->register_hook('ehlo', 'helo_handler');
@@ -290,17 +288,6 @@ sub populate_tests {
     };
 };
 
-sub init_resolver {
-    my $self = shift;
-    return $self->{_resolver} if $self->{_resolver};
-    $self->log( LOGDEBUG, "initializing Net::DNS::Resolver");
-    $self->{_resolver} = Net::DNS::Resolver->new(dnsrch => 0);
-    my $timeout = $self->{_args}{timeout} || 5;
-    $self->{_resolver}->tcp_timeout($timeout);
-    $self->{_resolver}->udp_timeout($timeout);
-    return $self->{_resolver};
-};
-
 sub is_in_badhelo {
     my ( $self, $host ) = @_;
 
@@ -451,8 +438,10 @@ sub no_reverse_dns {
 sub no_matching_dns {
     my ( $self, $host ) = @_;
 
-# this is called iprev, or "Forward-confirmed reverse DNS" and is discussed in RFC 5451
+# this is called iprev, or "Forward-confirmed reverse DNS" and is discussed
-# consider adding header: Authentication-Results
+# in RFC 5451. FCrDNS is done for the remote IP in the fcrdns plugin. Here
+# we do it on the HELO hostname.
+# consider adding status to Authentication-Results header
 
     if ( $self->connection->notes('helo_forward_match') &&
          $self->connection->notes('helo_reverse_match') ) {

plugins/rhsbl

@@ -33,26 +33,28 @@ use Qpsmtpd::Constants;
 sub register {
     my ($self, $qp ) = (shift, shift);
 
-    my $denial;
     if ( @_ == 1 ) {
-        $denial = shift;
+        $self->legacy_positional_args( @_ );
-        if ( defined $denial && $denial =~ /^disconnect$/i ) {
-            $self->{_args}{reject_type} = 'disconnect';
-        }
-        else {
-            $self->{_args}{reject_type} = 'perm';
-        }
     }
     else {
         $self->{_args} = { @_ };
     };
 
-    if ( ! defined $self->{_args}{reject}  ) {
+    $self->{_args}{reject} = 1 if ! defined $self->{_args}{reject};
-        $self->{_args}{reject} = 1;
-    };
     $self->{_args}{reject_type} ||= 'perm';
 }
 
+sub legacy_positional_args {
+    my ($self, $denial) = @_;
+
+    if ( defined $denial && $denial =~ /^disconnect$/i ) {
+        $self->{_args}{reject_type} = 'disconnect';
+    }
+    else {
+        $self->{_args}{reject_type} = 'perm';
+    }
+};
+
 sub hook_mail {
     my ($self, $transaction, $sender, %param) = @_;
 
@@ -135,14 +137,3 @@ sub populate_zones {
     return %rhsbl_zones;
 };
 
-sub init_resolver {
-    my $self = shift;
-    return $self->{_resolver} if $self->{_resolver};
-    $self->log( LOGDEBUG, "initializing Net::DNS::Resolver");
-    $self->{_resolver} = Net::DNS::Resolver->new(dnsrch => 0);
-    my $timeout = $self->{_args}{timeout} || 8;
-    $self->{_resolver}->tcp_timeout($timeout);
-    $self->{_resolver}->udp_timeout($timeout);
-    return $self->{_resolver};
-};
-