Implement Origin allow/block lists for Chrome Extensions in Network Service CORS

[Takashi Toyoshima]

Hi NetworkService and Extensions developers,

This is heads-up to share our current implementation status, and a mail thread to discuss design details, (asked in a code review, crrev.com/c/11863821186382).

As some of you may already know, I and yhirano@ have worked on out-of-render CORS or CORS in Network Service [design doc]. Major implementation was already done, and now we started digging details. The biggest remaining part is the origin allow/block lists for Chrome Extensions.

I already have a working implementation here, crrev.com/c/11863821186382, and base design is discussed at Origin Allow and Block lists in SecurityPolicy section in the design doc.

For NetworkService experts:

In the code review, I realized that I didn't understand the right way to access relevant NetworkContext interface, e.g. from platform apps, that do not use the default storage partition. I'm not sure if there are other exceptional cases, maybe need to consider incognito mode case?

At this moment, I haven't implemented the access lists for NetworkService disabled mode. So, I also want to know which implementation is returned from the default storage partition if the NetworkService is disabled.

For Extensions experts:

I'm afraid that I haven't caught other code path to update the access list. Please review my design doc.

Thanks,

--

Takashi Toyoshima
Software Engineer, Google

[Matt Menke]

With the network service disable, content/ will create a bogus NetworkContext that just wraps the URLRequestContext.  Some things will work (Like using the HostResolver APIs, and creating URLLoaderFactories), but other APIs are use at your own risk.  Chrome overrides this behavior, and creates "mostly normal" NetworkContexts that have a couple chrome objects injected (Like a NetworkDelegate, still required for extensions to work, unfortunately)

--
You received this message because you are subscribed to the Google Groups "network-service-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to network-service...@chromium.org.
To post to this group, send email to network-s...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/network-service-dev/CAFWCB1%3DxkyE6uDkHKKtWo-w8vq73Hc5Ev%3D%3DnWnop7X9SEubTJA%40mail.gmail.com.

[Takashi Toyoshima]

On Tue, Aug 28, 2018 at 11:38 PM Matt Menke <mme...@chromium.org> wrote:

With the network service disable, content/ will create a bogus NetworkContext that just wraps the URLRequestContext.  Some things will work (Like using the HostResolver APIs, and creating URLLoaderFactories), but other APIs are use at your own risk.  Chrome overrides this behavior, and creates "mostly normal" NetworkContexts that have a couple chrome objects injected (Like a NetworkDelegate, still required for extensions to work, unfortunately)

Thanks. What I really want is finding a place to have a per-profile basis allow/block lists. For legacy code path, we create CORSURLLoaderFactory instances in ResourceMessageFilter, and need to carry an accessor of the lists to the CORSURLLoaderFactory ctor. So, can I have such per-profile lists in the URLRequestContext, and pass an accessor to the ResourceMessageFilter via StoragePartition? Or is there any other better place to have it?

 

On Tue, Aug 28, 2018 at 9:32 AM Takashi Toyoshima <toyo...@chromium.org> wrote:

Hi NetworkService and Extensions developers,

This is heads-up to share our current implementation status, and a mail thread to discuss design details, (asked in a code review, crrev.com/c/11863821186382).

As some of you may already know, I and yhirano@ have worked on out-of-render CORS or CORS in Network Service [design doc]. Major implementation was already done, and now we started digging details. The biggest remaining part is the origin allow/block lists for Chrome Extensions.

I already have a working implementation here, crrev.com/c/11863821186382, and base design is discussed at Origin Allow and Block lists in SecurityPolicy section in the design doc.

For NetworkService experts:

In the code review, I realized that I didn't understand the right way to access relevant NetworkContext interface, e.g. from platform apps, that do not use the default storage partition. I'm not sure if there are other exceptional cases, maybe need to consider incognito mode case?

At this moment, I haven't implemented the access lists for NetworkService disabled mode. So, I also want to know which implementation is returned from the default storage partition if the NetworkService is disabled.

For Extensions experts:

I'm afraid that I haven't caught other code path to update the access list. Please review my design doc.

Thanks,

--

Takashi Toyoshima
Software Engineer, Google

--
You received this message because you are subscribed to the Google Groups "network-service-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to network-service...@chromium.org.
To post to this group, send email to network-s...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/network-service-dev/CAFWCB1%3DxkyE6uDkHKKtWo-w8vq73Hc5Ev%3D%3DnWnop7X9SEubTJA%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "network-service-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to network-service...@chromium.org.
To post to this group, send email to network-s...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/network-service-dev/CAEK7mvoQCXirfYXq1w%3DAKh-CrN_D_oFWB%2BmshfLwcmDnFbYaxQ%40mail.gmail.com.

[Matt Menke]

On Wed, Aug 29, 2018 at 6:29 AM Takashi Toyoshima <toyo...@chromium.org> wrote:

