other extension devs have also noticed that client[*].google.com origins appear to be used broadly for things that should be intercept-able: https://groups.google.com/a/chromium.org/d/msg/chromium-extensions/lJfiGYpnIq4/vwSBV8hrK3IJ, https://groups.google.com/a/chromium.org/forum/#!topic/chromium-extensions/-CEsQjfZLaI

-Yan

Oof.

The reason we block those is largely laid out in the function comment:

    // Check for "clients[0-9]*.google.com" hosts.
    // This protects requests to several internal services such as sync,
    // extension update pings, captive portal detection, fraudulent certificate
    // reporting, autofill and others.

We *shouldn't* let extensions intercept those requests due to the fairly massive risks.

I think this is something where, like Mike suggests, we need to clarify what exactly these origins may or may not be used for.

Thanks for taking a look!
> We *shouldn't* let extensions intercept those requests due to the fairly massive risks.

Yup. Understood and agreed. Internal Chrome requests should remain opaque to extensions.

One option is to more clearly enumerate the services we expect Chrome to be using, and narrow the carveouts to path-level rather than origin-level. That requires a bit more discipline about the services we rely on, but seems like it would lead to better outcomes. I think there's already some work in this direction via the `DefineNetworkTrafficAnnotation` additions that might make things simpler...

Or, if all teams have named their directories as well as the GMail team has, we can simply add `&& !base::StartsWith(url.path_piece(), "/ads/", base::CompareCase::SENSITIVE)` to the various checks. :)

Being more disciplined about which paths we use in chrome sounds great to me, but I'm worried that getting to that state will be painful (and might involve costly mistakes.)

from the same function:
    // This protects requests to safe browsing, link doctor, and possibly
    // others.

"possibly others" does not instill much confidence in our ability to pinpoint which services we rely on being protected... :)

Agreed that network annotations could help us here.

Currently, we define the "target" field (website, Google service, other). The "Google service" target should cover all internal requests that go to Google (but not all internal requests as they might go elsewhere), and should not cover content area requests for clients*.google.com, since those would be classified as "website".

However, currently the entire annotation is extracted except the ID; this would require us to leave that annotation field in. +rhalavati@ to please check if this makes sense.

But this might not be ready for M60. At the very moment, I'm not sure if there is a better differentiator than path, as Mike suggested.

> we need to clarify what exactly these origins may or may not be used for
IIRC clients* is not just for Chrome: it is for serving content from any automated or non-user-facing AJAX-like queries, as a way to isolate www.google.com from vulnerabilities or accidental DoS. It is used by several teams outside of the browser: http://shortn/_4dwFzlb0XF (sorry, Googlers-only).

I don't think we'll have any trying to maintain a list of exceptions like /ads/. It's probably more feasible to have Chrome be more disciplined as suggested. I feel that we should be able to discover all the URLs Chrome is using with some CodeSearching - I recall that there was an initiative (from Opera?) a while ago to move all such URLs into constants files. (Caveat: There's also the chance of a component-extension or default component using such a URL.)

It seems like annotations are the way to go here.

Mea culpa. When I wrote that code, I had the apparently wrong assumption that client == Chrome. I wonder whether we should - as a quick fix - keep the current regex but whitelist all requests from renderers to be exposed to the web request API. But I have no idea whether this would whitelist too much. My concern is that the network request annotations might slip the M60 feature freeze.

Given the point in #2 (the need to protect Chrome-internal requests from extensions), and given the limitations of distinguishing request type by hostname, URL path, or with annotations: Is it possible to instead have Chrome-internal requests use a totally different request context, one that doesn't have extensions hooks installed?

I'm ignorant about the design and implementation of this stuff, so maybe my question is off-base. But it seems like nobody has raised or addressed the possibility yet.

I don't think I have anything to contribute in addition to the people already active here.

Attaching a sample extension that just writes the network requests observed by the extension to console.log of the extension's background page. This can be used to manually test the proposed effect of https://codereview.chromium.org/2845943003:

1) Install the extension and open the console of its background page.
2) Open https://example.com, go to dev tools and the console, execute
   window.fetch("https://clients1.google.com", {mode: 'no-cors'})
   .then(response=>response.blob())
   .then(r=>console.log(r));
3) Observe that the console of 1) shows the request to https://clients1.google.com
4) Repeat 2) for chrome://settings instead of https://example.com
5) Observe that the console of 1) does not show the request to https://clients1.google.com

Deleted: webrequestlogging.zip 821 bytes

webrequestlogging.zip 821 bytes Download

battre@ has a patch up for this; passing ownership to him.

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

