w3c / webappsec-secure-contexts

WebAppSec Secure Contexts
https://w3c.github.io/webappsec-secure-contexts/
Other
33 stars 38 forks source link

Checking the opener browsing context is broken #18

Closed jwatt closed 8 years ago

jwatt commented 8 years ago

Right now https://w3c.github.io/webappsec-secure-contexts/#gather-ancestors says to include the opener browsing context.

In the case that an http page opens a window and loads a data: URI it should be clear that the data: URI is not secure. If the http page is replaced by an https page then at that point the algo will claim the window the data: URI was loaded in is secure though.

bzbarsky commented 8 years ago

This presumably applies to blob: URIs too, not just data:, right?

mikewest commented 8 years ago

If the opener browsing context navigates, it would no longer be the "creator Document" right? Since we're looping through creator Documents, I think this is taken care of in the spec (though I think Chrome's implementation is incorrect) in the case you've noted).

jwatt commented 8 years ago

If the opener browsing context navigates, it would no longer be the "creator Document" right?

This sentence doesn't quite compute since browsing contexts are not documents. The opener browsing context's active document would no longer be the creator Document, but that doesn't seem to change the outcome of the secure context algorithms. That algo (bullet numbering different due to markdown limitations; it should start at 3):

  1. While document has a creator Document creator:
    1. If creator’s browsing context is an ancestor browsing context or opener browsing context of document:
      1. Insert creator’s relevant settings object into ancestors, unless creator is an IFrame srcdoc Document.
      2. Let document be creator.
    2. Otherwise, exit this loop.

As far as I can see from the HTML5 spec, a document in a popup always has the same creator Document (the opener browser context's active document at the time the popup opened) regardless of whether the opener browser context is navigated. Implementation decisions (such as free'ing the memory used by a Document) may mean it is no longer accessible, but there's nothing in the spec that I see that says that is the case. So I think in step (1) 'creator' is always the same object regardless of opener browser context navigation. In step (i), whether or not 'creator' is the active document or not, it still has the same browser context, so again we proceed.

mikewest commented 8 years ago

I think we're agreeing with each other? If we're not agreeing with each other, I'd appreciate your input on a change to the spec text that would make it do the right thing in the cases you're thinking of.

bzbarsky commented 8 years ago

It does sound like you're agreeing with each other, but there's another fundamental problem here: "creator Document" as currently defined in the HTML spec is leaky in a bad way because it forces that document to be kept alive while the created document is alive, no? This is pretty suboptimal, especially because this sort of thing can chain, so documents that the user long since doesn't care about in any way are forced to be kept alive. Does any UA actually implement that sort of leak? Pinging @annevk

bzbarsky commented 8 years ago

And more to the point, it seems like in practice UAs would need to optimize things somehow by snapshotting things from the "creator document" and allowing it to go away. To the extent that the secure context spec doesn't allow that, we have a problem.

annevk commented 8 years ago

Yeah, I think the HTML Standard just uses it as a concept of convenience since we only ever use it for the origin and referrer. We should probably point out that we don't expect it to be kept alive for those purposes, right? I can file an issue against HTML on that.

bzbarsky commented 8 years ago

That's sort of the issue: once you have it as a concept of convenience, people start using it for other things like the secure context stuff here...

It might be better to define an "opener document info" thing which stores the things we really care about.

annevk commented 8 years ago

Well, but we do alias the origin, so as long as that document is alive, it actually matters. I guess if you alias an alias it still points to the original in a way so maybe that's okay...

bzbarsky commented 8 years ago

Aliasing the origin is not a problem, because the origin is immutable. Aliasing the effective script origin, if that's happening, is a bigger deal. But note that the current aliasing setup in HTML is pretty weird to start with (e.g. it's asymmetric), which is another longstanding issue with it...

annevk commented 8 years ago

(Both are aliased.) Is that issue filed? I don't remember running across it.

bzbarsky commented 8 years ago

I don't know. I recall reporting it, but it might have been in email form... Filed https://github.com/whatwg/html/issues/794

jwatt commented 8 years ago

I think we're agreeing with each other?

Yeah, I was confused. Sorry about that.

jwatt commented 8 years ago

there's another fundamental problem here: "creator Document" as currently defined in the HTML spec is leaky in a bad way because it forces that document to be kept alive while the created document is alive, no?

This assumes that secure context status can change, or more particularly that document.domain changes affect API exposure in auxiliary browser contexts. I'm hoping that's no longer going to be the case. See Anne's comment and my comment following it at:

https://github.com/w3c/webappsec-secure-contexts/issues/10#issuecomment-182737583

bzbarsky commented 8 years ago

or more particularly that document.domain changes affect API exposure in auxiliary browser contexts.

Right. If that changes, things are simpler.

jwatt commented 8 years ago

Whether or not we end up restricting API based on document.domain, the work for that has been split out by @annevk into https://github.com/whatwg/html/issues/829