Test failure with extra headers in message

Added test case to trunk. This seems to be fixed as of at least 0.4.2 - please confirm that test case would trigger the bug. What version was it broken with? Changing the status to fixed (if I'm allowed).

Oops, found typo in test case. Now test case confirms bug. :-}

I split out a select_headers function, which should help debug this.

It fails when there is more than one header field with the same name and different values. I can simplify the test case now.

Problem is that verify calls hash_headers(), which hashes repeat headers from last to first, whereas sign does its own header hashing, which hashes repeat headers from first to last.

Fixed sign to call hash_headers just like verify. Test case now passes.

Fix sign to use hash_headers like verify does.

So they are both last to first now? I'm curious why you did last to first? The DKIM RFCs don't help answering this question for two headers with the same name, but for different headers it mandates hashing using the ordering in h= (rfc 4871 3.7). Based on that, I would have thought first to last would be more 'expected'.

Thoughts?

The h= discussion in 3.5 is even stronger. "The field MUST contain the complete list of header fields in the order presented to the signing algorithm." Those two together (3.5 and 3.7) I think mean it has to be first to last. (nevermind if i misunderstood your comment, I didn't check the code).

For completeness ...

If it's the same header repeated, last to first is correct:

Signers choosing to sign an existing header field that occurs more than once in the message (such as Received) MUST sign the physically last instance of that header field in the header block. Signers wishing to sign multiple instances of such a header field MUST include the header field name multiple times in the h= tag of the DKIM-Signature header field, and MUST sign such header fields in order from the bottom of the header field block to the top. The signer MAY include more instances of a header field name in h= than there are actual corresponding header fields to indicate that additional header fields of that name SHOULD NOT be added.

Yes, both sign repeated headers last to first now according to spec.

However, the old code removed non-existing headers from include_headers. The new code does not. This means that verify will fail if any header mentioned in include_headers but not in the signed message (or mentioned in include_headers more times than it occurs in the message) is subsequently added to the message. This is documented feature in the RFC, so I think this is the way specifying an exact list with include_headers should work.

The solution for bug #799175 will provide SHOULD and FROZEN header lists, which sign the listed headers N and N+1 times respectively, where N is the number of occurrences in the message.