From 644709e6ae500cdc4bee3e85a5e896a703379fe6 Mon Sep 17 00:00:00 2001
From: Matt Simerson <matt@tnpi.net>
Date: Fri, 22 Jun 2012 20:03:47 -0400
Subject: [PATCH] badmailfrom: remove rcpt hook (uses naughty instead)

---
 plugins/check_badmailfrom        | 46 +++++++++-----------------------
 t/plugin_tests/check_badmailfrom | 23 +++++++++++-----
 2 files changed, 30 insertions(+), 39 deletions(-)

diff --git a/plugins/check_badmailfrom b/plugins/check_badmailfrom
index f4d1d84..47aa425 100644
--- a/plugins/check_badmailfrom
+++ b/plugins/check_badmailfrom
@@ -48,12 +48,6 @@ anywhere in the string.
  ^admin.*\.ppoonn400\.com$
 
 
-=head1 NOTES
-
-According to the SMTP protocol, we can't reject until after the RCPT
-stage, so store it until later.
-
-
 =head1 AUTHORS
 
 2002 - Jim Winstead - initial author of badmailfrom
@@ -65,16 +59,10 @@ stage, so store it until later.
 =cut
 
 sub register {
-    my ($self,$qp) = shift, shift;
+    my ($self,$qp) = (shift, shift);
     $self->{_args} = { @_ };
 
-    # preserve legacy "reject during rcpt" behavior
     $self->{_args}{reject} = 1 if ! defined $self->{_args}{reject};
-
-    return if ! $self->{_args}{reject};               # reject 0, log only
-    return if   $self->{_args}{reject} eq 'naughty';  # naughty will reject
-
-    $self->register_hook('rcpt', 'rcpt_handler');
 };
 
 sub hook_mail {
@@ -86,7 +74,6 @@ sub hook_mail {
     if ( defined $self->{_badmailfrom_config} ) {       # testing
         @badmailfrom = @{$self->{_badmailfrom_config}};
     };
-
     return DECLINED if $self->is_immune_sender( $sender, \@badmailfrom );
 
     my $host = lc $sender->host;
@@ -98,20 +85,22 @@ sub hook_mail {
         next unless $bad;
         next unless $self->is_match( $from, $bad, $host );
         $reason ||= "Your envelope sender is in my badmailfrom list";
-        $self->connection->notes('naughty', $reason);
+        $self->connection->notes('karma', ($self->connection->notes('karma') || 0) - 1);
+        return $self->get_reject( $reason );
     }
-    if ( ! $self->connection->notes('naughty') ) {
-        $self->log(LOGINFO, "pass");
-    };
+
+    $self->log(LOGINFO, "pass");
     return DECLINED;
 }
 
 sub is_match {
     my ( $self, $from, $bad, $host ) = @_;
 
-    if ( $bad =~ /[\/\^\$\*\+]/ ) {  # it's a regexp
-        $self->log(LOGDEBUG, "badmailfrom pattern ($bad) match for $from");
-        return 1 if $from =~ /$bad/;
+    if ( $bad =~ /[\/\^\$\*\+\!\%\?\\]/ ) {  # it's a regexp
+        if ( $from =~ /$bad/ ) {
+            $self->log(LOGDEBUG, "badmailfrom pattern ($bad) match for $from");
+            return 1;
+        };
         return;
     };
 
@@ -128,30 +117,21 @@ sub is_match {
     return 1;
 };
 
-sub rcpt_handler {
-    my ($self, $transaction, $rcpt, %param) = @_;
-
-    my $note = $self->connection->notes('naughty') or return (DECLINED);
-
-    $self->log(LOGINFO, "fail, $note");
-    return (DENY, $note);
-}
-
 sub is_immune_sender {
     my ($self, $sender, $badmf ) = @_;
 
     if ( ! scalar @$badmf ) {
-        $self->log(LOGDEBUG, 'skip: empty list');
+        $self->log(LOGDEBUG, 'skip, empty list');
         return 1;
     };
 
     if ( ! $sender || $sender->format eq '<>' ) {
-        $self->log(LOGDEBUG, 'skip: null sender');
+        $self->log(LOGDEBUG, 'skip, null sender');
         return 1;
     };
 
     if ( ! $sender->host || ! $sender->user ) {
-        $self->log(LOGDEBUG, 'skip: missing user or host');
+        $self->log(LOGDEBUG, 'skip, missing user or host');
         return 1;
     };
 
diff --git a/t/plugin_tests/check_badmailfrom b/t/plugin_tests/check_badmailfrom
index e80e0fb..042d3d0 100644
--- a/t/plugin_tests/check_badmailfrom
+++ b/t/plugin_tests/check_badmailfrom
@@ -4,6 +4,7 @@ use strict;
 use Data::Dumper;
 
 use Qpsmtpd::Address;
+use Qpsmtpd::Constants;
 
 sub register_tests {
     my $self = shift;
@@ -42,6 +43,8 @@ sub test_badmailfrom_is_immune_sender {
 sub test_badmailfrom_hook_mail {
     my $self = shift;
 
+    $self->_reset_connection_flags();
+
     my $transaction = $self->qp->transaction;
 
     my $test_email = 'matt@test.com';
@@ -49,16 +52,16 @@ sub test_badmailfrom_hook_mail {
     $transaction->sender($address);
 
     $self->{_badmailfrom_config} = ['matt@test.net','matt@test.com'];
-    $transaction->notes('badmailfrom', '');
+    $transaction->notes('naughty', '');
     my ($r, $err) = $self->hook_mail( $transaction, $address );
-    cmp_ok( $r, '==', 901, "hook_mail rc");
-    cmp_ok( $err, 'eq', 'Your envelope sender is in my badmailfrom list', "hook_mail: default reason");
+    cmp_ok( $r, '==', DENY, "hook_mail rc");
+    cmp_ok( $err, 'eq', 'Your envelope sender is in my badmailfrom list', "default reason");
 
     $self->{_badmailfrom_config} = ['matt@test.net','matt@test.com Yer a spammin bastert'];
-    $transaction->notes('badmailfrom', '');
+    $transaction->notes('naughty', '');
     ($r, $err) = $self->hook_mail( $transaction, $address );
-    cmp_ok( $r, '==', 901, "hook_mail rc");
-    cmp_ok( $err, 'eq', 'Yer a spammin bastert', "hook_mail: custom reason");
+    cmp_ok( $r, '==', DENY, "hook_mail rc");
+    cmp_ok( $err, 'eq', 'Yer a spammin bastert', "custom reason");
 };
 
 sub test_badmailfrom_match {
@@ -88,3 +91,11 @@ sub test_badmailfrom_match {
         "check_badmailfrom pattern non-match");
 };
 
+sub _reset_connection_flags {
+    my $self = shift;
+    $self->qp->connection->relay_client(0);
+    $self->qp->connection->notes('whitelisthost', 0);
+    $self->connection->notes('naughty',0);
+    $self->connection->notes('rejected', 0);
+};
+