From 296219c6039390a839456eba5a95db8e4374c706 Mon Sep 17 00:00:00 2001
From: Matt Simerson <matt@tnpi.net>
Date: Sun, 4 Jan 2015 01:49:38 -0500
Subject: [PATCH] fcrdns: add tests and improved localhost detection

---
 plugins/fcrdns        | 34 ++++++++++++------------
 t/config/plugins      |  1 +
 t/plugin_tests/fcrdns | 60 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 18 deletions(-)
 create mode 100644 t/plugin_tests/fcrdns

diff --git a/plugins/fcrdns b/plugins/fcrdns
index 18df058..fd5c564 100644
--- a/plugins/fcrdns
+++ b/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
-    foreach my $test (qw/ is_valid_localhost is_not_fqdn /) {
+    # run cheap tests before the more expensive DNS tests
+    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') {
-        $self->log(LOGDEBUG, "pass, remote_host is localhost");
-        return 1;
-    };
+    return 0 if ! $rh;
+    return 0 if lc $self->qp->connection->remote_host ne 'localhost';
 
     $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;
-    return 1 if $host eq 'Unknown';    # QP assigns this to a "no DNS result"
+    my $host = $self->qp->connection->remote_host or return 0;
+    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;
 }
 
diff --git a/t/config/plugins b/t/config/plugins
index d2f9b25..4e85186 100644
--- a/t/config/plugins
+++ b/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
diff --git a/t/plugin_tests/fcrdns b/t/plugin_tests/fcrdns
new file mode 100644
index 0000000..acd655e
--- /dev/null
+++ b/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");
+    }
+}