New issue
Advanced search Search tips

Issue 853256 link

Starred by 5 users

Issue metadata

Status: WontFix
Owner:
Closed: Sep 28
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

NetworkService: Remove ClientHints headers when cookie are not allowed.

Project Member Reported by mmenke@chromium.org, Jun 15 2018

Issue description

ResourceDispatchHostDelegate calls client_hints::RequestBeginning when a request starts, which remove client hints headers if cookies are not allowed to the initial URL (Strangely, if seems to leave on the header if there's a redirect to a URL where cookies are not allowed, from a URL where they are).  We should implement this logic for the network service.

Marking this as a Canary blocker out of paranoia, though I have no idea what the motivation for this logic was.
 
This is not mandated by the spec, so I do not think it should be blocker for network servicification canary. In fact, recently there were talks about removing the client hints tie up with cookie permissions.

I did not add the logic to remove cookies on redirect since if the original origin had permissions to obtain client hints (because the origin had JavaScript + cookie permissions), then they could have obtained them and passed them on to the next redirect target via other means (e.g., query params).  It would be cleaner to remove them on redirect, but it's not super necessary.
Labels: -Pri-1 -Proj-Servicification-Canary Pri-2
Owner: tbansal@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by mmenke@chromium.org, Jun 15 2018

Labels: -Pri-2 Proj-Servicification Pri-3
Thanks so much for the context!

Comment 4 by jam@chromium.org, Jun 18 2018

Cc: cduvall@chromium.org
+Clark who was looking at the ClientHintsBrowserTest browser tests failures.

Comment 5 by mmenke@chromium.org, Jun 18 2018

Summary: NetworkService: Remove ClientHints headers when cookie are not allowed. (was: NetworkService: Remove ClientHints headers when cookie are now allowed.)
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9526eeed27257de3e0187968fbb9402a13de911b

commit 9526eeed27257de3e0187968fbb9402a13de911b
Author: Clark DuVall <cduvall@chromium.org>
Date: Mon Jun 18 20:20:02 2018

Use new bug for client hints browser test failures in network service

Bug:  853256 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ibb5641a86c89739ef67d9ff8df86bff3f81c3ec0
Reviewed-on: https://chromium-review.googlesource.com/1104950
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568134}
[modify] https://crrev.com/9526eeed27257de3e0187968fbb9402a13de911b/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Comment 7 by dxie@chromium.org, Jun 19 2018

Labels: Hotlist-KnownIssue OS-Chrome OS-Linux OS-Mac OS-Windows
Status: Started (was: Assigned)
Labels: -Hotlist-KnownIssue
Labels: ReleaseBlock-Stable
Project Member

Comment 10 by sheriffbot@chromium.org, Sep 7

Cc: dxie@google.com
This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Pri-3 Hotlist-KnownIssue Proj-Servicification-Stable Pri-1
Project Member

Comment 12 by sheriffbot@chromium.org, Sep 10

This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -ReleaseBlock-Stable
Labels: -Pri-1 Pri-3
This is not Proj-Servicification-Stable blocker as this behavior is not mandated by the specs (see #1 above).
Thanks for the clarification @tbansal. For my understanding, curious why this used cookie permissions? i.e. a page could just load the data using JS and then make an XHR to send it right?

If this behavior isn't mandated by spec, and per comment 14 it's not Proj-Servicification-Stable, do we need to do this at all? Should the old path stop doing this instead?
> a page could just load the data using JS and then make an XHR to send it right?

Yes, but the subset of client hints that are sent can be used to track the user. e.g., if there are N client hints, then it makes it possible for a origin to track 2^N unique users even if the origin does not have cookies permission.

> If this behavior isn't mandated by spec

The spec does not require this. I think we had initially added this check for additional privacy protection. It's possible to implement this using URLLoaderThrottle and implementing the functionality in WillRedirectRequest(): https://cs.chromium.org/chromium/src/content/public/common/url_loader_throttle.cc?type=cs&g=0&l=36

For now, it's not a big concern since the number of client hints (value of N) is relatively small (7).

The client can still track the user by sending this data via XHR, so I'm still not understanding how the cookies check protects the user?
Remove ClientHints headers when cookie are not allowed handles the case when the origin has JavaScript privilege, but not cookie permissions.

Also, the subset of client hint headers that the origin has opted-in can allow origin to track 2^N users. This is irrespective of the values of the client hints themselves (which they can obtain using JavaScript). That means even if all client hints had the same value for all users, it would still be possible for the origin to track 2^N users. We want to prevent that from happening at least when the origin does not have cookie privileges.
"Remove ClientHints headers when cookie are not allowed handles the case when the origin has JavaScript privilege, but not cookie permissions."

What I mean in my questions is what is this trying to protect against? If the origin has JS privilege but not cookie permissions, then why does it matter if the client hints data isn't sent in request headers, if the page can just send the data to the server in an XHR?
There are 2 distinct pieces of information: (i) The value of the different client hints themselves (ii) The subset of client hints that the origin has opted-in. The origin can choose different subsets for different users to track up to 2^N users. 

With only the JavaScript permission, the origin can get (i), but not (ii).

Now, it's probably possible to argue that (i) itself exposes enough data that we do not need to worry about (ii). Is that what you were referring to?
Status: WontFix (was: Started)
Marking as WontFix as per https://bugs.chromium.org/p/chromium/issues/detail?id=880806#c18. 

The plan is to remove cookie checks on non-NS path. I will file a separate issue for that.

Sign in to add a comment