You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@spamassassin.apache.org on 2020/06/07 11:31:20 UTC

[Bug 7823] New: HashBL.pm confusingly not only looking at headers

https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7823

            Bug ID: 7823
           Summary: HashBL.pm confusingly not only looking at headers
           Product: Spamassassin
           Version: 3.4.4
          Hardware: PC
                OS: Windows NT
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Plugins
          Assignee: dev@spamassassin.apache.org
          Reporter: mgrant@grant.org
  Target Milestone: Undefined

The docs for HashBL.pm say you should use it like this:

header   HASHBL_EMAIL   eval:check_hashbl_emails('ebl.msbl.org.')
describe HASHBL_EMAIL   Message contains email address found on the EBL
score    HASHBL_EMAIL   9.0

This implies that it is going to look at the headers of messages.

However, the check_hashbl_emails function has multiple optional arguments.  The
second arg is called 'HEADERS' in the docs and allows you to specifiy which
headers to look at.  You can also specify 'body' which makes it look at the
body.

The default in check_hashbl_emails() is:
  $from = 'ALLFROM/Reply-To/body' if !$from;

which, if you don't specify a HEADER arg to search through though the body of
the messages.  THIS IS BIZZARLY OVERLOADED.  

I did not know the best way to fix this.  The config rule type should be
'header' or 'body' or 'full' and that should dictate which part of the message
it looks through.  The HEADER arg could still filter out specific headers.

The doc section of the module should also be cleaned up to reflect this.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7823] HashBL.pm confusingly not only looking at headers

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7823

--- Comment #5 from Henrik Krohns <ap...@hege.li> ---
(In reply to Michael Grant from comment #4)
> Ok good to know, how about for now stating this in the docs at the top of
> the HashBL.pm module?  You could argue that this is common knowledge but I
> didn't know it!

You are right. Of course there are many more similar examples, so it would need
to be mentioned everywhere. Which should not hurt.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7823] HashBL.pm confusingly not only looking at headers

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7823

Michael Grant <mg...@grant.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mgrant@grant.org

--- Comment #1 from Michael Grant <mg...@grant.org> ---
By the way, changing rule type to 'full' doesn't fully work.  The report I get
comes out like:

        *  9.0 HASHBL_EMAIL Message contains email address found on the EBL
        *      []

so it knows there's something on the black list but it doesn't list them.  I
did not debug this to understand why.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7823] HashBL.pm confusingly not only looking at headers

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7823

Henrik Krohns <ap...@hege.li> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

--- Comment #9 from Henrik Krohns <ap...@hege.li> ---
(In reply to RW from comment #8)
> (In reply to Michael Grant from comment #0)
>  
> > header   HASHBL_EMAIL   eval:check_hashbl_emails('ebl.msbl.org.')
> >...
> > This implies that it is going to look at the headers of messages.
> 
> AFAIK it implies that points from this rule will count as header points
> rather than body points. See Mail::SpamAssassin::Plugin::AutoLearnThreshold

This is good point, it might make sense to split the body check to a body
check_hashbl_emails_body eval.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7823] HashBL.pm confusingly not only looking at headers

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7823

--- Comment #4 from Michael Grant <mg...@grant.org> ---
Ok good to know, how about for now stating this in the docs at the top of the
HashBL.pm module?  You could argue that this is common knowledge but I didn't
know it!

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7823] HashBL.pm confusingly not only looking at headers

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7823

Henrik Krohns <ap...@hege.li> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Henrik Krohns <ap...@hege.li> ---
(In reply to Michael Grant from comment #4)
> Ok good to know, how about for now stating this in the docs at the top of
> the HashBL.pm module?  You could argue that this is common knowledge but I
> didn't know it!

I'm only looking at the docs now and I see that it's already documented, quite
clearly directly in chech_hashbl_emails example. I'm going to close this since
there isn't really anything to fix.


    header RULE check_hashbl_emails('bl.example.com/A', 'OPTS', 'HEADERS/body',
'^127\.')

        Check email addresses from DNS list, "body" can be specified along with
headers to search body for emails. Optional subtest regexp to match DNS answer.
Note that eval rule type must always be "header".

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7823] HashBL.pm confusingly not only looking at headers

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7823

--- Comment #6 from Henrik Krohns <ap...@hege.li> ---
I've also created Bug 7825, such mistakes should be easy to prevent.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7823] HashBL.pm confusingly not only looking at headers

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7823

Henrik Krohns <ap...@hege.li> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|REOPENED                    |RESOLVED

--- Comment #11 from Henrik Krohns <ap...@hege.li> ---
I don't think there's anything more we can do here, so closing.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7823] HashBL.pm confusingly not only looking at headers

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7823

--- Comment #10 from Henrik Krohns <ap...@hege.li> ---
(In reply to Henrik Krohns from comment #9)
>
> This is good point, it might make sense to split the body check to a body
> check_hashbl_emails_body eval.

Then again, both rules might hit the same email from headers and body,
resulting in higher score unless meta is used. This is awkward too.

Doesn't seem sensible to change to rule to "full" either, it seems
AutoLearnThreshold doesn't even consider scores from full rules. :-O

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7823] HashBL.pm confusingly not only looking at headers

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7823

Henrik Krohns <ap...@hege.li> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |apache@hege.li

--- Comment #2 from Henrik Krohns <ap...@hege.li> ---
(In reply to Michael Grant from comment #0)
> The docs for HashBL.pm say you should use it like this:
> 
> header   HASHBL_EMAIL   eval:check_hashbl_emails('ebl.msbl.org.')
>
> This implies that it is going to look at the headers of messages.

Sadly this is not true. Eval functions can and do many things, regardless of
what the "definition" is. This is the way SA has always worked.

> I did not know the best way to fix this.  The config rule type should be
> 'header' or 'body' or 'full' and that should dictate which part of the
> message it looks through.  The HEADER arg could still filter out specific
> headers.

You can't have multiple identically named eval functions. Then you would need
all separate check_hashbl_emails_header etc functions. I did the best I could
designing this with limited time and SA legacy restrictions.

I'll keep this open if there's good suggestions on configuration format,
there's still time to improve it in trunk before 4.0.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7823] HashBL.pm confusingly not only looking at headers

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7823

RW <rw...@googlemail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rwmaillists@googlemail.com

--- Comment #8 from RW <rw...@googlemail.com> ---
(In reply to Michael Grant from comment #0)

> header   HASHBL_EMAIL   eval:check_hashbl_emails('ebl.msbl.org.')
>...
> This implies that it is going to look at the headers of messages.

AFAIK it implies that points from this rule will count as header points rather
than body points. See Mail::SpamAssassin::Plugin::AutoLearnThreshold

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7823] HashBL.pm confusingly not only looking at headers

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7823

--- Comment #3 from Henrik Krohns <ap...@hege.li> ---
(In reply to Michael Grant from comment #1)
> By the way, changing rule type to 'full' doesn't fully work.

Of course it doesn't. You can NOT change the rule definition to something else.
You must use what the documentation specifies. Changing the type messes up SA
internals.

-- 
You are receiving this mail because:
You are the assignee for the bug.