New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 827582 link

Starred by 11 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 29
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Handle header modification edge cases with WebRequest + Network Service

Project Member Reported by roc...@chromium.org, Mar 30 2018

Issue description

There are at least two known edge cases that require resolution before we can say WebRequest fully works with Network Service:

1. Set-Cookie response header modification

This is where an extension modifies the Set-Cookie header of a response, thereby controlling the value of the cookie stored by the network stack. We suspect there is no interesting use of this outside of Chrome OS (login?) SAML support.

2. Removal of User-Agent or Accept-Language headers

Extensions may remove these headers from outgoing requests. This is supported with the non-Network-Service stack because header modification can be done after the stack has injected default values for these special headers if they were missing. With Network Service, we don't have that ability and would like to avoid introducing it.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 2 2018

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

commit 82a4879309dcd9a03f0756fdef2a452b9c5139fa
Author: Ken Rockot <rockot@chromium.org>
Date: Mon Apr 02 20:13:59 2018

Instrument WebRequest header modifications

This is to gain some more understanding about how request and response
header modifications are used in practice. See bug for more details.

Bug:  827582 
Change-Id: I589483173d16988f1c0676cd0a6d768da12e1037
Reviewed-on: https://chromium-review.googlesource.com/988237
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547504}
[modify] https://crrev.com/82a4879309dcd9a03f0756fdef2a452b9c5139fa/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/82a4879309dcd9a03f0756fdef2a452b9c5139fa/extensions/browser/api/web_request/web_request_api_helpers.cc
[modify] https://crrev.com/82a4879309dcd9a03f0756fdef2a452b9c5139fa/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/82a4879309dcd9a03f0756fdef2a452b9c5139fa/tools/metrics/histograms/histograms.xml

Comment 2 by roc...@chromium.org, May 29 2018

Cc: karandeepb@chromium.org rdevlin....@chromium.org jam@chromium.org
Based on the histograms collected in [1] I think it's safe to assert that:

  - Nobody relies on the ability to delete Accept-Language or User-Agent headers (as a reminder, simply *modifying* the headers is not a problem).
  - Nobody** relies on modifying Set-Cookie in response headers.

** Except for SAML login on Chrome OS. I'm actually not sure why we don't see any data reflecting that known usage. Possibly this is because it only affects enterprise users and maybe we don't get much (any?) metrics data from enterprise users?

[1] https://uma.googleplex.com/histograms?endDate=20180527&dayCount=28&histograms=Extensions.WebRequest.ModifiedResponseHeaders%2CExtensions.WebRequest.OnBeforeSendHeadersEventResponse%2CExtensions.WebRequest.OnHeadersReceivedEventResponse%2CExtensions.WebRequest.SpecialHeadersRemoved&fixupData=true&showMax=true&filters=channel%2Ceq%2C4%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial

Comment 3 by dxie@chromium.org, May 29 2018

Labels: Hotlist-KnownIssue
Cc: roc...@chromium.org
Owner: ----
Status: Available (was: Assigned)
Labels: Proj-Servicification-Canary
Adding the canary label for visibility, though I'm not sure whether or not this really needs to block it.

Comment 6 by dxie@chromium.org, Jun 8 2018

Labels: OS-Chrome OS-Windows OS-Mac OS-Linux

Comment 7 by dxie@google.com, Jun 27 2018

Labels: -Hotlist-KnownIssue -Proj-Servicification-Canary Proj-Servicification

Comment 8 Deleted

Cc: cduvall@chromium.org
Labels: OS-Linux OS-Mac OS-Windows
It looks like there may be a similar problem when using AddResponseCookie/EditResponseCookie/RemoveResponseCookie from declarativeWebRequest. These three actions modify the Set-Cookie header of the response[1], but it happens after the cookies are set[2] when the network service is enabled.