commit 14ca4ff2719095e31e2f4cc49d1d5e8d5af175ce
Author: battre <battre@chromium.org>
Date: Thu May 11 14:31:48 2017

Limit protection of clients[0-9]*.google.com to requests from browser.

The WebRequest API protected network requests to clients[0-9]*.google.com from
being visible or modified by extensions. This was built under the assumption
that these domains are only used by Chrome internally, which was invalid. This
CL restrictes the protection to only those network requests that originate from
the browser. Requests from the renderers are exposed to extensions.

BUG= 715184 
TEST= https://crbug.com/715184#c12 

Review-Url: https://codereview.chromium.org/2845943003
Cr-Commit-Position: refs/heads/master@{#470949}

[modify] https://crrev.com/14ca4ff2719095e31e2f4cc49d1d5e8d5af175ce/extensions/browser/api/web_request/web_request_permissions.cc
[modify] https://crrev.com/14ca4ff2719095e31e2f4cc49d1d5e8d5af175ce/extensions/browser/api/web_request/web_request_permissions.h
[modify] https://crrev.com/14ca4ff2719095e31e2f4cc49d1d5e8d5af175ce/extensions/browser/api/web_request/web_request_permissions_unittest.cc

As per Comment# 12 tested the issue on Windows, Mac and Linux on Chrome Dev# 60.0.3100.0 and observing the following error in console. 
Attaching a screenshot for reference.
@mkwst -- Could you please let us know what the expected behavior which would help us triage further.
Thanks in Advance.

Deleted: 715184.png 121 KB

	715184.png 121 KB View Download

msrchandra@: That looks correct to me. The request to `client1,google.com` is showing up in the extension, which wasn't happening before.

Dear release managers, WDYT about merging this change back to beta? (battre@: I think you're busy at the moment, so I can handle the merge if folks approve it.)

Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

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

commit 210763c75ffa0245904ff82e31d6e08e5b28ed89
Author: Mike West <mkwst@google.com>
Date: Tue May 16 11:26:57 2017

Limit protection of clients[0-9]*.google.com to requests from browser.

The WebRequest API protected network requests to clients[0-9]*.google.com from
being visible or modified by extensions. This was built under the assumption
that these domains are only used by Chrome internally, which was invalid. This
CL restrictes the protection to only those network requests that originate from
the browser. Requests from the renderers are exposed to extensions.

BUG= 715184 
TEST= https://crbug.com/715184#c12 

Review-Url: https://codereview.chromium.org/2845943003
Cr-Original-Commit-Position: refs/heads/master@{#470949}
Review-Url: https://codereview.chromium.org/2886573003 .
Cr-Commit-Position: refs/branch-heads/3071@{#578}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/210763c75ffa0245904ff82e31d6e08e5b28ed89/extensions/browser/api/web_request/web_request_permissions.cc
[modify] https://crrev.com/210763c75ffa0245904ff82e31d6e08e5b28ed89/extensions/browser/api/web_request/web_request_permissions.h
[modify] https://crrev.com/210763c75ffa0245904ff82e31d6e08e5b28ed89/extensions/browser/api/web_request/web_request_permissions_unittest.cc

@mkwst - Thanks for the update.
As per Comment# 17, the issue is found to be fixed on Chrome Dev# 60.0.3100.0 on Windows, Mac, Linux and Chrome OS (Candy).
Hence adding TE-Verified labels

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

commit d6a0a2e456e9f9e938d72c80a681ea701bdec8ec
Author: battre <battre@chromium.org>
Date: Tue May 16 17:13:47 2017

Integration test for protecting clients*.google.com

BUG= 715184 

Review-Url: https://codereview.chromium.org/2876653003
Cr-Commit-Position: refs/heads/master@{#472141}

[modify] https://crrev.com/d6a0a2e456e9f9e938d72c80a681ea701bdec8ec/chrome/browser/extensions/api/web_request/web_request_apitest.cc
[add] https://crrev.com/d6a0a2e456e9f9e938d72c80a681ea701bdec8ec/chrome/test/data/extensions/api_test/webrequest_clients_google_com/background.js
[add] https://crrev.com/d6a0a2e456e9f9e938d72c80a681ea701bdec8ec/chrome/test/data/extensions/api_test/webrequest_clients_google_com/manifest.json

Verified this issue on Mac OS 10.12, Ubuntu 14.04 and Windows-10 using chrome latest beta M59-59.0.3071.61 by following steps mentioned in the comment #12, Observed `client1,google.com` is showing up in the extension as expected by pasting step=2 code. Hence adding TE-Verified label.