On Tue, Aug 28, 2018 at 11:38 PM Matt Menke <mme...@chromium.org> wrote:

With the network service disable, content/ will create a bogus NetworkContext that just wraps the URLRequestContext.  Some things will work (Like using the HostResolver APIs, and creating URLLoaderFactories), but other APIs are use at your own risk.  Chrome overrides this behavior, and creates "mostly normal" NetworkContexts that have a couple chrome objects injected (Like a NetworkDelegate, still required for extensions to work, unfortunately)

Thanks. What I really want is finding a place to have a per-profile basis allow/block lists. For legacy code path, we create CORSURLLoaderFactory instances in ResourceMessageFilter, and need to carry an accessor of the lists to the CORSURLLoaderFactory ctor. So, can I have such per-profile lists in the URLRequestContext, and pass an accessor to the ResourceMessageFilter via StoragePartition? Or is there any other better place to have it?

I'm not familiar with CORSURLLoaderFactory in the legacy path, unfortunately.  Both ResourceDispatcherHostDelegate and ChromeNetworkDelegate are still in use in the legacy path, so you could use those...

Or if we could hook up the logic to the NetworkContext::ContextNetworkDelegate in the NetworkService then we could just hook it up there, and the code would affect both the NetworkService enabled and disabled paths, and we wouldn't need to implement two different ways of hooking up the filters.  The lists would affect chrome-internal requests there, too, though, which we may not want.

[Takashi Toyoshima]

On Wed, Aug 29, 2018 at 10:23 PM Matt Menke <mme...@chromium.org> wrote:

On Wed, Aug 29, 2018 at 6:29 AM Takashi Toyoshima <toyo...@chromium.org> wrote:

On Tue, Aug 28, 2018 at 11:38 PM Matt Menke <mme...@chromium.org> wrote:

With the network service disable, content/ will create a bogus NetworkContext that just wraps the URLRequestContext.  Some things will work (Like using the HostResolver APIs, and creating URLLoaderFactories), but other APIs are use at your own risk.  Chrome overrides this behavior, and creates "mostly normal" NetworkContexts that have a couple chrome objects injected (Like a NetworkDelegate, still required for extensions to work, unfortunately)

Thanks. What I really want is finding a place to have a per-profile basis allow/block lists. For legacy code path, we create CORSURLLoaderFactory instances in ResourceMessageFilter, and need to carry an accessor of the lists to the CORSURLLoaderFactory ctor. So, can I have such per-profile lists in the URLRequestContext, and pass an accessor to the ResourceMessageFilter via StoragePartition? Or is there any other better place to have it?

I'm not familiar with CORSURLLoaderFactory in the legacy path, unfortunately.  Both ResourceDispatcherHostDelegate and ChromeNetworkDelegate are still in use in the legacy path, so you could use those...

At this moment, CORSURLLoaderFactory just wraps the existing URLLoaderFactory and uses the URLLoader internally as is to make actual network requests. Everything related to CORS are implemented inside a proxy class CORSURLLoader. So if IIUC, I can not put my code in those Delegate classes you suggested. But, eventually, once we shipped OOR-CORS, we want to move CORSURLLoader implementation into the existing URLLoader and those Delegate classes. But I think it's a next step.

To find another plan, I quickly check existing code. My assumptions are

 - BrowserContext is per-profile (Profile is a subclass of it)

 - BrowserContext has a default StoragePartitionImpl for usual use cases, but another instance will be used for platform apps, or cases for the site isolation.

 - It's safe to provide a raw pointer from the StoragePartitionImpl to the ReosurceMessageFilter

 - They (or related parts especially on BC) run on the I/O thread

And, my proposed possible design is here. It puts the OriginAccessList into the StoragePartition.

[Plumbing for the legacy code path]: https://docs.google.com/document/d/1JNmUcvbw2UcjfdI2uyUpveHXCbae-DQ1n8d_sVs5fLg/edit?pli=1#heading=h.2ziwyagffze0

[Matt Menke]

On Thu, Aug 30, 2018 at 7:04 AM Takashi Toyoshima <toyo...@chromium.org> wrote:

On Wed, Aug 29, 2018 at 10:23 PM Matt Menke <mme...@chromium.org> wrote:

On Wed, Aug 29, 2018 at 6:29 AM Takashi Toyoshima <toyo...@chromium.org> wrote:

On Tue, Aug 28, 2018 at 11:38 PM Matt Menke <mme...@chromium.org> wrote:

With the network service disable, content/ will create a bogus NetworkContext that just wraps the URLRequestContext.  Some things will work (Like using the HostResolver APIs, and creating URLLoaderFactories), but other APIs are use at your own risk.  Chrome overrides this behavior, and creates "mostly normal" NetworkContexts that have a couple chrome objects injected (Like a NetworkDelegate, still required for extensions to work, unfortunately)