1. https://cs.chromium.org/chromium/src/extensions/browser/api/web_request/web_request_api_helpers.cc?l=969&rcl=d0f45007eac9033c8480329e88922e36b8c098fa
2. https://cs.chromium.org/chromium/src/net/url_request/url_request_http_job.cc?l=793&rcl=4273437267c208d21f3a5dba963673501cc72c95
Note that declarativeWebRequest hasn't launched, and probably won't (instead, we're focusing on declarativeNetRequest).  Depending on how all the timing works out, I don't think we necessarily need to address those cases if it's painful.
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 24

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

commit a2d2076ea20a077a80cac20d8cc4be90061cb4c8
Author: Clark DuVall <cduvall@chromium.org>
Date: Tue Jul 24 01:28:38 2018

Fix ExtensionWebRequestApiTest.WebRequestDeclarative2 with network service

This test was modifying Set-Cookie and removing User-Agent, which is no
longer supported with the network service (see  crbug.com/827582 ).

Bug:  827582 ,  721414 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I977bcc767188eafb76c6d560bb959871ed73808a
Reviewed-on: https://chromium-review.googlesource.com/1145941
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577395}
[modify] https://crrev.com/a2d2076ea20a077a80cac20d8cc4be90061cb4c8/chrome/browser/extensions/api/web_request/web_request_apitest.cc
[modify] https://crrev.com/a2d2076ea20a077a80cac20d8cc4be90061cb4c8/chrome/test/data/extensions/api_test/webrequest/test_declarative2.js
[modify] https://crrev.com/a2d2076ea20a077a80cac20d8cc4be90061cb4c8/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc
[modify] https://crrev.com/a2d2076ea20a077a80cac20d8cc4be90061cb4c8/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Labels: -Proj-Servicification-Canary
Since we think these should be pretty rare, not blocking canary on them. During trials, we'll see if we get any bug reports that are caused by this.
Labels: knon
Labels: -knon Hotlist-KnownIssue
Status: WontFix (was: Available)
Marking as WontFix because of data from the field:
https://uma.googleplex.com/p/chrome/histograms?endDate=latest&dayCount=28&histograms=Extensions.WebRequest.ModifiedResponseHeaders%2CExtensions.WebRequest.SpecialHeadersRemoved&fixupData=true&showMax=true&filters=platform%2Ceq%2CW%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial

Set-Cookie addition by extensions: over the last month, we didn't get any UMA of this happening.
AcceptLanguage removal: 0.00000024%
UserAgent removal: 0.0000035%
jam@: The original bug mentioned that the Set-Cookie option is used by Chrome OS SAML support. Has that been ported over?
The SAML support is tracked in bug 887061 and is being worked on now.
Cc: krajshree@chromium.org
 Issue 898461  has been merged into this issue.
> Marking as WontFix because of data from the field:

Hi all,

I think this is a wrong metrics to check. Set-Cookie might not be modified too often, but it might be analyzed in order to discover trackers.

At least one extension relies on inspecting Set-Cookie to do its work -- it is EFF's Privacy Badger with almost a million active users.
Another example is AdGuard Chrome extension with over 5 million active users:
https://github.com/AdguardTeam/AdguardBrowserExtension/issues/961

We were about to introduce a declarative tracking cookie prevention mechanism, and the implementation relies on inspecting set-cookie headers.
The cookies API allows getting notifications of when cookies are modified and allows changing them: https://developer.chrome.com/extensions/cookies#event-onChanged
> The cookies API allows getting notifications of when cookies are modified and allows changing them: https://developer.chrome.com/extensions/cookies#event-onChanged

Sure, but unfortunately, it is not the same as inspecting Set-Cookie as the context will be lost. For instance, this API does not allow detecting what exact request was used to set the cookie which is crucial for tracking detection heuristic used by Privacy Badger.
I am looking into it further and it seems the problem is not just in the headers missing from responses.

There are quite a few headers missing from the onBeforeSendHeaders callbacks as well. For instance, these headers are missing: referer, cookie, accept, accept-language, etc, etc.
Just in case, here is the full list of issues.

1. Some headers can be neither inspected nor modified. I've been able to identify the following cases:
2. onBeforeSendHeaders: referer, cookie, accept-*
3. onHeadersReceived: set-cookie
4. This change will interfere with some of the existing popular browser extensions (i.e. Privacy Badger, AdGuard, maybe others - I did just a quick check).

Is there any chance you'll reconsider?
Would it be sufficient for your use case if we exposed the set-cookie header to onHeadersReceived, but still did not allow modification?
> Would it be sufficient for your use case if we exposed the set-cookie header to onHeadersReceived, but still did not allow modification?

