Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Can we expose Crypto/SubtleCrypto to all Worklet / all scopes? #338

Open
xyaoinum opened this issue Apr 13, 2023 · 2 comments · May be fixed by #361
Open

Can we expose Crypto/SubtleCrypto to all Worklet / all scopes? #338

xyaoinum opened this issue Apr 13, 2023 · 2 comments · May be fixed by #361

Comments

@xyaoinum
Copy link

xyaoinum commented Apr 13, 2023 •
edited
Loading

Is there any concern exposing Crypto/SubtleCrypto to all scopes, or all Worklet scopes, rather than just (Window, Worker)?

This is a desirable feature for the shared storage worklet (e.g. will make the aggregation key generation process easier).
@xyaoinum xyaoinum changed the title Expose Crypto/SubtleCrypto to all scopes Expose Crypto/SubtleCrypto to SharedStorageWorklet Apr 13, 2023
@xyaoinum xyaoinum changed the title Expose Crypto/SubtleCrypto to SharedStorageWorklet Can we expose Crypto/SubtleCrypto to all Worklet / all scopes? Apr 13, 2023
aarongable pushed a commit to chromium/chromium that referenced this issue Apr 14, 2023
@yaoxiachromium
[shared storage] expose Crypto/TextEncoder/TextDecoder to SharedStora…
0b8bf03 
…geWorklet

It can make things easier inside the worklet (e.g. the aggregation key
generation process), and there should be no security concerns exposing
those (spec already says TextEncoder/Decoder should be exposed to
"*": https://encoding.spec.whatwg.org/#interface-textdecoder. Also created an issue for Crypto:
w3c/webcrypto#338 proposing exposing to
SharedStorageWorklet as well)

Override `ExecutionContext::IsSecureContext()` to always return true:
the shared storage worklet's is always (considered) a secure context
because it originates from a secure Window context. But the existing shared storage worklet architecture is currently populated with a
nullptr SecurityContext, so it's not compatible with the IDL Extended
Attribute `SecureContext`. We need to override it to make it work with
the `SecureContext` attribute.

Bug: 1414951
Change-Id: I940e850a8da42aa8b45950aebe3eb27546ab17ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4423770
Reviewed-by: Adam Rice <ricea@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Alex Turner <alexmt@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1130458}
@twiss
Copy link
Member

twiss commented Apr 19, 2023

Hey 👋 Apologies for the delay. Personally I don't see an issue with this, though it might be good to bring it up in the WebAppSec WG, to see if anyone else sees any concerns. (I can do so at some point, unless you want to do so?)

@annevk
Copy link
Member

annevk commented Apr 25, 2023

A problem with ShadowRealm exposure might be that this feature is conditional upon secure contexts. Perhaps that ought to be considered separately from worklets.

Also, do we have use cases for all worklets or just this one?

cc @Ms2ger

@twiss twiss linked a pull request Jan 23, 2024 that will close this issue
Open
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow using Crypto.getRandomValues() in Shadow Realms Igalia/webcrypto
3 participants
@twiss @annevk @xyaoinum