Subject: Re: [websec] I-D Action: draft-ietf-websec-key-pinning-12.txt

From: Yoav Nir <[email-masked]>
Date: Thu, 1 May 2014 11:34:33 +0300

On May 1, 2014, at 12:41 AM, Trevor Perrin <[email-masked]> wrote:

> On Mon, Apr 28, 2014 at 11:24 PM, Yoav Nir <[email-masked]> wrote:
>> 
>> Having reviewed this, I think this version addresses all the issues raised during WGLC.
> 
> The added sentence in 2.7 on preloaded/dynamic pins doesn't match the
> rest of the text, which is written to mandate specific behavior.  If
> we're not going to discuss implementation alternatives, we should just
> delete most of 2.7 and let implementations do what they want.

Why? All the new sentence says is “The Effective Pin Date of built-in pin lists is UA implementation-defined”. Implementations are still required to treat the pre-loaded pins as though they have some effective date and determine precedence by whichever has a later date.  Suppose Chrome will have a pin list that is downloaded from Google every hour. What date is assigned to the pins in that list could be part of the format, or it could be assumed to be a week old, depending on how Google updates this file.

> The text in 2.3.2 on PKP-RO doesn't really match the rest of the doc.
> Most of the draft is written as if there is a single header, set of
> Pinning Metadata, and "Known Pinned Host" state.  But 2.3.2 doubles
> those things, which causes many minor ambiguities, e.g.:
> 
> * 2.2.1 - "a Pinned Host SHOULD include in its response exactly one
> PKP header field"
> 
> * 2.3.1 - "the UA MUST process only the first such header field"
> 
> * 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? (or
> new "report-only" pins)?
> 
> * 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".  It's unclear whether a "report-only"
> pin counts as a "Pinned Host" for the purpose of other TLS errors.
> 
> The draft could use an editing/cleanup pass to clarify all this.
> 
> Also, some of the issues I've been bringing up since July need to be addressed:
> 
> * Interaction with cookies needs discussion, at least in security
> considerations.  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.

While there are similar concerns, I don’t see the interaction. If you are example.com, and badguy controls badguy.example.com, then they’re going to get your example.com cookies unless you can block it with scoping.  HPKP doesn’t help you there. You *can* block them with HPKP if their control on badguy.example.com relies on a mis-issued certificate and you restrict them with the includeSubdomains directive.

> 
> * "Pinning Policy" and "Pinning Metadata" are synonyms?
> * 2.3.1 ... "or," ... "Otherwise" is confusing
> * 2.6 - is pin validation performed on resumed TLS connections?
> * 3 - can UAs provide additional JSON fields inside the report?
> * The term "validated certificate chain" is not defined
> * An IANA registry is needed for directives
> * The acknowledgement for "pin break codes" is ancient/irrelevant
> 
> 
> Trevor

---

From: Trevor Perrin <[email-masked]>
Date: Thu, 1 May 2014 16:55:59 -0700

On Thu, May 1, 2014 at 1:34 AM, Yoav Nir <[email-masked]> wrote:
> Why? All the new sentence says is “The Effective Pin Date of built-in pin lists is UA implementation-defined”. Implementations are still required to treat the pre-loaded pins as though they have some effective date and determine precedence by whichever has a later date.

That's a bad requirement, as it forces implementations to track the
Effective Pin Date for dynamic pins (the Effective Expiration Date is
not sufficient).

Also, we talked about and I thought agreed to allow implementations
freedom to decide how to handle preloaded/dynamic pin co-existence, as
long as they don't apply a pin that was never asserted.  For example,
I described a reasonable implementation which this text would
disallow:

http://www.ietf.org/mail-archive/web/websec/current/msg02016.html

I thought we had agreement to allow things like that:

Yoav:
"Yet a fourth way is to observer that none of this affects
interoperability in any way and leave it up to the UA vendor to choose
their way. The web site operator should only include valid pins in
their HPKP headers, and they should also enter only valid pins in
pre-loaded lists. They should also deal with the fact that don't
control usage patterns, so anytime they pin something for X seconds,
their website MUST conform to that pin for at least those X seconds or
bad things will happen."
http://www.ietf.org/mail-archive/web/websec/current/msg02019.html

