These fixes are now available in a new version. You can see the changes at:

https://code.google.com/p/key-pinning-draft/source/list

On Mon, Aug 4, 2014 at 8:49 AM, Yoav Nir <[email protected]> wrote:
> [ with no hats on ]
>
> inline
>
> On Jul 31, 2014, at 10:27 PM, Yoav Nir <[email protected]> wrote:
>> On Jul 31, 2014, at 4:05 PM, Elwyn Davies <[email protected]> wrote:
>>
>>> I am the assigned Gen-ART reviewer for this draft. For background on
>>> Gen-ART, please see the FAQ at
>>>
>>> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
>>>
>>> Please resolve these comments along with any other Last Call comments
>>> you may receive.
>>>
>>> Document: draft-ietf-websec-key-pinning-19.txt
>>> Reviewer: Elwyn Davies
>>> Review Date: 31 July 2014
>>> IETF LC End Date: 1 August 2014
>>> IESG Telechat date: (if known) -
>>>
>>> Summary:
>>> Almost ready.  There are some minor issues some of which may be as a result 
>>> of some
>>> misunderstanding on my part.  The descriptive text in the early part of s2 
>>> is missing
>>> some definitions that make it unclear until they appear later on.  This 
>>> makes the early
>>> descriptions more confusing that illuminating in places.  Suggestions in 
>>> the detailed
>>> comments below.
>
> [YN] I believe the missing definitions you’re talking about are Pin, Pin 
> Validation. I think anyone who reads this specification is familiar enough 
> with HTTP to know what request, response, UA and directive mean. If so, I 
> suggest that these can be defined in 1-2 sentences in section 1, even if 
> they’re better explained later on.
>
>>> One thing that is not fully clear to me and could probably be explained to 
>>> help others
>>> is the start up of the pinning mechanisms for a given host domain.  AFAICS 
>>> Pin validation
>>> would possibly not be carried out on a first connection to a domain when 
>>> there are no
>>> preconfigured Pins.  I am not clear if this adds anything to the risk of a 
>>> MITM attack or
>>> does it in any way negate the value of the whole pinning process?  I was 
>>> not clear if
>>> an effective Pin validation should be carried out during the first 
>>> connection when the
>>> UA receives the host's Pins for the first time and decides that it is now a 
>>> Pinned Host,
>>> in that the document doesn't state that the connection is terminated if the 
>>> setting up
>>> of the Pinned host fails because the certs don't validate.
>
> Key pinning is a TOFU (trust on first use) mechanism. As such, the first time 
> a UA connects to a domain there is no validation. A MitM attacking such a 
> first connection will not be discovered. Worse, such a MitM can inject its 
> own PKP header into the HTTP stream, and pin the UA to its own keys. Note 
> that in order to pin false information, the attacker would have to be able to 
> produce an error-free connection. Without compromising a trusted CA, this 
> should not be possible, and the best that attackers can accomplish is to use 
> an invalid TLS certificate, leading to an interstitial warning page.
>
> This is apparent from section 2.3.1, but perhaps deserves a short paragraph 
> in the introduction.
>
>>> Major issues:
>>> None
>>>
>>> Minor issues:
>>> s1: The term "Pin" (as a noun) is not explicitly defined. The definition 
>>> doesn't appear
>>> until s2.4.
>>>
>>> s2.1.1: I'm not sure if this could be an issue.. should a maximum possible 
>>> value for max_age
>>> be specified to avoid UA's being cluttered up with old Pins - this might 
>>> possible be a DoS
>>> attack vehicle but I am not sure (yet) if it could be. OK.. so s2.3.3 talks 
>>> about limits.
>>> A pointer to this discussion would be useful here
>>>
>>> s2.2.1: Does this response behaviour apply to all possible request types? 
>>> Once a server has sent a
>>> Pin header should it send it again on all subsequent requests on the same 
>>> TLS connection or is
>>> that a choice?  Given the "SHOULD" in s2.2.1, what are the circumstances in 
>>> which the server should
>>> refrain from sending the Pins? [I first thought about 'Redirects' but 
>>> realized that that was probably
>>> a really good time to send Pins!]
>
> This has been discussed, and because of all kinds of weird deployments, such 
> as with multiplexed servers behind a load-balancer the only viable way was to 
> send the PKP in every response. Some (me!) were concerned about the overhead 
> of these large-ish headers, but we heard from people who actually run servers 
> that a header this small is inconsequential. The draft for HTTP/2 makes this 
> less of an issue, as repeated headers are efficiently encoded by referring 
> back to previous header sets.
>
>>> s2.3.1/s2.4: S2.4 states that hash algorithms might be deprecated in 
>>> future.  Presumably a
>>> deprecated algorithm should be treated as an unrecognized directive or some 
>>> such to avoid
>>> downgrade attacks.  Probably worth being explicit about this.  Also this is 
>>> potentially
>>> incompatible with the statement that 'UAs MUST recognize "sha256"' (Para 3 
>>> of s2.4).
>>> This results in a potential downgrade attack when and if SHA256 is deemed 
>>> to be no longer
>>> cryptographically effective. I think this statement can be removed as 
>>> presently a UA
>>> has no other option if it is to implement the specification.
>>>
>>> s2.6:
>>>>   Note that the UA MUST perform Pin Validation when setting up the TLS
>>>>   session, before beginning an HTTP conversation over the TLS channel.
>>> I suspect I am confused: If a UA is making its first connection to a host 
>>> for which it doesn't
>>> have a preconfigured Pin, then it won't get the Pin(s) from the host until 
>>> it has set up the
>>> TLS connection and received the response to the request at the HTTP 
>>> protocol level.  In that case
>>> Pin validation will pass by default (subject to local policy perhaps) since 
>>> the cache doesn't have
>>> entries for the host.  Presumably the UA should then perform Pin validation 
>>> if it has passed by
>>> default during TLS setup (assuming that this is possible given the 
>>> layering) or does the UA have to
>>> terminate the session and restart it so that Pin validation can be 
>>> performed?  The second case may
>>> give scope for a DoS attack.  Or is it the case that Pin validation is not 
>>> needed on the first
>>> connection... I don't see why this shouldn't be done but I may not 
>>> understand the problem.  I think
>>> some clarification about the startup of the process is needed.
>>> Nits/editorial comments:
>>>
>>> s1, last para: s/toegether/together/, s/but is possible/but it is possible/
>>> s2.1: It would be good to expand the term OWS.
>>>
>>> s2.1, Figure 1 caption: The acronym HPKP needs expanding.
>>>
>>> s2.1, 2nd para after numbered bullets:
>>> It is not the definition of hash algorithms that is relevant, but allowing 
>>> them to be
>>> used in pin-directives thus:
>>> OLD:
>>> additional algorithms may be defined in the future
>>> NEW:
>>> additional algorithms may be allowed for use in this context in future
>>>
>>> Also the implication of the "sha256" name should be explained precisely -
>>> i.e, that the SHA256 hash algorithm will be used, and a suitable reference
>>> for SHA256 should be given. (Again this doesn't happen until s2.4).
>>>
>>> And finally the "Fingerprint" of what SPKI? Defining Pin formally as noted 
>>> above would help!
>>>
>>> s2.1, last para: s/hahs/hash/
>>>
>>> s4.2/Figure 8: The first line of text is too wide.
>>>
>>> s5, para 1: Is it really "HSTS or HPKP"?  I thought it would be "HSTS 
>>> combined with HPKP".
>>>
>>> s6: Needs to be more precise about *which* message headers repository is to 
>>> be updated! Presumably
>>> the permanent one at 
>>> http://www.iana.org/assignments/message-headers/message-headers.xml#perm-headers.
>>>
>>> Also there may be some of the questions in s8.3.1 of RFC 7231 that need 
>>> specific answers.
>>>
>>> s5, 2nd top level bullet: Expand SNI acronym.
>>>
>>
>
> _______________________________________________
> websec mailing list
> [email protected]
> https://www.ietf.org/mailman/listinfo/websec

_______________________________________________
websec mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/websec

Reply via email to