[ietf-dkim] Re: Last Call: 'DomainKeys Identified Mail (DKIM) Signatures' to Proposed Standard (draft-ietf-dkim-base)

[Eric Allman]

Tony,

I decided to limit this response to you and ietf-dkim since your 
comments are primarily editorial rather than substantive.

Also, I apologize to all of you for going silent.  I've been 
traveling a huge amount and haven't had much time to work on IETF 
stuff.  At the moment I'm staying at a friend's house in Tokyo and 
borrowing his excellent connectivity and Cinema display to try to 
catch up somewhat.

--On November 7, 2006 10:45:47 PM -0500 Tony Hansen <tony at att.com> 
wrote:

> I have various minor nits with the base document. Overall I
> consider the document ready to go; these nits can be taken care of
> during AUTH48.
>
> 	Tony Hansen
> 	tony at att.com
>
> 1. Introduction
>
>   o archival is not a design goal
>     ^^^^^^^^
>
> All of the other bullet items have full sentences, but this one does
> not. (Archival is an adjective, not a noun.) Suggestions are
> replacing "archival" with "message archiving" or "archiving".

Done.  I went with "message archiving".

> 3.2 Tag=Value Lists
>
> < however, no encoding may include the semicolon (";")
>
> This is slightly ambiguous, as it could be read as saying that the
> value being encoded cannot have a semicolon in it, as opposed to
> the encoded version of the value. I suggest that this be changed to
> read
>
>> however, no encoding may use an unencoded semicolon (";")
> or
>> however, no encoding may produce an unencoded semicolon (";")

I realized that the two clauses aren't really related, so I changed 
this to

        The name of the tag will determine the encoding of each
        value.  Unencoded semicolon (";") characters MUST NOT occur
        in the tag value, since that separates tag-specs.