Trevor:
"Your "fourth way" is well-put, and I agree - all of these seem valid
implementations which should be allowed."
http://www.ietf.org/mail-archive/web/websec/current/msg02020.html

Chris Palmer:
"I have been thinking that this 4th way is the way to go."
http://www.ietf.org/mail-archive/web/websec/current/msg02022.html

Dan Veditz:
"The pre-load list seems outside the mechanism covered by the spec. At
best the spec could mention it in a non-normative section as something
UAs can do to cover the first-visit gap, but there are several valid
ways such a list could be implemented."
http://www.ietf.org/mail-archive/web/websec/current/msg02028.html

You suggested a text modification which you maybe thought implemented
that, and which Tom, Tobias, and Chris seconded:
"Does anyone (and that includes the authors) object to relaxing the
requirements in section 2.7, so that the choice of when the static
pins are believed to have been observed is left to the implementer?
If not, we'll resolve it that way."
http://www.ietf.org/mail-archive/web/websec/current/msg02021.html

But that doesn't satisfy what I think people wanted, so I think we
should strip down 2.7 to something like your text above on the "fourth
way".

> While there are similar concerns, I don’t see the interaction. If you are example.com, and badguy controls badguy.example.com, then they’re going to get your example.com cookies unless you can bloc…

The interaction is that the security you were hoping HPKP would give
you for example.com will be largely ineffective if a badguy can forge
a cert for badguy.example.com and you don't defend this with either
(a) includeSubdomains or (b) cookie-scoping to the domain (which
doesn't work on IE).

So this is a security consideration deployers of pinning need to be aware of.

Anyways, below is the text from TACK's security consideration which
isn't perfect, but something like this should be added:

   HTTP cookies [RFC6265] set by a pinned host can be stolen by a
   network attacker who can forge web and DNS responses so as to cause a
   client to send the cookies to a phony subdomain of the pinned host.
   To prevent this, TACK HTTPS servers SHOULD set the "secure" attribute
   and omit the "domain" attribute on all security-sensitive cookies,
   such as session cookies.  These settings tell the browser that the
   cookie should only be presented back to the originating host (not its
   subdomains), and should only be sent over HTTPS (not HTTP) [RFC6265].

Trevor

---

From: Chris Palmer <[email-masked]>
Date: Tue, 13 May 2014 14:09:34 -0700

For your consideration:

PKP vs. PKP-RO:
https://code.google.com/p/key-pinning-draft/source/detail?r=994a00dc31bf2cca6f3edea29871a6a4f18090f9

Interactions with built-in pins:
https://code.google.com/p/key-pinning-draft/source/detail?r=bbf42b1e5e9b49a8cdf193f9c7fe230d0d290543

Cookie security considerations:
https://code.google.com/p/key-pinning-draft/source/detail?r=7b4474e7d3666f1aebb3f1bcde69bf552aa65d78

On Thu, May 1, 2014 at 4:55 PM, Trevor Perrin <[email-masked]> wrote:
> _______________________________________________
> websec mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/websec

---

From: Trevor Perrin <[email-masked]>
Date: Mon, 19 May 2014 23:28:20 -0700

> https://code.google.com/p/key-pinning-draft/source/detail?r=994a00dc31bf2cca6f3edea29871a6a4f18090f9

The new text about PKP-RO in 2.5 (quoted below) seems to say that a
PKP-RO header is only evaluated against the current connection, not
stored as a pin.  I thought we decided the opposite (which is what I
think 2.3.2 is saying):

2.3.2 (existing text):
  If a Host sets both the Public-Key-Pins header and the Public-Key-
   Pins-Report-Only header, the UA MUST note and enforce Pin Validation
   as specified by the Public-Key-Pins header, and SHOULD note the Pins
   and directives given in the Public-Key-Pins-Report-Only header.

2.5 (new text):
    The UA SHOULD NOT note any pins or other policy expressed in the PKP-
    RO response header field.

> https://code.google.com/p/key-pinning-draft/source/detail?r=7b4474e7d3666f1aebb3f1bcde69bf552aa65d78

Those look OK to me.

Trevor