Support workers in privacy algos #238

kenchris · 2023-10-10T10:55:25Z

Split up existing algos and add substeps to get owner document for dedicated workers.
Add steps for dedicated workers and shared workers

Preview | Diff

index.html

rakuco · 2023-10-13T12:50:56Z

index.html

+                        Assert: |owner| is {{Document}}.
+                      </li>
+                      <li>
+                        If the result of running [=passes window privacy test=] with |observer| and |owner|'s


This algorithm is returning true if "passes window privacy test" is true for any owner, so I'd like to confirm if this is indeed the intention.

This means that if I have, for instance:

Window W1 showing https://foo.com/a.html that spawns a shared worker

Window W2 showing https://bar.com/b.html which has an iframe containing https://foo.com/a.html which uses the same shared worker spawned above.

I have window W1 open and showing in my screen.

I have window W2 open and next to W1, but the focus is on the top-level frame, which is not same origin with the iframe.

This means that the iframe in W2 can potentially have access to updates and readings from the shared worker that would not be available if it had been using the API directly rather than via a shared worker.

I don't have a lot of experience with shared workers to know if this is common or OK, but it'd be good to mention this in https://w3c.github.io/compute-pressure/#security-and-privacy-considerations (perhaps in https://w3c.github.io/compute-pressure/#same-origin-restriction but maybe elsewhere).

I think that is OK. We want to enable usage in iframes and that additionally depends on the embedder allowing the feature policy for the frame. The only way the W2 would have access is if it collaborates with the iframe to get that info

This brings up an interesting topic in that the permissions policy API does not apply to workers at the moment IIRC, so this means that not-same origin iframe can use the Compute Pressure API in a worker/shared worker but would not be able to do so if it was using it directly from its code, right?

Also, my concern wasn't about W2 specifically but rather if it's OK that the iframe in W2 would receive updates from its worker.

Yes, I think it is OK. It was one of the core use-cases we had from Zoom: it's going to be used from iframes and they want to share the worker across instances.

Also we are not showing a permissions prompt for the feature which is what Google wanted. Users won't understand what it means to use computer pressure

But yeah, it would also be weird if the shared worker cannot get data because one embedder didn't allow it.

I really don't know what to do in this situation.

I guess we could check them all and the app code would have to make sure to check policy before deciding to use a shared worker, and then document that in an example

Isn't it dangerous if it's up to the app as well?

My concern here is that apps could work around the privacy checks being defined for windows -- that is, if a page uses the API directly and the privacy checks would fail under certain circumstances, all it takes is using the API from a shared worker and having another tab/window loading the same shared worker and passing the privacy checks for the first page to bypass the mitigations.

Yeah so let's require that all in 'owner set' have appropriate permissions policy

wangw-1991 · 2023-10-16T07:52:48Z

Looks good to me. The discussion about workers are intersting.

index.html

rakuco

This new version looks nice.

I think I've managed to resolve all the discussion points that have been addressed, and the only one that's left is the choice of whether one document passing the "may receive data" check being enough for a worker or not. In the comments you mentioned wanting to change this, but the PR does the opposite, so it's not clear to me what the approach is going to be.

kenchris · 2023-10-16T17:01:15Z

I think there is a difference between the implicit focus check for data delivery or whether a worker should be allowed access at all via permission policy. I think we can make the permission policy check separately from this PR

rakuco · 2023-10-17T13:35:50Z

I think we can make the permission policy check separately from this PR

Doesn't this (i.e. doing a permissions policy check or something else entirely separately) leave the privacy checks in windows open to being bypassed?

kenchris · 2023-10-17T13:42:06Z

Permission policy checks (set by site and embedder and rarely changed at runtimw) would normally be checked at start of observation, and unless we want to adjust at runtime, this should be fine. We could always stop observation if these are changed at runtime, but probably not worth the effort

rakuco · 2023-10-17T15:46:02Z

Yeah, I usually see those checks when an object is constructed or a resource will be acquired.

Before lgtm'ing this PR, I just want to make sure that it's not a problem that a window which would normally fail to receive new data because of a failed "may receive data" check can receive said data given the conditions described in #238 (comment).

kenchris · 2023-10-17T17:47:46Z

I don't consider that a problem, but see it as implicit focus. There is an iframe with implicit focus relaying that to the shared worker

rakuco · 2023-10-17T18:22:36Z

OK then. Let's go!

kenchris · 2023-10-18T11:26:47Z

@himorin build is failing as https://respec.org/w3c/groups/ isn't loading!

