HTTPS-only mode alert shown in frames: adds exception for secure top-level site, possible clickjacking
Categories
(Core :: DOM: Security, defect, P3)
Tracking
()
People
(Reporter: dveditz, Assigned: maltejur)
References
Details
(Keywords: csectype-clickjacking, sec-low, Whiteboard: [domsecurity-active][adv-main133-])
Attachments
(3 files)
If https-only is turned on and we're unable to upgrade a framed http: site, we show the "HTTPS-Only Mode Alert" error page. I don't think we want to do that because normally the mixed-content blocker would block that document with no fuss (other than a console log error).
Even if we want to show that page, the exception button doesn't really help
- it adds an exception for the top-level https: page, not the framed domain
- it then reloads the page, which is now exempt from https-only upgrading, so the frames are now blocked by the mixed-content blocker. If there were multiple frames and some were successfully upgraded before then the page will be worse now.
The button in that page is potentially clickjack-able so I've made this a security bug for now. I haven't found any real harms but it makes me nervous. A site could prevent having its contents upgraded, but then the mixed-content will either upgrade them anyway or block them. A framed site could try to get its framer exempted, which might make it look broken if it was relying on https-only? But we're the only browser with that feature so that seems unlikely.
NB: I'm saying "frame" but we should see if there are differences in <embed> and <object> too.
See bug 1888695 for a similar issue with the XFO-blocked page
Reporter | ||
Updated•10 months ago
|
Assignee | ||
Comment 1•10 months ago
•
|
||
(In reply to Daniel Veditz [:dveditz] from comment #0)
If https-only is turned on and we're unable to upgrade a framed http: site, we show the "HTTPS-Only Mode Alert" error page. I don't think we want to do that because normally the mixed-content blocker would block that document with no fuss (other than a console log error).
I think I agree with that. We would still need to keep the logic for the error page though, as users can opt out of mixed content blocking per site via the lock icon in the address bar, and then we would still want this error page. I will ask Christoph if he knows about any edgecases because of which we currently still display this error page with mixed content blocking enabled.
Alternatively, we could also consider just dropping the option to exempt the top-level page from inside the iframe entirely, even with mixed content blocking disabled. That would make things a lot simpler for us, and may also help fix some other edgecases like the one in Bug 1887796. Users could then still disable HTTPS-Only for the top-level page via the lock icon, like they already have to do for other subresources like images. We could possibly also consider automatically adding a HTTPS-Only exception for the subresources if the user manually disables mixed content protection on a site through the lock icon.
A framed site could try to get its framer exempted, which might make it look broken if it was relying on https-only?
Note that due to permission implementation quirks, it can only exempt subresources of its framing top-level page (which with mixed content blocking means basically nothing). This is due to the exemption being added for the https:// principal of the top-level site, while regular HTTPS-Only exemptions have a http:// schemes (because that is the scheme that URLs of top-level loads have when HTTPS-Only needs to check if they are exempted).
Can you also CC me on Bug 1888695 you mentioned Dan?
Reporter | ||
Comment 2•10 months ago
|
||
Can you also CC me on Bug 1888695 you mentioned Dan?
Yes, sorry, thought I did that.
Assignee | ||
Updated•10 months ago
|
Assignee | ||
Comment 3•10 months ago
•
|
||
So I looked a bit further into the clickjacking part, and I don't think there is any risk here, as:
- As previously mentioned, the exception will only be added for the https:// principal, effectively only exempting subresource loads on the page from HTTPS-Only, not the top-level host itself
- Mixed Content Level 2 should then handle these subresources instead, either by blocking or upgrading them, which is almost the same behavior as HTTPS-Only. The only difference is that active mixed content won't be upgraded anymore.
So this would only be a problem when mixed content level 2 is disabled. For that case I have set up a small testcase here (also see the attached video), where the user could actually be tricked into allowing mixed passive content to load.
While writing that testcase, I also noticed two unrelated bugs, which I will file seperately:
- Despite a HTTPS-Only exception being added for the top-level page, it is not showing up in the lock icon, only in the settings (you can also see this in the video).
- The mixed content blocking opt-out in the lock icon ("Disable protection for now") doesn't actually seem to do anything when mixed content lvl2 is enabled. At least passive mixed content will still either be upgraded or blocked.
Assignee | ||
Comment 4•10 months ago
|
||
After talking with Christoph, the intent behind the HTTPS-Only error page in a iframe originally was probably to exempt the HTTP version of the top-level page, and then also reload it via HTTP. But that is not what was done in the end. And considering the clickjacking risk talked about in this bug, it probably also isn't a good idea to switch to that behavior now.
We instead came to the conclusion that it probably is most consistent to not display a button to exempt the top-level page from HTTPS-Only in the iframe. Instead, the user can disable HTTPS-Only under the lock icon, the same way they would need to do it for mixed content images that got blocked by HTTPS-Only. I will look into doing that.
Also slightly related: If we remove this button, there will be no way anymore to add a HTTPS-Only exception with a https:// scheme, i.e. excepting the subresources from HTTPS-Only on a secure page. In most other places this ability was already removed in Bug 1757297. So this change would also remove a edgecase we did not handle properly before.
Reporter | ||
Updated•10 months ago
|
Assignee | ||
Comment 5•9 months ago
|
||
Rationale for this can be read in Bug 1909396, but the main reason is that the iframe will get blocked regardless by mixed content blocking.
Comment 6•8 months ago
|
||
There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:maltejur, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.
Assignee | ||
Comment 7•8 months ago
|
||
The patch is still missing tests, I have set it to "changes planned" now.
![]() |
||
Comment 9•7 months ago
|
||
Comment 10•7 months ago
|
||
:dveditz should this ride the trains with Fx133 or should it be uplift to Fx132/esr128?
It's a sec-low and looks like it has string changes so double checking with you first
Reporter | ||
Comment 11•7 months ago
|
||
It should be fine to ride the trains and leave time for the localization process to work. If we do want to make sure this fix gets into ESR-128 we could easily write a variant patch without the explanation string. I'll defer to Freddy, though, about whether we do want to fix this there or not.
Updated•7 months ago
|
Updated•7 months ago
|
Updated•6 months ago
|
Reporter | ||
Updated•27 days ago
|
Description
•