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

clarify secure context requirements in Handle Fetch #890

Closed
wanderview opened this Issue Apr 28, 2016 · 16 comments

Comments

Projects
None yet
6 participants
@wanderview
Copy link
Member

wanderview commented Apr 28, 2016

Handle Fetch step 12.1 says this for non-subresource requests:

If client is not a secure context, return null.

The client in this case is the window that initiated the request.

For some cases this makes sense. For example, a nested iframe making the request to load its document. This check should prevent an insecure parent document from creating a nested https iframe in order to postMessage() to the server. That's good.

It doesn't seem to make sense, though, for things like navigations. If I'm on http://foo.com and follow a link to https://bar.com, I should totally be able to get an intercepted and controlled https://bar.com.

Can we just use the concept of "reserved" or "target" Client here instead? I think maybe "target" client would be right. (Assuming an about:blank in a new tab would pass the secure context check by being a local url.)

@mkruisselbrink

This comment has been minimized.

Copy link
Collaborator

mkruisselbrink commented Apr 28, 2016

It doesn't even make sense for "target client". The way things like window.open are specified for example the "noopener" flag isn't applied until after fetch has completed. And that noopener flag can be the difference between the target client being a secure context or not.

But this is kind of the same issue I was trying to figure out for Link headers (how to specify that Link: rel=serviceworker headers should only be processed for top-level requests that will result in a secure context). I somehow didn't realize that we already have this problem with Handle Fetch. I don't think there is a way with the way the fetch, html and secure contexts spec are currently written to properly specify this. But it certainly seems likely a request will need access to both the client that initiated a request and the client that could possibly be created using the result of the request.

@jakearchibald jakearchibald added this to the Version 1 milestone Jul 25, 2016

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Jul 25, 2016

Great catch. And ughhghghgh. Of course, a navigation to a secure origin with no means to communicate with the window that initiated the request should be considered secure.

I'm grumpy about the target="_blank" case here. Seems shitty that another website can stop my service worker working. We might need some kind of registration option to declaratively disown opener.

Related, but won't work for us w3c/webappsec#517

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Jul 25, 2016

Somewhat heretically proposing .opener isn't taken into consideration when determining secure context. w3c/webappsec-secure-contexts#42

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Jul 25, 2016

We may need an API to declaratively "disown" opener for all controlled pages. This would allow SW pages to work when linked to via target="blank". Could even be the default.

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Jul 26, 2016

Yeah, making opting into service workers not set the opener for the controlled globals might be nice.

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Jul 26, 2016

According to Mike West opener is used by oauth flows. Need to think about that.

@delapuente

This comment has been minimized.

Copy link

delapuente commented Jul 26, 2016

I'm not happy with a non HTTPS site to prevent part of the functionality of my site if this does not introduce an attack risk. Why iframing my site would prevent my SW from work? Is it to prevent abusing Cache API?

If I'm missing something, I would prefer to nullify .opener if the destination page is controlled by any Service Worker.

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Jul 27, 2016

Why iframing my site would prevent my SW from work? Is it to prevent abusing Cache API?

I believe it's to stop the non-secure origin getting data from powerful features by poseMessage-talking to the iframe.

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Jul 27, 2016

Pre F2F notes: I don't think any browser implements the target="_blank" non-secure thing. Do we think they will? How should we prepare for it?

@delapuente

This comment has been minimized.

Copy link

delapuente commented Jul 27, 2016

I believe it's to stop the non-secure origin getting data from powerful features by poseMessage-talking to the iframe.

Notice the non-secure origin can not get data this way, getting data is only possible if the safe origin decides to send that data via postMessage() to .opener / .parent. Preventing HTTPS sites to communicate with HTTP sites seems to be another different issues.

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Jul 27, 2016

Right, but that's still what they're trying to prevent. Sites were avoiding going full-HTTPS by just having a single proxy-page on HTTPS and using postMessage to pass the data.

@delapuente

This comment has been minimized.

Copy link

delapuente commented Jul 27, 2016

Enforcing HTTPS usage because we want to force them to adopt HTTPS does not feel right, don't you think? Anyway, if the HTTPS proxy is mine, why not to simply use the secured domain for my app and, if it is anyone else property, why this person would want to collaborate with me?

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Jul 28, 2016

F2F:

  • We should fix the spec here - I should be able to navigate from a non-secure origin to a secure one and use my service worker
  • In the target="_blank" case, if the secure context spec is going to continue like that, we should ditch opener for service worker controlled pages
@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Aug 3, 2016

Refining the above, seems like we should auto-disown opener for service worker controlled pages if opener is a non-secure context.

@jungkees

This comment has been minimized.

Copy link
Collaborator

jungkees commented Aug 23, 2016

Changing planChange summary:

  • Remove the secure context check of the source client in Handle Fetch step 12.1
    • The original intention was to check it against the target clientreserved client, but we don't have any control over the target clientreserved client at this stage. (A reserved client is a temporary environment settings object that has no JS realm and the global at this point.) So we check if the environment's creation URL is a potentially trustworthy URL in navigation cases, instead.
  • Add a step (to HTML navigation algorithm) that checks whether the newly created environment settings object (target clientthe environment settings object created for the Document loading) is secure context
    • If it's not a secure context, it won't get ditches the controller gotten from SW's Handle Fetch steps.
    • Check if this behavior is reasonable: the main resource has already been fetched through SW interactionIt went through "potentially trustworthy URL" with the environment's creation URL for the main resource fetch, which is the best it can do with the given information.
  • Add a step (to HTML navigation algorithm) that checks whether the source client that triggered this navigation is secure context
    • If it's not a secure context and the target clientreserved client is not controlled, disown the window.opener of the target clientreserved client

This change requires progress on #870 is done.

Self-note: don't forget to check the about:blank case and the worker client case.

@jungkees

This comment has been minimized.

Copy link
Collaborator

jungkees commented Nov 4, 2016

See the summary in #890 (comment) Closing.

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