himorin · 2023-10-18T14:06:57Z

@himorin build is failing as https://respec.org/w3c/groups/ isn't loading!

some other colleagues has reported on that, and someone in W3C is contacting them. so, please wait a bit for further update.

Privacy test is done in Blink currently. However, we can not get enough information in Blink to implement the improved privacy test. This CL moves privacy test from Blink to content/browser. This CL also deletes privacy test related WPTs, because we use MockPressureService in WPTs so these WPTs are no longer valid. TODO: add privacy test related unittests in content/browser. This CL also renames PassesPrivacyTest() to HasImplicitFocus() according to the spec change [1]. [1] w3c/compute-pressure#238 Bug: none Change-Id: I8c087951f5b804235a2dde4fa4e315159f400cc7

Privacy test is done in Blink currently. However, we can not get enough information in Blink to implement the improved privacy test. This CL moves privacy test from Blink to content/browser. This CL also deletes privacy test related WPTs and adds corresponding unit tests, because we use MockPressureService in WPTs so these WPTs are no longer valid. This CL also renames PassesPrivacyTest() to HasImplicitFocus() according to the spec change [1]. [1] w3c/compute-pressure#238 Bug: none Change-Id: I8c087951f5b804235a2dde4fa4e315159f400cc7

Privacy test is done in Blink currently. However, we can not get enough information in Blink to implement the improved privacy test [1]. This CL moves privacy test from Blink to content/browser. This CL also deletes privacy test related WPTs and adds corresponding unit tests, because we use MockPressureService in WPTs so these WPTs are no longer valid. This CL also renames PassesPrivacyTest() to HasImplicitFocus() according to the spec change [1]. [1] w3c/compute-pressure#238 Bug: 1500467, 1425053 Change-Id: I8c087951f5b804235a2dde4fa4e315159f400cc7

Privacy test is done in Blink currently. However, we can not get enough information in Blink to implement the improved privacy test [1]. This CL moves privacy test from Blink to content/browser. This CL also deletes privacy test related WPTs and adds corresponding browser tests, because we use MockPressureService in WPTs so these WPTs are no longer valid. This CL also renames PassesPrivacyTest() to HasImplicitFocus() according to the spec change [1]. [1] w3c/compute-pressure#238 Bug: 1500467, 1425053 Change-Id: I8c087951f5b804235a2dde4fa4e315159f400cc7

Privacy test is done in Blink currently. However, we can not get enough information in Blink to implement the improved privacy test [1]. This CL moves privacy test from Blink to content/browser. This CL also deletes privacy test related WPTs and adds corresponding browser tests, because we use MockPressureService in WPTs so these WPTs are no longer valid. This CL also renames PassesPrivacyTest() to HasImplicitFocus() according to the spec change [1]. [1] w3c/compute-pressure#238 Bug: 1500467, 1396177 Change-Id: I8c087951f5b804235a2dde4fa4e315159f400cc7

Privacy test is done in Blink currently. However, we can not get enough information in Blink to implement the improved privacy test [1]. This CL moves privacy test from Blink to content/browser. This CL also deletes privacy test related WPTs and adds corresponding browser tests, because we use MockPressureService in WPTs so these WPTs are no longer valid. This CL also renames PassesPrivacyTest() to HasImplicitFocus() according to the spec change [1]. [1] w3c/compute-pressure#238 Bug: 1500467, 1396177 Change-Id: I8c087951f5b804235a2dde4fa4e315159f400cc7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4956687 Commit-Queue: Wei4 Wang <wei4.wang@intel.com> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1237389}

…m Blink to content/browser., a=testonly Automatic update from web-platform-tests [ComputePressure] Move privacy tests from Blink to content/browser. Privacy test is done in Blink currently. However, we can not get enough information in Blink to implement the improved privacy test [1]. This CL moves privacy test from Blink to content/browser. This CL also deletes privacy test related WPTs and adds corresponding browser tests, because we use MockPressureService in WPTs so these WPTs are no longer valid. This CL also renames PassesPrivacyTest() to HasImplicitFocus() according to the spec change [1]. [1] w3c/compute-pressure#238 Bug: 1500467, 1396177 Change-Id: I8c087951f5b804235a2dde4fa4e315159f400cc7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4956687 Commit-Queue: Wei4 Wang <wei4.wang@intel.com> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1237389} -- wpt-commits: 985dd254555907cabf68224a7db1d5a62e4b989e wpt-pr: 42653

