Control blink features from the browser

[Jiewei Qian]

Hi,

We are prototyping Chrome OS System Extensions (SE). SE is built on top of blink to provide tighter integration with Chrome OS.

I have a question about letting the browser conditionally enable blink features.

Specifically, we have some blink features implemented in blink/renderer/extensions. We want to let the browser process conditionally enable a subset of them, based on the type of SWX being rendered (i.e. when committing a document load, or when starting a service worker). For example, only expose FooAPI (but not BarAPI) WebIDL bindings for Foo Manager extension type.

I want to know if it makes sense to make runtime_enabled_features configurable by the browser process per rendering context: Perhaps blink/public can expose a enum list of runtime_enabled_features, the embedder then passes a list of enums to enable features.

Alternatively, we can use ContextEnabled, or RuntimeEnabled with origin trials. 

All of the options above require blink/{platform, core} code to refer to things in blink/renderer/extensions (e.g. runtime_enabled_features.json5 contains a ChromeOSExtensionFooAPI entry). They won’t need link time dependency. I want to know if this is acceptable because platform and core aren’t supposed to depend on extensions.

What's the right way forward?

Cheers,

Jiewei

[Kentaro Hara]

I want to know if it makes sense to make runtime_enabled_features configurable by the browser process per rendering context: Perhaps blink/public can expose a enum list of runtime_enabled_features, the embedder then passes a list of enums to enable features.

Can you expose EnableXXX() methods to blink/public/platform/web_runtime_features.h just like other features are doing?

All of the options above require blink/{platform, core} code to refer to things in blink/renderer/extensions (e.g. runtime_enabled_features.json5 contains a ChromeOSExtensionFooAPI entry). They won’t need link time dependency. I want to know if this is acceptable because platform and core aren’t supposed to depend on extensions.

Would you help me understand why blink/{platform,core} needs to depend on blink/renderer/extensions/?

For example, accessibility/ is implemented in blink/renderer/modules/ but EnableAccessibilityObjectModel() is working without violating the layering principle.

--

Kentaro Hara, Tokyo

[Jiewei Qian]

Replied inline.

On Tue, Sep 28, 2021 at 6:01 PM Kentaro Hara <har...@google.com> wrote:

I want to know if it makes sense to make runtime_enabled_features configurable by the browser process per rendering context: Perhaps blink/public can expose a enum list of runtime_enabled_features, the embedder then passes a list of enums to enable features.

Can you expose EnableXXX() methods to blink/public/platform/web_runtime_features.h just like other features are doing?

I'm not sure if WebRuntimeFeatures applies per document. Will it reset if the document navigates away or if the renderer process is reused?

I think it's technically similar to ContextEnabled. But adding one EnableXXX() per feature isn't very scalable to me. We'll need to add corresponding Mojo calls in content/ and chrome/ for each feature. In our prototype, a given System Extension needs N blink features. Sending those N IPC calls from the browser to blink isn't ideal.

 

All of the options above require blink/{platform, core} code to refer to things in blink/renderer/extensions (e.g. runtime_enabled_features.json5 contains a ChromeOSExtensionFooAPI entry). They won’t need link time dependency. I want to know if this is acceptable because platform and core aren’t supposed to depend on extensions.

Would you help me understand why blink/{platform,core} needs to depend on blink/renderer/extensions/?

For example, accessibility/ is implemented in blink/renderer/modules/ but EnableAccessibilityObjectModel() is working without violating the layering principle.

My concern is putting code that refers to (has knowledge of) blink/renderer/extension feature names in blink/renderer/{platform, core}.

For example,

In blink/public: web_runtime_features.h

EnableChromeOSFooAPI();

EnableChromeOSBarAPI();

Here ChromeOS**API refers to blink/renderer/extensions concepts.

[Kentaro Hara]

I'm not sure if WebRuntimeFeatures applies per document. Will it reset if the document navigates away or if the renderer process is reused?