I guess it'd be sufficient for most of the usecases as modification via chrome.cookies is still possible.

But what about other headers (talking about onBeforeSendHeaders)? Should I file a new bug report or is it a part of this one?
Labels: -Pri-2 Pri-1
Owner: cduvall@chromium.org
Status: Assigned (was: WontFix)
Opening this bug again. cduvall@, can you clarify the behavior differences with the non-network service path again. The original bug report stated that it's not possible to modify Set-cookie response header (in onHeadersReceived) and delete User-Agent and Accept-Language request header (in onBeforeSendHeaders). Some questions:

1) Is the Set-Cookie response header visible in OnResponseStarted?
2) Is the User-Agent and Accept-Language request header visible in onSendHeaders?
3) c#25,26 also describes changes in behavior for referrer and accept header, which the original bug report didn't. Can you look into this?
I did some digging on this yesterday, here are my findings:

1) Set-Cookie is not visible. This is because we do not send set-cookie over IPC for security/privacy reasons, so it does not make the hop from network process to browser process where the web request code intercepts it: https://cs.chromium.org/chromium/src/services/network/public/cpp/net_ipc_param_traits.cc?l=203&rcl=6f13e53095d799d228a67b8a7233c3e023a13e63

2) User-Agent/Accept-Language may be visible if set outside the net stack, but if the default is used (which is set in the net stack), it will not be visible. Headers are set here (Accept-Language set in AddExtraHeaders()): https://cs.chromium.org/chromium/src/net/url_request/url_request_http_job.cc?l=439&rcl=fdea8bed4c4f917e882dd70165ad5b244e28b262

3) Referrer, Cookie, and Accept-Encoding are set in the same place linked above, so are not visible.

The current implementation of web request in network service sends the OnBeforeSendHeaders event before the request goes to the network process, which is why these headers are not visible. We would need to rethink the design a bit if we wanted to make these available.

I believe the restrictions on sending Set-Cookie over IPC are mainly to avoid sending it to a renderer, so it should be fine to send it to the browser process.
Cc: dxie@chromium.org
Talked to cduvall@ offline. It seems like our assumptions around the behavior differences between the network service and non-network service path were not correct. (See the original bug report and c#30). The same assumptions were used for the privacy review for network service. We should revisit before launching network service.
Labels: Proj-Servicification-Stable
Does my problem with referer header refers to this bug?
https://stackoverflow.com/questions/53153057/referer-header-is-no-longer-sent-by-chrome
Project Member

Comment 35 by bugdroid1@chromium.org, Nov 8

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

commit 2452f7df8eed3df44cf5852081909a5d9d08956a
Author: Clark DuVall <cduvall@chromium.org>
Date: Thu Nov 08 01:10:49 2018

Add histograms for tracking header webRequest modifications

These will help us know how often these special headers are changed
or removed. Also added a histogram to track the number of users with
a webRequest extension installed.

Bug:  827582 
Change-Id: Ia4e53d528363eb29a9271e6b635b97dd90117a7d
Reviewed-on: https://chromium-review.googlesource.com/c/1320469
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606267}
[modify] https://crrev.com/2452f7df8eed3df44cf5852081909a5d9d08956a/chrome/browser/extensions/api/web_request/web_request_apitest.cc
[modify] https://crrev.com/2452f7df8eed3df44cf5852081909a5d9d08956a/chrome/browser/extensions/installed_loader.cc
[modify] https://crrev.com/2452f7df8eed3df44cf5852081909a5d9d08956a/extensions/browser/api/web_request/web_request_api_helpers.cc
[modify] https://crrev.com/2452f7df8eed3df44cf5852081909a5d9d08956a/extensions/browser/api/web_request/web_request_api_helpers.h
[modify] https://crrev.com/2452f7df8eed3df44cf5852081909a5d9d08956a/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
[modify] https://crrev.com/2452f7df8eed3df44cf5852081909a5d9d08956a/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/2452f7df8eed3df44cf5852081909a5d9d08956a/tools/metrics/histograms/histograms.xml

Labels: M-72
Project Member

Comment 37 by bugdroid1@chromium.org, Nov 13

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