> 3.3.1 The rsa-sha1 Signing Algorithm
> 3.3.2 The rsa-sha256 Signing Algorithm
>
> < (defined in PKCS#1
> < (actually PKCS#1
>
> This wording is different between these two sections and could lead
> someone to wonder what is different in their treatment. I suggest
> that they be made the same.

I changed them both to "defined in".

> 3.3.3 Key sizes
>
> < See [RFC3766] for further discussion of selecting
>> See [RFC3766] for further discussion on selecting

Done.

> 3.5 The DKIM-Signature header field
>
> < after the body of the message;
>
> We no longer include the body of the message in the digest
> calculation. I suggest changing this fragment to:
>
>> after the rest of the header fields being signed;


> 5.4 Determine the header fields to Sign
>
> < Signers MAY claim to have signed ...
> < ...
> < Signers choosing to sign ....
> < The signer MAY include more instances of a header field ...
>
> The sentence "The signer MAY" is at the end of the paragraph
> starting "Signers choosing". It would make more sense to move this
> information to the paragraph starting "Signers MAY claim".

I'm not sure I agree on this one.  The previous paragraph hasn't 
talked about signing multiple instances of the same header, so the 
change you propose will create a forward reference.

> 6.1.3 Compute the Verification
>
> < 3. Verify that the hash of the canonicalized message body
> computed ...
>
> What happens if the hash does not match? Suggested addition is:
>
>> If the hash does not match, the verifier SHOULD ignore the
>> signature and return PERMFAIL (body hash did not verify).

Done.

> < used the "N-4" trick ... informative note in Section 5.5.
>
> The informative note is really in Section 3.4.5, but is not
> referred to there as 'the "N-4" trick'. The reader may have a
> problem following this reference. I suggest you change it to this:
>
>> used the "N-4" trick (omitting the final "--CRLF") ... informative
>> note in Section 3.4.5.

Done.

> 6.2 Communicate Verification Results
>
> < Verifiers may wish to delete existing results header fields after
> < verification and before adding a new header field to circumvent
> this < attack.
>
> This sentence is a bit awkward. I suggest his instead:
>
>> To circumvent this attack, verifiers may wish to delete existing
>> results header fields after verification and before adding a new
>> header field .

Done.

> A.1 The user composes an email and A.2 The email is signed
>
> The message as shown in these two sections are not identical. In
> particular, the indentation before "Joe" is different in the two
> examples. The A.2 and A.3 examples are consistent, so I suggest
> that the example in A.1 be changed. An alternative is to provide an
> explicit dump of the example in A.1 so the exact bytes are known,
> such as:
>
> 0000000   F   r   o   m   :       J   o   e       S   i   x   P   a
> c 0000020   k       <   j   o   e   @   f   o   o   t   b   a   l
> l   . 0000040   e   x   a   m   p   l   e   .   c   o   m   >  \r
> \n   T   o 0000060   :       S   u   z   i   e       Q       <   s
> u   z   i   e 0000100   @   s   h   o   p   p   i   n   g   .   e
> x   a   m   p   l 0000120   e   .   n   e   t   >  \r  \n   S   u
> b   j   e   c   t   : 0000140       I   s       d   i   n   n   e
> r       r   e   a   d   y 0000160   ?  \r  \n   D   a   t   e   :
> F   r   i   ,       1   1 0000200       J   u   l       2   0   0
> 3       2   1   :   0   0   : 0000220   3   7       -   0   7   0
> 0       (   P   D   T   )  \r  \n 0000240   M   e   s   s   a   g
> e   -   I   D   :       <   2   0   0 0000260   3   0   7   1   2
> 0   4   0   0   3   7   .   4   6   3   4 0000300   1   .   5   F
> 8   J   @   f   o   o   t   b   a   l   l   . 0000320   e   x   a
> m   p   l   e   .   c   o   m   >  \r  \n  \r  \n 0000340   H   i
> .  \r  \n  \r  \n   W   e       l   o   s   t       t 0000360   h
> e       g   a   m   e   .       A   r   e       y   o   u 0000400
> h   u   n   g   r   y       y   e   t   ?  \r  \n  \r  \n 0000420
> J   o   e   .  \r  \n
> 0000433

I've fixed the examples to match rather than adding the dump, since 
that would need to be done in either case.

> A.2 The email is signed
>
> The hashes in this section are not what would be produced by the
> sample private key found in Appendix C. (This was discussed at one
> of the past meetings.) Change bh= and b= to the following values:
>
> bh=jpltwNFTq83Bkjt/Y2ekyqr/+i296daNkFZSdaz8VCY=;
>
> b=bnUoMBPJ5wBigyZG2V4OG2JxLWJATkSkb9Ig+8OAu3cE2x/er+B7Tp1a1kEwZKdOt
> lTHlvF4JKg6
> RZUbN5urRJoaiD4RiSbf8D6fmMHtzEn8/OHpTCcdLOJaTp8/mKz69/RpatVBas2OqWa
> s7jrlaLGf HdBktHs6fxOzzAB7Wro=;

Argh!  I thought I had gotten these.

It turns out that the spacing in the DKIM-Signature: header field 
also differed between A.2 and A.3.
>
> A.3 The email signature is verified
>
> < The result ... is stored in the "Authentication-Results" header
> < ...
> < X-Authentication-Results: shopping.example.net
> <    header.from=joe at football.example.com; dkim=pass
>
> These are not consistent, and imply the pre-existence of the A-R
> header. Also, it is not the header.from value that has been
> authenticated, but the i= value within the dkim-signature. I
> suggest that these lines be changed to:
>
> < The result ... could be stored in a header named something like
> < "Dkim-Authentication-Results"
> < ...
> < Dkim-Authentication-Results: shopping.example.net
> <    header.dkim.i=joe at football.example.com

I rephrased this as: ``The result of the query and subsequent 
verification of the signature is stored (in this example) in the 
"X-Authentication-Results" header field line.''  Do you think that's 
good enough?

Thanks for your comments and obviously very detailed reading.

eric