Ah, WebRuntimeFeatures is per process. If you need a feature switch per document, ContextEnabled will be the right choice.

But adding one EnableXXX() per feature isn't very scalable to me. We'll need to add corresponding Mojo calls in content/ and chrome/ for each feature. In our prototype, a given System Extension needs N blink features. Sending those N IPC calls from the browser to blink isn't ideal.

How many features do you need to control independently? If N features are always enabled at the same time, you can add one IPC to enable them.

My concern is putting code that refers to (has knowledge of) blink/renderer/extension feature names in blink/renderer/{platform, core}.

For example,
In blink/public: web_runtime_features.h
EnableChromeOSFooAPI(); 

EnableChromeOSBarAPI(); 

 

Here ChromeOS**API refers to blink/renderer/extensions concepts.

Yeah, this is not nice but it's not a new problem. web_runtime_features.h already has many names of core / modules concepts. I'm okay with adding EnableChromeOSFooAPI() for now and revisiting it in the future :)

--

Kentaro Hara, Tokyo

[Dave Tapuska]

This sounds similar to the bindings policy that is enabled for mojo js, webui, etc.

Is this only different in that you are implementing it inside blink? Perhaps the bindings policy approach could be adjusted so that it takes enablement inside blink into account. Then perhaps things like the DOMAutomationController could be as extensions inside blink for onion soup.

dave.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jyG3beB7OfPYbjR7OxfmoXiL%3DXeeOeJV9rwKsmh_BaX9Q%40mail.gmail.com.

[Nasko Oskov]

On Tue, 28 Sept 2021 at 07:45, Dave Tapuska <dtap...@chromium.org> wrote:

This sounds similar to the bindings policy that is enabled for mojo js, webui, etc.

We (security team) would like to move away from bindings policy and have the browser process communicate this information at commit time. It is just a low priority item that is constantly starved for time. If we don't pile up more usage of this, it will help us in the long run avoiding to migrate more code. Is it possible to use the RenderFrameObserver's ReadyToCommitNavigation interface to enable/disable these APIs?

 

Is this only different in that you are implementing it inside blink? Perhaps the bindings policy approach could be adjusted so that it takes enablement inside blink into account. Then perhaps things like the DOMAutomationController could be as extensions inside blink for onion soup.1

dave.

On Tue, Sep 28, 2021 at 10:32 AM 'Kentaro Hara' via platform-architecture-dev <platform-arc...@chromium.org> wrote:

I'm not sure if WebRuntimeFeatures applies per document. Will it reset if the document navigates away or if the renderer process is reused?

Ah, WebRuntimeFeatures is per process. If you need a feature switch per document, ContextEnabled will be the right choice.

Will different system extensions reuse the same renderer process? I would expect no and I would expect each system extension to get its own process. If that's the case, I wonder if it makes a difference whether it is per-document or per-process.

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAHgVhZVp1%2Bkv3%3DYqcsX1WQ6fcape2k0WcKrJxSL_HDEyME627A%40mail.gmail.com.

[Jiewei Qian]

Ah, WebRuntimeFeatures is per process. If you need a feature switch per document, ContextEnabled will be the right choice.

I see, thanks for clarification.

How many features do you need to control independently? If N features are always enabled at the same time, you can add one IPC to enable them.

We aren't sure about this at the moment. I'd expect we'll need at least one EnableXXXApi for each System Extension type. My thought is to decouple extension type with blink features, in case we need to combine different features for a new extension type. For example, FooType uses FooApi1 and FooApi2. BarType uses BarApi1 and BarApi2. And we want to introduce a FooBarType which uses FooApi1 and BarApi2.

Currently, we have two prototype extension types. Their type -> feature is 1:1. So Enable{Foo,Bar}Api works. But I expect the need for fine-grain control in the future.

 

Is it possible to use the RenderFrameObserver's ReadyToCommitNavigation interface to enable/disable these APIs?

That's my current plan.

