Upgrade insecure requests: Pipe navigational hosts down into nested documents.

Upgrade insecure requests: Pipe navigational hosts down into nested documents.

After [1], we need to track hosts (including ancestor hosts) that have
set the 'upgrade-insecure-requests' directive in their respective policies
in order to correctly upgrade navigational requests to one of those
hosts.

This patch adds a 'HashSet<unsigned>' to SecurityContext that holds the
hashes of the hosts which have opted-into such treatment, ensures that
the set is correctly populated when creating a Document or applying a
policy, and uses the set to make decisions about navigational upgrades
inside ResourceFetcher.

[1]: ttps://github.com/w3c/webappsec/commit/f947b75e9b906c53d0bd6e66ca59b60bfe0aa20e

BUG=455674
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=191421

[Mike West, 2015-03-05 13:25:08]

mkwst@chromium.org changed reviewers:
+ yoav@yoav.ws

[Mike West, 2015-03-05 13:25:08]

Depends on https://codereview.chromium.org/983533005/ and
https://codereview.chromium.org/980213002.

Hi Yoav! Boy, I bet you're interested in reviewing more code! At the airport!
Or, you know, tomorrow or next week. :)

-mike

[Yoav Weiss, 2015-03-05 14:58:10]

On 2015/03/05 13:25:08, Mike West wrote:
> Depends on https://codereview.chromium.org/983533005/ and
> https://codereview.chromium.org/980213002.
> 
> Hi Yoav! Boy, I bet you're interested in reviewing more code! At the airport!
> Or, you know, tomorrow or next week. :)
> 
> -mike

In-the-air review!

Before I dive into the code, I'm not sure regarding the benefits of this.
What does MegaCorp get from pinning/remembering Upgrade instead of using HSTS?
Is this just a close-gap to help people address only browsers that support
Upgrade with HSTS-like functionality?
If so, can't they use the Prefer header to do the same on the server-side? That
way, we only remember one thing, rather than two.

Or, I'm completely off base (which due to the time/jetlag/elavation is fairly
possible).

[Mike West, 2015-03-05 15:11:38]

On 2015/03/05 at 14:58:10, yoav wrote:
> In-the-air review!

Wow! Technology. :)

> Before I dive into the code, I'm not sure regarding the benefits of this.
> What does MegaCorp get from pinning/remembering Upgrade instead of using HSTS?
Is this just a close-gap to help people address only browsers that support
Upgrade with HSTS-like functionality?

HSTS has side-effects (like draconian error handling for expired certs) that
(the EFF claims) make it more difficult to adopt for their target audience
(Let's Encrypt). There's some detail at
https://lists.w3.org/Archives/Public/public-webappsec/2015Feb/0043.html, but
it's a lot of thread to read.

> If so, can't they use the Prefer header to do the same on the server-side?
That way, we only remember one thing, rather than two.

They could, but then they'd be exposed to SSL-stripping attacks. It would be
nice if a protected resource could avoid that risk for users who support the
feature.

> Or, I'm completely off base (which due to the time/jetlag/elavation is fairly
possible).

You're not. I'm not entirely convinced that this is valuable in isolation from
HSTS, but it's also not terribly complex to implement. *shrug*

[Yoav Weiss, 2015-03-05 15:33:17]

On 2015/03/05 15:11:38, Mike West wrote:
> On 2015/03/05 at 14:58:10, yoav wrote:
> > In-the-air review!
> 
> Wow! Technology. :)
> 
> > Before I dive into the code, I'm not sure regarding the benefits of this.
> > What does MegaCorp get from pinning/remembering Upgrade instead of using
HSTS?
> Is this just a close-gap to help people address only browsers that support
> Upgrade with HSTS-like functionality?
> 
> HSTS has side-effects (like draconian error handling for expired certs) that
> (the EFF claims) make it more difficult to adopt for their target audience
> (Let's Encrypt). There's some detail at
> https://lists.w3.org/Archives/Public/public-webappsec/2015Feb/0043.html, but
> it's a lot of thread to read.

Can't read the thread now, but did we consider relaxing these side-effects, to
make HSTS more useable?

> 
> > If so, can't they use the Prefer header to do the same on the server-side?
> That way, we only remember one thing, rather than two.
> 
> They could, but then they'd be exposed to SSL-stripping attacks. It would be
> nice if a protected resource could avoid that risk for users who support the
> feature.

What I'm thinking of is a user that accesses a Web site for the first time using
http (with a prefer header),
and since it supports upgrade, gets an upgrade policy as well as an HSTS header.
Then followup navigations happen over https, and can't be stripped.

Is that scenario still vulnerable to stripping attacks? Or maybe HSTS is too
strict here?

> 
> > Or, I'm completely off base (which due to the time/jetlag/elavation is
fairly
> possible).
> 
> You're not. I'm not entirely convinced that this is valuable in isolation from
> HSTS, but it's also not terribly complex to implement. *shrug*

Just to make sure I understand the change, is SecurityContext persisted across
navigations/renderers? Or is this change aimed mainly at sub-frames and followup
navigations?

[Mike West, 2015-03-05 15:49:28]

