[Bug 5662] [review] DKIM plugin overhaul - whitelisting and terminology

[bu...@bugzilla.spamassassin.org]

http://issues.apache.org/SpamAssassin/show_bug.cgi?id=5662





------- Additional Comments From spamassassin@dostech.ca  2007-12-16 15:32 -------
Sorry for dragging my ass on this... I still haven't tried this out, I've only
looked over the patch quickly a couple weeks ago.

I few issues I've spotted so far:

 - $scan->{dkim_key_testing} is always 0
 - $scan->{match_in_whitelist_auth} could be named better so that it's not open
   to easy naming collision with development in other modules; perhaps just 
   prepend "dkim_" to "match_in_whitelist_"

and what I'm really concerned about:

 - it's now impossible to use DKIM for whitelisting only by setting the scores 
   for the DKIM rules to 0; this forces all DKIM signed messages to be 
   processed... something that isn't exactly light-weight; many, many ESPs are 
   extermely adverse to the crypto load incurred for (currently) no benefit 
   (DKIM pass/verified/whatever doesn't currently get you anything without 
   reputation or a whitelist) so not being able to restrict processing to 
   whitelist eligible messages is not good and definitely something I don't want
   to see changed midway through a stable branch

     - this, by the way, is why the non-intuitive whitelist loops seemed to be 
       inside out... I should have documented that, sorry :(

Also... I'm not sure exactly how much the plugin has changed to accommodate the
current SSP RFC, but with all of the recent debate about SSP issues on the DKIM
list I'm hesitant to make major SSP related changes at this time.  Either an
outline by Mark about what exactly was changed in regards to SSP, or me getting
around to reviewing what was changed myself will be required to figure out
exactly how hesitant I am to make this change now, especially mid way through a
stable branch.

More also... while I'm thinking of it... we really need to revert the change to
the DKIM_POLICY_SIGNSOME eval that causes it to use the default policy, rather
than the explicit policy as the former is absolutely useless (at least without a
way for rules to differentiate between the former and latter).  The current way
is as silly as having a rules like PLAIN_TEXT_EMAIL and NON_PLAIN_TEXT_EMAIL and
having PLAIN_TEXT_EMAIL fire regardless of whether there's a content type that
says its plain text.  The current situation was brought on solely by a way too
literal interpretation of the DKIM RFC.  SA rules shouldn't have to act as the
output of an RFC compliance validator.



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