commit 19c3e55b279c0ab189705875d14d2b4cff54e895
Author: Clark DuVall <cduvall@chromium.org>
Date: Tue Nov 13 18:40:48 2018

Record request timing metrics for webRequest

Records two metrics, one to track request time when at least one
listener is registered, and another to record request time when the
request has been blocked on a listener.

Bug:  827582 
Change-Id: I938cb971a40a638295b13a27f9c1d77f6e6d2107
Reviewed-on: https://chromium-review.googlesource.com/c/1327149
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607669}
[modify] https://crrev.com/19c3e55b279c0ab189705875d14d2b4cff54e895/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/19c3e55b279c0ab189705875d14d2b4cff54e895/extensions/browser/api/web_request/web_request_time_tracker.cc
[modify] https://crrev.com/19c3e55b279c0ab189705875d14d2b4cff54e895/extensions/browser/api/web_request/web_request_time_tracker.h
[modify] https://crrev.com/19c3e55b279c0ab189705875d14d2b4cff54e895/extensions/browser/api/web_request/web_request_time_tracker_unittest.cc
[modify] https://crrev.com/19c3e55b279c0ab189705875d14d2b4cff54e895/tools/metrics/histograms/histograms.xml

Labels: Merge-Request-71
Requesting merge of changes in comment 35 (2452f7df8eed3df44cf5852081909a5d9d08956a) and comment 37 (19c3e55b279c0ab189705875d14d2b4cff54e895) to M71. These are histogram changes that will help us gather data in stable.
Project Member

Comment 39 by sheriffbot@chromium.org, Nov 14

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Are the changes listed at #38 looking good in canary and full safe to merge?
Yes both are looking good in canary and should be safe.
Labels: -Merge-Review-71 Merge-Approved-71
Approving merge for CLs listed at #38 based on comment #38 and #41. Please merge ASAP. Thank you.
Project Member

Comment 43 by bugdroid1@chromium.org, Nov 14

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b8938673e64dd856673a041e299396ef8687f65f

commit b8938673e64dd856673a041e299396ef8687f65f
Author: Clark DuVall <cduvall@chromium.org>
Date: Wed Nov 14 22:06:51 2018

Add histograms for tracking header webRequest modifications

These will help us know how often these special headers are changed
or removed. Also added a histogram to track the number of users with
a webRequest extension installed.

Bug:  827582 
Change-Id: Ia4e53d528363eb29a9271e6b635b97dd90117a7d
Reviewed-on: https://chromium-review.googlesource.com/c/1320469
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#606267}(cherry picked from commit 2452f7df8eed3df44cf5852081909a5d9d08956a)
Reviewed-on: https://chromium-review.googlesource.com/c/1336371
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#685}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/b8938673e64dd856673a041e299396ef8687f65f/chrome/browser/extensions/api/web_request/web_request_apitest.cc
[modify] https://crrev.com/b8938673e64dd856673a041e299396ef8687f65f/chrome/browser/extensions/installed_loader.cc
[modify] https://crrev.com/b8938673e64dd856673a041e299396ef8687f65f/extensions/browser/api/web_request/web_request_api_helpers.cc
[modify] https://crrev.com/b8938673e64dd856673a041e299396ef8687f65f/extensions/browser/api/web_request/web_request_api_helpers.h
[modify] https://crrev.com/b8938673e64dd856673a041e299396ef8687f65f/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
[modify] https://crrev.com/b8938673e64dd856673a041e299396ef8687f65f/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/b8938673e64dd856673a041e299396ef8687f65f/tools/metrics/histograms/histograms.xml

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/b8938673e64dd856673a041e299396ef8687f65f

Commit: b8938673e64dd856673a041e299396ef8687f65f
Author: cduvall@chromium.org
Commiter: cduvall@chromium.org
Date: 2018-11-14 22:06:51 +0000 UTC

Add histograms for tracking header webRequest modifications

These will help us know how often these special headers are changed
or removed. Also added a histogram to track the number of users with
a webRequest extension installed.

Bug:  827582 
Change-Id: Ia4e53d528363eb29a9271e6b635b97dd90117a7d
Reviewed-on: https://chromium-review.googlesource.com/c/1320469
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#606267}(cherry picked from commit 2452f7df8eed3df44cf5852081909a5d9d08956a)
Reviewed-on: https://chromium-review.googlesource.com/c/1336371
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#685}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/ee875f722f90b37562b1b0339539dc16fec08421

