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





------- Additional Comments From [EMAIL PROTECTED]  2007-12-24 07:11 -------
Comment #10 From Daryl C. W. O'Shea
> - $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_"

Fixed.

Comment #14 From Daryl C. W. O'Shea
> You have to disable the WHITELIST_FROM_DKIM rule (and the DEF_ one), too, to
> completely disable DKIM checks.  I had been supplying a patch to some ESP
> customers that only ran the DKIM check (do the crypto and DNS checks) if the
> address appeared in the DKIM whitelist config.  It makes a significant
> difference it processing time (at least wall clock wise).  I thought
> I had merged this into our codebase, it turns out I never did.  [...]
>From myself:
> This hasn't changed. In 3.2.3, the DKIM check is performed even if
> DKIM_SIGNED, DKIM_VERIFIED, DKIM_POLICY_SIGNALL, DKIM_POLICY_SIGNSOME,
> and DKIM_POLICY_TESTING are zero, as long as USER_IN_DKIM_WHITELIST
> or USER_IN_DEF_DKIM_WL are nonzero - even if a sender address is NOT
> found in a whitelist. So are you asking to make it a new feature?
>From Daryl:
> Sure, for 3.3. I honestly thought I had merged that into 3.1 way back,
> sorry. Ignore this concern too.

Actually, it turned out it was really easy to do so with the current code.
In _check_dkim_whitelist() just had to move collecting of applicable wl
addresses in front of a call to check_dkim_valid, conditionalized only
if the list of applicable wl entries is nonempty. (by applicable I mean:
entries matching the From address pattern (first parameter) in a wl entry,
but not yet checked whether the signature is valid or whether the signing
identity matches the second parameter in a wl entry).

> 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

Actually, that change (where an absent policy defaults to singsome, as it 
should) occurred in the underlying Mail::DKIM, not in the Plugin::DKIM.

As the DKIM_POLICY_SIGNSOME is useless, either a rule score could be zeroed,
or the check_dkim_signsome eval could always return false. I chose the letter,
now in the most recent patch and in trunk.

> OK, I'm confused by your "Reworked whitelisting to match the new
> documentation and expected use as per SSP draft" statement in comment #0.
> Could you elaborate on that? Pointing out, in comment #5, that RFC 5016
> has been published led me to believe that there was something of substance
> to your initial comment.

Not related to RFC 5016, just mentioned it, considering someone might
find it interesting.

The underlying reason for my whitelisting change was a fact that SSP
distinguishes between a first-party signature (also called "author signature"
or now disfavoured "originator signature") (where From matches a signing 
identity), apart from a third-party signatures (e.g. from a mailing list).
The old code was unable to express whitelisting based on a first-party
signature.

Reapeating from my comment #2:

- whitelisting first-party signatures based on a wildcarded domain
  does not work, e.g.:
    whitelist_from_dkim  [EMAIL PROTECTED]
  This is because the identity as stored in a whitelist is '*.cam.ac.uk'
  and does not literally match a signing identity e.g. 'department.cam.ac.uk'.
  For checking first-party signatures a check must me made against the
  specific author address in a mail which matched the wildcard address
  (first parameter in whitelist_from_dkim).


Here is a summary of terminology changes (reflected in variable and
in sub names, in comments and in POD section):
- has a concept of a first-party signature (= author signature);
- recognize the fact that a message may have more than one signature;
- renamed verified -> valid (in a backwards compatible way);
  semantics of 'verified' is that signatures went through a
  verification process; the outcome of which is either valid,
  failed/invalid, or none
- renamed $message to $verifier - it holds a DKIM verifier object
  with its methods, not a message;
- use name 'author' consistently for an address in a From header filed,
  for consistency with rfc4871, with SSP draft and rfc2822/2822upd;
- although the SSP draft still uses a term 'originator signature', this
  term was found inconsistent with rfc2822 (where it covers From, Sender
  and Reply-To header fields), so the DKIM WG now uses term author signature,
  which is now used in the DKIM plugin;

Summary of functional changes:
- as per #2, whitelisting can imply first-party signatures (when a second
  parameter is absent);
- as per #2, matching on identity (second parameter) was too permissive;
- disabled check_dkim_signsome (always returns false);
- supports multiple signatures for whitelisting (requires Mail::DKIM 0.29
  or later);
- new eval rule check_dkim_valid_author_sig returns true if a message
  contains a valid author signature (nor just any valid signature);
  (this was an easy byproduct of a fix to: a policy lookup should not be
  suppressed on 3rd-party signatures);
- new eval rule check_dkim_valid is an alias for check_dkim_verified misnomer;
- new tags available: _DKIMIDENTITY_ and _DKIMDOMAIN_;
- significant verification speedup on large messages by avoiding feeding
  a message to Mail::DKIM line-by-line (speedup only on versions of 
  Mail::DKIM 0.29_1 and later, i.e. the pre-releases of 0.30)

When reviewing, I suggest to look at the resulting code (almost the same
as in trunk) and not to bother figuring out the diffs. Or just enable
the 'dkim' debug areas and watch the log.



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

Reply via email to