From b89a6d9e4cffbf03fe69026d8ec0ec2041d8d22f Mon Sep 17 00:00:00 2001 From: John Peacock Date: Mon, 20 Mar 2006 16:47:05 +0000 Subject: [PATCH] * plugins/queue/smtp-forward s/register/init/ to match new plugin style (jpeacock) * lib/Qpsmtpd/Address.pm t/qpsmtpd-address.t Ill-formed addresses should return null not partial garbage. Resolves https://rt.perl.org/rt3/Ticket/Display.html?id=38746 Patch by Hanno Hecker. * plugins/virus/clamav Clamav alternate config file. Resolves https://rt.perl.org/rt3/Ticket/Display.html?id=38736 Patch by Robin Bowes. * lib/Qpsmtpd/SMTP.pm lib/Qpsmtpd.pm Return multiline responses from plugins. Resolves https://rt.perl.org/rt3/Ticket/Display.html?id=38741 Patch by Charlie Brady. git-svn-id: https://svn.perl.org/qpsmtpd/branches/0.3x@630 958fd67b-6ff1-0310-b445-bb7760255be9 --- lib/Qpsmtpd.pm | 1 + lib/Qpsmtpd/Address.pm | 3 +- lib/Qpsmtpd/SMTP.pm | 138 +++++++++++++++++++++---------------- plugins/queue/smtp-forward | 2 +- plugins/virus/clamav | 19 ++++- t/qpsmtpd-address.t | 7 +- 6 files changed, 103 insertions(+), 67 deletions(-) diff --git a/lib/Qpsmtpd.pm b/lib/Qpsmtpd.pm index f294ca3..a7ae15e 100644 --- a/lib/Qpsmtpd.pm +++ b/lib/Qpsmtpd.pm @@ -348,6 +348,7 @@ sub run_hooks { last unless $r[0] == DECLINED; } $r[0] = DECLINED if not defined $r[0]; + @r = map { split /\n/ } @r; return @r; } return (0, ''); diff --git a/lib/Qpsmtpd/Address.pm b/lib/Qpsmtpd/Address.pm index 9d68c7c..f1381e1 100644 --- a/lib/Qpsmtpd/Address.pm +++ b/lib/Qpsmtpd/Address.pm @@ -60,7 +60,8 @@ sub new { my ($class, $user, $host) = @_; my $self = {}; if ($user =~ /^<(.*)>$/ ) { - ($user, $host) = $class->canonify($user) + ($user, $host) = $class->canonify($user); + return undef unless defined $user; } elsif ( not defined $host ) { my $address = $user; diff --git a/lib/Qpsmtpd/SMTP.pm b/lib/Qpsmtpd/SMTP.pm index 34cf37a..5b350ac 100644 --- a/lib/Qpsmtpd/SMTP.pm +++ b/lib/Qpsmtpd/SMTP.pm @@ -51,13 +51,14 @@ sub dispatch { $self->{_counter}++; if ($cmd !~ /^(\w{1,12})$/ or !exists $self->{_commands}->{$1}) { - my ($rc, $msg) = $self->run_hooks("unrecognized_command", $cmd, @_); + my ($rc, @msg) = $self->run_hooks("unrecognized_command", $cmd, @_); + @msg = map { split /\n/ } @msg; if ($rc == DENY_DISCONNECT) { - $self->respond(521, $msg); + $self->respond(521, @msg); $self->disconnect; } elsif ($rc == DENY) { - $self->respond(500, $msg); + $self->respond(500, @msg); } elsif ($rc == DONE) { 1; @@ -91,13 +92,15 @@ sub start_conversation { my $self = shift; # this should maybe be called something else than "connect", see # lib/Qpsmtpd/TcpServer.pm for more confusion. - my ($rc, $msg) = $self->run_hooks("connect"); + my ($rc, @msg) = $self->run_hooks("connect"); if ($rc == DENY) { - $self->respond(550, ($msg || 'Connection from you denied, bye bye.')); + $msg[0] ||= 'Connection from you denied, bye bye.'; + $self->respond(550, @msg); return $rc; } elsif ($rc == DENYSOFT) { - $self->respond(450, ($msg || 'Connection from you temporarily denied, bye bye.')); + $msg[0] ||= 'Connection from you temporarily denied, bye bye.'; + $self->respond(450, @msg); return $rc; } elsif ($rc == DONE) { @@ -146,18 +149,18 @@ sub helo { my $conn = $self->connection; return $self->respond (503, "but you already said HELO ...") if $conn->hello; - my ($rc, $msg) = $self->run_hooks("helo", $hello_host, @stuff); + my ($rc, @msg) = $self->run_hooks("helo", $hello_host, @stuff); if ($rc == DONE) { # do nothing } elsif ($rc == DENY) { - $self->respond(550, $msg); + $self->respond(550, @msg); } elsif ($rc == DENYSOFT) { - $self->respond(450, $msg); + $self->respond(450, @msg); } elsif ($rc == DENY_DISCONNECT) { - $self->respond(550, $msg); + $self->respond(550, @msg); $self->disconnect; } elsif ($rc == DENYSOFT_DISCONNECT) { - $self->respond(450, $msg); + $self->respond(450, @msg); $self->disconnect; } else { $conn->hello("helo"); @@ -174,18 +177,18 @@ sub ehlo { my $conn = $self->connection; return $self->respond (503, "but you already said HELO ...") if $conn->hello; - my ($rc, $msg) = $self->run_hooks("ehlo", $hello_host, @stuff); + my ($rc, @msg) = $self->run_hooks("ehlo", $hello_host, @stuff); if ($rc == DONE) { # do nothing } elsif ($rc == DENY) { - $self->respond(550, $msg); + $self->respond(550, @msg); } elsif ($rc == DENYSOFT) { - $self->respond(450, $msg); + $self->respond(450, @msg); } elsif ($rc == DENY_DISCONNECT) { - $self->respond(550, $msg); + $self->respond(550, @msg); $self->disconnect; } elsif ($rc == DENYSOFT_DISCONNECT) { - $self->respond(450, $msg); + $self->respond(450, @msg); $self->disconnect; } else { $conn->hello("ehlo"); @@ -285,30 +288,30 @@ sub mail { } return $self->respond(501, "could not parse your mail from command") unless $from; - my ($rc, $msg) = $self->run_hooks("mail", $from); + my ($rc, @msg) = $self->run_hooks("mail", $from); if ($rc == DONE) { return 1; } elsif ($rc == DENY) { - $msg ||= $from->format . ', denied'; - $self->log(LOGINFO, "deny mail from " . $from->format . " ($msg)"); - $self->respond(550, $msg); + $msg[0] ||= $from->format . ', denied'; + $self->log(LOGINFO, "deny mail from " . $from->format . " (@msg)"); + $self->respond(550, @msg); } elsif ($rc == DENYSOFT) { - $msg ||= $from->format . ', temporarily denied'; - $self->log(LOGINFO, "denysoft mail from " . $from->format . " ($msg)"); - $self->respond(450, $msg); + $msg[0] ||= $from->format . ', temporarily denied'; + $self->log(LOGINFO, "denysoft mail from " . $from->format . " (@msg)"); + $self->respond(450, @msg); } elsif ($rc == DENY_DISCONNECT) { - $msg ||= $from->format . ', denied'; - $self->log(LOGINFO, "deny mail from " . $from->format . " ($msg)"); - $self->respond(550, $msg); + $msg[0] ||= $from->format . ', denied'; + $self->log(LOGINFO, "deny mail from " . $from->format . " (@msg)"); + $self->respond(550, @msg); $self->disconnect; } elsif ($rc == DENYSOFT_DISCONNECT) { - $msg ||= $from->format . ', temporarily denied'; - $self->log(LOGINFO, "denysoft mail from " . $from->format . " ($msg)"); - $self->respond(421, $msg); + $msg[0] ||= $from->format . ', temporarily denied'; + $self->log(LOGINFO, "denysoft mail from " . $from->format . " (@msg)"); + $self->respond(421, @msg); $self->disconnect; } else { # includes OK @@ -331,28 +334,28 @@ sub rcpt { return $self->respond(501, "could not parse recipient") unless $rcpt; - my ($rc, $msg) = $self->run_hooks("rcpt", $rcpt); + my ($rc, @msg) = $self->run_hooks("rcpt", $rcpt); if ($rc == DONE) { return 1; } elsif ($rc == DENY) { - $msg ||= 'relaying denied'; - $self->respond(550, $msg); + $msg[0] ||= 'relaying denied'; + $self->respond(550, @msg); } elsif ($rc == DENYSOFT) { - $msg ||= 'relaying denied'; - return $self->respond(450, $msg); + $msg[0] ||= 'relaying denied'; + return $self->respond(450, @msg); } elsif ($rc == DENY_DISCONNECT) { - $msg ||= 'delivery denied'; - $self->log(LOGINFO, "delivery denied ($msg)"); - $self->respond(550, $msg); + $msg[0] ||= 'delivery denied'; + $self->log(LOGINFO, "delivery denied (@msg)"); + $self->respond(550, @msg); $self->disconnect; } elsif ($rc == DENYSOFT_DISCONNECT) { - $msg ||= 'relaying denied'; - $self->log(LOGINFO, "delivery denied ($msg)"); - $self->respond(421, $msg); + $msg[0] ||= 'relaying denied'; + $self->log(LOGINFO, "delivery denied (@msg)"); + $self->respond(421, @msg); $self->disconnect; } elsif ($rc == OK) { @@ -388,17 +391,19 @@ sub vrfy { # documented in RFC2821#3.5.1 # I also don't think it provides all the proper result codes. - my ($rc, $msg) = $self->run_hooks("vrfy"); + my ($rc, @msg) = $self->run_hooks("vrfy"); if ($rc == DONE) { return 1; } elsif ($rc == DENY) { - $self->respond(554, $msg || "Access Denied"); + $msg[0] ||= "Access Denied"; + $self->respond(554, @msg); $self->reset_transaction(); return 1; } elsif ($rc == OK) { - $self->respond(250, $msg || "User OK"); + $msg[0] ||= "User OK"; + $self->respond(250, @msg); return 1; } else { # $rc == DECLINED or anything else @@ -415,9 +420,10 @@ sub rset { sub quit { my $self = shift; - my ($rc, $msg) = $self->run_hooks("quit"); + my ($rc, @msg) = $self->run_hooks("quit"); if ($rc != DONE) { - $self->respond(221, $self->config('me') . " closing connection. Have a wonderful day."); + $msg[0] ||= $self->config('me') . " closing connection. Have a wonderful day."; + $self->respond(221, @msg); } $self->disconnect(); } @@ -430,27 +436,31 @@ sub disconnect { sub data { my $self = shift; - my ($rc, $msg) = $self->run_hooks("data"); + my ($rc, @msg) = $self->run_hooks("data"); if ($rc == DONE) { return 1; } elsif ($rc == DENY) { - $self->respond(554, $msg || "Message denied"); + $msg[0] ||= "Message denied"; + $self->respond(554, @msg); $self->reset_transaction(); return 1; } elsif ($rc == DENYSOFT) { - $self->respond(451, $msg || "Message denied temporarily"); + $msg[0] ||= "Message denied temporarily"; + $self->respond(451, @msg); $self->reset_transaction(); return 1; } elsif ($rc == DENY_DISCONNECT) { - $self->respond(554, $msg || "Message denied"); + $msg[0] ||= "Message denied"; + $self->respond(554, @msg); $self->disconnect; return 1; } elsif ($rc == DENYSOFT_DISCONNECT) { - $self->respond(421, $msg || "Message denied temporarily"); + $msg[0] ||= "Message denied temporarily"; + $self->respond(421, @msg); $self->disconnect; return 1; } @@ -547,15 +557,17 @@ sub data { #$self->respond(550, $self->transaction->blocked),return 1 if ($self->transaction->blocked); $self->respond(552, "Message too big!"),return 1 if $max_size and $size > $max_size; - ($rc, $msg) = $self->run_hooks("data_post"); + ($rc, @msg) = $self->run_hooks("data_post"); if ($rc == DONE) { return 1; } elsif ($rc == DENY) { - $self->respond(552, $msg || "Message denied"); + $msg[0] ||= "Message denied"; + $self->respond(552, @msg); } elsif ($rc == DENYSOFT) { - $self->respond(452, $msg || "Message denied temporarily"); + $msg[0] ||= "Message denied temporarily"; + $self->respond(452, @msg); } else { $self->queue($self->transaction); @@ -579,7 +591,7 @@ sub queue { my ($self, $transaction) = @_; # First fire any queue_pre hooks - my ($rc, $msg) = $self->run_hooks("queue_pre"); + my ($rc, @msg) = $self->run_hooks("queue_pre"); if ($rc == DONE) { return 1; } @@ -589,26 +601,30 @@ sub queue { } # If we got this far, run the queue hooks - ($rc, $msg) = $self->run_hooks("queue"); + ($rc, @msg) = $self->run_hooks("queue"); if ($rc == DONE) { return 1; } elsif ($rc == OK) { - $self->respond(250, ($msg || 'Queued')); + $msg[0] ||= 'Queued'; + $self->respond(250, @msg); } elsif ($rc == DENY) { - $self->respond(552, $msg || "Message denied"); + $msg[0] ||= 'Message denied'; + $self->respond(552, @msg); } elsif ($rc == DENYSOFT) { - $self->respond(452, $msg || "Message denied temporarily"); + $msg[0] ||= 'Message denied temporarily'; + $self->respond(452, @msg); } else { - $self->respond(451, $msg || "Queuing declined or disabled; try again later" ); + $msg[0] ||= 'Queuing declined or disabled; try again later'; + $self->respond(451, @msg); } # And finally run any queue_post hooks - ($rc, $msg) = $self->run_hooks("queue_post"); - $self->log(LOGERROR, $msg) unless ($rc == OK or $rc == 0); + ($rc, @msg) = $self->run_hooks("queue_post"); + $self->log(LOGERROR, @msg) unless ($rc == OK or $rc == 0); } diff --git a/plugins/queue/smtp-forward b/plugins/queue/smtp-forward index 1d56a6f..f7e212b 100644 --- a/plugins/queue/smtp-forward +++ b/plugins/queue/smtp-forward @@ -21,7 +21,7 @@ Optionally you can also add a port: use Net::SMTP; -sub register { +sub init { my ($self, $qp, @args) = @_; if (@args > 0) { diff --git a/plugins/virus/clamav b/plugins/virus/clamav index 85a928a..b16d1cb 100644 --- a/plugins/virus/clamav +++ b/plugins/virus/clamav @@ -27,6 +27,13 @@ Path to the clamav commandline scanner. Mail will be passed to the clamav scanner in Berkeley mbox format (that is, with a "From " line). See the discussion below on which commandline scanner to use. +=item clamd_conf=I (e.g. I) + +Path to the clamd configuration file. Passed as an argument to the +command-line scanner (--config-file=I). + +The default value is '/etc/clamd.conf'. + =item action=EI | IE (e.g. I) Selects an action to take when an inbound message is found to be infected. @@ -120,6 +127,9 @@ sub register { elsif (/^clamscan_path=(\/[\/\-\_\.a-z0-9A-Z]*)$/) { $self->{_clamscan_loc} = $1; } + elsif (/^clamd_conf=(\/[\/\-\_\.a-z0-9A-Z]*)$/) { + $self->{_clamd_conf} = "$1"; + } elsif (/^tmp_dir=(\/[\/\-\_\.a-z0-9A-Z]*)$/) { $self->{_spool_dir} = $1; } @@ -138,6 +148,7 @@ sub register { $self->{_max_size} ||= 512 * 1024; $self->{_spool_dir} ||= $self->spool_dir(); $self->{_back_compat} ||= ''; # make sure something is set + $self->{_clamd_conf} ||= '/etc/clamd/conf'; # make sure something is set unless ($self->{_spool_dir}) { $self->log(LOGERROR, "No spool dir configuration found"); @@ -172,9 +183,11 @@ sub hook_data_post { } # Now do the actual scanning! - my $cmd = $self->{_clamscan_loc}." --stdout " - .$self->{_back_compat} - ." --disable-summary $filename 2>&1"; + my $cmd = $self->{_clamscan_loc} + . " --stdout " + . $self->{_back_compat} + . " --config-file=" . $self->{_clamd_conf} + . " --disable-summary $filename 2>&1"; $self->log(LOGDEBUG, "Running: $cmd"); my $output = `$cmd`; diff --git a/t/qpsmtpd-address.t b/t/qpsmtpd-address.t index c08d44b..599a4af 100644 --- a/t/qpsmtpd-address.t +++ b/t/qpsmtpd-address.t @@ -2,7 +2,7 @@ use strict; $^W = 1; -use Test::More tests => 29; +use Test::More qw/no_plan/; BEGIN { use_ok('Qpsmtpd::Address'); @@ -101,3 +101,8 @@ my @test_list = sort @unsorted_list; is_deeply( \@test_list, \@sorted_list, "sort via overloaded 'cmp' operator"); +# RT#38746 - non-RFC compliant address should return undef + +$as=''; +$ao = Qpsmtpd::Address->new($as); +is ($ao, undef, "illegal $as");