#1 saying "proof" and "non-repudiation" in the abstract is a
mistake and potentially misleading. Rephase to use less difficult
terms, e.g. talk about signatures being evidence rather than proof.
Given the sensitivity over the exact wording of the abstract, I'm not
willing to make this change without discussion of some proposed
wording.
#2 1.1, 1st set of bullets. The biggest difference between dkim and
s/mime or pgp signatures IMO is that with dkim we do not expect
that signature failure will lead to message (delivery) failure,
whereas with s/mime or pgp we do. I'd mention this in a bullet.
There seems to be a difference of opinion on this, so I'll leave it
for now.
#3 1.1, 2nd set of bullets. dkim *does* require a ttp - the DNS.
Better to say that dkim requires no *new* ttp.
I don't see DNS as a "third party" in the same sense as a CA for
certs. Yes, DNS has to work, but it isn't a third party (unless you
want to count the root servers, I suppose). By this logic, we should
also include the multiple third parties that run the routers and all
the rest of the infrastructure.
#4 1.1, last para. Seems a bit early to introduce selectors here -
was there a reason (that still applies)? If not, then maybe this
can be deleted now.
I don't see it as necessary. Deleted.
#4 3.2, 1st sentence. "multiple key signing/verfication algorithms"
is not a good phrase, suggest changing to "multiple digital
signature algorithms" instead.
I hope you mean 3.3, otherwise you may be reading a very out of date
draft. Changed.
#5 3.3.1 and 3.3.1, phraseology is still a bit odd. Suggest
changing from: "That hash is then signed by the signer using the
RSA algorithm (defined in PKCS#1 version 1.5 [RFC3447]; in
particular see section 5.2) with an exponent of 65537 as the
crypt-alg and the signer's private key. The hash MUST NOT be
truncated or converted into any form other than the native binary
form before being signed." ...to... "The signature is calculated
using the RSA algorithm with a fixed public exponent of 65537 - if
a different public exponent is required, then a new DKIM signing
algorithm must be defined."
Adding the specific reference was specifically requested by another
reviewer --- in fact, I think your proposal changes it back to almost
exactly what was objected to before. I think this requires some
discussion.
#6 3.3.3 suggest s/"do not understand"/"cannot verify"/
I changed this to "do not implement". "Cannot verify" is too vague:
there are lots of reasons a verifier might not be able to verify a
signature.
#7 3.3.4 says "RSA keys of at least 1024 bits" and similar. The 1024
refers to the modulus length so that would be more correctly stated
as "RSA keys with modulus at leat 1024 bits long". Similar fixes
could be done with other parts of the same section.
I would like some discussion on this. This might be more technically
accurate for a cryto expert, but I know that if I as a non-crypto guy
read this, I would immediately ask myself "now, is that the same as
the key length or something else altogether?"
#8 3.3.4 would be a little clearer if it said that "Verifier
security policies may use the length..."
Agreed.
#9 3.4 s/result in an authentication failure/result in signature
verification failure/ since dkim isn't authenticating, strictly
speaking.
It seems pretty pedantic, but OK, changed.
#10 3.4.5, 1st sentence: s/and verified/and to be verified/
or just drop the last two words.
I just dropped them.
#11 3.4.5, end of 1st informative note: s/ignore the tag/ignore
content after the indicated length/ Reason - if the ignore the tag
then they won't verify the signature.
Actually, in our early discussion over this we actually did mean that
the verifier can simply ignore the tag, and yes, it won't verify.
Some people deemed that to be a feature, not a bug.
#12 3.4.5, 3rd para: s/body length count is made after/body
length count is calculated after/
Done.
#13 3.4.5, 2nd informative note: I think this just repeats stuff in
the 1st informative note and could be deleted.
The point of this note was to provide a clear rationale, which
doesn't really seem to be fully done in the first (implementation)
note. I moved it up before the first note and deleted the first
sentence of the implementation note, so it now reads:
INFORMATIVE RATIONALE: This capability is provided because it
is very common for mailing lists to add trailers to messages
(e.g., instructions how to get off the list). Until those
messages are also signed, the body length count is a useful
tool for the verifier since it may as a matter of policy
accept messages having valid signatures with extraneous data.
INFORMATIVE IMPLEMENTATION NOTE: Using body length limits
enables an attack in which an attacker modifies a message to
include content that solely benefits the attacker. It is
possible for the appended content to completely replace the
original content in the end recipient's eyes and to defeat
duplicate message detection algorithms. To avoid this attack,
signers should be wary of using this tag, and verifiers might
wish to ignore the tag or remove text that appears after the
specified content length, perhaps based on other criteria.
Does this address your comment adequately?
#14 3.4.5, last informative note: again I think this could be
deleted.
I deleted this and we'll see if anyone screams. There are so many
notes in that section because the "l= is evil" faction wanted to make
it very clear that you can get yourself into a heap of trouble by
using it inappropriately.
#15 3.5 "q=" the text says "dns/txt" but the ABNF contains "txt/dns"
and the text description isn't clear (enough) that "dns" is allowed
and means "dns/txt".
Whoops, the ABNF was wrong. And actually "dns" by itself is /not/
allowed; note the sentence reading "The only option defined for the
"dns" query type is "txt", which MUST be included."
#16 3.5, example at end. This example doesn't contain a "bh=", it'd
be better if it did since that's REQUIRED.
Right you are. It was also missing a semicolon after the z= value.
#17 3.6.1 talks about "the response" which is dns specific, and I
thought we didn't want to be here, so s/response/record/ maybe?
Works for me.
#18 3.6.2.1 and elsewhere, I see ""_domainkey"" and other instances
of double-double quotes. Can't see why those are there.
Someone else reported the same thing and I couldn't find it --- but
this time I did (use of spanx with style=verb causes quoting in plain
text). It should be fixed now.
#19, 3.7. This talks about two hashes all the time, which is
correct, but a bit misleading since any sensible API will include
both hashing and private key application inside a "sign()"
function. The end result is that the dkim programmer calls "hash()"
once and either "sign()" or "verify()" once. Suggest adding a
sentence along these lines to clear that up. If someone ever
calculated "sign(hash())" then they'd be wrong and this text sort of
suggests that. Similar comment about the pseudo-code at the end
where I'd rather see something like:
body-hash = hash( canon_body )
signature = sign( canon_header || DKIM-SIG )
I disagree. This section has nothing to do with what an API would
look like, it's about what algorithms have to be applied and when.
For example, I can easily see your change being interpreted to mean
that you don't hash the header at all.
#20, 3.7, 4th last para. This sort-of threw me a bit since it seems
like an MUA instruction. Is it? If so, then maybe move to the
appendix/overview? This is the one starting "When calculating the
hash on messages..."
No, this is not an MUA function --- it is perfectly reasonable for a
gateway to convert an 8-bit message to a 7-bit form long after the
MUA is done with it, and dot-stuffing definitely has nothing
whatsoever to do with the MUA.
#21, 3.7, last para. "sans" is nice, but would it confuse
non-franglish speakers?
Actually it's an English word, derived from Latin, as in "sans
serif". It is also a French word, and came into Middle English from
Old French. But if other English speakers want me to change it, Ita
Sit (Latin for "so be it", although we more commonly use the word
"amen", although that has another connotation).
#22, 5.2, last para. Change "when the selector containing the
corresponding public key is expected to be removed before the
verifier...", to, "when the public key in the relevant selector is
expected to be revoked before the verifier..." since we're not
recommending removing the selector but rather zeroing out the key.
Is that what we are recommending? I thought that omitting the key
was a way of saying "we're sorry, but we had to pull that key
unexpectedly." In fact, the usual way of dropping a key would be to
drop the record entirely. Otherwise you would have ever-growing name
spaces.
#23, 5.5, last 2 paras about "l=". I think this has all already
been said, so it could be removed from here.
I would consider removing the informative note, but I think it is
completely reasonable to include all the steps in the section on
"Compute the Message Hash and Signature", even if they do duplicate
some other text.
#24, Section 6, 1st sentence says "may expire a public key"
s/expire/revoke/ since dkim, (properly), doesn't do expiry, only
revocation.
See my comments on #22.
#25, 6.1, 1st note, "other clues" is a bit opaque - include a
reference to somewhere where the reader can find those clues or
maybe reword.
I changed this to read "INFORMATIVE IMPLEMENTATION NOTE: Verifiers
might use the order as a clue to signing order in the absence of any
other information. However, other clues as to the semantics of
multiple signatures (such as correlating the signing host with
Received headers) may also be considered."
#26, 6.1 "; this is local policy..." that "this" is a maybe a tad
ambiguous, suggest replacing with "; such treatment is a matter
for local policy..."
Done.
#27, 6.1.3, list item #1 says "create a canonicalised copy" which
isn't necessary - all you need to is feed the right bits through
the hashing, so an entire copy need never be created. Suggest
rewording to say "re-create the canonicalized octets" or something.
(Or, add an informative note along the above lines.)
This is a description of an algorithm, not an implementation. I'll
put in an implementation note, but frankly getting this wording
sufficiently unambiguous has proven challenging, and I'm reluctant to
wade in again without a really good reason. I will change the intro
to read "... consists of actions semantically equivalent to the
following steps." I can also add something in the Implementer's Note
if you feel it needs more.
#28, 6.2. The reference to ID-AUTH-RES might be better off being
removed for process reasons.
Even as an example? It's informational, not normative.
#29, 8.1.1. Missing the example.
Right, thanks. I added a paragraph reading:
For example, if an attacker can append information to a
text/html body part, they may be able to exploit a bug in
some MUAs that continue to read after a </html> marker, and
thus display HTML text on top of already displayed text. If
a message has a multipart/alternative body part, they might
be able to add a new body part that is preferred by the
displaying MTA.
#30, 8.2. This section mainly applies to MUA signing, which is not
the main use-case, and so the threat is less serious. Suggest
saying that at the start of the section along with a sentence
like: "Protection of private keys on servers can be easily achieved
though the use of specialised crytographic hardware."
Added.
#31, A.2, the example has no "bh=". Needs recalculating. And would
be better if the example inlcuded real signatures and the
corresponding public key record too.
I did get a (correct) bh= value included, but haven't recomputed the
signature (it's getting late today; I'll try to get to that tomorrow).
#32, A.3, the authentication results header should not, IMO be
included here.
I think it is very important to make it clear how one might convey
results to higher levels. Otherwise all of A.3 doesn't have any
reasonable example. It doesn't have to be Authentication-Results,
just something.
#33, Appendix B. I expected to see a bit here about the type of
setup we have at IETF meetings, where the IETF's MTA signs and the
verifier has to handle the From being nothing like the signer
identity.
Sorry, I don't see any signatures coming from the IETF MTA. Can you
be more specific?
#34, Appendix C. I think that this can be deleted. Others may
disagree. In any case, the text says 768 and the command line 1024,
no password handling is shown and the last part could confuse since
that signature is not usable for DKIM. So if this isn't deleted,
then a bunch of changes are needed.
I fixed the discrepancy, but I don't think it should be deleted.
It's non-normative and will help people understand how to use the
standard, which I think is A Good Thing.
I'm not sure what you mean about password handling. I just ran
through these instructions, and there are no passwords requested.
Total Nits
----------
#1 When the [[]] text disappears there'll be nothing between
"introduction" and "overview" - suggest getting rid of 1.1 and just
having the text be the introduction.
Works for me.
#2 1.1, 2nd set of bullets. The last point here would be better in
the 1st set of bullets.
Makes sense. Done.
#3 2.3, last sentence. Does the single abnf definition here offer
value? If so, then why not one for FWS too? Suggest deleting this.
I think the ABNF does have value. We don't include it for FWS
because that's imported from another document (RFC2822); the ABNF is
there.
#4 3.5, 3rd para: is "the null string" well enough defined? Maybe
"an empty string" would be better?
Done.
#5 3.5, last para before tags says: "Defined tags are described
below." which is sort of obvious. Suggest deleting it.
Done (but why do I have the creepy feeling someone is going to
object?).
#6 3.6.1 "k=" says that the public key is in the "p=" value, but its
actually the modulus.
I guess I'm confused. If this isn't the public key, what is?
#7 3.6, 3rd last para s/string of characters/string of octets/
3.7 perhaps? I've made that change.
Thanks for your comments.
eric
_______________________________________________
NOTE WELL: This list operates according to
http://mipassoc.org/dkim/ietf-list-rules.html