w3c / webappsec-secure-contexts

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

<iframe sandbox srcdoc="foo"> on a secure page should be secure #5

Closed bzbarsky closed 8 years ago

bzbarsky commented 8 years ago

This may not end up being an issue depending on how https://github.com/w3c/webappsec-secure-contexts/issues/4 is resolved, but chances are it will be.

The intent of this spec is that on https://foo <iframe sandbox src="https://foo"> be considered secure. But then <iframe sandbox srcdoc="whatever"> should also be considered secure.

And chances are, so should <iframe sandbox src="data:stuff"> and probably <iframe src="data:stuff">, right?

mikewest commented 8 years ago

I'm fairly certain this is already dealt with via the HTTPS State flag on the global object. That is, https://w3c.github.io/webappsec-secure-contexts/#settings-object step 4.1 skips the check when we're dealing with these origins, assuming they're nested inside secure origins (because the flag is persisted when documents are created (see https://html.spec.whatwg.org/multipage/browsers.html#initialise-the-document-object, and then things like https://html.spec.whatwg.org/#process-the-iframe-attributes)). I hope we caught all the places we need to in order to shove this flag around.

The case that's more interesting is when we're secure because we're http://127.0.0.1 or similar. I think we deal with this correctly today by skipping over srcdoc documents in https://w3c.github.io/webappsec-secure-contexts/#gather-ancestors.

data is more interesting. We explicitly exclude data: and javascript: origins in the note at the top of https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy. I'm willing to believe that that's a bad idea, but I do think it's at least defined behavior. :)

annevk commented 8 years ago

What is a data or javascript origin? There's no such thing.

mikewest commented 8 years ago

Indeed. I should have just copy/pasted from the note: "The origin of data: and javascript: URLs is an opaque identifier, which will not be considered potentially secure."

annevk commented 8 years ago

For data URLs at least Fetch does set HTTPS state on the response based on who initiated the fetch. So something seems amiss.

mikewest commented 8 years ago

I see. That's a mismatch between Chrome and Fetch, then. We've traditionally had problems identifying "who identified the fetch" when it comes to data navigations in frames.

bzbarsky commented 8 years ago

I'm fairly certain this is already dealt with via the HTTPS State flag on the global object.

Ah, that's why that check is there. OK.

The case that's more interesting is when we're secure because we're http://127.0.0.1 or similar.

Yes.

I think we deal with this correctly today by skipping over srcdoc documents in https://w3c.github.io/webappsec-secure-contexts/#gather-ancestors.

It's not. That skipping makes a descendant of a srcdoc document ignore it for purposes of this stuff, but still leaves an <iframe sandbox srcdoc> on http://localhost insecure (or rather undefined due to https://github.com/w3c/webappsec-secure-contexts/issues/4).

We explicitly exclude data: and javascript: origins in the note at the top of https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy.

It's an informative note. It's helpful for understanding intent, but the normative language is what needs to define the actual behavior. Right now nothing normative says anything about what happens with opaque identifier origins; the relevant algorithms pretend they don't exist. Again, see https://github.com/w3c/webappsec-secure-contexts/issues/4.

mikewest commented 8 years ago

It's not. That skipping makes a descendant of a srcdoc document ignore it for purposes of this stuff, but still leaves an <iframe sandbox srcdoc> on http://localhost insecure (or rather undefined due to #4).

Let's assume I deal with #4 in some stunningly intelligent way.

In this case, I'd like the following to happen:

  1. https://w3c.github.io/webappsec-secure-contexts/#settings-object creates a list of documents to examine that includes the top-level document, but not the srcdoc document. So:
    1. Step 1 of that algorithm should create an empty list
    2. We should add the worker global in step 2
    3. https://w3c.github.io/webappsec-secure-contexts/#gather-ancestors step 1 should only add the given document's settings object if it's not a srcdoc.
  2. The top-level document either:
    1. Has an origin which is a tuple of (http, 127.0.0.1, 80) (which is not "potentially secure", but is "potentially trustworthy").
    2. Is sandboxed (perhaps via CSP), in which case we read the document's address' origin in step 4.3 of https://w3c.github.io/webappsec-secure-contexts/#settings-object, so we're back at a tuple of (http, 127.0.0.1, 80).

I'll go write something up for #4 in the hopes of making these statements true.

mikewest commented 8 years ago

https://github.com/w3c/webappsec-secure-contexts/commit/834f9f4d0202ef890fcaa26bb04f8205454a3b80 should take care of the bits in 1 above. I think it's accurate, but I'd appreciate feedback as to whether it addresses your concerns.

bzbarsky commented 8 years ago

There's also the case of the top-level document being sandboxed but loaded from a URI has a unique identifier origin, no? Most simply consider a sandboxed https:// page doing a window.open("data:stuff"). Should that be considered secure? If not, why not?

bzbarsky commented 8 years ago

