On Fri, Nov 29, 2013 at 4:14 PM, Ryan Sleevi <[email protected]> wrote:
> On Fri, November 29, 2013 12:24 pm, Trevor Perrin wrote:
>> FEEDBACK ON DRAFT-07, STILL RELEVANT
>> ====
>>
>> * Interaction with cookies needs discussion. Cookie scoping rules
>> pose a serious problem for pinning, e.g. a pin at "example.com" could
>> be undermined by a MITM inventing a "badguy.example.com" and using it
>> to steal or force cookies.
>
> I still disagree that the *pin* can be undermined.
Semantics aside, there should be Security Considerations about cookies.
>> * Why is there a "Public-Key-Pins-Report-Only" header instead of a
>> "report-only" directive? Most of the document is written as if there
>> was a single "PKP header field", so a directive would make more sense.
>
> Similar to CSP. A PKP might specify a looser-policy, while PKP-Report-Only
> is used to test a more restrictive policy for deployment.
So the UA is going to maintain 2 stores of pins ("PKP" and "PKP-R-O"),
and process the "PKP" and "PKP-R-O" headers only against the
appropriate store?
That means asserting PKP:<new-pin> isn't sufficient to overwrite
existing pins. You'd also have to assert PKP-R-O:max-age=0.
Seems OK, but none of this is in the draft.
>> * In draft-05, client processing was changed from noting a single
>> "expiry" value to noting two values: "Effective Pin Date" and
>> "max-age". The previous approach was simpler, stored less data, and
>> was more aligned with HSTS.
>>
>> * Section 2.3.1 fails to update the Effective Date of a noted pin
>> when it is noted again.
>>
>> * Section 2.6 mandates "When a UA connects to a Pinned Host, if the
>> TLS connection has errors, the UA MUST terminate the connection
>> without allowing the user to proceed". HSTS allows the server to
>> specify this, so it seems unnecessary and inflexible to mandate it
>> here. It's also unclear whether a "report-only" pin counts as a
>> "Pinned Host".
>
> This is a feature, not a bug.
>
> PKP is useful without mandating HSTS. However, the security advantages of
> PKP can be undermined, much the way HSTS's can, if the UA allows users to
> bypass.
I think it's a mistake to mandate UI here. "Users clicking through
legitimate security errors" is one risk. "Users being denied access
to legitimate sites due to pinning failures" is another.
It's hard to know how to balance these. We should allow UAs latitude.
I suggest changing the text from MUST to SHOULD, at minimum.
>> * UA processing rules are confusingly spread across the document.
>> For example, 2.3.1 and 2.5 describe the same process so should be
>> combined.
>>
>> * Section 2.5 - Does a failed validation of a "report-only" pin count
>> as an "error" that will inhibit noting of new pins in the connection?
>
> No. Happy to clarify that.
Should failed validation of a "report-only" pin inhibit noting of new
"report-only" pins?
>> * Specified UA behavior with multiple header fields is contradictory:
>> - 2.1.3: "If a Host sets both the Public-Key-Pins header and the
>> Public-Key-
>> Pins-Report-Only header, the UA MUST NOT enforce Pin Validation, and
>> MUST note only the pins and directives given in the Public-Key-Pins-
>> Report-Only header."
>> - 2.3.1: "If a UA receives more than one PKP header field in an HTTP
>> response message over secure transport, then the UA MUST
>> process only the first such header field."
>
> Not contradictory, but ambiguous. The presence of > 1 PKP or > 1 PKP-R-O
> cause the first (PKP || PKP-R-O) field to be processed, and the rest
> ignored. However, if both a PKP and PKP-R-O field are present, they are
> both processed (independently).
>
> The ambiguity is the term "PKP header field", which may be alternatively
> written "more than one PKP header field or more than one PKP-R-O header
> field". Does that satisfy your concern?
The draft is generally written as if there was a only single "header"
and a single stored "entry" for any site, so these sorts of changes
will probably need to be made elsewhere.
>> * Section 3 - Should the failure report contain the certificates sent
>> by the server, as well as the validated chain? Could help debugging
>> and attack analysis.
>>
>> * Why is SHA1 supported? If the reason is size, why not remove it
>> but allow the server to pin to a truncated hash (e.g. pin to the first
>> 16 or 20 bytes of SHA256)
>
> I won't attempt to assume any argument for you, but it sounds like you
> oppose SHA-1. Perhaps you could state that, and your reasons why.
It's weaker than SHA-256. I don't see a reason to bother with it.
It seems better to steer everyone towards one hash algorithm, to make
comparing pins easier.
>> * Should there be a list of "excluded" keys that must not appear in
>> the "validated certificate chain"? Chrome's preloaded pinning
>> supports this, apparently to exclude 3rd-party subCAs that might have
>> been issued under a pinned CA.
>
> This was a temporary experiment that is no longer in use.
>
> If such a feature is desirable, it seems like something that could be
> addressed with additional directives in a later spec.
OK.
>> * Would it be better to use the term "pin" for the entire record
>> associating (hostname, HPKP data), instead of calling each individual
>> hash a "pin"? The hostname is "pinned" to the 1-of-n key set as a
>> whole, not each key individually.
>>
>> * Should the header name be changed once this draft stabilizes, to
>> ensure no bad interactions with UAs that implemented earlier drafts?
>
> This seems premature at best, but more likely an unwarranted and
> unnecessary change.
Chromium has had evolving, partial support for HPKP for some time.
Have these partial versions been shipping?
Are they likely to be "in the wild", with odd behavior that
HPKP-asserting sites might trigger?
Trevor
_______________________________________________
websec mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/websec
