Bug 6522 - HELO_DYNAMIC_SPLIT_IP suboptimal
Summary: HELO_DYNAMIC_SPLIT_IP suboptimal
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Rules (show other bugs)
Version: 3.3.1
Hardware: PC Linux
: P2 minor
Target Milestone: Undefined
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-09 20:25 UTC by Cedric Knight
Modified: 2011-12-13 18:30 UTC (History)
2 users (show)


Attachment Type Modified Status Actions Submitter/CLA Status
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 Cedric Knight 2010-12-09 20:25:59 UTC 
HELO_DYNAMIC_SPLIT_IP peculiarly will hit "HELO 12.34.56.78.ahost.com", but not "HELO 12.3.45.67.ahost.com" (that is, with the second octet being only one decimal digit).  RCVD_NUMERIC_HELO will trigger on both, and also "HELO 12.34.56.78", which can give rise to some high scores when there is a rare FP.

The comment for the rule gives a case which does not start with a dotted quad address at all:
# 118.Red-80-35-201.pooles.rima-tde.net
header HELO_DYNAMIC_SPLIT_IP X-Spam-Relays-Untrusted =~ /^[^\]]+ helo=\d+\.\S+\d+[^\d\s]\d+[^\d\s]\d+[^\d\s]/

I would suggest replacing this with a rule that excludes the dotted-quad HELOs caught by RCVD_NUMERIC_HELO.  I have tried this, and it hits more spam than the existing version, and no more ham.

header CK_HELO_DYNAMIC_SPLIT_IP X-Spam-Relays-Untrusted =~ /^[^\]]+ helo=(?!(?:\d+\.){4})\d+[^\d\s]+\d+[^\d\s]\d+[^\d\s]\d+[^\d\s]/

I also submit the following related rules as getting a good S/O and covering dynamic HELOs not otherwise caught:
header CK_HELO_DYNAMIC_POOL   X-Spam-Relays-Untrusted =~ /^[^\]]+ helo=(?=\S*(?:pool|dyna|lease|dial|dip|static))\S*\d+[^\d\s]+\d+[^\]]+ auth= /i

header   __HELO_MISC_IP        X-Spam-Relays-Untrusted =~ /^[^\]]+ helo=[^ a-z\?]\S{0,30}(?:\d{1,3}[^\d]){4}[^\]]+ auth= /
meta     HELO_MISC_IP          (__HELO_MISC_IP && !HELO_DYNAMIC_IPADDR && !HELO_DYNAMIC_IPADDR2 && !FH_HELO_EQ_D_D_D_D && !HELO_DYNAMIC_SPLIT_IP && !HELO_DYNAMIC_HCC && !HELO_DYNAMIC_DIALIN && ((TVD_RCVD_IP4 + TVD_RCVD_IP + RCVD_NUMERIC_HELO) <2))
Comment 1 Kevin A. McGrail 2011-11-01 22:34:41 UTC 
I've added these rules for sandbox testing.

If they show good results, you are suggesting HELO_DYNAMIC_SPLIT_IP is replaced, correct?

svn commit -m 'Dynamic Helo Rules for Testing per Bug 6522'
Adding         rulesrc/sandbox/kmcgrail/20_fake_helo_tests.cf
Transmitting file data .
Committed revision 1196336.
Comment 2 Matus UHLAR - fantomas 2011-12-12 09:24:36 UTC 
Are you sure that a host having "static" has to be detected as dynamic by CK_HELO_DYNAMIC_POOL?
Unless this rule is supposed to detect something different than it appears (generic IP's), I suggest to remove the "static" from it.
Comment 3 Kevin A. McGrail 2011-12-12 15:28:05 UTC 
(In reply to comment #2)
> Are you sure that a host having "static" has to be detected as dynamic by
> CK_HELO_DYNAMIC_POOL?
> Unless this rule is supposed to detect something different than it appears
> (generic IP's), I suggest to remove the "static" from it.

I would argue static implies a generic rPTR.  Have you ever used it in a real rPTR?
Comment 4 Matus UHLAR - fantomas 2011-12-13 08:06:25 UTC 
Then, please rename CK_HELO_DYNAMIC_POOL to CK_HELO_GENERIC as they do NOT indicate dynamic addresses. 
There is a big difference between generic DNS names and dynamic pools.
Comment 5 Kevin A. McGrail 2011-12-13 18:30:01 UTC 
(In reply to comment #4)
> Then, please rename CK_HELO_DYNAMIC_POOL to CK_HELO_GENERIC as they do NOT
> indicate dynamic addresses. 
> There is a big difference between generic DNS names and dynamic pools.

Makes sense to me.  Good input!

svn commit -m 'renaming CK_HELO_DYNAMIC_POOL to CK_HELO_GENERIC to better reflect the tests actual implementation - bug 6522' 20_fake_helo_tests.cf 
Sending        20_fake_helo_tests.cf
Transmitting file data .
Committed revision 1213831.

regards,
KAM