Merge pull request #95 from msimerson/helo

helo: make NXDOMAIN time out faster

lib/Qpsmtpd/Utils.pm

@@ -1,6 +1,8 @@
 package Qpsmtpd::Utils;
 use strict;
 
+use Net::IP;
+
 sub tildeexp {
     my ($self, $path) = @_;
     $path =~ s{^~([^/]*)} {  
@@ -20,4 +22,18 @@ sub is_localhost {
     return;
 }
 
+sub is_valid_ip {
+    my ($self, $ip) = @_;
+
+    if (Net::IP::ip_is_ipv4($ip)) {
+        return if $ip eq '0.0.0.0';
+        return if $ip eq '255.255.255.255';
+        return if $ip =~ /255/;
+        return 1;
+    };
+    return 1 if Net::IP::ip_is_ipv6($ip);
+
+    return;
+}
+
 1;

plugins/helo

@@ -227,7 +227,10 @@ additional check ideas from Haraka helo plugin
 use strict;
 use warnings;
 
+use Net::IP;
+
 use Qpsmtpd::Constants;
+use Qpsmtpd::Utils;
 
 sub register {
     my ($self, $qp) = (shift, shift);
@@ -354,8 +357,7 @@ sub invalid_localhost {
 
 sub is_plain_ip {
     my ($self, $host) = @_;
-    return if $host =~ /[^\d\.]+/;    # has chars other than digits and a dot
+    return if !Qpsmtpd::Utils->is_valid_ip($host);
-    return if $host !~ m/^(\d{1,3}\.){3}\d{1,3}$/;
 
     $self->log(LOGDEBUG, "fail, plain IP");
     return ("Plain IP is invalid HELO hostname (RFC 2821)", "plain IP");
@@ -363,7 +365,11 @@ sub is_plain_ip {
 
 sub is_address_literal {
     my ($self, $host) = @_;
-    return if $host !~ m/^\[(\d{1,3}\.){3}\d{1,3}\]$/;
+
+    my ($ip) = $host =~ /^\[(.*)\]/;  # strip off any brackets
+    return if !$ip;   # no brackets, not a literal
+
+    return if !Qpsmtpd::Utils->is_valid_ip($ip);
 
     $self->log(LOGDEBUG, "fail, bracketed IP");
     return ("RFC 2821 allows an address literal, but we do not",
@@ -372,9 +378,9 @@ sub is_address_literal {
 
 sub is_forged_literal {
     my ($self, $host) = @_;
-    return if $host !~ m/^\[(\d{1,3}\.){3}\d{1,3}\]$/;
+    return if !Qpsmtpd::Utils->is_valid_ip($host);
 
-  # should we add exceptions for reserved internal IP space? (192.168,10., etc?)
+  # should we add exceptions for reserved internal IP space? (192.168,10., etc)
     $host = substr $host, 1, -1;
     return if $host eq $self->qp->connection->remote_ip;
     return ("Forged IPs not accepted here", "forged IP literal");
@@ -401,7 +407,7 @@ sub no_forward_dns {
     my $res = $self->init_resolver();
 
     $host = "$host." if $host !~ /\.$/;    # fully qualify name
-    my $query = $res->search($host);
+    my $query = $res->query($host);
 
     if (!$query) {
         if ($res->errorstring eq 'NXDOMAIN') {
@@ -486,15 +492,25 @@ sub check_ip_match {
     my $self = shift;
     my $ip = shift or return;
 
-    if ($ip eq $self->qp->connection->remote_ip) {
+    my $rip = $self->qp->connection->remote_ip;
+    if ($ip eq $rip) {
         $self->log(LOGDEBUG, "forward ip match");
         $self->connection->notes('helo_forward_match', 1);
         return;
     }
 
-    my $dns_net = join('.', (split(/\./, $ip))[0, 1, 2]);
+    my ($dns_net, $rem_net);
-    my $rem_net =
+    if ($ip =~ /:/) {
-      join('.', (split(/\./, $self->qp->connection->remote_ip))[0, 1, 2]);
+        if ($ip  =~ /::/) { $ip  = Net::IP::ip_expand_address($ip,  6); }
+        if ($rip =~ /::/) { $rip = Net::IP::ip_expand_address($rip, 6); }
+
+        $dns_net = join(':', (split(/:/, $ip ))[0, 1, 2, 3, 4, 5]);
+        $rem_net = join(':', (split(/:/, $rip))[0, 1, 2, 3, 4, 5]);
+    }
+    else {
+        $dns_net = join('.', (split(/\./, $ip))[0, 1, 2]);
+        $rem_net = join('.', (split(/\./, $rip))[0, 1, 2]);
+    }
 
     if ($dns_net eq $rem_net) {
         $self->log(LOGNOTICE, "forward network match");

plugins/resolvable_fromhost

@@ -183,13 +183,12 @@ sub check_dns {
 
 sub ip_is_valid {
     my ($self, $ip) = @_;
-    my ($net, $mask);
     ### while (($net,$mask) = each %invalid) {
     ###      ... does NOT reset to beginning, will start on
     ###      2nd invocation after where it denied the first time..., so
     ###      2nd time the same "MAIL FROM" would be accepted!
-    foreach $net (keys %invalid) {
+    foreach my $net (keys %invalid) {
-        $mask = $invalid{$net};
+        my $mask = $invalid{$net};
         $mask = pack "B32", "1" x ($mask) . "0" x (32 - $mask);
         return if $net eq join('.', unpack("C4", inet_aton($ip) & $mask));
     }

t/plugin_tests/helo

@@ -18,7 +18,7 @@ sub register_tests {
     $self->register_test('test_no_reverse_dns', 3);
     $self->register_test('test_no_matching_dns', 2);
     $self->register_test('test_helo_handler', 1);
-    $self->register_test('test_check_ip_match', 3);
+    $self->register_test('test_check_ip_match', 6);
     $self->register_test('test_check_name_match', 3);
 }
 
@@ -82,26 +82,26 @@ sub test_invalid_localhost {
 sub test_is_plain_ip {
     my $self = shift;
 
-    my ($err, $why) = $self->is_plain_ip('0.0.0.0');
+    my ($err, $why) = $self->is_plain_ip('1.0.0.0');
     ok( $err, "plain IP, $why");
 
-    ($err, $why) = $self->is_plain_ip('255.255.255.255');
+    ($err, $why) = $self->is_plain_ip('254.254.254.254');
     ok( $err, "plain IP, $why");
 
-    ($err, $why) = $self->is_plain_ip('[255.255.255.255]');
+    ($err, $why) = $self->is_plain_ip('[254.254.254.254]');
     ok( ! $err, "address literal");
 };
 
 sub test_is_address_literal {
     my $self = shift;
 
-    my ($err, $why) = $self->is_address_literal('[0.0.0.0]');
+    my ($err, $why) = $self->is_address_literal('[1.0.0.0]');
     ok( $err, "plain IP, $why");
 
-    ($err, $why) = $self->is_address_literal('[255.255.255.255]');
+    ($err, $why) = $self->is_address_literal('[254.254.254.254]');
     ok( $err, "plain IP, $why");
 
-    ($err, $why) = $self->is_address_literal('255.255.255.255');
+    ($err, $why) = $self->is_address_literal('254.254.254.254');
     ok( ! $err, "address literal");
 };
 
@@ -112,8 +112,8 @@ sub test_no_forward_dns {
     ok( ! $err, "perl.org");
 
     # reserved .test TLD: http://tools.ietf.org/html/rfc2606
-    ($err, $why) = $self->no_forward_dns('perl.org.test');
+    ($err, $why) = $self->no_forward_dns('perl.test');
-    ok( $err, "test.perl.org.test");
+    ok( $err, "perl.test");
 };
 
 sub test_no_reverse_dns {
@@ -125,8 +125,8 @@ sub test_no_reverse_dns {
     ($err, $why) = $self->no_reverse_dns('test-host', '192.0.2.1');
     ok( $err, "192.0.2.1, $why");
 
-    ($err, $why) = $self->no_reverse_dns('mail.theartfarm.com', '208.75.177.101');
+    ($err, $why) = $self->no_reverse_dns('mail.theartfarm.com', '66.128.51.165');
-    ok( ! $err, "208.75.177.101");
+    ok( ! $err, "66.128.51.165");
 };
 
 sub test_no_matching_dns {
@@ -146,19 +146,32 @@ sub test_no_matching_dns {
 sub test_check_ip_match {
     my $self = shift;
 
-    $self->qp->connection->remote_ip('192.0.2.1');
+    my @good_tests = (
+        { ip => '192.0.2.1',   ip2 => '192.0.2.1',   r => 'exact'   },
+        { ip => '192.0.2.1',   ip2 => '192.0.2.2',   r => 'network' },
+        { ip => '2001:db8::1', ip2 => '2001:db8::1', r => 'exact'   },
+        { ip => '2001:db8::1', ip2 => '2001:db8::2', r => 'network' },
+    );
 
-    $self->connection->notes('helo_forward_match', 0);
+    my @bad_tests = (
-    $self->check_ip_match('192.0.2.1');
+        { ip => '192.0.2.1',   ip2 => '192.0.1.1',   r => 'miss'   },
-    ok( $self->connection->notes('helo_forward_match'), "exact");
+        { ip => '2001:db8::1', ip2 => '2001:db7::1', r => 'miss'   },
+    );
 
-    $self->connection->notes('helo_forward_match', 0);
+    foreach my $t ( @good_tests ) {
-    $self->check_ip_match('192.0.2.2');
+        $self->qp->connection->remote_ip($t->{ip});
-    ok( $self->connection->notes('helo_forward_match'), "network");
+        $self->connection->notes('helo_forward_match', 0);
+        $self->check_ip_match($t->{ip2});
+        ok( $self->connection->notes('helo_forward_match'), $t->{r});
+    };
 
-    $self->connection->notes('helo_forward_match', 0);
+    foreach my $t ( @bad_tests ) {
-    $self->check_ip_match('192.0.1.1');
+        $self->qp->connection->remote_ip($t->{ip});
-    ok( ! $self->connection->notes('helo_forward_match'), "miss");
+
+        $self->connection->notes('helo_forward_match', 0);
+        $self->check_ip_match($t->{ip2});
+        ok( ! $self->connection->notes('helo_forward_match'), $t->{r});
+    };
 };
 
 sub test_check_name_match {

t/qpsmtpd-utils.t

@@ -12,9 +12,22 @@ my $utils = bless {}, 'Qpsmtpd::Utils';
 
 __tildeexp();
 __is_localhost();
+__is_valid_ip();
 
 done_testing();
 
+sub __is_valid_ip {
+    my @good = qw/ 1.2.3.4 1.0.0.0 254.254.254.254 2001:db8:ffff:ffff:ffff:ffff:ffff:ffff /;
+    foreach my $ip ( @good ) {
+        ok( $utils->is_valid_ip($ip), "is_valid_ip: $ip");
+    }
+
+    my @bad = qw/ 1.2.3.256 256.1.1.1 2001:db8:ffff:ffff:ffff:ffff:ffff:fffj /;
+    foreach my $ip ( @bad ) {
+        ok( !$utils->is_valid_ip($ip), "is_valid_ip, neg: $ip");
+    }
+};
+
 sub __is_localhost {
 
     for my $local_ip (qw/ 127.0.0.1 ::1 2607:f060:b008:feed::127.0.0.1 127.0.0.2 /) {