On Nov 13, 2012, at 5:49 PM, Michael Holzt <mich...@holzt.de> wrote:

> The sender_permitted_from plugin will hang in an endless loop for
> IPv6 clients. This is because of faulty code which only knows how
> to handle IPv4 addresses.
> 
> The attached patch fixes this problem.
> 
> Regards,
> Michael


Tis true, tis true. 

The existing SPF plugin is IPv6 broken, and your patch does make it less 
broken. Which is good, but it left behind a few IPv6 issues. Namely:

1. whacking off 16 bits at a time makes for a very inflexible configuration
2. no support for compressed IPv6 addresses 
        Can we be certain that $client_ip will always be expanded?
3. after an IPv6 address has had it's last : removed, it's no longer recognized 
(within the loop) as IPv6. That is likely to create future problems.

With that in mind, I made the following changes to qpsmtpd-dev and committed 
them. It checks for compressed IPs and expands them if necessary. It does the 
IPv6 check outside the while loop, so chopping the address won't cause 
surprising behavior. It strips off a nibble at a time instead of a group. This 
allows much finer control over relayclients.

https://github.com/msimerson/qpsmtpd-dev/commit/d1bb2d949ba4f570e087941820eb0baddc18ed23

Matt


--- a/plugins/sender_permitted_from
+++ b/plugins/sender_permitted_from
@@ -59,6 +59,8 @@ use warnings;
 #use Mail::SPF 2.000;   # eval'ed in ->register
 use Qpsmtpd::Constants;
 
+use Net::IP;
+
 sub register {
     my ($self, $qp, %args) = @_;
     eval 'use Mail::SPF';
@@ -237,13 +239,27 @@ sub is_in_relayclients {
     my $more_relay_clients = $self->qp->config('morerelayclients', 'map');
     my %relay_clients      = map { $_ => 1 } @relay_clients;
 
+    my $ipv6 = $client_ip =~ /:/ ? 1 : 0;
+
+    if ( $ipv6 && $client_ip =~ /::/ ) {  # IPv6 compressed notation
+        $client_ip = Net::IP::ip_expand_address($client_ip,6);
+    };
+
     while ($client_ip) {
         if ( exists $relay_clients{$client_ip} ||
              exists $more_relay_clients->{$client_ip} ) {
             $self->log( LOGDEBUG, "skip, IP in relayclients" );
             return 1;
         };
-        $client_ip =~ s/\d+\.?$// or last;   # strip off another 8 bits
+
+        # added IPv6 support (Michael Holzt - 2012-11-14)
+        if ( $ipv6 ) {
+            $client_ip =~ s/[0-9a-f]:*$//;       # strip off another nibble
+            chop $client_ip if ':' eq substr($client_ip, -1, 1);
+        }
+        else {
+            $client_ip =~ s/\d+\.?$// or last;   # strip off another 8 bits
+        }
     }
     return;
 };


Reply via email to