fcrdns: add tests and improved localhost detection

plugins/fcrdns

@@ -141,22 +141,20 @@ sub register {
         $self->{_args}{reject} = 0;
     }
 
-    $self->register_hook('connect',   'connect_handler');
+    $self->register_hook('connect', 'fcrdns_tests');
 }
 
-sub connect_handler {
+sub fcrdns_tests {
     my ($self) = @_;
 
     return DECLINED if $self->is_immune();
 
-    # run a couple cheap tests before the more expensive DNS tests
+    # run cheap tests before the more expensive DNS tests
-    foreach my $test (qw/ is_valid_localhost is_not_fqdn /) {
+    foreach my $test (
+        qw/ is_valid_localhost is_fqdn has_reverse_dns has_forward_dns / ) {
         $self->$test() or return DECLINED;
     }
 
-    $self->has_reverse_dns() or return DECLINED;
-    $self->has_forward_dns() or return DECLINED;
-
     $self->log(LOGINFO, "pass");
     return DECLINED;
 }
@@ -168,35 +166,35 @@ sub is_valid_localhost {
         $self->adjust_karma(1);
         $self->log(LOGDEBUG, "pass, is localhost");
         return 1;
-    };
+    }
 
     my $rh = $self->qp->connection->remote_host;
-    if ($rh && lc $self->qp->connection->remote_host eq 'localhost') {
+    return 0 if ! $rh;
-        $self->log(LOGDEBUG, "pass, remote_host is localhost");
+    return 0 if lc $self->qp->connection->remote_host ne 'localhost';
-        return 1;
-    };
 
     $self->adjust_karma(-1);
     $self->log(LOGINFO, "fail, not localhost");
-    return;
+    return 0;
 }
 
-sub is_not_fqdn {
+sub is_fqdn {
     my ($self) = @_;
-    my $host = $self->qp->connection->remote_host or return 1;
+    my $host = $self->qp->connection->remote_host or return 0;
-    return 1 if $host eq 'Unknown';    # QP assigns this to a "no DNS result"
+    return 0 if $host eq 'Unknown';    # QP assigns this to a "no DNS result"
 
     # Since QP looked it up, perform some quick validation
     if ($host !~ /\./) {               # has no dots
         $self->adjust_karma(-1);
         $self->log(LOGINFO, "fail, not FQDN");
-        return;
+        return 0;
     }
     if ($host =~ /[^a-zA-Z0-9\-\.]/) {
         $self->adjust_karma(-1);
         $self->log(LOGINFO, "fail, invalid FQDN chars");
-        return;
+        return 0;
     }
+
+    $self->log(LOGDEBUG, "pass, is FQDN");
     return 1;
 }
 

t/config/plugins

@@ -20,6 +20,7 @@ hosts_allow
 ident/geoip
 ident/p0f /tmp/.p0f_socket version 3
 connection_time
+fcrdns
 
 # enable to accept MAIL FROM:/RCPT TO: addresses without surrounding <>
 dont_require_anglebrackets

t/plugin_tests/fcrdns

@@ -0,0 +1,60 @@
+#!perl -w
+
+use strict;
+#use POSIX qw(strftime);
+
+#use Qpsmtpd::Address;
+use Qpsmtpd::Constants;
+
+my $test_email = 'matt@tnpi.net';
+
+sub register_tests {
+    my $self = shift;
+
+    $self->register_test('_is_valid_localhost');
+    $self->register_test('_is_fqdn');
+}
+
+
+sub _is_valid_localhost {
+    my $self = shift;
+    my @passes = qw/ 127.0.0.1 127.1.1.1 127.255.255.255 /;
+    my @fails  = qw/ 128.0.0.1 126.1.1.1 /;
+
+    foreach my $pass (@passes) {
+        $self->qp->connection->remote_ip($pass);
+        cmp_ok( $self->is_valid_localhost(), '==', 1, "$pass, true");
+    }
+
+    foreach my $fail (@fails) {
+        $self->qp->connection->remote_ip($fail);
+        cmp_ok( $self->is_valid_localhost(), '==', 0, "$fail, false");
+    }
+
+    $self->qp->connection->remote_host('localhost');
+    cmp_ok( $self->is_valid_localhost(), '==', 0, "localhost, non-loopback IP, false");
+
+    $self->qp->connection->remote_ip('127.0.0.1');
+    $self->qp->connection->remote_host('localhost');
+    cmp_ok( $self->is_valid_localhost(), '==', 1, "localhost, loop IP, true");
+
+    $self->qp->connection->remote_ip('::1');
+    $self->qp->connection->remote_host('localhost');
+    cmp_ok( $self->is_valid_localhost(), '==', 1, "localhost, IPv6 loop, true");
+}
+
+sub _is_fqdn {
+    my $self = shift;
+    my @passes = qw/ foo.com example.com /;
+    my @fails = qw/ com net edu boogers /;
+
+    foreach my $pass (@passes) {
+        $self->qp->connection->remote_host($pass);
+        cmp_ok( $self->is_fqdn(), '==', 1, "$pass, true");
+    }
+
+    foreach my $fail (@fails) {
+        $self->qp->connection->remote_host($fail);
+        cmp_ok( $self->is_fqdn(), '==', 0, "$fail, false");
+    }
+}