Subject: Re: [secdir] draft-ietf-dane-openpgpkey-06 SECDIR Review

From: Donald Eastlake <[email-masked]>
Date: Wed, 27 Apr 2016 20:24:40 -0400

Hi Paul,

I have reviewed all change from -06 through the current -10 and I am
satisfied with the resolution of my comments.

Thanks,
Donald
=============================
 Donald E. Eastlake 3rd   +1-508-333-2270 (cell)
 155 Beaver Street, Milford, MA 01757 USA
 [email-masked]

On Wed, Jan 27, 2016 at 6:06 AM, Paul Wouters <[email-masked]> wrote:
>
> Hi Donald,
>
> Thanks for the secdir review. I've incorporated your suggestions and
> hopefully resolved your issues in the -07 draft I just posted at
>
> https://tools.ietf.org/html/draft-ietf-dane-openpgpkey-07
>
>
> Comments inline below.
>
>
>> I think this draft is not ready for publication. It probably minimal
>> technical changes but there are significant wording problems with it.
>>
>> Security:
>> ------------
>>
>> This document is "DANE for OpenPGP ..." but never says how what it
>> documents is a use of DANE or what DANE is. While there is a reference
>> to RFC 6698, at a minimum the DANE acronym should be expanded at first
>> use and/or in Section 1.2. Preferably two or three sentences should be
>> added to fix this gap.
>
>
> I added a sentence to the introduction:
>
> DNSSEC Authentication of Networked Entities ("DANE") is a method
> for publishing public keys and certificates in DNS.
>
>> I am concerned about the use of the words "validate" and "verify" in
>> this document in a wide variety of different ways, and in particular
>> their use in connection with OPENPGPKEY RRs. The ordinary and usual
>> meaning of these words, when they are not qualified in some way, is
>> that something is completely valid/verified for use and can be used
>> without further checking. But that isn't what seems to be meant in
>> this document. Here it just seems to sometimes mean that it has
>> validated under DNSSEC. It might also mean that it is of valid syntax
>> and a bit more -- the document is unclear on whether that is included.
>> But the use of these words for OPENPGPKEY RRs seems to exclude the
>> validation under the web of trust or human judgement even though that
>> step is mandated at a couple of places in the document.
>
>
> The term "verify" is in common use with OpenPPGP, for instance using
> the gnupg command where the command is "gpg --verify". I have made
> some changes to avoid possible confusion. I have changed the usage
> of validation or verification in the context of DNSSEC to consitently
> be "DNSSEC validation". I've also changed the word "verification" when
> used in a context that is not related to OpenPGP. (for instance by
> changing "source ip verification" to "source ip confirmation").
>
>> Looking at Section 5, the "obtain or verify" in the first sentence
>> seems odd. Shouldn't it use "and" and be more like "obtain and DNSSEC
>> verify"? And in the following sentence, I would say "... ; if DNSSEC
>> validation reaches ..." Also, if you are going to start talking about
>> a specific DNSSEC state name as is done here, there should be a
>> reference to the specific DNSSEC RFC where that state name is defined.
>
>
> Changed to:
>
> The OPENPGPKEY record allows an application or service to obtain an
> OpenPGP public key and use it for verifying a digital signature or
> encrypting a message to the public key. The DNS answer MUST pass
> DNSSEC validation; if DNSSEC validation reaches any state other than
> "Secure" (as specified in RFC-4035), the DNSSEC validation MUST be
> treated as a failure.
>
> RFF-4035 has been added as a normative reference.
>
>> In Section 5.1, in the first sentence, I would say "to seek" rather
>> than "to discover". "discover" makes it sound like it will always
>> un-cover/find something; also I think it would be a bit better to say
>> "corresponding to" rather than "belongs to".
>
>
> Changed as suggested.
>
>> The last sentence in 5.1
>> has too many confusing "this"s. Suggest, assuming I have correctly
>> understood what you want to say, replacing the current last sentence
>> with "An application whose attempt fails to retrieve a DNSSEC verified
>> OPENPGPKEY RR from the DNS should remember that failure for some time
>> to avoid sending out a DNS request for each email message the
>> application is sending out; such DNS requests constitute a privacy
>> leak".
>
>
> Changed.
>
>> I suggest changing the title of Section 5.2 to "Confirming that an
>> OpenPGP key is current" since that is what it is about, not about
>> general validity.
>
>
> Changed.
>
>> The third sentence of Section 5.2 ("If verifying ...
>> a failure") is unclear and not grammatical. Trying to re-write this
>> third sentence I come up with "If a locally stored OpenPGP public key
>> is found to be different from an OpenPGP retrieved from the DNS and
>> DNSSEC verified as described herein, then ...." But I don't understand
>> this and don't understand what the "..." should be.
>
>
> I have changed it to:
>
> If the locally stored OpenPGP public key is different from the DNSSEC
> validated OpenPGP public key currently published in DNS, the verification
> MUST be treated as a failure unless if the locally stored OpenPGP key
> signed the newly published OpenPGP public key found in DNS.
>
>> Can't there can be
>> multiple good OpenPGP keys for the same email address?
>
>
> Yes, there can be. But a locally stored OpenPGP public key must be
> considered more secure than a new one observed in DNS, until a human
> has confirmed the new key. Unless the old key has confirmed the new key.
>
>> What if one key
>> is stored locally and you retrieve two keys, one of which is equal to
>> the local key and one of which is different? Presumably it depends on
>> the local/user's policy what to do in such a case of different keys.
>
>
> What I tried to accomplish is that if you have a local key, any key
> update must be confirmed by the old key or the user. Switching keys
> without further confirmation is just too dangerous.
>
>> How is it helpful to say "the verification MUST be treated as a
>> failure"? (This certainly further confuses what "verification" means
>> in this document.)
>
>
> The presence of a new key might mean the old (locally stored) key
> was compromised. But the new key cannot just be trusted even if it
> passed DNSSEC validation. Unless the old key signed the new key,
> human intervention should be used to resolve the conflict. By saying
> "verification failure" it blocks the application from sending the data
> encrypted to either key - and require a user to resolve the situation.
>
>> It is not clear exactly what that means but if it
>> says that a DNSSEC verified OpenPGP key retrieved from the DNS should
>> be dropped/ignored, why is that always the right thing?
>
>
> I did not mean say drop or ignored. A conflict of keys should be
> resolved by the user.
>
>> In the second sentence of the first paragraph of Section 7, what does
>> the initial "It" stand for?
>
>
> Changed to:
>
> DANE for OpenPGP as specified in this document is a solution aimed to
> ease obtaining someone's public key.  Without manual verification of
> the OpenPGP key obtained via DANE, this retrieved key should only be
> used for encryption if the only other alternative is sending the message
> in plaintext.
>
>> If you were faked-out and believed a false key so email was encrypted
>> to the bad guy and could not be read by the intended recipient, I
>> would say that was worse than plaintext.
>
>
> That really depends on the situation. If an attacker can replace
> OPENPGPKEY records, they can most likely also change MX records
> to just get everything.
>
>
>> This paragraph goes on to
>> talk about active attacks, which usually. in the email context, refers
>> to active attacks on the email on the wire, but I would guess this
>> text is actually talking about active attacks in the form of storing a
>> wrong key in the DNS...
>
>
> The active attacks refer to everything that can cause a third party to
> gain access to the unencrypted email content.
>
>> In re Section 7.5, why isn't the domain name included in the hash? It
>> seems to improve security a little and the effort is small.
>
>
> As stated in that section 7.5:
>
>    The domain name part of the email address is not used as part of the
>    hash so that hashes can be used in multiple zones deployed using
>    DNAME [RFC6672]
>
>> Other:
>> --------
>>
>>   Section 1:
>>
>> The references for Secure DNS should be given when Secure DNS is first
>> mentioned on page 3.
>
>
> Done.
>
>>   Section 1.1:
>>
>> I do not think there is such a thing as an "Experimental RRtype". It
>> would be better to say something like "This document specifies an
>> RRtype whose use is Experimental."
>
>
> Done.
>
>> I don't quite grok the use of "generality of" on page 4. Perhaps it
>> should be replaced with "diffuse support of" or something.
>
>
> Changed to "general application"
>
>>   Section 2:
>>
>> As long as you are bothering to say that the OPENPGPKEY RR has no
>> special TTL requirements, you might as well say it has no special
>> Additional section retrieval requirements, since I think that is the
>> most common type of RR special processing. But I think the lack of
>> such special requirements is the default so you could probably just
>> leave these negative statements out.
>
>
> Done.
>
>>   Section 2.3:
>>
>> "textual zone files" -> "master files [RFC1035]" and add [RFC1035] to
>> the normative references.
>
>
> Done.
>
>>   Section 3:
>>
>> The following statement seems at least a little misleading:
>>     The DNS does not allow the use of all characters that are supported
>>     in the "local-part" of email addresses as defined in [RFC5322] and
>>     [RFC6530].
>> DNS is binary clean. What left hand side characters allowed in
>> [RFC5322] are now allowed in DNS? Seems to me that only international
>> text as such [RFC6530] is a problem for DNS.
>
>
> And the "." or a NULL. There is also the case sensitivity argument
> raised by some.
>
>> Probably the first bullet should be split in two. The first time I
>> read it, it seemed that the first sentence was talking about some
>> encodings. Then the second sentence talks about other encodings and
>> says they are hashed. So, of course, I thought that the encodings
>> talked about in the first sentence were not hashed. But the example
>> appears to show that the current text had conveyed the wrong thing to
>> me and that it is always hashes. I suggest that after "If it is
>> written in another encoding it should be converted to UTF-8" be
>> followed by a period and then there should be a new bullet item
>> talking about hashing, etc., to make it clear that the hashing, etc.,
>
>
> Done.
>
>> apply to all encodings in the first bullet. Furthermore, I don't
>> understand why the  text fragment I quote says "should" rather than
>> "must" or perhaps just replace "should be" with "is".
>
>
> Done (with "is")
>
>> Then we get to the truncation. "Truncation comes from the right-most
>> octets." is completely ambiguous. At a minimum, a word needs to be
>> added so it says "Truncation comes from using the right-most octets."
>> or "Truncation comes from dropping the right-most octets."
>> Alternatively some other non-ambiguous wording is needed.
>
>
> Actually I think the whole two sentence that start from this can be
> dropped. These add no new information.
>
>> Presumably it is believed that the probability of a hash collision is
>> small enough that it can be ignored. If so, it wouldn't hurt to say
>> so.
>
>
> Added to the security section:
>
> In theory, two different local-parts could hash to the same value. This
> document assumes that such a hash collision has a negliable chance of
> happening.
>
>> Section 7:
>>
>> The last paragraph of Section 7 seems to equate "Organizations" and
>> "mail servers". Suggest recasting the second sentence as "Mail servers
>> of such organizations MAY optionally re-encrypt a received message to
>> an individual's OpenPGP key.".
>
>
> Done.
>
>>   Section 7.1:
>>
>> Again, I assume "indeterminate" and "bogus" are used in their DNSSEC
>> meaning. So there needs to be a reference here to the DNSSEC RFC that
>> explains those words.
>
>
> Done, Added pointer to RFC-4035
>
>> Author's Address:
>>
>> I understand that many do not agree with me but I believe that first
>> page authors should normally list a postal address and a telephone
>> number to which a message could be sent or at which a message could be
>> left for them in addition to an email address. This section looks like
>> schlock corner cutting to me.
>
>
> I do not agree. Any address and phone number listed would be too
> ephemeral or too generic to be of value to anyone.
>
>> Trivia:
>> --------
>>
>> "twart" -> "thwart" and "twarts" -> "thwarts"
>
>
> Fixed.
>
>> Section 6: "properties are not exported" -> "properties not be
>> exported" and in the following sentence "have" -> "has"
>
>
> Fixed.
>
>> Section 7: "direct" -> "ask" (a mail client has no power to order the
>> user to do anything)
>
>
> Fixed. Also changed "human" to "user" everywhere.
>
>> Section 7.1: 5th paragraph, "sent" -> "send"
>
>
> Fixed.
>
> Paul