On Jun 1, 2012, at 3:30 PM, Steve Kemp wrote:
> On Fri Jun 01, 2012 at 15:17:51 -0700, Matt Simerson wrote:
>
>> Introducing zombies and a reaper.
>
> Cute names..
Thanks.
>> Notice the bolded lines. Instead of rejecting the message early,
>> plugins like dnsbl and karma can zombify it. To increase efficiency,
>> other plugins detect the zombie state and skip processing.
>
> This seems to be a specific case of the general idea to lower
> overhead:
Aye.
But reducing code and simplifying plugins is just as important. This
implementation adds more control and flexibility to plugins without additional
code. For example, now it's possible (without writing code) to configure all
plugins to reject early or late by setting them all to <reject zombie>. Some
plugins already allow this ability, many do not, and all of them become simpler
with this.
> * Avoid processing messages once you've decided they are spam.
>
> Your idea is nice, and it does work well in practise if you can
> skip later plugins based on earlier ones.
And we can! It gets really simple if there is a method available to all
plugins like this one:
sub is_immune {
my $self = shift;
if ( $self->connection->notes('zombie') ) {
$self->log(LOGINFO, "skip, zombie");
return 1;
};
if ( $self->connection->notes('rejected') ) {
$self->log(LOGINFO, "skip, already rejected");
return 1;
};
if ( $self->qp->connection->relay_client() ) {
$self->log(LOGINFO, "skip, relay client");
return 1;
};
if ( $self->qp->connection->notes('whitelisthost') ) {
$self->log(LOGINFO, "skip, whitelisted host");
return 1;
};
if ( $self->qp->transaction->notes('whitelistsender') ) {
$self->log(LOGINFO, "skip, whitelisted sender");
return 1;
};
return;
};
and most plugins called it like this:
return DECLINED if $self->is_immune();
Several plugins already have their own versions of is_immune. As I've
refactored and added tests to them, I've abstracted their immunity tests into
is_immune methods within several of the plugins. Centralizing them further
simplifies a bunch of plugins.
> Especially if you skip resource-intensive ones - such as dspam, clamav, etc.
Yup. Part of the motivation for this plugin was to short circuit all the
intermediate plugins and handlers so I can feed the message to sa-learn and
dspam. Until dspam is trained, that's a very important step in training it. But
there's no gain in validating the HELO name, SPF, or DomainKeys. This plugin
and associated changes adds that flexibility while reducing the code and
complexity of the plugins.
> But a more general approach is to set a note "rejected" => 1, and
> have all plugins skip processing if that is set *except* a final
> whitelist plugin which might revert the decision.
I think moving the conditions for bypassing plugin processing into a
centralized is_immune() method as I have done is substantially more general. It
means adding only a single line of code to each plugin, and is_immune isn't
specific to any single note. It works equally well for whitelists, relay
clients, your rejects, and my zombies. Since many plugins already have code to
bypass processing for relay clients and whitelists, this solution reduces the
code and simplify the plugins as well.
> This approach is implemented here:
>
> http://www.steve.org.uk/Software/ms-lite/
I obviously like the rejected idea. I don't like the 'rejected' note name
though, because the rejected term is already overloaded. It means many
different things in different contexts within qpsmtpd. It's also somewhat
counterintuitive when a plugin is handling a connection that had already been
rejected. Oh, but wait, "rejected" doesn't really mean "rejected" any more.
Redefining terms adds complexity, which I've been eschewing.
Why hasn't your rejected idea been worked into qpsmtpd yet?
Matt
