From 1910fabf0ee1fce907fa23fa3afb9822e4c3ca9f Mon Sep 17 00:00:00 2001
From: Matt Simerson <matt@tnpi.net>
Date: Sat, 5 May 2012 01:04:33 -0400
Subject: [PATCH] badmailfromto: added strictures, tests, and

rearranged portionsn of logic for ease of reading
---
 plugins/check_badmailfromto        | 66 ++++++++++++++++++++----------
 t/plugin_tests/check_badmailfromto | 36 ++++++++++++++++
 2 files changed, 81 insertions(+), 21 deletions(-)
 create mode 100644 t/plugin_tests/check_badmailfromto

diff --git a/plugins/check_badmailfromto b/plugins/check_badmailfromto
index a57f6f6..3a39874 100644
--- a/plugins/check_badmailfromto
+++ b/plugins/check_badmailfromto
@@ -17,14 +17,14 @@ Based heavily on check_badmailfrom.
 
 =cut
 
+use strict;
+use Qpsmtpd::Constants;
+
 sub hook_mail {
   my ($self, $transaction, $sender, %param) = @_;
 
-  my @badmailfromto = $self->qp->config("badmailfromto")
-    or return (DECLINED);
-
-  return (DECLINED) unless ($sender->format ne "<>"
-			    and $sender->host && $sender->user);
+    my @badmailfromto = $self->qp->config("badmailfromto");
+    return DECLINED if $self->is_sender_immune( $sender, \@badmailfromto );
 
   my $host = lc $sender->host;
   my $from = lc($sender->user) . '@' . $host;
@@ -33,27 +33,51 @@ sub hook_mail {
     $bad =~ s/^\s*(\S+).*/$1/;
     next unless $bad;
     $bad = lc $bad;
-    $self->log(LOGWARN, "Bad badmailfromto config: No \@ sign in $bad") and next unless $bad =~ m/\@/;
-    $transaction->notes('badmailfromto', "$bad")
-      if ($bad eq $from)
-      || (substr($bad,0,1) eq '@' && $bad eq "\@$host");
+    if ( $bad !~ m/\@/ ) {
+        $self->log(LOGWARN, 'badmailfromto: bad config, no @ sign in '. $bad);
+        next;
+    };
+    if ( $bad eq $from || (substr($bad,0,1) eq '@' && $bad eq "\@$host") ) {
+        $transaction->notes('badmailfromto', $bad);
+    };
   }
   return (DECLINED);
 }
 
 sub hook_rcpt {
-  my ($self, $transaction, $rcpt, %param) = @_;
-  my $recipient = lc($rcpt->user) . '@' . lc($rcpt->host);
-  my $sender = $transaction->notes('badmailfromto');
-  if ($sender) {
-    my @badmailfromto = $self->qp->config("badmailfromto")
-      or return (DECLINED);
+    my ($self, $transaction, $rcpt, %param) = @_;
+    my $recipient = lc($rcpt->user) . '@' . lc($rcpt->host);
+    my $sender = $transaction->notes('badmailfromto') or do {
+        $self->log(LOGDEBUG, "pass: sender not listed");
+        return (DECLINED);
+    };
 
-    foreach (@badmailfromto) {
-      my ($from, $to) = m/^\s*(\S+)\t(\S+).*/;
-      return (DENY, "mail to $recipient not accepted here")
-        if lc($from) eq $sender and lc($to) eq $recipient;
+    foreach ( $self->qp->config("badmailfromto") ) {
+        my ($from, $to) = m/^\s*(\S+)\t(\S+).*/;
+        return (DENY, "mail to $recipient not accepted here")
+            if lc($from) eq $sender && lc($to) eq $recipient;
     }
-  }
-  return (DECLINED);
+    $self->log(LOGDEBUG, "pass: recipient not listed");
+    return (DECLINED);
 }
+
+sub is_sender_immune {
+    my ($self, $sender, $badmf ) = @_;
+
+    if ( ! scalar @$badmf ) {
+        $self->log(LOGDEBUG, 'skip: empty list');
+        return 1;
+    };
+
+    if ( ! $sender || $sender->format eq '<>' ) {
+        $self->log(LOGDEBUG, 'skip: null sender');
+        return 1;
+    };
+
+    if ( ! $sender->host || ! $sender->user ) {
+        $self->log(LOGDEBUG, 'skip: missing user or host');
+        return 1;
+    };
+
+    return;
+};
diff --git a/t/plugin_tests/check_badmailfromto b/t/plugin_tests/check_badmailfromto
new file mode 100644
index 0000000..73d9bb9
--- /dev/null
+++ b/t/plugin_tests/check_badmailfromto
@@ -0,0 +1,36 @@
+#!perl -w
+
+use strict;
+use Data::Dumper;
+
+use Qpsmtpd::Address;
+
+sub register_tests {
+    my $self = shift;
+
+    $self->register_test("test_badmailfromto_is_sender_immune", 5);
+}
+
+sub test_badmailfromto_is_sender_immune {
+    my $self = shift;
+
+    my $transaction = $self->qp->transaction;
+    my $test_email = 'matt@test.com';
+    $transaction->sender( Qpsmtpd::Address->new( "<$test_email>" ) );
+    ok( $self->is_sender_immune( $transaction->sender, [] ), "is_immune, empty list");
+
+    $transaction->sender( Qpsmtpd::Address->new( '<>' ) );
+    ok( $self->is_sender_immune( $transaction->sender, ['bad@example.com'] ), "is_immune, null sender");
+
+    my $address = Qpsmtpd::Address->new( '<matt@>' );
+    $transaction->sender($address);
+    ok( $self->is_sender_immune( $transaction->sender, ['bad@example.com'] ), "is_immune, missing host");
+
+    $address = Qpsmtpd::Address->new( '<@example.com>' );
+    $transaction->sender($address);
+    ok( $self->is_sender_immune( $transaction->sender, ['bad@example.com'] ), "is_immune, missing user");
+
+    $transaction->sender( Qpsmtpd::Address->new( '<matt@example.com>' ) );
+    ok( ! $self->is_sender_immune( $transaction->sender, ['bad@example.com'] ), "is_immune, false");
+};
+