SPF plugin: refactored, tests, new config option

added POD description of spfquery note

changed spf_deny -> reject  (and offered 4 more options, see POD for reject)
	backwards compatible with old config settings
	replicates qmail-smtpd SPF patch behavior

improved logging (again)

uses a stringy eval 'use Mail::SPF' in the register sub. If missing, warn and log the error, and don't register any hooks. This is much nicer error than the current, "*** Remote host closed connection unexpectedly." broken mail server that results from enabling the SPF plugin without Mail::SPF installed.

background: I noticed I was deferring valid emails with the SPF plugin at 'spf_deny 1', and without changing the code, there wasn't a way to change how ~all records were handled. This provides that flexibility.

Changes

@@ -1,6 +1,8 @@
 
 Next Version
 
+  sender_permitted_from. see UPGRADING (Matt Simerson)
+
   dspam plugin added (Matt Simerson)
 
   p0f version 3 supported and new default. see UPGRADING (Matt Simerson)

UPGRADING

@@ -3,6 +3,8 @@ When upgrading from:
 
 v 0.84 or below
 
+SPF plugin: spf_deny setting deprecated. Use reject N setting instead, which provides administrators with more granular control over SPF. For backward compatibility, a spf_deny setting of 1 is mapped to 'reject 3' and a 'spf_deny 2' is mapped to 'reject 4'.
+
 p0f plugin: now defaults to p0f v3
 
 Upgrade p0f to version 3 or add 'version 2' to your p0f line in config/plugins. perldoc plugins/ident/p0f for more details.

plugins/sender_permitted_from

@@ -12,20 +12,41 @@ Prevents email sender address spoofing by checking the SPF policy of the purport
 
 Sender Policy Framework (SPF) is an e-mail validation system designed to prevent spam by addressing source address spoofing. SPF allows administrators to specify which hosts are allowed to send e-mail from a given domain by creating a specific SPF record in the public DNS. Mail exchangers then use the DNS to check that mail from a given domain is being sent by a host sanctioned by that domain's administrators. -- http://en.wikipedia.org/wiki/Sender_Policy_Framework
 
+The results of a SPF query are stored in a transaction note named 'spfquery';
+
 =head1 CONFIGURATION
 
 In config/plugins, add arguments to the sender_permitted_from line.
 
-  sender_permitted_from spf_deny 1
+  sender_permitted_from reject 3
 
-=head2 spf_deny
+=head2 reject
 
-Setting spf_deny to 0 will prevent emails from being rejected, even if they fail SPF checks. sfp_deny 1 is the default, and a reasonable setting. It temporarily defers connections (4xx) that have soft SFP failures and only rejects (5xx) messages when the sending domains policy suggests it. Settings spf_deny to 2 is more aggressive and will cause soft failures to be rejected permanently.
+Set to a value between 1 and 6 to enable the following SPF behaviors:
 
-See also http://spf.pobox.com/
+ 1 annotate-only, add Received-SPF header, no rejections.
+ 2 defer on DNS failures. Assure there's always a meaningful SPF header.
+ 3 rejected if SPF record says 'fail'
+ 4 stricter reject. Also rejects 'softfail'
+ 5 reject 'neutral'
+ 6 reject if no SPF records, or a syntax error
+
+Most sites should start at level 3. It temporarily defers connections (4xx) that have soft SFP failures and only rejects (5xx) messages when the sending domains policy suggests it.
+
+SPF levels above 4 are for crusaders who don't mind rejecting some valid mail when the sending server administrator hasn't dotted his i's and crossed his t's. May the deities bless theirobsessive little hearts.
+
+=head1 SEE ALSO
+
+ http://spf.pobox.com/
+ http://en.wikipedia.org/wiki/Sender_Policy_Framework
+
+=head1 ACKNOWLDGEMENTS
+
+The reject options are modeled after, and aim to match the functionality of those found in the SPF patch for qmail-smtpd.
 
 =head1 AUTHOR
 
+Matt Simerson - 2002 - increased policy options from 3 to 6
 Matt Simerson - 2011 - rewrote using Mail::SPF
 
 Matt Sergeant - 2003 - initial plugin
@@ -33,55 +54,57 @@ Matt Sergeant - 2003 - initial plugin
 =cut
 
 use strict;
