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

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

Closed
bzbarsky opened this issue Oct 21, 2015 · 20 comments
Closed

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

bzbarsky opened this issue Oct 21, 2015 · 20 comments

Comments

@bzbarsky
Copy link

This may not end up being an issue depending on how #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
Copy link
Member

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
Copy link
Member

annevk commented Oct 21, 2015

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

@mikewest
Copy link
Member

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
Copy link
Member

annevk commented Oct 21, 2015

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

@mikewest
Copy link
Member

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
Copy link
Author

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 #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 #4.

@mikewest
Copy link
Member

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:
  3. Has an origin which is a tuple of (http, 127.0.0.1, 80) (which is not "potentially secure", but is "potentially trustworthy").
  4. 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
Copy link
Member

834f9f4 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
Copy link
Author

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
Copy link
Author

834f9f4 should take care of the bits in 1 above.

Looks good to me.

@mikewest
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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 Origin of non-initial about:blank doesn't seem to be defined whatwg/html#255).

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

  1. 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
Copy link
Author

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

@annevk
Copy link
Member

annevk commented Oct 28, 2015

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
Copy link
Contributor

jwatt commented Nov 3, 2015

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
Copy link
Author

bzbarsky commented Nov 3, 2015

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
Copy link
Contributor

jwatt commented Mar 11, 2016

I think modulo whatwg/html#255 and whatwg/html#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
Copy link
Member

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.

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

No branches or pull requests

4 participants