Yoav, is a revised I-D forthcoming?  Are there further comments about
Elwyn's review?

Barry

On Thu, Jul 31, 2014 at 3:27 PM, Yoav Nir <[email protected]> wrote:
> Hi, Elwyn
>
> [ adding the WebSec mailing list ]
>
> Thanks for the review. I think the editorial comments and the un-expanded 
> initialisms are not controversial. The text in section 4.1 contains our 
> hard-earned consensus about maximum max-age. Perhaps your comment about 
> section 2.1.1 can be accommodated by pointing to section 4.1 from section 
> 2.1.1.
>
> As for the rest, I will leave up to the group, but I might also comment as an 
> individual.
>
> To the WebSec group: please send in comments about this. A GenArt review 
> (just like *Dir reviews that are likely to come in the next few days) should 
> be addressed just like any other comments. Pushing back is fine if we think 
> they are inappropriate, but comments about lack of clarity tend to be correct 
> when they're made by someone outside the working group - someone who reads 
> the document without having participated in the discussion on-list and at 
> meetings. Making changes is still up to the working group, so please go over 
> Elwyn's comments.
>
> Yoav
> As co-chair and document shepherd.
>
> 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.
>>
>> 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.
>>
>> 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!]
>>
>> 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

Reply via email to