whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.11k stars 2.67k forks source link

Windows opened via <a target=_blank> should not have an opener by default #4078

Closed cdumez closed 5 years ago

cdumez commented 6 years ago

Windows opened via <a target=_blank> currently have an opener unless specified otherwise via rel="noopener". While most developers expect a window opened via window.open() to have an opener, I believe most do not necessarily realize the same applies to windows opened via <a target=_blank>. It is unfortunately too rare to see Web developers use rel="noopener" in cases where it can and should definitely be used, even on top Web sites (see for example articles on Google News).

As a result, I would argue that we should switch the default behavior so that windows opened via <a target=_blank> do not get an opener, except if the developers explicitly asks for one via rel="opener".

This change would be beneficial for security: both in the general Web security aspect (e.g. not being able to navigate one's opener) but also for process isolation. For engines such as WebKit which do not currently support out of process iframes, this would allow for process swapping in more cases.

I understand this change could be risky for a compatibility point of view. It is hard - however - for me to tell how risky it would really be. Also, there would still be a way for Web content to get an opener if they really wanted to, either by using window.open(), or rel="opener".

We are considering experimenting with this new behavior in WebKit (via Safari Technology Preview) so I wanted to file this issue beforehand to gather some feedback and get a feel of how other browser vendors feel about this. For example, if you know of a good reason why we should definitely not do this, we'd really like know :)

cdumez commented 6 years ago

cc @annevk @domenic @mystor

geoffreygaren commented 6 years ago

There's a lot of discussion online by web developers who were surprised by the default opener policy. It's a common source of security bugs.

https://news.ycombinator.com/item?id=11553740 https://www.reddit.com/r/programming/comments/4zikpx/the_target_blank_vulnerability_by_example/ https://mathiasbynens.github.io/rel-noopener/

domenic commented 6 years ago

Previous discussions wherein we considered variants of this too risky to attempt: https://github.com/whatwg/html/issues/2119 https://github.com/whatwg/html/issues/2047 . I'm not sure anyone considered limiting it only to target=_blank.

If you search for "opener" on this tracker there are a bunch of still-open issues. Some are in this area but a lot seem to be about the spec being unclear/broken around browsing context names and opener-disowning.

/cc @whatwg/security and @csreis

annevk commented 6 years ago

To rephrase OP, the suggestion here would be that target=_blank results in a new top-level browsing context (i.e., not an auxiliary browsing context).

(I like it, if web-compatible.)

csreis commented 6 years ago

I'd love to see this as well. Looking at #2047, I guess the main question is whether OAuth flows use target=_blank or not (and if so, if they could be updated to use named windows). /cc @hillbrad for thoughts on that, and @mikewest for thoughts on making noopener the default behavior in the target=_blank case.

cdumez commented 6 years ago

I did some very early testing with regards to OAuth, I tried both Facebook and Google Oauth on several top sites and both still seem to work (the newly opened window still gets an opener). They are likely relying on window.open() rather than <a target=_blank>. This seems encouraging.

URLs tested:

bzbarsky commented 6 years ago

This sounds like an interesting proposal. Would the newly opened thing also not be targetable by name from the page that opened it, to sever links in both directions?

geoffreygaren commented 6 years ago

@bzbarsky Yes the proposal is to sever the link in both directions. I think that’s what the “top-level browsing context” phrase attempts to specify.

Note that the newly opened thing initially has no targetable name (because it is _blank), so observable cases of this detail are probably rare — but I guess the loaded page could in theory set window.name, making the behavior observable.

bzbarsky commented 6 years ago

but I guess the loaded page could in theory set window.name

Right, that would be the case to worry about.

annevk commented 6 years ago

New top-level browsing context was indeed meant to indicate a new name bucket as well.

Digression on name buckets

There are some oddities with rel=noopener and names: #1826 (I think that's a bug and we should always create a new top-level browsing context when it's used).

Defining the name bucket is #1440 and to some extent #313 is also about that. For both having a complete description of what browsers do today would help a lot (or pointers to the relevant algorithms).

cdumez commented 6 years ago

Will experiment with the new behavior in WebKit via https://bugs.webkit.org/show_bug.cgi?id=190481.

cdumez commented 6 years ago

Safari Technology Preview 68 contains this behavior change. https://webkit.org/blog/8475/release-notes-for-safari-technology-preview-68/

sashafirsov commented 6 years ago

The target name concept for cross-documents control, communication, etc is broken from very beginning as given a global naming scope for all sites. I think this security gap could be solved by applying the app scope on top of name. So when window is opened or addressed the name is used only inside of this scope. Other little patches like adding extra parameter to anchors are the scotch patches on bumper. The problem should be addressed on fundamental principles level. If some one curious, defined scoping model is implemented in epa-wg/embed-page. Of course it is a draft to start with.

annevk commented 5 years ago

@cdumez was it intentional to exclude <area> and <form> btw? It would be nice to be consistent among those I think.

cdumez commented 5 years ago

Yes, it was intentional, to limit the risk of Web breakage. Although I agree with you that all these should be consistent, I'd like to do it incrementally so we can identify which ones are OK (or not) in terms of Web compatibility.

For now, there was no reported regression that I know of from changing only <a>'s behavior in STP 68, which is encouraging.

I am not super worried about <area> but I do believe that <form> brings additional compatibility risk.

annevk commented 5 years ago

https://bugzilla.mozilla.org/show_bug.cgi?id=1503681 adds wpt tentative tests for <a> and <area>. We're also gonna try <form>, but separately. Fixing #2983 would be good for <form> too, if we indeed keep the rel=opener pattern.

Another thing that came up is that adding noreferrer to window.open() might be good.

mystor commented 5 years ago

One concern which came up in our bug (which @annevk has linked above) was the interaction of multiple opener noopener and noreferrer flags. We could:

  1. Apply the rules in order, so "opener noopener" is noopener, while "noopener opener" is opener. This has an interesting interaction for "noreferrer opener" which would be that the referrer wouldn't be sent, but the opener would be present.
  2. Specify a pecking order, and always use the most specific one, for example "opener" < "noopener" < "noreferrer". This would mean that if "noreferrer" is written, it wins, and "opener" only wins if nothing is written.
annevk commented 5 years ago

I tried to clarify the semantics of rel in d324aebd1350c93e268b836d47c1c8db39328690. I don't think we should do 1 as currently alternate stylesheet and stylesheet alternate are identical. It seems weird to let order matter sometimes.

annevk commented 5 years ago

What's needed here:

Equivalent changes for form can be done as part of #2983.