…m Blink to content/browser., a=testonly Automatic update from web-platform-tests [ComputePressure] Move privacy tests from Blink to content/browser. Privacy test is done in Blink currently. However, we can not get enough information in Blink to implement the improved privacy test [1]. This CL moves privacy test from Blink to content/browser. This CL also deletes privacy test related WPTs and adds corresponding browser tests, because we use MockPressureService in WPTs so these WPTs are no longer valid. This CL also renames PassesPrivacyTest() to HasImplicitFocus() according to the spec change [1]. [1] w3c/compute-pressure#238 Bug: 1500467, 1396177 Change-Id: I8c087951f5b804235a2dde4fa4e315159f400cc7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4956687 Commit-Queue: Wei4 Wang <wei4.wangintel.com> Reviewed-by: Reilly Grant <reillygchromium.org> Cr-Commit-Position: refs/heads/main{#1237389} -- wpt-commits: 985dd254555907cabf68224a7db1d5a62e4b989e wpt-pr: 42653 UltraBlame original commit: 182628bdd37a61f67c7575788276650441985494

…m Blink to content/browser., a=testonly Automatic update from web-platform-tests [ComputePressure] Move privacy tests from Blink to content/browser. Privacy test is done in Blink currently. However, we can not get enough information in Blink to implement the improved privacy test [1]. This CL moves privacy test from Blink to content/browser. This CL also deletes privacy test related WPTs and adds corresponding browser tests, because we use MockPressureService in WPTs so these WPTs are no longer valid. This CL also renames PassesPrivacyTest() to HasImplicitFocus() according to the spec change [1]. [1] w3c/compute-pressure#238 Bug: 1500467, 1396177 Change-Id: I8c087951f5b804235a2dde4fa4e315159f400cc7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4956687 Commit-Queue: Wei4 Wang <wei4.wang@intel.com> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1237389} -- wpt-commits: 985dd254555907cabf68224a7db1d5a62e4b989e wpt-pr: 42653

kenchris force-pushed the main branch from 2fddced to 87f360c Compare October 10, 2023 11:08

Support workers in privacy algos

aac2c45

kenchris force-pushed the main branch from 87f360c to aac2c45 Compare October 10, 2023 11:13

rakuco reviewed Oct 13, 2023

View reviewed changes

Fix comments from Raphael

bf532c0

Fix: dedicated workers can be created from shared ones

add2e53

arskama reviewed Oct 16, 2023

View reviewed changes

index.html Outdated Show resolved Hide resolved

Fix typo

033bf28

rakuco reviewed Oct 16, 2023

View reviewed changes

rakuco approved these changes Oct 17, 2023

View reviewed changes

kenchris merged commit 3aa342e into w3c:main Oct 18, 2023

chromium-wpt-export-bot mentioned this pull request Oct 20, 2023

[ComputePressure] Move privacy tests from Blink to content/browser. web-platform-tests/wpt#42653

Merged

arskama mentioned this pull request Oct 27, 2023

Same-origin check for workers #215

Closed

Support workers in privacy algos #238

Support workers in privacy algos #238

Uh oh!

Conversation

kenchris commented Oct 10, 2023 • edited by pr-preview bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rakuco Oct 13, 2023

Choose a reason for hiding this comment

Uh oh!

kenchris Oct 13, 2023

Choose a reason for hiding this comment

Uh oh!

rakuco Oct 13, 2023

Choose a reason for hiding this comment

Uh oh!

kenchris Oct 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kenchris Oct 13, 2023

Choose a reason for hiding this comment

Uh oh!

kenchris Oct 13, 2023

Choose a reason for hiding this comment

Uh oh!

rakuco Oct 13, 2023

Choose a reason for hiding this comment

Uh oh!

kenchris Oct 13, 2023

Choose a reason for hiding this comment

Uh oh!

wangw-1991 commented Oct 16, 2023

Uh oh!

Uh oh!

rakuco left a comment

Choose a reason for hiding this comment

Uh oh!

kenchris commented Oct 16, 2023

Uh oh!

rakuco commented Oct 17, 2023

Uh oh!

kenchris commented Oct 17, 2023

Uh oh!

rakuco commented Oct 17, 2023

Uh oh!

kenchris commented Oct 17, 2023

Uh oh!

rakuco commented Oct 17, 2023

Uh oh!

kenchris commented Oct 18, 2023

Uh oh!

himorin commented Oct 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kenchris commented Oct 10, 2023 •

edited by pr-preview bot

Loading

kenchris Oct 13, 2023 •

edited

Loading