Bug 5102 - [Review] FORGED_HOTMAIL_RCVD false positive
Summary: [Review] FORGED_HOTMAIL_RCVD false positive
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Rules (Eval Tests) (show other bugs)
Version: 3.1.5
Hardware: Other Linux
: P5 normal
Target Milestone: 3.2.1
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-09-18 16:37 UTC by Tony Finch
Modified: 2007-06-07 01:28 UTC (History)
0 users


Attachment Type Modified Status Actions Submitter/CLA Status
Example Received: headers text/plain None Tony Finch [HasCLA]
Proposed patch patch None Tony Finch [HasCLA]
better (less picky) regex patch None Tony Finch [HasCLA]
proposed patch (draconian regex) patch None Alex Bramley [NoCLA]
that change, as a patch against 3.2.0 patch None Justin Mason [HasCLA]
"that change" without the unused capture patch None Daryl C. W. O'Shea [HasCLA]
Show Obsolete (4) 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 Tony Finch 2006-09-18 16:37:13 UTC 
The Received: header checks in _check_for_forged_hotmail_received_headers() do
not allow for legitimate email from Hotmail received by Exim (and perhaps other
MTAs).
Comment 1 Tony Finch 2006-09-18 16:39:04 UTC 
Created attachment 3697 [details]
Example Received: headers
Comment 2 Tony Finch 2006-09-18 16:40:14 UTC 
Created attachment 3698 [details]
Proposed patch
Comment 3 Tony Finch 2006-09-18 17:36:50 UTC 
Created attachment 3699 [details]
better (less picky) regex
Comment 4 Alex Bramley 2006-10-30 09:34:44 UTC 
Created attachment 3738 [details]
proposed patch (draconian regex)

Also noticed this issue, but I created a fairly draconian regex instead. Hope
this is fixed in the next release...

--alex
Comment 5 Tony Finch 2007-05-02 07:47:19 UTC 
Fix committed to trunk.
Comment 6 Justin Mason 2007-05-02 07:53:12 UTC 
did we miss this in 3.2.0?  drat :(

might be worth backporting to 3.2.1 as well.
Comment 7 Tony Finch 2007-05-02 08:11:56 UTC 
My bad - should have committed it ages ago
Comment 8 Tony Finch 2007-05-02 08:18:17 UTC 
Fix committed to 3.2 branch
Comment 9 Justin Mason 2007-05-02 08:25:22 UTC 
Tony -- hold on -- could you revert that change to 3.2?

the 3.2 branch is now in R-T-C mode (although I haven't officially announced
it yet), so we'll need to vote on the patch before it can be applied.

Just revert r534510, attach it as a diff to this bug, and set the milestone
to "3.2.1" for voting...
Comment 10 Tony Finch 2007-05-02 08:34:51 UTC 
Reopen for 3.2.1 review
Comment 11 Justin Mason 2007-05-02 08:56:04 UTC 
setting to 3.2.1.  looks like you weren't in the "EditBugs" bugzilla group --
you should be able to set milestones now...
Comment 12 Tony Finch 2007-05-02 09:03:24 UTC 
Looks good from here. Thanks for your help.
Comment 13 Sidney Markowitz 2007-06-04 21:42:56 UTC 
This looks ready for review and voting for 3.2.1 (This is not a vote)
Comment 14 Sidney Markowitz 2007-06-04 22:39:47 UTC 
This looks like it ought to already have two votes, from Tony and Justin, but I
can't tell exactly which patch to vote on. Tony or Justin, can you make that
clear? Then we can get it into 3.2.1.
Comment 15 Tony Finch 2007-06-05 03:05:46 UTC 
(In reply to comment #14)
> This looks like it ought to already have two votes, from Tony and Justin, but I
> can't tell exactly which patch to vote on. Tony or Justin, can you make that
> clear? Then we can get it into 3.2.1.

I intended to merge this commit from the trunk to 3.2:

http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/HeaderEval.pm?
r1=495220&r2=534488
Comment 16 Justin Mason 2007-06-06 13:54:11 UTC 
Created attachment 3971 [details]
that change, as a patch against 3.2.0

here's the change as a patch attachment.  Tony, that's the easiest way to do
patch reviews, we find, since we can then (a) test it ourselves if we feel like
it and (b) be sure that the change will still apply correctly against current
head of the branch.

my vote: +1
Comment 17 Sidney Markowitz 2007-06-06 14:33:08 UTC 
+1

Tony, normally the vote of the committer submitting the fix is implicit, but in
this case Justin actually put together the patch file we are voting on so if you
want to vote for this to bring us up to the required three votes, please do so.
Comment 18 Daryl C. W. O'Shea 2007-06-06 20:26:10 UTC 
+  if ($rcvd =~ /from (\S*\.)?hotmail.com \(\S+\.hotmail(?:\.msn)?\.com[ \)]/ &&
$ip)


+1.  Feel free to add a '?:' to eliminate the capture though.
Comment 19 Sidney Markowitz 2007-06-06 20:51:16 UTC 
> Feel free to add a '?:' to eliminate the capture though

Put it up as a patch and I'll vote on again. That way it will be official and
have two votes ready when Justin wakes up this morning (his time) and sees it.
Comment 20 Daryl C. W. O'Shea 2007-06-06 20:56:38 UTC 
Created attachment 3974 [details]
"that change" without the unused capture
Comment 21 Sidney Markowitz 2007-06-06 21:49:04 UTC 
+1 on the new patch without the capture

By the way I notice that it was committed to trunk already as r545055.
Comment 22 Tony Finch 2007-06-07 01:13:49 UTC 
+1
Comment 23 Sidney Markowitz 2007-06-07 01:28:15 UTC 
Committed to 3.2 branch revision 545100.