Open jakearchibald opened 3 years ago
Something I haven't looked at yet: When going back()
, do the srcdoc
iframes take their CSP rules from the parent as it is now, or as it was when the srcdoc
page was created?
/cc @domfarolino @antosart for the CSP rules question. I am pretty sure that has a canonical answer as to how it should be, given recent policy container work.
I believe the answer for CSP rules depends on the answer to the main question (whether the srcdoc
content is reloaded when navigating back or not).
If we store the content of srcdoc
in history, we should also store the policies. If we reload the srcdoc
attribute (which might have changed) on history navigations, then I guess we should also re-inherit the policies from the parent.
I believe the currently specced behaviour is to store the policies in history for srcdoc. This is also implemented in chrome.
(This is not tested for CSP at the moment I believe, although it is for referrer-policy, see the file referrer-policy/generic/inheritance/iframe-inheritance-history-about-srcdoc.html)
If we store the content of
srcdoc
in history, we should also store the policies. If we reload thesrcdoc
attribute (which might have changed) on history navigations, then I guess we should also re-inherit the policies from the parent.
Agreed.
I believe the currently specced behaviour is to store the policies in history for srcdoc.
The spec doesn't seem to cover restoring srcdoc pages at all, unless I'm missing something.
This is also implemented in chrome.
I don't think so. See the tests above – it seems like Chrome goes back to the srcdoc defined in the page.
I tested Firefox, and it does seem to store the CSP rules along with the srcdoc src, so it's like it stores the whole response. That's what I'll spec for now, and we can look at it again later if needed.
I believe the currently specced behaviour is to store the policies in history for srcdoc.
The spec doesn't seem to cover restoring srcdoc pages at all, unless I'm missing something.
Sorry, I just meant w.r.t. policy inheritance (which might never be called because the spec does not cover navigating back to srcdoc). And I actually was mistaken: we explicitly excluded srcdoc when storing policies in history
This is also implemented in chrome.
I don't think so. See the tests above – it seems like Chrome goes back to the srcdoc defined in the page.
Also here I was referring to policies. Chrome reloads the srcdoc content from the srcdoc
attribute, but stores and reloads the policies from history.
Back to the original question: if we store the content of the srcdoc
attribute in history, then we should probably store other attributes as well (I am thinking at sandbox
for example, or allow
). I am not totally sure I am convinced that that is better than reloading everything on history navigations.
I think it comes down to what you'd expect to happen here:
Should '/a' be sandboxed?
Another good question! I am not sure.
For history navigations to srcdoc it seemed to be slightly more clear to me, in order to avoid the asymmetry of getting one attribute (srcdoc) back from history while re-parsing another one (sandbox) from the
Yeah it's a tough one, and I don't know enough about sandboxing. How about:
It seems weird to load a non-sandboxed page into an iframe that was born with a sandbox attribute. Maybe reconnection just shouldn't happen in that case.
I think it comes down to what you'd expect to happen here:
That's a tough one. On the one hand, I would expect '/a' not to be sandboxed since it was originally not sandboxed when loaded. On the other hand, I worry that that could allow some sort of sandbox escape. E.g. maybe the outer page expects that once it puts the sandbox attribute on the iframe, it's safe from its contents.
Maybe reconnection just shouldn't happen in that case.
This is my instinct.
So, to summarize:
I think that sounds pretty reasonable.
cc @smaug----
- Policies, including sandboxing policies (and probably anonymous iframes if they become a thing) are stored in history at creation-time
I think that's still a bit weird in this case:
If we're saying that policies are stored with the history entry, that means loading page1 without sandboxing, into an iframe with the sandbox attribute set. Isn't that… confusing?
If we were starting again, I think I'd want iframe history to be cleared if safety features changed.
Yeah it is. I wonder if clearing history upon such an action would break anything in practice. Failing that, using the sandboxing rules at the time navigate is invoked is prolly best for that case. Is this something we should consider doing better on with new features such as https://github.com/camillelamy/explainers/blob/master/anonymous_iframes.md? No breakage there yet.
cc @camillelamy
I think we're envisioning to look at the attribute every time we navigate, whether history or not. I thought this is what sandbox was doing? I would find it weird that an attribute set by the parent of the iframe for security purposes could be overwritten by the iframe's action of going back. I reckon that this looks weird from the iframe point-of-view, however in this case the iframe parent's choice of security attribute should be respected. Otherwise, we might risk a sandbox escape, or loading a regular document in a crossOriginIsolated context by error for the anonymous iframe case.
@camillelamy yeah, I think that's a good fallback. The question is whether setting sandbox
or credentials
should reset the history as such actions impact the integrity of the nested browsing context.
Although I'm not entirely sure how that would work. In the example from https://github.com/whatwg/html/issues/6809#issuecomment-884227242 it would be reset in step 2, but presumably you still need a current history entry so would it really solve anything? @jakearchibald what was the model you had in mind?
@camillelamy
I think we're envisioning to look at the attribute every time we navigate, whether history or not.
Doesn't that leave you with the other problem?
This results in the risky page being reparented in an iframe without sandboxing.
Or:
This results in the risky page loading in an unsandboxed iframe.
@annevk
In the example from #6809 (comment) it would be reset in step 2, but presumably you still need a current history entry so would it really solve anything? @jakearchibald what was the model you had in mind?
I was thinking the history clearing would happen in step 3, as that's the moment new sandboxing rules would be applied to the iframe. The spec would go "wait, the policy has changed!" and cycle the navigable before continuing (as if the iframe had been removed and readded in the same place).
@camillelamy
I think we're envisioning to look at the attribute every time we navigate, whether history or not.
Doesn't that leave you with the other problem?
- Page has an iframe without the sandbox attr.
- Page wants to load a risky page in the iframe, so it sets the sandbox attr.
- Page loads the risky page in the iframe.
- Top level navigation.
- Back.
This results in the risky page being reparented in an iframe without sandboxing.
If there is a top-level navigation, then back, we would reload the top-level document. It might then recreate or not recreate the iframe, with whatever sandbox flag it wants, and then load whatever it wants inside. I don't see how this necessarily leads to reparenting the risky page.
Or:
- Page has an iframe with the sandbox attr, containing a risky page.
- Page wants to load a safe page in the iframe, so it removes the sandbox attr.
- Page loads safe page the iframe.
- Back.
This results in the risky page loading in an unsandboxed iframe.
I would argue that it is probably not good security practice to set sandbox flags on iframe and then remove them...
For the history clearing part, would this apply to the history list the browser maintains (not really specced AFAIU) or just the history API available to the document? Because if the later, wouldn't there still be a problem when the user clicks on the back button?
@camillelamy
I don't see how this necessarily leads to reparenting the risky page.
Unfortunately that's what happens. To reproduce:
"one" will be reparented into the second iframe, but it's no longer sandboxed.
Reparenting logic is different across browsers, and unspec'd. In Firefox it seems to just use index in the source to reconnect iframes, so it's fairly possible to end up with content reloading in the wrong iframe, which may have a weaker policy.
At a minimum, I think we need to avoid reparenting in cases like this, although I'm not 100% sure what the logic would be.
I would argue that it is probably not good security practice to set sandbox flags on iframe and then remove them
Yeah, I think you're right. Maybe solving the reparenting case is enough. Although, the "clearing history" fix seems easy enough, and if it protects folks, it might be worth it?
For the history clearing part, would this apply to the history list the browser maintains (not really specced AFAIU) or just the history API available to the document?
It'd be the browser history list (I'm working on a better spec for that in https://github.com/whatwg/html/pull/6315). But, it'd work the same as removing and readding the iframe, since that flushes iframe history.
Treating it as if the iframe was removed an re-added when we change security properties of the frame seems reasonable to me.
cc @ArthurSonzogni @antosart
cc @iVanlIsh
Firefox behavior of storing srcdoc into the history entry is interesting.
At some point, we are going to isolate sandboxed document in their own process. Since we are going to make srcdoc content to move across processes, we might benefits from copying Firefox behavior here along the way. I remember in this internal doc, I mentioned doing it as an alternative: https://docs.google.com/document/d/1YHWET0-Q8fh-i78DCMYowZFXtaXogYrIEhcKHKcCZbA/edit#heading=h.42t66nhl3wkw but said this would introduce a breaking change with this scenario:
Set srcdoc = ”A”
Set src = “https://example.
Set srcdoc = ”B”
history.back(-2)
Actually, by reading this thread, I realize the breaking change would actually mean converging with Firefox here. That might turns out to be a nice outcome. +CC @wjmaclean, Alex Moshchuk (can't find GitHub handle right now), @csreis. FYI.
Some more findings:
Test:
Chrome, Firefox: add replace Safari: replace replace
It seems like Chrome & Firefox treat it as a replacement since the url is about:srcset
in each case. I'm not sure why Safari does what it does.
There's a good case to be made for each srcdoc change being an 'add', but I guess spec'ing what Chrome & Firefox do is fine.
Test:
Chrome, Firefox: add add Safari: replace replace
Chrome and Firefox are consistent with the previous finding. In this case it's an 'add' because the url is changing from about:srcset#yo
to about:srcset
, which is consistent with the findings in https://github.com/whatwg/html/issues/6682.
Chrome: The back button doesn't work as I'd expect, but it's consistent with findings in the OP.
Firefox: The back button takes you to an error page. I guess it doesn't like about:srcdoc#yo
, even though it was fine with it previously.
I'll spec it so the back button changes the document, like we see from Firefox apart from the error case.
PSA: 1) Firefox stores the srcdoc content in history and reloads it from history. 2) Chrome reloads the srcdoc content from the iframe srcdoc attribute (which might have changed in between).
It seems we might have reached the conclusion to move toward Firefox behavior. If yes, it means restoring the PolicyContainer from history also makes some sense. Anyway, it seems this is already what Chrome and Firefox are doing: I checked behavior for CSP (see wpt results).
I initially wanted to align Chrome with the spec, but I am now believing I should align the spec/WPT with Chrome and Firefox. I am going to propose a PR removing step 2 from: https://html.spec.whatwg.org/multipage/origin.html#requires-storing-the-policy-container-in-history Please let me know if you disagree.
So what are all the behaviors this bug is covering?
I will work on some WPTs. If someone could help clarify the desired resolution for sandboxing that'd be great as well.
Summary of the sandbox options:
Pros: It's what browsers do now!
Risk: If a developer loads an unsafe page in a sandboxed iframe, then removes sandbox and loads a safe page, it's possible through traversal that the unsafe page loads without a sandbox.
Risk: If a page loads with a safe page in an iframe, then adds a sandbox to load an unsafe page, it's possible through iframe reparenting that the unsafe page loads without a sandbox.
Risk: A page loads with an unsafe page in an iframe. It's possible through iframe reparenting, that the heuristics don't quite work, and the unsafe page ends up reparenting into a 'different' iframe without a sandbox.
Risk: Inconsistent results if we start pulling iframe documents from bfcache.
Pros: Follows what we've decided to do with srcdoc and other policy state.
Risk: It's a change in behaviour, so could be a compat issue.
Risk: The developer sets a sandbox
attribute, then traverses the iframe, and is confused that the rules aren't applied. I don't think this is a security risk. This seems less risky than option 1. Although the developer may expect stronger sandboxing to be in place, since it's a traversal it means they must have already loaded the page with no sandbox.
This solution has two parts:
Pros: Avoids pages loading with unexpected sandbox rules.
Risk: It's a change in behaviour, so could be a compat issue.
Maybe option 2 is the right balance of safe & simple?
I and @petervanderbeken happened to discuss about this and also prefer option 2 of those three options (and the way Firefox stores srcdoc). Should be relatively easy to implement and hopefully not too surprising for the web devs.
First up is a simple one: Does setting
srcdoc
replace the current history entry or add a new one?Chrome & Firefox: Replace if the current document url is
about:srcdoc
, otherwise add. Safari: Always replace.I'm pretty sure Chrome & Firefox are doing the right thing here and it's somewhat spec'd.
How is a
srcdoc
entry stored in session history? It's currently unspec'd.Test:
This creates history entries
[srcdoc1, 'one', srcdoc2]
in Chrome/Firefox and[srcdoc1, srcdoc2]
in Safari.Each browser behaves differently when traversing to the srcdoc entries.
Chrome: Recreates the document using the
srcdoc
of the iframe. As insrcdoc1
andsrcdoc2
use the same source, which is different to the sourcesrcdoc1
used when it was first created.Firefox: Seems to store the
srcdoc
source along with the history entry, sosrcdoc1
andsrcdoc2
use different source, which is each the source they were created with.Safari: Restores the original history entries in these places, as if the
srcdoc
stuff never happened.Another test:
back()
Chrome: iframe empty. I guess this is because it looks at the
srcdoc
of the iframe which is empty, since the page has reloaded.Firefox: Restores the iframe to its previous content.
Safari: Restores the iframe as if setting
srcdoc
never happened.This seems consistent with the findings above. Firefox's model seems to make the most sense to me. Chrome's behaviour is ok I guess. Safari's behaviour is plain weird.