Commit: ee875f722f90b37562b1b0339539dc16fec08421
Author: cduvall@chromium.org
Commiter: cduvall@chromium.org
Date: 2018-11-14 22:08:40 +0000 UTC

Record request timing metrics for webRequest

Records two metrics, one to track request time when at least one
listener is registered, and another to record request time when the
request has been blocked on a listener.

Bug:  827582 
Change-Id: I938cb971a40a638295b13a27f9c1d77f6e6d2107
Reviewed-on: https://chromium-review.googlesource.com/c/1327149
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#607669}(cherry picked from commit 19c3e55b279c0ab189705875d14d2b4cff54e895)
Reviewed-on: https://chromium-review.googlesource.com/c/1336372
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#686}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Project Member

Comment 46 by bugdroid1@chromium.org, Nov 14

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

commit ee875f722f90b37562b1b0339539dc16fec08421
Author: Clark DuVall <cduvall@chromium.org>
Date: Wed Nov 14 22:08:40 2018

Record request timing metrics for webRequest

Records two metrics, one to track request time when at least one
listener is registered, and another to record request time when the
request has been blocked on a listener.

Bug:  827582 
Change-Id: I938cb971a40a638295b13a27f9c1d77f6e6d2107
Reviewed-on: https://chromium-review.googlesource.com/c/1327149
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#607669}(cherry picked from commit 19c3e55b279c0ab189705875d14d2b4cff54e895)
Reviewed-on: https://chromium-review.googlesource.com/c/1336372
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#686}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/ee875f722f90b37562b1b0339539dc16fec08421/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/ee875f722f90b37562b1b0339539dc16fec08421/extensions/browser/api/web_request/web_request_time_tracker.cc
[modify] https://crrev.com/ee875f722f90b37562b1b0339539dc16fec08421/extensions/browser/api/web_request/web_request_time_tracker.h
[modify] https://crrev.com/ee875f722f90b37562b1b0339539dc16fec08421/extensions/browser/api/web_request/web_request_time_tracker_unittest.cc
[modify] https://crrev.com/ee875f722f90b37562b1b0339539dc16fec08421/tools/metrics/histograms/histograms.xml

Hello,

Is there any change in Chrome Canary? I've just checked it on Version 72.0.3623.0 (Official Build) canary (64-bit) and the headers are still invisible to extensions.

