Subject: Re: [websec] Comments on draft-ietf-websec-key-pinning-00

From: Chris Palmer <[email-masked]>
Date: Thu, 8 Dec 2011 18:25:13 -0800

On Tue, Nov 29, 2011 at 7:44 PM, Manger, James H
<[email-masked]> wrote:

> First, nice work.

Thanks!

> > If the Public-Key-Pins response header field does not meet all three
> > of these criteria [error-free TLS; current key; backup pin], the UA MUST
> > NOT note the host as a Pinned Host,
> > and MUST discard any previously set Pinning Metadata for that host in
> > its non-volatile store.
>
> This seems to make it too easy for an attacker to force a UA to discard
> previously set pins for a host, removing the protection they offered. If an
> attacker inserts a Public-Key-Pins HTTP response header into an HTTP (not
> HTTPS) response from the server it will fail the 1st criteria (over an
> error-free TLS connection). Is that all it takes to turn off pinning for
> that host?

I think I can satisfy your concern by adding a sentence saying that
UAs MUST only pay attention to the Public-Key-Pins response header
field if the connection is over TLS. I think that is implicit now,
e.g. in the first criterion:

"""The UA MUST note the pins if and only if it received the
Public-Key-Pins response header field over an error-free TLS
connection."""

I'll change it to:

"""The UA MUST note the pins if and only if it received the
Public-Key-Pins response header field over an error-free TLS
connection. The UAs MUST ignore Public-Key-Pins response header fields
received on HTTP (non-HTTPS) connections."""

How is that?

> > if the TLS connection has errors… the UA might… allow the user the option
> > of continuing with the connection anyway
> > If the connection has no errors, the UA will then apply a new
> > correctness check: Pin Validation.
>
> Does this mean pin validation is bypassed if the cert was, say, expired and
> the user said continue anyway?
>
> It doesn’t look right if the UA MUST fail with a non-recoverable error if
> pin validation fails, but an attacker can cause an earlier failure (eg
> expired cert) and users will be allowed to click-through that warning and
> get to the site. Or the user can click-through a recoverable cert error; pin
> validation is then bypassed; then a Public-Key-Pins header fails the
> error-free TLS connection criteria so the UA removes all pinning for this
> site.

Good point. I have changed the paragraph to:

<t>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
anyway. (This behavior is the same as that required by <xref
target="hsts-draft"/>.</t>

> Might be worth explicitly mentioning that only certificates that are
> actually used in the validated certificate chain should be used in Pin
> Validation, which is not necessarily all the certificates in a TLS
> Certificate message (even if the two are supposed to be the same).

Good point again. Added this language:

<t>If the connection has no errors, the UA will then apply a new correctness
check: Pin Validation. To perform Pin Validation, the UA will compute the
fingerprints of the SPKI structures in each certificate in the host's
validated certificate chain. (The UA ignores superfluous certificates in the
chain that do not form part of the validating chain.) The UA will then check
that the set of these fingerprints intersects the set of fingerprints in
that host's Pinning Metadata. If there is set intersection, the UA continues
with the connection as normal. Otherwise, the UA MUST treat this Pin
Failure as a non-recoverable error.</t>

>   For hosts that are Known HSTS Hosts the UA MUST terminate
>  the connection in case of TLS errors, as required by [hsts-draft].
> It is ok to refer to HSTS but this spec shouldn’t repeat a “MUST” from HSTS.

That text is gone now.

> §2.1 “Response Header Field Syntax”: The ABNF and the examples don’t match
> at all.

Weird; I thought I updated the examples to match the ABNF but I guess
not. Fixing that now...

> Typo: §2.4 “Validating Pinned Connections”: “might or might now” should be
> “might or might not”

That text is gone now.

Thanks! I've added you to the Acknowledgements section.