Skip to main content

DomainKeys Identified Mail (DKIM) Signatures
draft-ietf-dkim-rfc4871bis-15

Yes

(Sean Turner)

No Objection

(Dan Romascanu)
(David Harrington)
(Gonzalo Camarillo)
(Ralph Droms)
(Robert Sparks)
(Ron Bonica)
(Wesley Eddy)

Note: This ballot was opened for revision 15 and is now closed.

Pete Resnick Former IESG member
(was Discuss) Yes
Yes (2011-06-29) Unknown
These first 15 comments are things that I think are potentially real problems, but I haven't heard enough back from the authors yet to know if they are worthy of a DISCUSSion.

1. Section 2.7: I don't understand what the word "module" means in this context. It sounds like some sort of specific implementation, not part of a protocol. I don't know what it means to deliver values to a module.

2. Section 3.5:

   v= Version (MUST be included).  This tag defines the version of this
      specification that applies to the signature record.  It MUST have
      the value "1".  Note that verifiers must do a string comparison on
      this value; for example, "1" is not the same as "1.0".

What is the intention of "string comparison" here? There's no case sensitivity to worry about. There's no character encoding to worry about. Why not instead say, "Note that verifiers must (MUST?) ensure that the value is '1'; for exmaple '1' is not the same as '1.0'"? (Seem similar text in 3.6.1.) What is this trying to convey.

Also, the value is not listed as "plain-text".

3. Section 3.5, "h=":

         INFORMATIVE EXPLANATION: By "signing" header fields that do not
         actually exist, a signer can allow a verifier to detect
         insertion of those header fields after signing.  However, since
         a signer cannot possibly know what header fields might be
         created in the future, and that some MUAs might present header
         fields that are embedded inside a message (e.g., as a message/
         rfc822 content type), the security of this solution is not
         total.

a. I don't understand what MUAs presenting "header fields that are embedded inside a message" has to do with anything.

b. I don't know what the words, "the security of this solution is not total" are supposed to mean. 

Don't you simply mean: "However, since a signer cannot possibly know what header fields might be defined in the future, this mechanism can't be used to prevent the addition of any possible unknown header fields."?

4. Section 3.5, "h=":

         INFORMATIVE EXPLANATION: The exclusion of the header field name
         and colon as well as the header field value for non-existent
         header fields allows detection of an attacker that inserts an
         actual header field with a null value.

I cannot parse that sentence. I have no idea what it means.

5. Section 3.7: "content-hash" is not defined.

6. Section 3.9:

a. Neither TEMPFAIL nor PERMAFAIL is defined at this point.

b. I have no idea what the "output of a DKIM verifier module" is supposed to mean.

I suspect that section 3.9 is at least misplaced in this document, and probably completely unnecessary as it sounds like implementation details.

7. Section 4.2, first INFORMATIVE NOTE: Why is this an informative note? It seems like good protocol adivce and therefore should say that signers SHOULD NOT sign exiting DKIM-Signauture fields.

8. Section 4.2, last paragraph: PERMFAIL is still not defined to this point.

I suspect sections 3.8 through all of section 4 belong after (or in) section 6.

9. Section 5.1: I don't know what the term "signing address" means.

10. Section 5.3:

   Therefore, a verifier
   SHOULD NOT validate a message that is not compliant with those
   specifications.

This section is about signing, not verifying. What is that sentence doing in there?

11. Section 5.4:

      INFORMATIVE ADMONITION: Despite the fact that [RFC5322] permits
      header fields to be reordered (with the exception of Received
      header fields)

5322 does *not* permit header fields to be reordered. It says:

   ...header fields SHOULD NOT be reordered when a message is transported
   or transformed.  More importantly, the trace header fields and resent
   header fields MUST NOT be reordered, and SHOULD be kept in blocks
   prepended to the message.

Suggest: "Despite the fact that [RFC5322] does not absolutely prohibit header fields to be reordered..."

12. Section 6.1:

   A verifier SHOULD NOT treat a message that has one or more bad
   signatures and no good signatures differently from a message with no
   signature at all; such treatment is a matter of local policy and is
   beyond the scope of this document.

The two clauses in that sentence seem contradictory. Is there an interoperability requirement that I not treat messages in a particular way, or is it a matter of local policy?