We already do similar things with EnableMojoJsBindingWithBroker:

SystemExtensionsFrameObserver::ReadyToCommitNavigation():

-> RenderFrameHost::EnableXYZApi()

-> Mojo IPC 

-> RenderFrameImpl::EnableXYZApi() { enable_xyz_api_ = true }

RenderFrameImpl::DidCreateScriptContext():

-> WebV8Features::EnableXYZApi()

Alternatively, we can use blink.mojom.CommitNavigationParams, like ForceEnableOriginTrials.

Will different system extensions reuse the same renderer process? I would expect no and I would expect each system extension to get its own process. If that's the case, I wonder if it makes a difference whether it is per-document or per-process.

Each system extension runs in its own process. 

Whether it makes a difference whether it is per-document or per-process.

 

Good point. But I don't know enough of the renderer process internals to answer this with certainty.

Though conceptually, I think a per-document feature control makes more sense. For example, an extension's foreground page (an app launched by users) may want different features, compared with its background page or service worker.

[Kentaro Hara]

We aren't sure about this at the moment. I'd expect we'll need at least one EnableXXXApi for each System Extension type. My thought is to decouple extension type with blink features, in case we need to combine different features for a new extension type. For example, FooType uses FooApi1 and FooApi2. BarType uses BarApi1 and BarApi2. And we want to introduce a FooBarType which uses FooApi1 and BarApi2.
Currently, we have two prototype extension types. Their type -> feature is 1:1. So Enable{Foo,Bar}Api works. But I expect the need for fine-grain control in the future.

I'd prefer going with this simple 1:1 mapping until the necessity to introduce a complex mapping arises :)

--

Kentaro Hara, Tokyo

[Nasko Oskov]

On Tue, 28 Sept 2021 at 21:24, Kentaro Hara <har...@google.com> wrote:

We aren't sure about this at the moment. I'd expect we'll need at least one EnableXXXApi for each System Extension type. My thought is to decouple extension type with blink features, in case we need to combine different features for a new extension type. For example, FooType uses FooApi1 and FooApi2. BarType uses BarApi1 and BarApi2. And we want to introduce a FooBarType which uses FooApi1 and BarApi2.
Currently, we have two prototype extension types. Their type -> feature is 1:1. So Enable{Foo,Bar}Api works. But I expect the need for fine-grain control in the future.

I'd prefer going with this simple 1:1 mapping until the necessity to introduce a complex mapping arises :)

On Wed, Sep 29, 2021 at 12:49 PM Jiewei Qian <q...@google.com> wrote:

Ah, WebRuntimeFeatures is per process. If you need a feature switch per document, ContextEnabled will be the right choice.

I see, thanks for clarification.

How many features do you need to control independently? If N features are always enabled at the same time, you can add one IPC to enable them.

We aren't sure about this at the moment. I'd expect we'll need at least one EnableXXXApi for each System Extension type. My thought is to decouple extension type with blink features, in case we need to combine different features for a new extension type. For example, FooType uses FooApi1 and FooApi2. BarType uses BarApi1 and BarApi2. And we want to introduce a FooBarType which uses FooApi1 and BarApi2.

Currently, we have two prototype extension types. Their type -> feature is 1:1. So Enable{Foo,Bar}Api works. But I expect the need for fine-grain control in the future.

 

Is it possible to use the RenderFrameObserver's ReadyToCommitNavigation interface to enable/disable these APIs?

That's my current plan.

We already do similar things with EnableMojoJsBindingWithBroker:

SystemExtensionsFrameObserver::ReadyToCommitNavigation():

-> RenderFrameHost::EnableXYZApi()

-> Mojo IPC 

-> RenderFrameImpl::EnableXYZApi() { enable_xyz_api_ = true }

I meant actually doing the observation on the renderer side instead of the browser side. This approach also works, but it seems strange to me to be adding state/APIs to RenderFrame* that are not part of the web platform. In general, we implement features on top of //content/.

 