-use Mail::SPF 2.000;
+use warnings;
+
+#use Mail::SPF 2.000;   # eval'ed in ->register
 use Qpsmtpd::Constants;
 
 sub register {
-    my ($self, $qp, @args) = @_;
+    my ($self, $qp, %args) = @_;
-    %{$self->{_args}} = @args;
+    eval "use Mail::SPF";
+    if ( $@ ) {
+        warn "skip: plugin disabled, could not find Mail::SPF\n";
+        $self->log(LOGERROR, "skip: plugin disabled, is Mail::SPF installed?");
+        return;
+    };
+    $self->{_args} = { %args };
+    if ( $self->{_args}{spf_deny} ) {
+        $self->{_args}{reject} = 3 if $self->{_args}{spf_deny} == 1;
+        $self->{_args}{reject} = 4 if $self->{_args}{spf_deny} == 2;
+    };
+    if ( ! $self->{_args}{reject} && $self->qp->config('spfbehavior') ) {
+        $self->{_args}{reject} = $self->qp->config('spfbehavior');
+    };
 }
 
 sub hook_mail {
     my ($self, $transaction, $sender, %param) = @_;
 
-    my $format    = $sender->format;
+    if ( ! $self->{_args}{reject} ) {
+        $self->log( LOGINFO, "skip: disabled in config" );
+        return (DECLINED);
+    };
+
+    my $format = $sender->format;
     if ( $format eq '<>' || ! $sender->host || ! $sender->user ) {
-        $self->log( LOGDEBUG, "pass: null sender" );
+        $self->log( LOGINFO, "skip: null sender" );
         return (DECLINED, "SPF - null sender");
     };
 
-    my $client_ip = $self->qp->connection->remote_ip;
+    if ( $self->is_relayclient() ) {
-    my $from      = $sender->user . '@' . lc($sender->host);
+        return (DECLINED, "SPF - relaying permitted");
-    my $helo      = $self->qp->connection->hello_host;
-
-    # If we are receiving from a relay permitted host, then we are probably
-    # not the delivery system, and so we shouldn't check
-    if ( $self->qp->connection->relay_client() ) {
-        $self->log( LOGDEBUG, "pass: relaying permitted (connection)" );
-        return (DECLINED, "SPF - relaying permitted")
     };
 
-    my @relay_clients      = $self->qp->config("relayclients");
+    my $client_ip  = $self->qp->connection->remote_ip;
-    my $more_relay_clients = $self->qp->config("morerelayclients", "map");
+    my $from       = $sender->user . '@' . lc($sender->host);
-    my %relay_clients      = map { $_ => 1 } @relay_clients;
+    my $helo       = $self->qp->connection->hello_host;
-    while ($client_ip) {
+    my $scope      = $from ? 'mfrom' : 'helo';
-        if ( exists $relay_clients{$client_ip} ||
+    my %req_params = ( versions   => [1, 2],        # optional
-             exists $more_relay_clients->{$client_ip} ) {
+                       scope      => $scope,
-            $self->log( LOGDEBUG, "pass: relaying permitted (config)" );
+                       ip_address => $client_ip,
-            return (DECLINED, "SPF - relaying permitted");
-        };
-        $client_ip =~ s/\d+\.?$//;    # strip off another 8 bits
-    }
-
-    my $scope = $from ? 'mfrom' : 'helo';
-    $client_ip = $self->qp->connection->remote_ip;
-    my %req_params = (
-        versions => [1, 2],           # optional
-        scope => $scope,
-        ip_address => $client_ip,
                      );
 
-    if ($scope =~ /mfrom|pra/) {
+    if ($scope =~ /^mfrom|pra$/) {
         $req_params{identity} = $from;
         $req_params{helo_identity} = $helo if $helo;
     }
@@ -95,44 +118,63 @@ sub hook_mail {
     my $result     = $spf_server->process($request);
 
     $transaction->notes('spfquery', $result);
-    $transaction->notes('spfcode', $result->code);
 
-    if ( $result->code eq 'pass' ) {    # this test passed
+    $self->log( LOGINFO, $result );
-        $self->log( LOGINFO, "pass" );
+
+    if ( $result->code eq 'pass' ) {
         return (OK);
     };
 
-    $self->log( LOGINFO, "fail: " . $result );
     return (DECLINED, "SPF - $result->code");
 }
 
 sub hook_rcpt {
     my ($self, $transaction, $rcpt, %param) = @_;
 
-    # special addresses don't get SPF-tested.
+    return DECLINED if $self->is_special_recipient( $rcpt );
-    return DECLINED
-      if $rcpt
-          and $rcpt->user
-          and $rcpt->user =~ /^(?:postmaster|abuse|mailer-daemon|root)$/i;
 
     my $result = $transaction->notes('spfquery') or return DECLINED;
     my $code   = $result->code;
     my $why    = $result->local_explanation;
-    my $deny   = $self->{_args}{spf_deny};
+    my $reject = $self->{_args}{reject};
 
-    return (DECLINED, "SPF - $code: $why")   if $code eq "pass";
+    if ( ! $code ) {
-    return (DECLINED, "SPF - $code, $why")   if !$deny;
+        return (DENYSOFT, "SPF - no response") if $reject >= 2;
-    return (DENYSOFT, "SPF - $code: $why")   if $code eq "error";
+        return (DECLINED, "SPF - no response");
-    return (DENY,     "SPF - forgery: $why") if $code eq 'fail';
+    };
 
-    if ($code eq "softfail") {
+    return (DECLINED, "SPF - $code: $why") if ! $reject;
-        return (DENY, "SPF probable forgery: $why") if $deny > 1;
+
-        return (DENYSOFT, "SPF probable forgery: $why");
+# SPF result codes: pass fail softfail neutral none error permerror temperror
+    if    ( $code eq 'pass' ) { }
+    elsif ( $code eq 'fail' ) {
+        return (DENY, "SPF - forgery: $why") if $reject >= 3;
+        return (DENYSOFT, "SPF - $code: $why") if $reject >= 2;
+    }
+    elsif ( $code eq 'softfail' ) {
+        return (DENY, "SPF - forgery: $why") if $reject >= 4;
+        return (DENYSOFT, "SPF - $code: $why") if $reject >= 3;
+    }
+    elsif ( $code eq 'neutral' ) {
+        return (DENY, "SPF - forgery: $why") if $reject >= 5;
+    }
+    elsif ( $code eq 'none' ) {
+        return (DENY, "SPF - forgery: $why") if $reject >= 6;
+    }
+    elsif ( $code eq 'error' ) {
+        return (DENY, "SPF - $code: $why") if $reject >= 6;
+        return (DENYSOFT, "SPF - $code: $why") if $reject >= 2;
+    }
+    elsif ( $code eq 'permerror' ) {
+        return (DENY, "SPF - $code: $why") if $reject >= 6;
+        return (DENYSOFT, "SPF - $code: $why") if $reject >= 2;
+    }
+    elsif ( $code eq 'temperror' ) {
+        return (DENYSOFT, "SPF - $code: $why") if $reject >= 2;
     }
 
     $self->log(LOGDEBUG, "result for $rcpt->address was $code: $why");
-
+    return (DECLINED, "SPF - $code: $why");
-    return (DECLINED, "SPF - $code, $why");
 }
 
 sub hook_data_post {
@@ -147,3 +189,49 @@ sub hook_data_post {
     return DECLINED;
 }
 
+sub is_relayclient {
+    my $self = shift;
+
+    # If we are receiving from a relay permitted host, then we are probably
+    # not the delivery system, and so we shouldn't check
+    if ( $self->qp->connection->relay_client() ) {
+        $self->log( LOGINFO, "skip: relaying permitted (relay_client)" );
+        return 1;
+    };
+
+    my $client_ip          = $self->qp->connection->remote_ip;
+    my @relay_clients      = $self->qp->config('relayclients');
+    my $more_relay_clients = $self->qp->config('morerelayclients', 'map');
+    my %relay_clients      = map { $_ => 1 } @relay_clients;
+
+    while ($client_ip) {
+        if ( exists $relay_clients{$client_ip} ||
+             exists $more_relay_clients->{$client_ip} ) {
+            $self->log( LOGDEBUG, "skip: relaying permitted (config)" );
+            return 1;
+        };
+        $client_ip =~ s/\d+\.?$// or last;   # strip off another 8 bits
+    }
+    return;
+};
+
+sub is_special_recipient {
+    my ($self, $rcpt) = @_;
+
+    if ( ! $rcpt ) {
+        $self->log(LOGINFO, "skip: missing recipient");
+        return 1;
+    };
+    if ( ! $rcpt->user ) {
+        $self->log(LOGINFO, "skip: missing user");
+        return 1;
+    };
+
+    # special addresses don't get SPF-tested.
+    if ( $rcpt->user =~ /^(?:postmaster|abuse|mailer-daemon|root)$/i ) {
+        $self->log(LOGINFO, "skip: special user (".$rcpt->user.")");
+        return 1;
+    };
+
+    return;
+};

t/plugin_tests/sender_permitted_from

@@ -0,0 +1,50 @@
+#!perl -w
+
+use strict;
+use warnings;
+
+use Qpsmtpd::Constants;
+
+my $r;
+
+sub register_tests {
+    my $self = shift;
+
+    eval 'use Mail::SPF';
+    return if $@;
+
+    $self->register_test('test_is_relayclient', 3);
+    $self->register_test('test_is_special_recipient', 5);
+}
+
+sub test_is_relayclient {
+    my $self = shift;
+
+    my $transaction = $self->qp->transaction;
+    ok( ! $self->is_relayclient( $transaction ),
+        "sender_permitted_from, is_relayclient -");
+
+    $self->qp->connection->relay_client(1);
+    ok( $self->is_relayclient( $transaction ),
+        "sender_permitted_from, is_relayclient +");
+
+    $self->qp->connection->relay_client(0);
+    $self->qp->connection->remote_ip('192.168.7.5');
+    my $client_ip = $self->qp->connection->remote_ip;
+    ok( $client_ip, "sender_permitted_from, relayclients ($client_ip)");
+};
+
+sub test_is_special_recipient {
+    my $self = shift;
+
+    my $transaction = $self->qp->transaction;
+    my $address     = Qpsmtpd::Address->new('user@example.com');
+
+    ok( ! $self->is_special_recipient( $address ), "is_special_recipient -");
+
+    foreach my $user ( qw/ postmaster abuse mailer-daemon root / ) {
+        $address = Qpsmtpd::Address->new("$user\@example.com");
+        ok( $self->is_special_recipient( $address ), "is_special_recipient ($user)");
+    };
+};
+