13. Section 6.1 (and subsections): This section is playing some sort of game. The beginning of 6.1 describes some "statuses" and says things like, "The '(explanation)' is not normative text; it is provided solely for clarification." If it wanted to give an algorithm for Verfiers to follow, where there was a certain state machine with states of "PERMFAIL" and "TEMPFAIL", that would have been OK. But it is clear from the use of the phrasing, for example, "return PERMFAIL (signature syntax error)", the document is trying to sneak in some sort of actual APIs into the protocol spec. I think this is bogus and would much prefer that these sections be rewritten to say, "enters a PERMFAIL state", perhaps labeling each paragraph with the explanation. For example, the first paragraph of 6.1.1 could read:

   Signature syntax
   
   Implementers MUST meticulously validate the format and values in the
   DKIM-Signature header field; any inconsistency or unexpected values
   MUST cause the header field to be completely ignored and the verifier
   enters a PERMFAIL state.  Being "liberal in what you accept" is
   definitely a bad strategy in this security context.  Note however that
   this does not include the existence of unknown tags in a DKIM-Signature
   header field, which are explicitly permitted.
   
   Version compatibility
   
   Verifiers MUST enter a PERMFAIL state when presented a DKIM-Signature
   header field with a "v=" tag that is inconsistent with this
   specification.

The current text is too cute by half, and I think it obfuscates the meaning.

14. Section 6.1, first "INFORMATIVE NOTE" on attack by many bad sigs: This is a security consideration instead, not an informative note. Should be a forward reference to section 8.3

15. Section 6.1.3: I don't understand the meaning of the note, "(note that this version does not actually need to be instantiated)".

These last 10 comments are simply stylistic and editorial stuff. If they make sense to fix, great. If not, I'm not concerned.

1. I find the use of the word "INFORMATIVE" throughout the document superfluous. No other word (e.g., NORMATIVE) is used in its place. I think they should all be removed. They serve no purpose.

2. The ABNF "0*" construct is not normally used. Just "*" is sufficient.

3. Section 3.4.4:

      INFORMATIVE NOTE: It should be noted that the relaxed body
      canonicalization algorithm may enable certain types of extremely
      crude "ASCII Art" attacks where a message may be conveyed by
      adjusting the spacing between words.  If this is a concern, the
      "simple" body canonicalization algorithm should be used instead.

I think this MITM attack (the ability to insert and delete whitespace to send coded ASCII Art messages to the recipient) is so far fetched as to not be worthy of mention. If the WG really thinks it is worthy of mention, it should go in security considerations.

4. Section 3.5: It would be nice to subsection each tag here. (e.g., "v=" could be 3.5.1, etc.)

5. Section 3.5, "h=":

It would be nice to add a sentence similar to the one in 5.4: "The field MAY contain multiple instances of a header field name; this means that multiple occurrences of the header field are being signed by the signing algorithm."

6. Section 3.6.1: It would be nice to subsection each of the tags.

7. Section 3.10: Is this not completely redundant with the text in 3.6.1 regarding "t=s"?

8. Section 4.1: The "INFORMATIVE EXAMPLES" don't need to be called out as such. The title of the section is "Example Scenarios". All of the text here is example, and as such it is all informative.

9. Section 5.1, INFORMATIVE NOTE: Instead of saying "Signing modules may be incorporated into any", how about "A signer can be implemented as part of any"?

10. Section 5.5: Though section 5.5 is titled "Recommended Signature Content", it is clear that the entire section is devoted to the topic of section 5.4: "Determine the Header Fields to Sign". These two sections should be combined, and might be subsections of a larger section. But it was very confusing to read these as separate.
Sean Turner Former IESG member
Yes
Yes () Unknown

                            
Stephen Farrell Former IESG member
Yes
Yes (2011-06-24) Unknown
Good job.

Other, than (6) & (8) below, which I'd ask that you check, I'm 
entirely fine if you totally ignore these - they're just the notes I 
made as I read the thing again (after not having had to do that 
for a while:-) and are basically all nitty suggestions.

(1) 2nd para of 2.6 refers to "i=" and "t=s" before those have
been introduced. I wouldn't suggest defining them here, so I'm
not sure what to change to make it better but maybe worth a look.
Maybe you could get rid of the tags though for example by saying: 

  "Note that acceptable values for the AUID may be constrained via
   a flag in public key record. (See Section 3.6.1)"