I have a change being reviewed right now (http://crrev.com/c/1338165) which will allow extensions to modify/view the headers if the 'extraHeaders' value is used in the extraInfoSpec of the listener. Once this is in Canary, you should be able to use this to match the previous behavior.

Example: https://chromium-review.googlesource.com/c/chromium/src/+/1338165/23/chrome/common/extensions/docs/examples/extensions/no_cookies/background.js
Project Member

Comment 49 by bugdroid1@chromium.org, Nov 27

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

commit a5e25b756922b12f8a4566bbefc6ef28a75ec9e3
Author: Clark DuVall <cduvall@chromium.org>
Date: Tue Nov 27 22:08:16 2018

Allow modifying/viewing special WebRequest headers with network service

This approach adds 2 extra process hops before the request, and once
the headers are received to allow modification of headers that are set
and used within //net. A new "extraHeaders" value is added to the
opt_extraInfoSpec enum of onBeforeSendHeaders/onSendHeaders/
onHeadersReceived, which will trigger this behavior. The process hops
will only be done if there is a listener with the "extraHeaders" option
that matches the request registered when the request starts.

To do the header modification, a TrustedURLLoaderHeaderClient can be
set on URLLoaderFactoryParams which will be called from the network
delegate at the required times if the kURLLoadOptionUseHeaderClient
option is set on the request. This interface has methods to modify
the request and response headers, which will pause the request until
the callback is called with the result of the header modification.
These methods expose the sensitive "set-cookie" response header
(which is normally stripped when response headers are serialized for
IPC) by serializing the full headers to a string before sending. This
is safe because only the browser process is able to set the header
client on the URLLoaderFactoryParams that will have access to this
header. The TrustedURLLoaderHeaderClient pipe should never be passed
to an untrusted process.

Bug:  827582 
Change-Id: If31ac82d91de1fd2c745a2f05ebbb14130713b3e
Reviewed-on: https://chromium-review.googlesource.com/c/1338165
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611357}
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/android_webview/browser/aw_content_browser_client.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/android_webview/browser/aw_content_browser_client.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/chrome/browser/extensions/api/web_request/web_request_api_unittest.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/chrome/browser/extensions/api/web_request/web_request_apitest.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/chrome/browser/extensions/chrome_content_browser_client_extensions_part.h
[add] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/chrome/common/extensions/docs/examples/extensions/no_cookies/background.js
[add] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/chrome/common/extensions/docs/examples/extensions/no_cookies/manifest.json
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/chrome/common/extensions/docs/templates/intros/webRequest.html
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/chrome/test/data/extensions/api_test/webrequest/framework.js
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/chrome/test/data/extensions/api_test/webrequest/test_blocking_cookie.js
[add] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/chrome/test/data/extensions/api_test/webrequest/test_extra_headers.html
[add] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/chrome/test/data/extensions/api_test/webrequest/test_extra_headers.js
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/browser/loader/navigation_url_loader_impl.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/browser/loader/navigation_url_loader_impl_unittest.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/browser/security_exploit_browsertest.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/browser/service_worker/embedded_worker_instance.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/browser/shared_worker/worker_script_fetch_initiator.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/public/browser/content_browser_client.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/public/browser/render_process_host.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/public/test/mock_render_process_host.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/public/test/mock_render_process_host.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/public/test/url_loader_interceptor.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/public/test/url_loader_interceptor.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/content/test/url_loader_interceptor_test.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/extensions/browser/api/web_request/web_request_api.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/extensions/browser/api/web_request/web_request_api_helpers.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/extensions/browser/api/web_request/web_request_api_helpers.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/extensions/browser/api/web_request/web_request_event_details.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/extensions/browser/url_loader_factory_manager.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/extensions/browser/url_loader_factory_manager.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/extensions/common/api/web_request.json
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/extensions/common/api/web_request_internal.json
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/extensions/shell/browser/shell_content_browser_client.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/extensions/shell/browser/shell_content_browser_client.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/services/network/BUILD.gn
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/services/network/network_context_unittest.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/services/network/network_service_network_delegate.cc
[add] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/services/network/pending_callback_chain.cc
[add] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/services/network/pending_callback_chain.h
[add] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/services/network/pending_callback_chain_unittest.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/services/network/public/mojom/url_loader_factory.mojom
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/services/network/url_loader.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/services/network/url_loader.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/services/network/url_loader_factory.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/services/network/url_loader_factory.h
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/services/network/url_loader_unittest.cc
[modify] https://crrev.com/a5e25b756922b12f8a4566bbefc6ef28a75ec9e3/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

This change is now in Canary. Please see the post in the chrome-extensions group for how to enable these headers in Chrome 72: https://groups.google.com/a/chromium.org/forum/#!topic/chromium-extensions/vYIaeezZwfQ
Project Member

Comment 52 by bugdroid1@chromium.org, Nov 29

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

commit 0990c9da8b96d96551a382b5e97de4958058dcd4
Author: Clark DuVall <cduvall@chromium.org>
Date: Thu Nov 29 00:04:48 2018

Add histogram for webRequest listeners added with each flag

Bug:  827582 
Change-Id: I86a4d5cba5901fb7430c554a4fc978789f51fb49
Reviewed-on: https://chromium-review.googlesource.com/c/1354418
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611944}
[modify] https://crrev.com/0990c9da8b96d96551a382b5e97de4958058dcd4/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/0990c9da8b96d96551a382b5e97de4958058dcd4/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/0990c9da8b96d96551a382b5e97de4958058dcd4/tools/metrics/histograms/histograms.xml

I can confirm that with extraHeaders it works as expected in the latest Chromium.

Thank you!
Status: Fixed (was: Assigned)
Marking this as fixed, feel free to re-open if extraHeaders is not working as expected.
 Issue 810905  has been merged into this issue.
Cc: viswa.karala@chromium.org swarnasree.mukkala@chromium.org
 Issue 914359  has been merged into this issue.

Sign in to add a comment