Bug 2163

I'm pro this idea, BTW.  One thing I've been wanting to do -- but without any
time to do it ;) -- is to open up SpamAssassin a little more for 2.70.

What I'm thinking is:

    - if it were possible to drop new .pm files (let's say, objects of type
      Mail::SpamAssassin::EvalTest) into /etc/mail/spamassassin to provide
      new eval function APIs, and

    - SpamAssassin provided clean documented OO APIs for third-party
      eval-test .pm objects to use, to access the body parts, MIME
      structure, headers, and parsed body text, and

    - possibly provide a sample of what such an EvalTest object is coded like 
      (even just on a webpage)

That would be a great way to do this.

BTW, Matt talked about this in his presentation to the Spam Conference in
January: 'SA is a framework for combining spam detection techniques'.

Opening up the framework to more sophisticated code-based tests would be great,
especially since we've put so much work into reliable message parsing that's
resilient against the attackers spammers use to render "real" MIME parsers
ineffective.  This way, we can make SA a usable *platform* for spam detection,
not just a spam detector on its own.

Any volunteers? ;)

I'd like to see this ability as well, to create variations of
check_for_to_in_subject (see for instance
http://bugzilla.spamassassin.org/show_bug.cgi?id=2638). As I reported on
SA-Talk, I see a lot of spam with my name or username, a comma, and whitespace
that I'd like to add a point or two for. Rod's idea of a "localeval" tag in a
rule sounds good to me.

I think the current status is that we're waiting for someone to free up some
time to work on "framework-izing" SA's internals; then this will fall out more
or less for free.  That hasn't happened yet. ;)

In the meantime, I think we'd be able to apply a patch.  However, it'd have to
be done like this in order to be future-proof:

- we have a base class, Mail::SpamAssassin::EvalTest .   This provides the
public APIs that local eval test implementations can use (e.g. getting headers,
getting body text in various formats, etc.)

- all local eval tests have to be subclasses of this.

- rather than defining a new prefix "localeval", we define a new test keyword,
"loadmodule" (in honour of Apache ;). e.g.

   header MY_EVAL      eval:myevalsub(args)
   loadmodule   /path/to/module/file.pm       (for full path specification)

or

   loadmodule   file.pm 

the latter will search in the system-wide config file locations (e.g.
/usr/share/spamassassin, /etc/mail/spamassassin, etc.,  but *not* the user home
dir by default, see below.)

- when "loadmodule" is found, it searches for the module.pm file, loads it, and
calls an init() function in that module to initialize it.  (this happens at
Mail::SpamAssassin::init() time, so once at boot for spamd.)

- that init() function will be passed the Mail::SpamAssassin object.  It must
create an instance of the local-eval class, then call
$sa->register_local_eval($instance, "nameofsub") where "nameofsub" is the name
of the eval test subroutine that this local-eval class will handle.   SA will
then know to call that method on that object when an eval test refers to
"eval:nameofsub(..args..)", and we don't need an extra "localeval" keyword.

- the init() function can also do any rule-specific initialization required;
e.g. loading dictionaries, etc.

- users can make their own eval test classes *only if* a new parameter,
allow_user_eval_rules is set to 1.  It's 0 by default for security (because
this is raw perl code being executed).

So that's how I see it so far -- that gives us a lot of flexibility now, but is
fast, extensible for future additions, and nicely OO (which is a big problem
with our current eval test model).

Sounds good.

I would assume that the semantics of allow_user_eval_rules is that it really
means allow a module owned by *another* user, allowing an end-user to run
spamassassin and use his own rules but preventing spamd (also running as a
mortal) from using his stuff.

Subject: Re: [SAdev]  local eval tests 

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


>I would assume that the semantics of allow_user_eval_rules is that it really
>means allow a module owned by *another* user, allowing an end-user to run
>spamassassin and use his own rules but preventing spamd (also running as a
>mortal) from using his stuff.

Well, it'd mean allowing "loadmodule" lines in ~/.spamassassin/user_prefs
to be acted upon.  "spamd" *does* run as the user; the problem is that
many sites don't want their users running *any* code on the mailserver,
and would keep allow_user_eval_rules set to 0.  That way only
admin-supplied local eval rules would be possible in that configuration.

- --j.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)
Comment: Exmh CVS

iD8DBQE/uVAYQTcbUG5Y7woRAtWQAJ9/7pumUSN1YhEw/Zuaus1F0K109ACgwESO
tw+WXaTtKnYjxf7isl/0XJA=
=etrL
-----END PGP SIGNATURE-----

Subject: Re: [SAdev]  local eval tests

FWIW, allowing user eval's is no more of a security hole that allowing
user rules. Both are run as the user, but can contain arbitrary
code. (Which reminds me, spamd should check that a user_prefs is not
world writable before using.)

> FWIW, allowing user eval's is no more of a security hole that allowing
> user rules. Both are run as the user, but can contain arbitrary
> code. 

There is a difference: user rules are unexploitable, as far as I can
tell, *from our investigations so far*.   So exploiting them would require
a bit of work, and may be limited in their usefulness.  However, user
evals are trivial to exploit; the .pm just has to contain "run { system
("/bin/sh"); }" or whatever to get shell access.

So, given this, I'd suggest that there is a difference.  If
I was installing SA on a "no shell access" machine, with a relatively
trusted group of users, I'd probably set

  allow_user_rules 1
  allow_user_eval_rules 0

due to the level of trust.

> (Which reminds me, spamd should check that a user_prefs is not
> world writable before using.)

good point...

oh BTW -- another reason to add a 'allow_user_eval_rules' instead of reusing
'allow_user_rules' is because an admin with 'allow_user_rules' set would
silently allow his users to run arbitrary perl code a lot more easily that
way, possibly without realising this, once they upgraded.   IMO it'd be
better to fail-safe on this one.

OK, plugin support is now in CVS.

allow_user_eval_rules is not yet implemented, BTW; only the system-wide rules
files can currently contain plugins, not user configs.

Here's a sample plugin class:

package MyPlugin;

use Mail::SpamAssassin::Plugin;
use vars qw(@ISA);
@ISA = qw(Mail::SpamAssassin::Plugin);

# constructor: register the eval rule
sub new {
  my $class = shift;
  my $mailsaobject = shift;

  # some boilerplate...
  $class = ref($class) || $class;
  my $self = $class->SUPER::new($mailsaobject);
  bless ($self, $class);

  # the important bit!
  $self->register_eval_rule ("check_for_foo");

  warn "JMD registered MyPlugin $self";
  return $self;
}

# and the eval rule itself
sub check_for_foo {
  my ($self, $permsgstatus) = @_;
  warn "JMD MyPlugin eval test called on: $self";
  # ... hard work goes here...
  return 1;
}

1;


To try this out, save this as e.g. /home/jm/ftp/spamassassin/plugintest.pm,
and write these lines to /etc/mail/spamassassin/plugintest.cf:

  loadplugin     MyPlugin /home/jm/ftp/spamassassin/plugintest.pm
  header         MY_PLUGIN_FOO eval:check_for_foo()


Note that that's just for eval rules; however, plugins can receive settings
from Conf.pm, and the design is such that they can receive other arbitrary
events from other parts of SA.  So it should be quite usable for other stuff,
given more instrumentation points in key functions...

justin: any reason that since plugin support is now in HEAD, the bug wasn't 
closed?

nope. ;)

Just lost the bug in the swamp that is bugzilla...