(2) end of p20, maybe s/are expected to/may/ since we're doing a
new spec now but not changing the version number.

(3) 2nd last para, p23: signing non-existent header fields does
not "prevent" later insertion, it allows detection of that after
the fact. Same issue with last para on p23 as well. The change is
obvious I guess if you want to make it. 

(4) p26, is "DNS TXT record" or "DNS TXT resource record" more
correct? (And if so, do we care or not:-) If the latter then the
same change would be needed in a few places.

(5) Would it be useful for the reader and/or IANA to note that
there are no new tags defined in section 7 compared to rfc4871?

(6) Is 7.9 actually correct? That IANA registry references 4871 
but should that be changed to this document or left alone? I've
no idea.

(7) Could/should appendix B.2.3 have an informative reference to
dkim-mailinglists? Since that's now approved, it won't add any
significant time for the RFC editor to do those together.  (I
think.)

(8) The last paragraph of appendix C is odd - maybe the reference
in there should be to rfc4870?
Adrian Farrel Former IESG member
No Objection
No Objection (2011-06-29) Unknown
I note that in Section 1.2 of RFC 4871 (May 2007) it says "there are currently over 70 million domains." So while Section 1.3 of this I-D is not wrong to say "there are currently over 70 million domains," it may be a significant underestimate.

I am not completely comfortable with the use of "normative" RFC 2119 language in ABNF comments. For example...
   dkim-safe-char        =  %x21-3A / %x3C / %x3E-7E
                               ; '!' - ':', '<', '>' - '~'
                               ; Characters not listed as "mail-safe" in
                               ; [RFC2049] are also NOT RECOMMENDED.

A terrible nit, but would you consider s/may/might/ in...
   o  The practical constraint that large (e.g., 4096 bit) keys may not
      fit within a 512-byte DNS UDP response packet
Dan Romascanu Former IESG member
No Objection
No Objection () Unknown

                            
David Harrington Former IESG member
No Objection
No Objection () Unknown

                            
Gonzalo Camarillo Former IESG member
No Objection
No Objection () Unknown

                            
Peter Saint-Andre Former IESG member
No Objection
No Objection (2011-06-30) Unknown
1. It's good to see draft-ietf-dkim-implementation-report, which has 
a thorough overview of the interoperability testing held in 2007.
However, that I-D does not discuss interoperability regarding the 
clarifications in RFC 5672. Are they covered here? Does the community 
have enough experience with them to enable us to say that there are at
least two interoperable implementations of RFC 5672?

2. In Section 3.2, why not reference RFC 4648, perhaps with text about 
line feeds (etc.), instead of Section 6.8 of RFC 2045?

3. In Section 3.2, we might consider adding an informative reference to  
RFC 3629 with regard to UTF-8.

4. In Secton 3.6.2, we might consider adding a normative reference to  
RFC 1464 with regard to DNS TXT RRs.

5. In Section 6.1, we might consider adding an informative reference to
RFC 4732 with regard to denial of service attacks. (Also Sections 8.3, 
8.7, 8.12.)

6. In Section 6.1.1, we might consider changing MAY to SHOULD here:

   Verifiers MAY ignore the DKIM-Signature header field if the domain
   used by the signer in the "d=" tag is not associated with a valid
   signing entity.  For example, signatures with "d=" values such as
   "com" and "co.uk" may be ignored.  The list of unacceptable domains
   SHOULD be configurable.

This seems like something we'd recommend, not describe as purely 
optional.

7. In Section 7, "one has been obsoleted" makes it sound as if a new 
tag has been defined to replace it (as in RFC 2026); we might consider
changing it to "one has been designated as historic".
Ralph Droms Former IESG member
No Objection
No Objection () Unknown

                            
Robert Sparks Former IESG member
No Objection
No Objection () Unknown

                            
Ron Bonica Former IESG member
No Objection
No Objection () Unknown

                            
Russ Housley Former IESG member
No Objection
No Objection (2011-06-29) Unknown
  Appendix E should probably include a pointer to the errata.
  Some documents have included a URL for this purpose.
Wesley Eddy Former IESG member
No Objection
No Objection () Unknown