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.

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

Synchronous hasStorageAccess? #146

Closed
johannhof opened this issue Dec 15, 2022 · 4 comments
Closed

Synchronous hasStorageAccess? #146

johannhof opened this issue Dec 15, 2022 · 4 comments
Labels
resolve before graduation These issues need to be resolved before the spec graduates from the CG

Comments

@johannhof
Copy link
Member

Given the changes in #138 and #141 it seems like we can access a static flag on the window/environment to determine storage access. If I'm not mistaken this would enable a sync hasStorageAccess call without the promise. If this is useful for developers we could consider adding a new attribute and deprecating regular Promise-based hSA?

@annevk annevk added the resolve before graduation These issues need to be resolved before the spec graduates from the CG label Dec 15, 2022
@bvandersloot-mozilla
Copy link
Collaborator

I like making this sync.

@cfredric
Copy link
Contributor

I think I'm supportive of making it sync.

My gut was to argue against it, since the environment's has storage access bool seems like a security-sensitive thing that Chromium would not want to store in a renderer (since we don't trust those processes). However, if we did store it in the renderer, a compromised renderer would only be able to modify the has storage access bool and shouldn't be able to forge a permission for the storage-access feature. So as long as the new parts of Fetch/Cookies (in #145) follow the determine the storage access policy algorithm (as written) and check the permission state (in addition to the has storage access bool), then a compromised renderer would gain access to unpartitioned cookies iff the user had already granted permission for that <top level, embed> pair. And in that case, the renderer could have gained access more easily by calling doc.requestStorageAccess() anyway, regardless of whether the bool is trusted or untrusted. So storing it somewhere other than the renderer process (where it's easy to implement a sync hasStorageAccess) doesn't actually accomplish anything for security.

So, I'm not seeing a reason not to support sync hSA.

@annevk
Copy link
Collaborator

annevk commented Dec 20, 2022

For a moment I thought this would lead to badness across processes, but hasStorageAccess() only gives you information about the current document (i.e., did requestStorageAccess() succeed). The permission is the one that travels across processes.

Still, I'm not sure we can change this API fundamentally. Developers presumably already rely on it. If they want something synchronous they can use a variable instead...

Options we do have:

  1. Change the task queueing. Resolving the promise directly ought to be compatible. However, we want to be sure this doesn't create issues for the future with buckets. Cannot immediately think of any issues though so this is probably okay.
  2. We could also try something fancier, where it waits for requestStorageAccess() to complete before resolving, if that was already invoked. Given that you can already build this yourself this is probably not that useful, but I wanted to mention it anyway.

@johannhof
Copy link
Member Author

I think sync hSA is off the table given the direction where going in #171

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolve before graduation These issues need to be resolved before the spec graduates from the CG
Projects
None yet
Development

No branches or pull requests

4 participants