RenderFrameImpl::DidCreateScriptContext():

-> WebV8Features::EnableXYZApi()

Alternatively, we can use blink.mojom.CommitNavigationParams, like ForceEnableOriginTrials.

 

I wouldn't expect navigation to know about CrOS specific APIs. It is a layering violation, isn't it?

Will different system extensions reuse the same renderer process? I would expect no and I would expect each system extension to get its own process. If that's the case, I wonder if it makes a difference whether it is per-document or per-process.

Each system extension runs in its own process. 

Whether it makes a difference whether it is per-document or per-process.

 

Good point. But I don't know enough of the renderer process internals to answer this with certainty.

Though conceptually, I think a per-document feature control makes more sense. For example, an extension's foreground page (an app launched by users) may want different features, compared with its background page or service worker.

 

In general, reasoning about security should be based on what privileges are given to what principals. We then map these principals to processes, as that is the granularity of which we can have security guarantees.

From security perspective, if they run under the same security principal, they can get all the APIs that security principal is allowed. How would you deny an API in the foreground page and allow it in the ServiceWorker?

[Kentaro Hara]

I meant actually doing the observation on the renderer side instead of the browser side. This approach also works, but it seems strange to me to be adding state/APIs to RenderFrame* that are not part of the web platform. In general, we implement features on top of //content/. 

 

I wouldn't expect navigation to know about CrOS specific APIs. It is a layering violation, isn't it?

+1.

The renderer should know that it is creating a frame for the Chrome OS extension (because the browser told it to the renderer at some earlier point), right? Then the renderer can control what features should be exposed to the frame.

From security perspective, if they run under the same security principal, they can get all the APIs that security principal is allowed. How would you deny an API in the foreground page and allow it in the ServiceWorker?

Good point :)

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAA%3DmyAtaNR%2By8ePa0VA-KKpwRqQNZWRTd43MGFC15Ls7pdZSeg%40mail.gmail.com.

--

Kentaro Hara, Tokyo

[Jiewei Qian]

Thanks for the comments.

 

I meant actually doing the observation on the renderer side instead of the browser side. This approach also works, but it seems strange to me to be adding state/APIs to RenderFrame* that are not part of the web platform. In general, we implement features on top of //content/. 

 

I agree that CrOS extensions shouldn't be mentioned in content/. But we'll still need content/ to pass information to blink (I don't think chrome/browser can talk to blink directly). 

I think content/ doesn't need to understand the information, passing it as-is to blink/ should be sufficient. 

As for observing on renderer-side: I'm not sure how that would work. Wouldn't this require the renderer to know about SE concepts (like its type)? If we put logic in content/renderer, wouldn't this be a layering violation as well?

 

 I wouldn't expect navigation to know about CrOS specific APIs. It is a layering violation, isn't it?

 

In the case of ForceEnabledOriginTrial, chrome/browser pass a Vector<String>, content/ essentially forwards it to blink, Then it's up to blink to interpret the strings and enable features.

 

From security perspective, if they run under the same security principal, they can get all the APIs that security principal is allowed.

 

I see. 

How about letting /chrome/browser set up command line arguments when we start a renderer process for SE? 

chrome/browser/system_extensions_observer.cc:

WillCreateRendererProcess(CommandLine* cmd_line) {

    cmd_line->AppendSwitches("enable-blink-features", "XYZ")

}

I'll try to piece together a short doc listing the options we explored so far next week.

 

[Jiewei Qian]

Hi all,

Here's a doc and our proposal, please take a look and let me know what you think.

https://docs.google.com/document/d/153Ic-e5p8oeevLysCpVUMDlmcbg81w3rfc3QVIH9KHc/edit?usp=sharing&resourcekey=0-5Qrvfab_X74WHHbcgLzVMw

In short, we propose to use blink runtime enabled features, and pass a list of feature names when service worker starts.

[Kentaro Hara]

The proposal LGTM :)

--

Kentaro Hara, Tokyo