looks like this will block the 3.3.0 recut.

I figured out how it got into the tarball, but not the sa-update channel.

I fixed this issue in svn right before the 3.3.0 cut.  Unfortunately the nightly auto-promote did not remove those __* rules from rules/active.list.  It seems anything listed in active.list goes into the rules tarball.  Subsequent auto-promotions cleaned it up from active.list so it was no longer an issue.

So we can safely cut the 3.3.0 release after verifying that rules/active.list contains what we want.

Leave this bug open to add further safeties to the update_stable scripts after 3.3.0.

T_KHOP_BOTNET_2 is a separate issue from ANBREP.

72_active.cf:meta	 KHOP_HELO_BOT	__HELO_NO_DOMAIN && T_KHOP_BOTNET_2
72_active.cf:describe KHOP_HELO_BOT	Suspect botnet sender claims no domain name

rulesrc/sandbox/khopesh/20_khop_general.cf:
meta     KHOP_HELO_BOT  __HELO_NO_DOMAIN && KHOP_BOTNET_2
describe KHOP_HELO_BOT  Suspect botnet sender claims no domain name

rulesrc/sandbox/khopesh/20_s25r.cf:
meta     KHOP_BOTNET_2  __LAST_EXTERNAL_RELAY_NO_AUTH && !(__FROM_FREEMAIL || __NOT_SPOOFED || __GREYLISTED) && (__S25R_3 || __S25R_4 || __S25R_5 || __S25R_6 || (RDNS_DYNAMIC + __S25R_1 + __S25R_2) > 1)
describe KHOP_BOTNET_2  Relay looks like a dynamic address
tflags   KHOP_BOTNET_2  nopublish

rules/active.list:
# good enough
KHOP_HELO_BOT

KHOP_HELO_BOT was auto-promoted, but it relies upon KHOP_BOTNET_2 which is marked as nopublish in khopesh's sandbox.  The active.list and meta boolean appear to be forcing T_KHOP_BOTNET_2 to be an active rule despite nopublish?

It appears the safest thing we can do now is switch KHOP_HELO_BOT to nopublish, remove it from rules/active.list.  Then we should be able to safely cut the 3.3.0.  We can decide if it is a good idea to enable KHOP_HELO_BOT and an appropriate score later.

Any objections?

This does suggest there is a bug having to do with auto-promotion, where auto-promote forcefully publishes a meta boolean dependent rule even if that dependency is nopublish.  Let us solve this for 3.3.1.

Created attachment 4654 [details]
Make KHOP_HELO_BOT nopublish, remove from active.list

Trivial patch.  Any objections?

I suspect with this patch and perhaps Bug #6295 we can safely cut 3.3.0.

+1 on the patch, in case it needs a vote

Sending        rules/active.list
Sending        rulesrc/sandbox/khopesh/20_khop_general.cf
Transmitting file data ..
Committed revision 901410.

ANBREP and T_KHOP_BOTNET_2 now removed from active.list.  It should now be safe to cut 3.3.0.

so a recut now will avoid the ANBREP rules issue entirely?

btw, rule changes do not require votes, and +1 on the patch anyway ;)

Created attachment 4655 [details]
mkrules_nopublish.t

here's a test script that exercises this bug.  I haven't been able to fix the issue though -- mkrules currently doesn't understand "nopublish" in itself -- the rules/active.list generator does -- but I think it does.

moving most remaining 3.3.0 bugs to 3.3.1 milestone

reassigning, too

nopublish rules for online updates that involve network traffic are particularly bad.  The current UPDATE version 912513 contains 22 nopublish rules, 14 of which are net rules -- including new DNSBLs like T_RCVD_IN_NIX_SPAM that result in additional DNS traffic.  Perhaps the recommendation should be for sa-update to be disabled until this bug is fixed.

As a large site, we keep external DNS traffic to an absolute minimum and rely on DNSBLs that are served up locally via rbldnsd to keep reasonable scantimes and not flood unsuspecting DNSBLs.