https://github.com/w3c/webappsec-secure-contexts/commit/834f9f4d0202ef890fcaa26bb04f8205454a3b80 should take care of the bits in 1 above.

Looks good to me.

mikewest commented 8 years ago

Most simply consider a sandboxed https:// page doing a window.open("data:stuff").

The current algorithm would consider it trustworthy if it was created by an https:// page, and untrustworthy otherwise (because Fetch sets its HTTPS state to the request's client's HTTPS state). shrug The current algorithm is self-consistent, and does have an answer for you, but it's not clear to me that it's a good one.

Chrome would consider that new window untrustworthy, basically because Chrome doesn't like data URLs. Even if we did consider it trustworthy, Chrome wouldn't support many interesting features in the data: window, as we'd give it a unique origin, and deny it access to most permission-based things that we tie to origins.

bzbarsky commented 8 years ago

because Fetch sets its HTTPS state to the request's client's HTTPS state

Ah, ok. It seems like that's the consistent thing to do to me, by the way: any time an origin would normally be aliased per spec, even if such aliasing is prevented by sandboxing, the HTTPS state should be copied. That would completely close up all the edge cases I can think of, as well as any future issues if new origin-sharing things are added.

mikewest commented 8 years ago

The patch which added HTTPS state to HTML should have covered all the cases in which new browsing contexts inherit details from their creator, and I believe that Anne has similarly covered all the right places in Fetch (see the first few items in https://fetch.spec.whatwg.org/#concept-basic-fetch, for example).

I'd love it if you'd spot-check specific instances that come to mind to verify that we've gotten things right.

bzbarsky commented 8 years ago

Ah, I had missed the HTTPS state stuff in Fetch. I clearly need to read that spec again.

I did just spot-check things, and I have two issues:

1) The origin inheritance and the HTTPS state inheritance are not defined in the same place, so it's hard to tell whether they're in sync or not. For example, the Fetch spec defines that about:blank inherits HTTPS state, but the HTML spec doesn't have that inheriting origin (which is a bug; see https://github.com/whatwg/html/issues/255).

Ideally both would be defined in the same place so it was clear how things work.

2) The Fetch spec defines things in terms of the fetch client, and I think this can cause various problems to creep in due to how HTML defines the fetch client. Here's an example:

Say I load an http:// page which is same-origin with me in browsing context A, then grab a function f from that page, then load an https:// page in browsing context A (which is not same-origin with me, obviously). Now I call function f. What function f does is set location.href on its window, which it's allowed to do cross-origin. The href setter on Location is defined at https://html.spec.whatwg.org/#location-object-setter-navigate and invokes https://html.spec.whatwg.org/#location-object-navigate which grabs the incumbent settings object (in this case that corresponds to f), gets its responsible browsing context (that's A in this case) and invokes https://html.spec.whatwg.org/#navigate. Step 15 of that algorithm is what performs the fetch, and sets it up in substep 3 of the "Otherwise" steps. It sets the "client" to "source browsing context's active document's Window object's environment settings object" or in other words the https:// page I loaded. So we will copy the HTTPS state from that page.

On the other hand, origin inheritance as defined in HTML, at least for the data: case, uses the incumbent settings object when the "navigate" algorithm is invoked. I expect about:blank will be defined similarly. This means that the origin inherited is that of the page f came from, while the HTTPS state is that of some totally unrelated (and indeed not necessarily under my control at all) page. This is clearly broken. Oh, and this is the very first case I spot-checked, though I admit it was the one I expected to be most likely to be broken.

I think that these are basically the same problem: we want to make sure that any time we inherit an origin from a settings object we also inherit the HTTPS state from the same settings object. Ideally in as obvious a way as possible (i.e. in the same exact spec, at the same exact place, where we just have one settings object we grab both pieces of information from). @annevk, can we get there?

bzbarsky commented 8 years ago

Oh, and all this stuff obviously needs tests. Lots and lots of tests.

annevk commented 8 years ago

Yeah, I think it's not just origin and HTTPS state, but also CSP policy, document's URL (maybe not always), and some other things. We should definitely try to get to a place where that is a single sane thing. Not sure about the timeframe though. Untangling HTML is hard.

jwatt commented 8 years ago

I think that these are basically the same problem: we want to make sure that any time we inherit an origin from a settings object we also inherit the HTTPS state from the same settings object.

I wonder if we can just use the origin's scheme in that case and do away with all the spec text about how to propagate HTTPS state.

bzbarsky commented 8 years ago

No, because that fails for things that are sandboxed. We want to propagate HTTPS state any time we would have inherited origin, even if we're not due to sandboxing.

jwatt commented 8 years ago

I think modulo https://github.com/whatwg/html/issues/255 and https://github.com/whatwg/html/issues/856 all the issues and scenarios raised here work or have been fixed. Can we close this or are there outstanding issues or work to do?

mikewest commented 8 years ago

No, I also agree that those bugs cover the work necessary, and that once they're resolved, Secure Contexts will be doing the right thing.