Skip to content

Network policies should be enforced asynchronously #161

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

Closed
sicking opened this issue Nov 13, 2015 · 5 comments
Closed

Network policies should be enforced asynchronously #161

sicking opened this issue Nov 13, 2015 · 5 comments

Comments

@sicking
Copy link

sicking commented Nov 13, 2015

We should make all network policies be enforced asynchronously. Including things like CSP, mixed-content-blocking and same-origin checks.

So for example

x = new XMLHttpRequest();
x.open("GET", uri);
x.send();

should not throw an exception from neither the .open call, nor the .send call even if uri violates CSP policies for the page. Instead we should treat the error as a network error.

There's a few reasons for this.

  • Authors have to deal with network requests failing asynchronously anyway. By also throwing exceptions we require authors to handle both synchronous and asynchronous errors.
  • Only some policies can be enforced synchronously. For example enforcing that a redirect follows same-origin policies can only be done after a network request, and so obviously has to be asynchronous. It would be better if all CSP policies are reported the same way, rather than have some reported synchronously and some asynchronously.
  • At least Gecko's network implementation is currently main-thread-only. Including essentially all of our uri handling. This means that we have to do things like CSP checks on the main thread. That makes it problematic to make these APIs throw on workers. It's certainly not impossible, but it adds significant performance and complexity overhead.
  • I'd like to move much more of our network security policies into our network stack. If these checks are asynchronous, it means that we can keep all communication with the network stack asynchronous and can report things like CSP errors and network errors through the same code paths. I.e. we can keep a simpler implementation which means fewer bugs. This is especially important since error conditions are notoriously poorly tested, and so having fewer error reporting paths is especially beneficial.

One example where synchronous errors are required is step 5 in:
https://html.spec.whatwg.org/multipage/workers.html#dom-worker
One problem there might simply be that workers need to integrate with the fetch spec.

@annevk
Copy link
Member

annevk commented Nov 13, 2015

This is already the case for XMLHttpRequest. Workers do indeed enforce some stuff early on that we could remove there easily if implementations are on board (although the data URL restriction is not a hook Fetch exposes). If this is about workers, filing an issue against whatwg/html would be more appropriate.

@sicking
Copy link
Author

sicking commented Nov 13, 2015

I guess the question is, is this true for the entire fetch algorithm? And does fetch enable enforcing same-origin? And would simply changing workers to use fetch and not duplicate any of the security checks that fetch does cause it to not do synchronous checks?

Regarding data, blink/webkit already don't do what the workers spec says. And I've not heard any plans that they will change.

My recommendation has for a long time been to make fetch have an argument for "data: inherits" vs. "data: creates a unique origin". Webkit and blink use the latter policy for <iframe>s. It's something I wished that gecko did too. And the former policy is obviously needed for things like <img> and <script>.

If you then combine the "data: creates a unique origin" with the "require same origin" policy that workers use, you get the behavior that webkit/blink has for workers. And the policy that I'd like to make gecko use for workers.

@annevk
Copy link
Member

annevk commented Nov 13, 2015

Fetch does enable enforcing same-origin (mode is same-origin) and fetch is asynchronous (unless told not to, of course). Workers already use fetch but workers also have synchronous checks since that is how the specification was written. I think there's various other features that do the same, e.g., WebSocket although that may not be the best example. But say, EventSource.

Fetch also has the data thing as the same-origin data-URL flag so we could probably use that indeed. But again, the question is whether we can change the way the worker constructors work or not.

@sicking
Copy link
Author

sicking commented Nov 13, 2015

Changing an API from throwing to not-throwing has generally been considered safe. Gecko is planning on making this change, in part to archive consistency since currently main-thread-workers throw but workers-in-workers fire an error event.

Anyhow, sounds like this is entirely a worker issue then.

@sicking sicking closed this as completed Nov 13, 2015
@sicking
Copy link
Author

sicking commented Nov 13, 2015

For anyone interested in following along: whatwg/html#337

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants