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

From: "Manger, James H" <[email-masked]>
Date: Wed, 30 Nov 2011 14:44:40 +1100

Comments on draft-ietf-websec-key-pinning-00:

First, nice work.

§2.3 "Noting Pins" 2nd-last paragraph:
  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?

§2.4 "Validating Pinned Connections":
   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.

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).
§2.4 "Validating Pinned Connections":
  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.

§2.1 "Response Header Field Syntax": The ABNF and the examples don't match at all.
ABNF suggests
 Public-Key-Pins: pin-sha1="4n972HfV354KP560yw4uqe/baXc="; ...
But example is
 Public-Key-Pins: pins=sha1-4n972HfV354KP560yw4uqe/baXc=, ...

Typo: §2.4 "Validating Pinned Connections": "might or might now" should be "might or might not"

--
James Manger