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

SECURE: determining if settings is secure in SharedWorker #406

Closed
wanderview opened this issue Jun 17, 2015 · 21 comments
Closed

SECURE: determining if settings is secure in SharedWorker #406

wanderview opened this issue Jun 17, 2015 · 21 comments
Assignees
Labels

Comments

@wanderview
Copy link
Member

Looking at the spec for determining if settings is secure:

https://w3c.github.io/webappsec/specs/powerfulfeatures/#settings-secure

It seems the intent is to walk up the parent/document chain to see if the root is secure. This works for windows, iframes, and dedicated workers.

But it doesn't seem to work for SharedWorker.

A SharedWorker does not have a single parent. It can have multiple documents attached that change over time.

If we need to restrict a feature based on trust in a SharedWorker, how should the trust be determined?

It seems we can't check all the documents because then it would oscillate back and forth between trusted and non-trusted as docs attached. It seems we can't just use the initial document, because then later attaching docs will see inconsistent behavior. It would also allow later docs to observe the GC of the SharedWorker from previous docs.

@annevk
Copy link
Member

annevk commented Jun 17, 2015

I think the best solution would be to figure out if we can restrict creation of SharedWorker in <iframe> to only work if the <iframe> is a secure context. (Perhaps also allow http doc -> http iframe, but not http doc -> https iframe.)

@annevk
Copy link
Member

annevk commented Jun 17, 2015

Otherwise SharedWorker can always be used as a way out for secure context features, which seems bad.

@annevk
Copy link
Member

annevk commented Jun 18, 2015

New idea for SharedWorker: the secure contextness of its creator adds to the scope of the SharedWorker, such that it can only be accessed from other secure contexts. And vice versa, if an insecure context created a SharedWorker that nonetheless went over HTTPS (due to HTTP -> HTTPS nesting), a secure context would simply end up creating a new SharedWorker rather than connecting to that one.

This seems like a reasonably safe extension of the way SharedWorker works and allows us to have truly secure SharedWorker instances.

@mikewest mikewest self-assigned this Jun 19, 2015
@mikewest
Copy link
Member

Splitting the SharedWorker based on the security of it's opening context makes sense to me, though it's a breaking change. I don't believe IE or Safari ever shipped SharedWorker, however, so the usage might be low enough for us to change the behavior.

@annevk
Copy link
Member

annevk commented Jun 20, 2015

I don't really see any other way for us to get actual secure contexts. Actually, this applies to service workers too, maybe? What if you have http -> https frame with associated service worker?

@mikewest
Copy link
Member

In the interests of being Good Enough™, the document currently doesn't split the worker. It does walk through all the currently attached Documents, however, which seems to be enough of a deterrent to make working around the restrictions with a Shared Worker a) require a new Window, and b) difficult and annoying. I think that's a high-enough bar.

It does mean that the Shared Worker will "oscillate back and forth" based on the currently-connected documents. That might mean that we should expose a "isSecure" bit on the global object, but I don't think it's fatal to the design. Anything permission-based can be either allowed or denied based on the asynchronous whim of the user, and this doesn't seem significantly distinct from that model.

WDYT?

@annevk
Copy link
Member

annevk commented Sep 11, 2015

I'm not a fan, but I agree we'll not make meaningful progress otherwise. So "secure context" will end up meaning "environment with its HTTPS state set to authenticated plus some ancestor checks".

@wanderview
Copy link
Member Author

Actually, this applies to service workers too, maybe? What if you have http -> https frame with associated service worker?

The security check for service workers is at registration time. It seems clear cut to determine the document security there.

It does mean that the Shared Worker will "oscillate back and forth" based on the currently-connected documents.

I'm sorry, but this seems like a really big footgun for developers. It will lead to heisenbugs.

Anything permission-based can be either allowed or denied based on the asynchronous whim of the user, and this doesn't seem significantly distinct from that model.

But we are using the security status to allow access to features that are not permission-based. For example Cache API. I think a site would be quite surprised if they could get a Cache object, start using it, but then have the Cache start failing halfway through.

Also, from an implementation point of view, it would require work to revoke privileges to all this stuff at any time during the workers lifetime. Currently we just check if its secure when the page starts using something.

Can we make SharedWorker never secure for now? And then require an additional API in the future to opt in to restrictive secure SharedWorkers to avoid breaking back-compat?

var sw = new SharedWorker(url); // never secure
var secureSW = new SharedWorker(url, { mode: 'secure' });

@mikewest
Copy link
Member

I think I could live with that. It might make folks sad to lose WebCrypto in a Shared Worker, but that might be a better outcome than dealing with oscillations. According to https://www.chromestatus.com/metrics/feature/timeline/popularity/5, Chrome users only see Shared Workers on 0.0584% of page views (and I'd posit that some significant percentage thereof is Google Docs, which denies framing, so...).

Anne?

@mikewest
Copy link
Member

A friendlier version of @wanderview's suggestion might be to prevent attachment from an insecure context to a secure shared worker context. That is, rather than flagging the shared worker via the constructor, we could set an immutable flag at creation time based on the opener context: secure contexts create secure shared workers, insecure contexts create insecure shared workers.

Secure context can connect to both secure and insecure shared workers (whose state remains stable regardless), but insecure contexts may only connect to insecure shared workers (I guess we'd throw if we couldn't connect?).

WDYT?

@wanderview
Copy link
Member Author

Secure context can connect to both secure and insecure shared workers (whose state remains stable regardless), but insecure contexts may only connect to insecure shared workers (I guess we'd throw if we couldn't connect?).

I guess I would prefer if secure could only attach to secure shared workers. Otherwise someone using a site with a mixed of secure and non-secure could see varying behavior based on timing.

  1. Insecure frame attaches to SharedWorker first, then site works because all frames can attach.
  2. Secure frame attaches to SharedWorker first, then site breaks because some frames can no longer attach.

Also, this still leaves open the door for @annevk's suggestion of spawning a separate shared worker for secure and insecure.

@mikewest
Copy link
Member

Makes sense.

I'd prefer to avoid spawning multiple shared workers, simply because it's complex, and would require changes to the origin model, but I agree that that's something that this implementation would leave open for future consideration.

@mikewest
Copy link
Member

It sounds like @slightlyoff is happy with this suggestion as well.

@slightlyoff
Copy link

Yes, seems great.

@diracdeltas
Copy link

+1

@dbaron
Copy link
Member

dbaron commented Sep 15, 2015

Does that require a change to the workers spec to be made/tracked?

@mikewest
Copy link
Member

@dbaron: Yes. This spec will monkey patch that spec. :)

@annevk
Copy link
Member

annevk commented Sep 15, 2015

I recommend just PR'ing directly: https://github.com/whatwg/html. This feature is non-controversial so should be easy enough to get through.

@mikewest
Copy link
Member

Yup. I'll send you a PR tomorrow.

I'm sure I'll be monkey patching in the short term because this is the W3C, and http://w3.org/TR/workers is incredibly outdated but still exists. :)

@annevk
Copy link
Member

annevk commented Sep 15, 2015

I recommend "monkey patching" that by telling folks where to look instead. Only monkey patching your bits without it having all the other fixes isn't helping anyone.

@mikewest
Copy link
Member

  1. I agree!
  2. Hopefully chatting with someone on that point today, as it's already becoming a problem for me with Upgrade-Insecure-Requests.

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

No branches or pull requests

6 participants