Bug 6132 - FreeMail plugin
Summary: FreeMail plugin
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Plugins (show other bugs)
Version: SVN Trunk (Latest Devel Version)
Hardware: All All
: P2 enhancement
Target Milestone: 3.3.0
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
: 6138 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-06-09 23:29 UTC by Henrik Krohns
Modified: 2009-07-23 04:52 UTC (History)
1 user (show)


Attachment Type Modified Status Actions Submitter/CLA Status
test mail, should fire T_FREEMAIL_FROM text/plain None Justin Mason [HasCLA]
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Henrik Krohns 2009-06-09 23:29:26 UTC 
I'm finally somewhat satisfied with the plugin, so let's discuss possible inclusion to SA.

http://sa.hege.li/FreeMail.pm
http://sa.hege.li/freemail_domains.cf
http://sa.hege.li/FreeMail_mass.cf

Firstly, can you do a masscheck with the special cf I made? It has some whitelisting rules and some other checks that seem to work nicely. Or could I have my own sandbox where I can test it (have some other rules also..)?

The code itself has some special stuff, like own tlds regex (can't use the one from PerMsgStatus.pm? can we make it available?). Also I include my own email regex, because SA functions can't be used, get_uri_detail_list can't distinguish some whitelisting methods like stripping emails inside <> and emails with "quote:" after them. If we want to make more use of SA, uri metadata should include surrounding text of the found emails.

Here is my quick mass check as reference. Corpus from last month, doesn't include botnet traffic (thus the nice spam ratio!), dups removed.

OVERALL    SPAM%     HAM%     S/O    RANK   SCORE  NAME
      0    12046   113896    0.096   0.00    0.00  (all messages)
0.00000   9.5647  90.4353    0.096   0.00    0.00  (all messages as %)
  1.807  18.7780   0.0123    0.999   1.00    1.00  FREEMAIL_REPLYTO
  1.822  18.7780   0.0290    0.998   0.95    1.00  __freemail_replyto
  0.148   1.5026   0.0053    0.997   0.89    1.00  FREEMAIL_FORGED_REPLYTO
  2.355  22.8541   0.1870    0.992   0.85    1.00  __freemail_reply
  0.016   0.1411   0.0026    0.982   0.83    1.00  FREEMAIL_SUBJECT
  2.671  20.8368   0.7498    0.965   0.75    1.00  FREEMAIL_FROM_END_DIGIT
  1.337  12.4523   0.1616    0.987   0.74    1.00  __freemail_hdr_replyto
  0.607   5.4292   0.0966    0.983   0.74    1.00  FREEMAIL_REPLY
  9.450  33.8868   6.8659    0.832   0.70    1.00  FREEMAIL_FROM
  0.055   0.0000   0.0606    0.000   0.35    1.00  __freemail_safe_ml3
  0.833   0.0249   0.9184    0.026   0.26    1.00  __freemail_safe_ml2
 25.218   0.1494  27.8693    0.005   0.17    1.00  __freemail_safe
 10.192   0.1245  11.2568    0.011   0.16    1.00  __freemail_safe_ml1
  1.471   0.0083   1.6260    0.005   0.10    1.00  __freemail_safe_rls
 16.046   0.0166  17.7416    0.001   0.00    1.00  __freemail_safe_fwd
Comment 1 Justin Mason 2009-06-10 05:09:59 UTC 
great news! setting milestone for the 3.3.0 release.  I don't have the time to respond or check it in right now but I'll come back to it rsn assuming nobody else does....
Comment 2 Karsten Bräckelmann 2009-06-10 08:10:12 UTC 
Didn't actually run any debugging, just poking at the code for some review, but there's a problem with the whitelisting in parse_config().

    if ($opts->{key} eq "freemail_whitelist") {
        foreach my $temp (split(/\s+/, $opts->{value})) {
            my $value = lc($temp);
            if ($temp =~ /\w[.@]\w/i) {
                $self->{freemail_domains}{$value} = 1;
            }

I believe that should be freemail_whitelist instead of freemail_domain here. Oh, and for consistency, you should match $value against the RE. ;)
Comment 3 Henrik Krohns 2009-06-10 08:53:45 UTC 
Right, fixed for 2.001 ;)
Comment 4 Justin Mason 2009-06-11 14:32:53 UTC 
cool. checked in:

: 71...; svn commit -m "bug 6132: add the FreeMail ruleset and plugin for testing, as requested. thanks to Henrik Krohns" rulesrc/sandbox/hege
Adding         rulesrc/sandbox/hege
Adding         rulesrc/sandbox/hege/20_FreeMail_domains.cf
Adding         rulesrc/sandbox/hege/20_FreeMail_mass.cf
Adding         rulesrc/sandbox/hege/FreeMail.pm
Transmitting file data ...
Committed revision 783928.
Comment 5 Justin Mason 2009-06-11 14:34:14 UTC 
Created attachment 4461 [details]
test mail, should fire T_FREEMAIL_FROM
Comment 6 Justin Mason 2009-06-11 14:35:25 UTC 
oh, btw:

'Or could I
have my own sandbox where I can test it (have some other rules also..)?'

I've checked it in under your name -- if you're interested, I'd be happy to start the SVN account creation process?
Comment 7 Justin Mason 2009-06-11 16:12:39 UTC 
btw there appears to be some issue in recreating the Mail::Spamassassin object, t/recreate.t is now failing with:

t/recreate....ok 1/9rules: failed to run T_FREEMAIL_FROM_END_DIGIT test, skipping:
        (Can't locate object method "check_freemail_from" via package "Mail::SpamAssassin::PerMsgStatus" at (eval 900) line 194.
)
rules: failed to run __freemail_reply test, skipping:
        (Can't locate object method "check_freemail_replyto" via package "Mail::SpamAssassin::PerMsgStatus" at (eval 900) line 239.
)
rules: failed to run T_FREEMAIL_SUBJECT test, skipping:
        (Can't locate object method "check_freemail_header" via package "Mail::SpamAssassin::PerMsgStatus" at (eval 900) line 684.
)
rules: failed to run __freemail_replyto test, skipping:
        (Can't locate object method "check_freemail_replyto" via package "Mail::SpamAssassin::PerMsgStatus" at (eval 900) line 1004.
)
rules: failed to run __freemail_hdr_replyto test, skipping:
        (Can't locate object method "check_freemail_header" via package "Mail::SpamAssassin::PerMsgStatus" at (eval 900) line 1224.
)
rules: failed to run T_FREEMAIL_FROM test, skipping:
        (Can't locate object method "check_freemail_from" via package "Mail::SpamAssassin::PerMsgStatus" at (eval 900) line 1494.
)
t/recreate....ok 3/9plugin: failed to parse plugin /home/jm/ftp/sa/trunk/t/../t/log/test_rules_copy/EmailBL.pm: Can't locate /home/jm/ftp/sa/trunk/t/../t/log/test_rules_copy/EmailBL.pm in @INC (@INC contains: ../blib/lib t . /home/jm/ftp/sa/trunk/blib/lib /home/jm/ftp/sa/trunk/blib/arch /etc/perl /usr/local/lib/perl/5.10.0 /usr/local/share/perl/5.10.0 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.10 /usr/share/perl/5.10 /usr/local/lib/site_perl) at ../blib/lib/Mail/SpamAssassin/PluginHandler.pm line 108.

[ignored warning, not recreate-related]
plugin: failed to parse plugin /home/jm/ftp/sa/trunk/t/../t/log/test_rules_copy/sandbox-hstern.pm: Can't locate /home/jm/ftp/sa/trunk/t/../t/log/test_rules_copy/sandbox-hstern.pm in @INC (@INC contains: ../blib/lib t . /home/jm/ftp/sa/trunk/blib/lib /home/jm/ftp/sa/trunk/blib/arch /etc/perl /usr/local/lib/perl/5.10.0 /usr/local/share/perl/5.10.0 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.10 /usr/share/perl/5.10 /usr/local/lib/site_perl) at ../blib/lib/Mail/SpamAssassin/PluginHandler.pm line 108.

[ignored warning, not recreate-related]
plugin: failed to parse plugin /home/jm/ftp/sa/trunk/t/../t/log/test_rules_copy/FreeMail.pm: Can't locate /home/jm/ftp/sa/trunk/t/../t/log/test_rules_copy/FreeMail.pm in @INC (@INC contains: ../blib/lib t . /home/jm/ftp/sa/trunk/blib/lib /home/jm/ftp/sa/trunk/blib/arch /etc/perl /usr/local/lib/perl/5.10.0 /usr/local/share/perl/5.10.0 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.10 /usr/share/perl/5.10 /usr/local/lib/site_perl) at ../blib/lib/Mail/SpamAssassin/PluginHandler.pm line 108.

[ignored warning, not recreate-related]
t/recreate....ok 5/9rules: failed to run T_FREEMAIL_FROM_END_DIGIT test, skipping:
        (Can't locate object method "check_freemail_from" via package "Mail::SpamAssassin::PerMsgStatus" at (eval 1856) line 194.
)
rules: failed to run __freemail_reply test, skipping:
        (Can't locate object method "check_freemail_replyto" via package "Mail::SpamAssassin::PerMsgStatus" at (eval 1856) line 239.
)
rules: failed to run T_FREEMAIL_SUBJECT test, skipping:
        (Can't locate object method "check_freemail_header" via package "Mail::SpamAssassin::PerMsgStatus" at (eval 1856) line 684.
)
rules: failed to run __freemail_replyto test, skipping:
        (Can't locate object method "check_freemail_replyto" via package "Mail::SpamAssassin::PerMsgStatus" at (eval 1856) line 1004.
)
rules: failed to run __freemail_hdr_replyto test, skipping:
        (Can't locate object method "check_freemail_header" via package "Mail::SpamAssassin::PerMsgStatus" at (eval 1856) line 1224.
)
rules: failed to run T_FREEMAIL_FROM test, skipping:
        (Can't locate object method "check_freemail_from" via package "Mail::SpamAssassin::PerMsgStatus" at (eval 1856) line 1494.
)
t/recreate....NOK 9/9# Failed test 9 in t/recreate.t at line 90
t/recreate....FAILED test 9
        Failed 1/9 tests, 88.89% okay
Failed Test  Stat Wstat Total Fail  List of Failed
-------------------------------------------------------------------------------
t/recreate.t                9    1  9
Failed 1/1 test scripts. 1/9 subtests failed.
Files=1, Tests=9,  3 wallclock secs ( 2.50 cusr +  0.10 csys =  2.60 CPU)
Failed 1/1 test programs. 1/9 subtests failed.
make: *** [test_dynamic] Error 255


I would guess there might be some kind of plugin lifecycle bug there...
Comment 8 Karsten Bräckelmann 2009-06-11 16:33:28 UTC 
The checkin from revision 783928 has the loadplugin line in 20_FreeMail_mass.cf, which lexicographically comes *after* 20_FreeMail_domains.cf.

The ifplugin in the latter will pretty much disable the plugin, cause there are no freemail_domain's loaded. Might also explain that recreation failure?
Comment 9 Justin Mason 2009-06-12 08:49:03 UTC 
(In reply to comment #8)
> The checkin from revision 783928 has the loadplugin line in
> 20_FreeMail_mass.cf, which lexicographically comes *after*
> 20_FreeMail_domains.cf.
> 
> The ifplugin in the latter will pretty much disable the plugin, cause there are
> no freemail_domain's loaded. Might also explain that recreation failure?

I think the "mkrules" compilation step from "make" rearranges them.
Comment 10 Sidney Markowitz 2009-06-19 15:41:00 UTC 
*** Bug 6138 has been marked as a duplicate of this bug. ***
Comment 11 Sidney Markowitz 2009-06-19 17:23:39 UTC 
20_FreeMail_mass.cf had a ifplugin line commented out. That made sense because
it was immediately preceded by a loadplugin, so obviously is not needed.

Unfortunately, what is obvious is not always true. When the rules in the file are moved out of that file into 70_sandbox.cf, the ifplugin becomes necessary during make test when FreeMail.pm may not be available to load.

Uncommenting the ifplugin and the associated endif seems to fix the failure.

Committed revision 786732.

Since I'm not familiar with this bug report as a whole, just trying to fix the
Hudson builds, could someone else decide if this is ready to be closed as
FIXED, assuming that Hudson finally stops complaining after this commit?
Comment 12 Justin Mason 2009-06-20 03:38:10 UTC 
I think that did it ;)
Comment 13 Justin Mason 2009-06-20 03:39:09 UTC 
oh wait.  we got sidetracked by the t/recreate.t issue.  this bug is for discussion of the FreeMail plugin itself!  reopening for that....
Comment 14 Justin Mason 2009-06-29 04:29:33 UTC 
so I'd like to add the plugin, but I guess it needs a viable host for the DNS zone first, right?
Comment 15 AXB 2009-06-29 04:35:54 UTC 
(In reply to comment #14)
> so I'd like to add the plugin, but I guess it needs a viable host for the DNS
> zone first, right?

You're confusing with EmailBL

This plugin doesn't do any lookups.
Its all "static"
Comment 16 Henrik Krohns 2009-06-29 04:38:22 UTC 
Yeah. No DNS.

One thing is needed:

freemail_domains.cf should be distributed with sa-update, but that's no problem right?
Comment 17 Justin Mason 2009-07-06 09:53:04 UTC 
(In reply to comment #16)
> Yeah. No DNS.

oh yeah.  right, duh. ;)

> One thing is needed:
> 
> freemail_domains.cf should be distributed with sa-update, but that's no problem
> right?

yep, no problem there.  OK, next task is to get it into lib/Mail/SpamAssassin with the rest of the "official" plugins etc.  should be easy enough!
Comment 18 Justin Mason 2009-07-23 04:52:08 UTC 
: 436...; svn commit -m "bug 6132: move the FreeMail ruleset and plugin to core distro" 
Sending        MANIFEST
Adding         lib/Mail/SpamAssassin/Plugin/FreeMail.pm
Adding         rules/20_freemail.cf
Adding         rules/20_freemail_domains.cf
Sending        rules/v330.pre
Deleting       rulesrc/sandbox/hege/20_FreeMail_domains.cf
Deleting       rulesrc/sandbox/hege/20_FreeMail_mass.cf
Deleting       rulesrc/sandbox/hege/FreeMail.pm
Transmitting file data ...
Committed revision 797029.