On 2015/03/05 at 15:33:17, yoav wrote:
> On 2015/03/05 15:11:38, Mike West wrote:
> > On 2015/03/05 at 14:58:10, yoav wrote:
> > > In-the-air review!
> > 
> > Wow! Technology. :)
> > 
> > > Before I dive into the code, I'm not sure regarding the benefits of this.
> > > What does MegaCorp get from pinning/remembering Upgrade instead of using
HSTS?
> > Is this just a close-gap to help people address only browsers that support
> > Upgrade with HSTS-like functionality?
> > 
> > HSTS has side-effects (like draconian error handling for expired certs) that
> > (the EFF claims) make it more difficult to adopt for their target audience
> > (Let's Encrypt). There's some detail at
> > https://lists.w3.org/Archives/Public/public-webappsec/2015Feb/0043.html, but
> > it's a lot of thread to read.
> 
> Can't read the thread now, but did we consider relaxing these side-effects, to
make HSTS more useable?
> 
> > 
> > > If so, can't they use the Prefer header to do the same on the server-side?
> > That way, we only remember one thing, rather than two.
> > 
> > They could, but then they'd be exposed to SSL-stripping attacks. It would be
> > nice if a protected resource could avoid that risk for users who support the
> > feature.
> 
> What I'm thinking of is a user that accesses a Web site for the first time
using http (with a prefer header),
> and since it supports upgrade, gets an upgrade policy as well as an HSTS
header.
> Then followup navigations happen over https, and can't be stripped.
> 
> Is that scenario still vulnerable to stripping attacks? Or maybe HSTS is too
strict here?

No, HSTS is the right thing to do, and I think the spec recommends exactly what
you outline here. HSTS just doesn't seem to be lined up well with the general
idea of this upgrade mechanism as a stopgap solution.

> 
> > 
> > > Or, I'm completely off base (which due to the time/jetlag/elavation is
fairly
> > possible).
> > 
> > You're not. I'm not entirely convinced that this is valuable in isolation
from
> > HSTS, but it's also not terribly complex to implement. *shrug*
> 
> Just to make sure I understand the change, is SecurityContext persisted across
navigations/renderers? Or is this change aimed mainly at sub-frames and followup
navigations?

This change is explicitly not persistent. The SecurityContext is the Document,
and it goes away whenever the user navigates. All this covers are navigation's
triggered from a Document whose policy is set, as well as its children browsing
contexts.

[Yoav Weiss, 2015-03-06 07:40:49]

On 2015/03/05 15:49:28, Mike West wrote:
> On 2015/03/05 at 15:33:17, yoav wrote:
> > On 2015/03/05 15:11:38, Mike West wrote:
> > > On 2015/03/05 at 14:58:10, yoav wrote:
> > > > In-the-air review!
> > > 
> > > Wow! Technology. :)
> > > 
> > > > Before I dive into the code, I'm not sure regarding the benefits of
this.
> > > > What does MegaCorp get from pinning/remembering Upgrade instead of using
> HSTS?
> > > Is this just a close-gap to help people address only browsers that support
> > > Upgrade with HSTS-like functionality?
> > > 
> > > HSTS has side-effects (like draconian error handling for expired certs)
that
> > > (the EFF claims) make it more difficult to adopt for their target audience
> > > (Let's Encrypt). There's some detail at
> > > https://lists.w3.org/Archives/Public/public-webappsec/2015Feb/0043.html,
but
> > > it's a lot of thread to read.
> > 
> > Can't read the thread now, but did we consider relaxing these side-effects,
to
> make HSTS more useable?
> > 
> > > 
> > > > If so, can't they use the Prefer header to do the same on the
server-side?
> > > That way, we only remember one thing, rather than two.
> > > 
> > > They could, but then they'd be exposed to SSL-stripping attacks. It would
be
> > > nice if a protected resource could avoid that risk for users who support
the
> > > feature.
> > 
> > What I'm thinking of is a user that accesses a Web site for the first time
> using http (with a prefer header),
> > and since it supports upgrade, gets an upgrade policy as well as an HSTS
> header.
> > Then followup navigations happen over https, and can't be stripped.
> > 
> > Is that scenario still vulnerable to stripping attacks? Or maybe HSTS is too
> strict here?
> 
> No, HSTS is the right thing to do, and I think the spec recommends exactly
what
> you outline here. HSTS just doesn't seem to be lined up well with the general
> idea of this upgrade mechanism as a stopgap solution.
> 
> > 
> > > 
> > > > Or, I'm completely off base (which due to the time/jetlag/elavation is
> fairly
> > > possible).
> > > 
> > > You're not. I'm not entirely convinced that this is valuable in isolation
> from
> > > HSTS, but it's also not terribly complex to implement. *shrug*
> > 
> > Just to make sure I understand the change, is SecurityContext persisted
across
> navigations/renderers? Or is this change aimed mainly at sub-frames and
followup
> navigations?
> 
> This change is explicitly not persistent. The SecurityContext is the Document,
> and it goes away whenever the user navigates. All this covers are navigation's
> triggered from a Document whose policy is set, as well as its children
browsing
> contexts.

OK, I missed that originally.

LGTM

[Mike West, 2015-03-06 07:42:16]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2015-03-06 07:42:36]

On 2015/03/06 at 07:40:49, yoav wrote:
> LGTM

Thanks. Glad you're back home safely. :)

[commit-bot: I haz the power, 2015-03-06 07:42:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/978323002/1

[commit-bot: I haz the power, 2015-03-06 10:26:24]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=191421

[Mike West, 2015-03-10 18:55:29]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/999473002/ by mkwst@chromium.org.

The reason for reverting is: Speculative revert to see if things stop crashing.
:)

BUG=465497.