New issue
Advanced search Search tips

Issue 843780 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Feature



Sign in to add a comment

Implement Feature Policy in dedicated workers

Project Member Reported by odejesush@chromium.org, May 16 2018

Issue description

Enable workers to use Feature Policy to check if features are enabled.
 

Comment 1 by bashi@chromium.org, May 17 2018

Status: Available (was: Untriaged)
Does anyone in your team plan to work on this? It would be helpful for us (worker team) to prioritize this if you could give us more context/clarification.
Yes, I'll provide some context. Currently I'm working on allowing Dedicated and Shared Workers to use the WebUSB API. WebUSB uses Feature Policy to determine whether a frame is allowed to use it or not. If a frame connects to a worker, the same Feature Policy should apply to the worker. As it is right now, if a frame is not allowed to use WebUSB by the Feature Policy, they would be able to access WebUSB anyways using a worker, since the worker is unable to check the Feature Policy. I'll provide a link to the relevant change:
https://chromium-review.googlesource.com/c/chromium/src/+/1045309/11..14/third_party/blink/renderer/modules/webusb/usb.cc.

In this change, in the IsFeatureEnabled() function, I have to skip the check for the worker contexts because it fails the DCHECK in FeaturePolicy::IsFeatureEnabledForOrigin:
https://cs.chromium.org/chromium/src/third_party/blink/common/feature_policy/feature_policy.cc?l=130.

As for if anyone on my team is planning to work on it, probably not. However, if I understood the problem correctly, it might be as simple as calling FeaturePolicy::CreateFromParentPolicy when the worker contexts are created, so perhaps I could attempt to implement it.
Dedicated worker support should be pretty easy to add -- the spec hasn't been written for it, but I've always considered it more-or-less given that we'd do it eventually.

Share workers are going to be more difficult -- Since feature policy works on a frame-by-frame basis, it might be possible for two different frames, with different permissions for USB, to try to connect to the same shared worker. I don't know what should happen in that case. Conceptually, I don't feel like a document in a frame like

<iframe allow="usb 'none'" src="https://some.cross.origin.domain/"></iframe>

should be allowed to open a shared worker, and have that worker use the WebUSB api, simply because the same site is open in another tab. (And be unable to use it in other circumstances), but I'm not sure how to restrict that, and still allow USB in shared workers generally.
Cc: reillyg@chromium.org
I agree. An idea that I have is that perhaps we shouldn't allow a frame with more restrictive policies to connect to a shared worker that was created from a frame with less restrictive policies. It would prevent two frames with opposing policies from connecting to the same shared worker. It wouldn't make much sense to have a worker that interacts with USB devices having a frame that restricts USB by Feature Policy to connect to the worker anyways.
Even without the use of workers isn't it possible for an frame to work around feature policy restrictions by communicating through BroadcastChannel with other frames on the same origin?
BroadcastChannel, I think, has roughly the same security properties as a shared worker; it's just a lower-overhead version that can only do message passing. So yes, in that case, if one frame has access to the information, it can share it with the others if it wishes to.

(The same could be done with an XHR or WebSocket to the origin server, or even localstorage, I expect).

That is, of course, restricted to information that can be sent over postMessage -- that covers a lot of ground, but not everything. You can't, for example, access fullscreen that way, or webvr. You could certainly pass geolocation info around. I'm not sure where WebUSB lies.

I'm also not sure yet what that means for feature policy in shared workers -- we have to accept that they are a channel for information to be shared from any frame that has access to that info. That shouldn't mean that they should grant access to an API even if no frame would otherwise have access.

We may just need to be very careful about how APIs available in shared workers are designed. As long as access to an API or sensor/device has to be initiated from a document, then maybe we don't need to gate access to other parts of the API on feature policy at all, and it becomes irrelevant whether or not it is usable inside of a shared worker.
WebUSB would need the user to grant access to a device using navigator.usb.requestDevice beforehand in the document. That API is not exposed in the supported workers, and USB devices are only retrieved using navigator.usb.getDevices. This API returns a list of devices that the origin already has permission to access.
I agree with iclelland@. We can assign to the Shared Worker the Feature Policy of the frame that launched it.
To implement this, my approach at the moment is to add the parent feature policy to the GlobalScopeCreationParams that is used by the workers when initializing their respective GlobalScope. For Dedicated Workers, the params are initialized in DedicatedWorker::CreateGlobalScopeCreationParams(), so the parent feature policy can be extracted here. In WorkerGlobalScope constructor, I can check if the feature policy is not null before using the InitializeFeaturePolicy function that's inherited from SecurityContext. For the container policy parameter, this could probably be an empty vector since the worker would inherit the policies directly from the context that started it.

What are your thoughts on this approach for Dedicated Workers?
I think that this will work for now. Worker scripts are always going to be same-origin with their opener, so there shouldn't be a difference between the opener and the worker.

In the future, we'll want to have the ability for a worker script to be delivered with a Feature-Policy header of its own, and possibly for the container policy to be overridden by the owner document. (All of that will have to be written into the spec first)
Owner: odejesush@chromium.org
Status: Started (was: Available)
(Worker bug triage) Looks like odejesush@ has a CL related to this issue:
https://chromium-review.googlesource.com/c/chromium/src/+/1091635
Project Member

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

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

commit cdf20cb9110eafe4e03df305d03fe04b2469a021
Author: Ovidio Henriquez <odejesush@chromium.org>
Date: Mon Jun 18 19:25:31 2018

Enable Dedicated Workers to inherit Feature Policy

This change enables Dedicated Workers to inherit the Feature Policy of
the context that created the worker.

Bug:  843780 
Change-Id: I2afaf7ee8547853a2f1c639961c78260efbd9633
Reviewed-on: https://chromium-review.googlesource.com/1091635
Commit-Queue: Ovidio Henriquez <odejesush@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Chong Zhang <chongz@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568102}
[modify] https://crrev.com/cdf20cb9110eafe4e03df305d03fe04b2469a021/third_party/blink/renderer/core/execution_context/security_context.cc
[modify] https://crrev.com/cdf20cb9110eafe4e03df305d03fe04b2469a021/third_party/blink/renderer/core/execution_context/security_context.h
[modify] https://crrev.com/cdf20cb9110eafe4e03df305d03fe04b2469a021/third_party/blink/renderer/core/workers/dedicated_worker.cc
[modify] https://crrev.com/cdf20cb9110eafe4e03df305d03fe04b2469a021/third_party/blink/renderer/core/workers/global_scope_creation_params.cc
[modify] https://crrev.com/cdf20cb9110eafe4e03df305d03fe04b2469a021/third_party/blink/renderer/core/workers/global_scope_creation_params.h
[modify] https://crrev.com/cdf20cb9110eafe4e03df305d03fe04b2469a021/third_party/blink/renderer/core/workers/worker_global_scope.cc
[modify] https://crrev.com/cdf20cb9110eafe4e03df305d03fe04b2469a021/third_party/blink/renderer/modules/webusb/usb.cc

Blocking: -837406
Owner: ----
Status: Available (was: Started)
I will set this back to available, since I don't plan to work on Feature Policy in Shared Workers.
Labels: M-69 Target-69
Owner: odejesush@chromium.org
Status: Fixed (was: Available)
Summary: Implement Feature Policy in dedicated workers (was: Implement Feature Policy in Web Workers.)
Let's narrow down the scope of this issue only to dedicated workers. I filed a separate issue for shared workers (issue 858918)

Sign in to add a comment