(In reply to comment #11)
> nopublish rules for online updates that involve network traffic are
> particularly bad.  The current UPDATE version 912513 contains 22 nopublish
> rules, 14 of which are net rules -- including new DNSBLs like

Ick.  I wasn't aware of this.  I saw a couple of comments via email that said "I fixed this" but it appears that it wasn't fixed, just worked around for a single day.

> T_RCVD_IN_NIX_SPAM that result in additional DNS traffic.  Perhaps the
> recommendation should be for sa-update to be disabled until this bug is fixed.

No, please don't promote that people should disable sa-update.  If people do, we won't be able to get the rules revoked by a subsequent update.

I'm looking into this now.

> As a large site, we keep external DNS traffic to an absolute minimum and rely
> on DNSBLs that are served up locally via rbldnsd to keep reasonable scantimes
> and not flood unsuspecting DNSBLs.

Hmm.  I'm not yet sure how to best handle this.  We want to be able to push out new DNSBLs via updates, but I completely understand (and live) your situation.

(In reply to comment #12)
> Hmm.  I'm not yet sure how to best handle this.  We want to be able to push out
> new DNSBLs via updates, but I completely understand (and live) your situation.

This hasn't been a significant issue in the past, since the DNSBLs in SpamAssassin have been rather static.  I believe most production DNSBLs in SA know that they'll be receiving a lot of traffic, perhaps from large sites that haven't yet disabled or modified the default rules.  If adding new DNSBLs becomes common in SA online updates, we'll likely just script a system where we're notified of additions.

(In reply to comment #13)
> If adding new DNSBLs
> becomes common in SA online updates, we'll likely just script a system where
> we're notified of additions.

Yeah, I pondering if there's something we can do for people who don't bother to try and detect new DNSBLs but shouldn't be querying public name servers.  Maybe there needs to be a config option to signify if you're someone who can or can not use public name servers.

Any who... I've committed what is probably a dirty hack to build/mkrules to avoid publishing these nopublish rules.

[dos@cyan trunk]$ svn ci -m 'bug 6297: HACK? prevent rules with "tflags nopublish" from appearing in 72_active.cf; note that this needs testing to see what happens if a meta rule does something like "&& !$rule", where $rule has tflags no publish'
Sending        build/mkrules
Transmitting file data .
Committed revision 915656.

(In reply to comment #14)
> Any who... I've committed what is probably a dirty hack to build/mkrules to
> avoid publishing these nopublish rules.
> 
> [dos@cyan trunk]$ svn ci -m 'bug 6297: HACK? prevent rules with "tflags
> nopublish" from appearing in 72_active.cf; note that this needs testing to see
> what happens if a meta rule does something like "&& !$rule", where $rule has
> tflags no publish'
> Sending        build/mkrules
> Transmitting file data .
> Committed revision 915656.

this came up yesterday with FORM_FRAUD_3 et al in rulesrc/sandbox/jhardin/20_lotsa_money.cf , which was a meta rule depending on a "tflags nopublish" rule.  t/basic_meta.t caught it, so we can detect it.

: 103...; svn commit -m "add more linting to the mkupdates sanity test stage, using the t/basic*.t t
est cases.  will catch 'tflags nopublish' leakage, bug 6297" build/mkupdates/                       
Sending        build/mkupdates/run_part2
Transmitting file data .
Committed revision 917941.



With this change, updates will not build if there's a "tflags nopublish" rule in the output.  moving this down the priority list as the key issue is now resolved, it's just a matter of finding a cleaner (ie. in mkrules) way to do it.

> With this change, updates will not build if there's a "tflags nopublish" rule
> in the output.  moving this down the priority list as the key issue is now
> resolved, it's just a matter of finding a cleaner (ie. in mkrules) way to do
> it.

I was wrong -- there's now a different script generating 3.3.x updates, /home/updatesd/svn/mkupdates-with-scores/mkupdate-with-scores ,  so this change needs to be merged into that too :(  bumping back up priority.

this doesn't need to block 3.3.1.  it's an ongoing issue, not release-related.

moving all open 3.3.1 bugs to 3.3.2

Moving back off of Security, which got changed by accident during the mass Target Milestone move.

Comment #14 said:
> Any who... I've committed what is probably a dirty hack to build/mkrules to
> avoid publishing these nopublish rules.

> [dos@cyan trunk]$ svn ci -m 'bug 6297: HACK? prevent rules with "tflags
> nopublish" from appearing in 72_active.cf; note that this needs testing to see
> what happens if a meta rule does something like "&& !$rule", where $rule has
> tflags no publish'
> Sending        build/mkrules
> Transmitting file data .
> Committed revision 915656.

Apparently this hack had the unpleasant side-effect of also removing certain rules from 70_sandbox.cf, meaning they are no longer in the masscheck.  This was filed as Bug #6527.

First pass considered resolved and the rest is a duplicate of bug 6527 where there is more information

(In reply to comment #22)
> First pass considered resolved and the rest is a duplicate of bug 6527 where
> there is more information

No, bug 6527 is an unwanted side-effect of the hack added in comment #14 here.  It seems the original problem of THIS bug was never fixed.

(In reply to comment #23)
> (In reply to comment #22)
> > First pass considered resolved and the rest is a duplicate of bug 6527 where
> > there is more information
> 
> No, bug 6527 is an unwanted side-effect of the hack added in comment #14 here. 
> It seems the original problem of THIS bug was never fixed.

The cf file in question was NOT marked nopublish. I have updated it as such and confirmed that make build_rules does not include it.  This bug is resolved though the related bug that 70_sandbox.cf is not correct in bug 6527 is very valid.