From ca96ddf4eb926b1d077b0b1b9a6e48a11345397d Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Thu, 11 Sep 2014 13:34:32 -0700 Subject: [PATCH] added Utils->is_valid_ip, IPv6 ready resolves Issue #82 --- lib/Qpsmtpd/Utils.pm | 16 +++++++++++++ plugins/helo | 34 ++++++++++++++++++++------- plugins/resolvable_fromhost | 5 ++-- t/plugin_tests/helo | 47 +++++++++++++++++++++++-------------- t/qpsmtpd-utils.t | 13 ++++++++++ 5 files changed, 86 insertions(+), 29 deletions(-) diff --git a/lib/Qpsmtpd/Utils.pm b/lib/Qpsmtpd/Utils.pm index edd8d4e..2aa3c0b 100644 --- a/lib/Qpsmtpd/Utils.pm +++ b/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; diff --git a/plugins/helo b/plugins/helo index d5e237d..92ac3eb 100644 --- a/plugins/helo +++ b/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 $host !~ m/^(\d{1,3}\.){3}\d{1,3}$/; + return if !Qpsmtpd::Utils->is_valid_ip($host); $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"); @@ -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 $rem_net = - join('.', (split(/\./, $self->qp->connection->remote_ip))[0, 1, 2]); + my ($dns_net, $rem_net); + if ($ip =~ /:/) { + 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"); diff --git a/plugins/resolvable_fromhost b/plugins/resolvable_fromhost index 9804705..d7824bf 100644 --- a/plugins/resolvable_fromhost +++ b/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) { - $mask = $invalid{$net}; + foreach my $net (keys %invalid) { + 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)); } diff --git a/t/plugin_tests/helo b/t/plugin_tests/helo index c2ac5f4..a9945a6 100644 --- a/t/plugin_tests/helo +++ b/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"); }; @@ -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); - $self->check_ip_match('192.0.2.1'); - ok( $self->connection->notes('helo_forward_match'), "exact"); + my @bad_tests = ( + { ip => '192.0.2.1', ip2 => '192.0.1.1', r => 'miss' }, + { ip => '2001:db8::1', ip2 => '2001:db7::1', r => 'miss' }, + ); - $self->connection->notes('helo_forward_match', 0); - $self->check_ip_match('192.0.2.2'); - ok( $self->connection->notes('helo_forward_match'), "network"); + foreach my $t ( @good_tests ) { + $self->qp->connection->remote_ip($t->{ip}); + $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); - $self->check_ip_match('192.0.1.1'); - ok( ! $self->connection->notes('helo_forward_match'), "miss"); + foreach my $t ( @bad_tests ) { + $self->qp->connection->remote_ip($t->{ip}); + + $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 { diff --git a/t/qpsmtpd-utils.t b/t/qpsmtpd-utils.t index 0d278b9..920c271 100644 --- a/t/qpsmtpd-utils.t +++ b/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 /) {