Loosen strict 'Secure' checks for non-overlapping paths.

Loosen strict 'Secure' checks for non-overlapping paths.

After a bit of discussion in [1], the latest strict 'Secure' draft [2]
relaxes the equivalency checks for strict 'Secure' enforcement to allow
a non-secure cookie whose path does not overlap an existing secure
cookie's path to be set.

That is, given a secure cookie at '/path', a non-secure cookie can be
set at '/not-path', but not at '/path/subpath'.

This carveout allows us to harden 'Secure' without breaking folks who
host both secure and non-secure applications on a single domain. That
turned out to be the root cause of the breakage Blizzard experienced
in https://crbug.com/580770.

[1]: https://github.com/httpwg/http-extensions/issues/223
[2]: https://tools.ietf.org/html/draft-ietf-httpbis-cookie-alone-01

BUG=580770
R=jww@chromium.org

Committed: https://crrev.com/2c857fb16cdef9783ba3caeb586b9a3648114b17
Cr-Commit-Position: refs/heads/master@{#417237}

[Mike West, 2016-09-05 10:01:57]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-05 10:02:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mike West, 2016-09-05 10:02:19]

Hey Joel, mind taking a look at this?

[commit-bot: I haz the power, 2016-09-05 11:16:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-05 11:16:11]

Dry run: This issue passed the CQ dry run.

[jww, 2016-09-06 22:45:10]

lgtm with minor nits.

https://codereview.chromium.org/2306393002/diff/1/net/cookies/canonical_cookie.h
File net/cookies/canonical_cookie.h (right):

https://codereview.chromium.org/2306393002/diff/1/net/cookies/canonical_cooki...
net/cookies/canonical_cookie.h:112: // cookie's path (as per 'IsOnPath()').
nit: Maybe make a more explicit note that this is not a symmetric check? It took
me a minute to fully grok that this is one purpose with the path change.

https://codereview.chromium.org/2306393002/diff/1/net/cookies/canonical_cooki...
File net/cookies/canonical_cookie_unittest.cc (right):

https://codereview.chromium.org/2306393002/diff/1/net/cookies/canonical_cooki...
net/cookies/canonical_cookie_unittest.cc:252:
EXPECT_TRUE(cookie->IsEquivalentForSecureCookieMatching(*other_cookie));
nit: In a few of these cases, it's probably worth doing the inverse comparison
as well, to validate the asymmetry of the function, i.e.:
EXPECT_FALSE(other_cookie->IsEquivalentForSecureCookieMatching(cookie));

[Mike West, 2016-09-07 10:59:57]

On 2016/09/06 at 22:45:10, jww wrote:
> lgtm with minor nits.
> 
>
https://codereview.chromium.org/2306393002/diff/1/net/cookies/canonical_cookie.h
> File net/cookies/canonical_cookie.h (right):
> 
>
https://codereview.chromium.org/2306393002/diff/1/net/cookies/canonical_cooki...
> net/cookies/canonical_cookie.h:112: // cookie's path (as per 'IsOnPath()').
> nit: Maybe make a more explicit note that this is not a symmetric check? It
took me a minute to fully grok that this is one purpose with the path change.
> 
>
https://codereview.chromium.org/2306393002/diff/1/net/cookies/canonical_cooki...
> File net/cookies/canonical_cookie_unittest.cc (right):
> 
>
https://codereview.chromium.org/2306393002/diff/1/net/cookies/canonical_cooki...
> net/cookies/canonical_cookie_unittest.cc:252:
EXPECT_TRUE(cookie->IsEquivalentForSecureCookieMatching(*other_cookie));
> nit: In a few of these cases, it's probably worth doing the inverse comparison
as well, to validate the asymmetry of the function, i.e.:
> EXPECT_FALSE(other_cookie->IsEquivalentForSecureCookieMatching(cookie));

Done and done. Thanks, Joel!

[Mike West, 2016-09-07 11:00:04]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2016-09-07 11:00:05]

The patchset sent to the CQ was uploaded after l-g-t-m from jww@chromium.org
Link to the patchset: https://codereview.chromium.org/2306393002/#ps20001
(title: "jww@")

[commit-bot: I haz the power, 2016-09-07 11:00:19]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 11:28:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-07 11:28:51]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Mike West, 2016-09-08 06:48:38]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2016-09-08 06:48:39]

The patchset sent to the CQ was uploaded after l-g-t-m from jww@chromium.org
Link to the patchset: https://codereview.chromium.org/2306393002/#ps40001
(title: "oops")

[commit-bot: I haz the power, 2016-09-08 06:48:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-08 09:18:40]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-08 09:20:49]

Description was changed from

==========
Loosen strict 'Secure' checks for non-overlapping paths.

After a bit of discussion in [1], the latest strict 'Secure' draft [2]
relaxes the equivalency checks for strict 'Secure' enforcement to allow
a non-secure cookie whose path does not overlap an existing secure
cookie's path to be set.

That is, given a secure cookie at '/path', a non-secure cookie can be
set at '/not-path', but not at '/path/subpath'.

This carveout allows us to harden 'Secure' without breaking folks who
host both secure and non-secure applications on a single domain. That
turned out to be the root cause of the breakage Blizzard experienced
in https://crbug.com/580770.

[1]: https://github.com/httpwg/http-extensions/issues/223
[2]: https://tools.ietf.org/html/draft-ietf-httpbis-cookie-alone-01

BUG=580770
R=jww@chromium.org
==========

to

==========
Loosen strict 'Secure' checks for non-overlapping paths.

After a bit of discussion in [1], the latest strict 'Secure' draft [2]
relaxes the equivalency checks for strict 'Secure' enforcement to allow
a non-secure cookie whose path does not overlap an existing secure
cookie's path to be set.

That is, given a secure cookie at '/path', a non-secure cookie can be
set at '/not-path', but not at '/path/subpath'.

This carveout allows us to harden 'Secure' without breaking folks who
host both secure and non-secure applications on a single domain. That
turned out to be the root cause of the breakage Blizzard experienced
in https://crbug.com/580770.

[1]: https://github.com/httpwg/http-extensions/issues/223
[2]: https://tools.ietf.org/html/draft-ietf-httpbis-cookie-alone-01

BUG=580770
R=jww@chromium.org

Committed: https://crrev.com/2c857fb16cdef9783ba3caeb586b9a3648114b17
Cr-Commit-Position: refs/heads/master@{#417237}
==========

[commit-bot: I haz the power, 2016-09-08 09:20:50]

Patchset 3 (id:??) landed as
https://crrev.com/2c857fb16cdef9783ba3caeb586b9a3648114b17
Cr-Commit-Position: refs/heads/master@{#417237}