Thanks. What I really want is finding a place to have a per-profile basis allow/block lists. For legacy code path, we create CORSURLLoaderFactory instances in ResourceMessageFilter, and need to carry an accessor of the lists to the CORSURLLoaderFactory ctor. So, can I have such per-profile lists in the URLRequestContext, and pass an accessor to the ResourceMessageFilter via StoragePartition? Or is there any other better place to have it?

I'm not familiar with CORSURLLoaderFactory in the legacy path, unfortunately.  Both ResourceDispatcherHostDelegate and ChromeNetworkDelegate are still in use in the legacy path, so you could use those...

At this moment, CORSURLLoaderFactory just wraps the existing URLLoaderFactory and uses the URLLoader internally as is to make actual network requests. Everything related to CORS are implemented inside a proxy class CORSURLLoader. So if IIUC, I can not put my code in those Delegate classes you suggested. But, eventually, once we shipped OOR-CORS, we want to move CORSURLLoader implementation into the existing URLLoader and those Delegate classes. But I think it's a next step.

To find another plan, I quickly check existing code. My assumptions are

 - BrowserContext is per-profile (Profile is a subclass of it)

 - BrowserContext has a default StoragePartitionImpl for usual use cases, but another instance will be used for platform apps, or cases for the site isolation.

 - It's safe to provide a raw pointer from the StoragePartitionImpl to the ReosurceMessageFilter

 - They (or related parts especially on BC) run on the I/O thread 

This is false.  The StoragePartition is on the UI thread.  ResourceMessageFilters are refcounted, so you should make no assumptions about their lifetimes.

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/network-service-dev/CAFWCB1%3DkohD6ABts0caZ_J5%3D73BdyFZMi5oP7YVM_z3F2cA09w%40mail.gmail.com.

[Takashi Toyoshima]

On Thu, Aug 30, 2018 at 10:41 PM Matt Menke <mme...@chromium.org> wrote:

On Thu, Aug 30, 2018 at 7:04 AM Takashi Toyoshima <toyo...@chromium.org> wrote:

On Wed, Aug 29, 2018 at 10:23 PM Matt Menke <mme...@chromium.org> wrote:

On Wed, Aug 29, 2018 at 6:29 AM Takashi Toyoshima <toyo...@chromium.org> wrote:

On Tue, Aug 28, 2018 at 11:38 PM Matt Menke <mme...@chromium.org> wrote:

With the network service disable, content/ will create a bogus NetworkContext that just wraps the URLRequestContext.  Some things will work (Like using the HostResolver APIs, and creating URLLoaderFactories), but other APIs are use at your own risk.  Chrome overrides this behavior, and creates "mostly normal" NetworkContexts that have a couple chrome objects injected (Like a NetworkDelegate, still required for extensions to work, unfortunately)

Thanks. What I really want is finding a place to have a per-profile basis allow/block lists. For legacy code path, we create CORSURLLoaderFactory instances in ResourceMessageFilter, and need to carry an accessor of the lists to the CORSURLLoaderFactory ctor. So, can I have such per-profile lists in the URLRequestContext, and pass an accessor to the ResourceMessageFilter via StoragePartition? Or is there any other better place to have it?

I'm not familiar with CORSURLLoaderFactory in the legacy path, unfortunately.  Both ResourceDispatcherHostDelegate and ChromeNetworkDelegate are still in use in the legacy path, so you could use those...

At this moment, CORSURLLoaderFactory just wraps the existing URLLoaderFactory and uses the URLLoader internally as is to make actual network requests. Everything related to CORS are implemented inside a proxy class CORSURLLoader. So if IIUC, I can not put my code in those Delegate classes you suggested. But, eventually, once we shipped OOR-CORS, we want to move CORSURLLoader implementation into the existing URLLoader and those Delegate classes. But I think it's a next step.

To find another plan, I quickly check existing code. My assumptions are

 - BrowserContext is per-profile (Profile is a subclass of it)

 - BrowserContext has a default StoragePartitionImpl for usual use cases, but another instance will be used for platform apps, or cases for the site isolation.

 - It's safe to provide a raw pointer from the StoragePartitionImpl to the ReosurceMessageFilter

 - They (or related parts especially on BC) run on the I/O thread 

This is false.  The StoragePartition is on the UI thread.  ResourceMessageFilters are refcounted, so you should make no assumptions about their lifetimes.

Thank you for important inputs. Now my design reflects these facts. The list is stored in BrowserContext as a RefCountedThreadSafe, and have thread-hop interfaces for the UI thread so to access the actual list instance only on the IO thread.

I sent a review request: https://chromium-review.googlesource.com/c/chromium/src/+/1196802

Thanks a lot!