Get rid of permission for DOM fullscreen

VERIFIED FIXED in Firefox 42

Status

()

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

(Blocks 2 bugs)

Trunk
Firefox 42
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox42 verified)

Details

Attachments

(3 attachments, 1 obsolete attachment)

After we implement the new transition for DOM fullscreen, we should be able to get rid of permission management for that API.
Depends on: 1160023
Blocks: 1160023
No longer depends on: 1160023
Blocks: 1183420
Assignee: nobody → quanxunzhen
Attachment #8633873 - Flags: review?(bugs)
I won't land this patchset before bug 1160014 finishes.
Comment on attachment 8633874 [details] [diff] [review]
patch 2 - remove permission from browser part

There are more stylesheets using the obscure-browser attribute. Please update those too.
Attachment #8633874 - Flags: review?(dao) → review-
Attachment #8633875 - Flags: review?(margaret.leibovic) → review+
Attachment #8633874 - Attachment is obsolete: true
Attachment #8634006 - Flags: review?(dao)
Attachment #8634006 - Flags: review?(dao) → review+
Comment on attachment 8633873 [details] [diff] [review]
patch 1 - remove permission from dom part

So someone from the security team has approved this new a bit scary behavior?
Attachment #8633873 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #7)
> Comment on attachment 8633873 [details] [diff] [review]
> patch 1 - remove permission from dom part
> 
> So someone from the security team has approved this new a bit scary behavior?

Yes, with the transition implemented in bug 1160014, which makes it clearer for entering fullscreen.
Blocks: 1121626
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #8)
> (In reply to Olli Pettay [:smaug] from comment #7)
> > Comment on attachment 8633873 [details] [diff] [review]
> > patch 1 - remove permission from dom part
> > 
> > So someone from the security team has approved this new a bit scary behavior?
> 
> Yes, with the transition implemented in bug 1160014, which makes it clearer
> for entering fullscreen.

Cool. To make it easier for interested folks, can you point us to exactly the place / comment where this got security sign-off?
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #9)
> Cool. To make it easier for interested folks, can you point us to exactly
> the place / comment where this got security sign-off?

It seems to me this decision has not been not published anywhere yet. Sheila Mooney told us in a private email that Daniel Veditz and Richard Barnes have agreed with this change. ni? them for confirmation.
Flags: needinfo?(smooney)
Flags: needinfo?(rlb)
Flags: needinfo?(dveditz)
Please review this change as part of a fullscreen re-implementation as discussed with the UX and Security teams and described with a video posted here:

http://people.mozilla.org/~mverdi/projects/fullscreen/

While this change might appear to be a reduction in security, the ultimate goal is to reduce reliance on plugins for fullscreen videos & games--a much bigger security win.
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #8)
> (In reply to Olli Pettay [:smaug] from comment #7)
> > Comment on attachment 8633873 [details] [diff] [review]
> > patch 1 - remove permission from dom part
> > 
> > So someone from the security team has approved this new a bit scary behavior?
> 
> Yes, with the transition implemented in bug 1160014, which makes it clearer
> for entering fullscreen.

While I am all for reducing permissions prompts, the fullscreen one was never that odious (especially since it only showed up after the fullscreen effect already happened).  I don't see how a transition reduces security concerns, though.  If the user is sitting at the computer, then yes, they'd notice a transition happened (but if it transitioned to something that looked suspiciously like a desktop, would they realize it's a web page).

However, it's trivial to have a good idea of when the user is inactive.  Waiting until the user has been inactive for, say, 30 minutes and then fullscreen-transitioning to something that looks like an OS-appropriate password prompt seems like a pretty trivial spoof.  Is this something we're comfortable with being a non-issue?
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #12)
> However, it's trivial to have a good idea of when the user is inactive. 
> Waiting until the user has been inactive for, say, 30 minutes and then
> fullscreen-transitioning to something that looks like an OS-appropriate
> password prompt seems like a pretty trivial spoof.  Is this something we're
> comfortable with being a non-issue?

We could probably display the "foo.com is now fullscreen" message until the user becomes active...
(In reply to Dão Gottwald [:dao] from comment #13)
> (In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #12)
> > However, it's trivial to have a good idea of when the user is inactive. 
> > Waiting until the user has been inactive for, say, 30 minutes and then
> > fullscreen-transitioning to something that looks like an OS-appropriate
> > password prompt seems like a pretty trivial spoof.  Is this something we're
> > comfortable with being a non-issue?
> 
> We could probably display the "foo.com is now fullscreen" message until the
> user becomes active...

That's a great idea.  Make sure that message stays on the screen and kick off the timer to fade it out only when user activity is seen.
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #14)
> (In reply to Dão Gottwald [:dao] from comment #13)
> > We could probably display the "foo.com is now fullscreen" message until the
> > user becomes active...
> 
> That's a great idea.  Make sure that message stays on the screen and kick
> off the timer to fade it out only when user activity is seen.

I like it too. I would start the timer from the last user gesture (ie. the click that entered fullscreen.) If that timer expires because fullscreen was delayed (eg. in the attack described,) to start the timer from the first user gesture after fullscreen was entered. I think this nicely keeps the friction low while protecting from spoofing.
Need to have some way to hide the warning using keyboard too, but yes, sounds like a rather nice UI.
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #12)
> While I am all for reducing permissions prompts, the fullscreen one was
> never that odious (especially since it only showed up after the fullscreen
> effect already happened).  I don't see how a transition reduces security
> concerns, though.  If the user is sitting at the computer, then yes, they'd
> notice a transition happened (but if it transitioned to something that
> looked suspiciously like a desktop, would they realize it's a web page).

We still shows a fullscreen warning.

> However, it's trivial to have a good idea of when the user is inactive. 
> Waiting until the user has been inactive for, say, 30 minutes and then
> fullscreen-transitioning to something that looks like an OS-appropriate
> password prompt seems like a pretty trivial spoof.  Is this something we're
> comfortable with being a non-issue?

Are we able to address this issue now with the permission requirement? If a website, which has been in fullscreen, wants, it can always switch to a UI like that.

Note that we still require one user input for requesting fullscreen, which means website is still not able to enter fullscreen without user's action, and transition enables users to see clearly how the change is related to their action.

(In reply to Dão Gottwald [:dao] from comment #13)
> We could probably display the "foo.com is now fullscreen" message until the
> user becomes active...

We can't. If a user is watching a movie, he/she may not interact with any input device for a very long time. We must not show such message for that. And also it is not possible for us to distinguish whether a user is still there, or has left the computer for a while, unless we do some face recognition from the camera.
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #17)
> (In reply to Dão Gottwald [:dao] from comment #13)
> > We could probably display the "foo.com is now fullscreen" message until the
> > user becomes active...
> 
> We can't. If a user is watching a movie, he/she may not interact with any
> input device for a very long time. We must not show such message for that.

In order to go fullscreen while watching a movie, users usually interact with the UI, which means we'd already consider them "active" at that point.
(In reply to Dão Gottwald [:dao] from comment #18)
> 
> In order to go fullscreen while watching a movie, users usually interact
> with the UI, which means we'd already consider them "active" at that point.

We always require a user input event for entering fullscreen. Website can never go to fullscreen if user doesn't do anything within one second before.

The patches here just removes the additional step where user needs to explicitly approve the permission.

The logic is that, if the transition and the warning are obvious enough, the website would not be able to control the screen without user's awareness, and thus we do not need to ask for explicit approval in addition.
This means the scenario from comment 12 isn't actually possible?
I thought the scenario from comment 12 is about, a website has entered fullscreen, and then turns to some spoof interface later, which is a case inevitably vulnerable even we ask for approval.

If that comment is about that a website can enter fullscreen anytime it wants, which is currently only blocked by the permission approval, then I have to say it is not that case.
OK, I've finished all patches for bug 1160014 (though Mac one and GTK one are still pending for review). Per my discussion with jet today, I'm going to land this bug first now.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b76ca34d720
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #10)
> (In reply to Mike Conley (:mconley) - Needinfo me! from comment #9)
> > Cool. To make it easier for interested folks, can you point us to exactly
> > the place / comment where this got security sign-off?
> 
> It seems to me this decision has not been not published anywhere yet. Sheila
> Mooney told us in a private email that Daniel Veditz and Richard Barnes have
> agreed with this change. ni? them for confirmation.

I thought you were part of the conversation too? Can't remember if it was in a bug or a mail thread, but we approved it given a mock up of a transition that showed
 * user action required
 * a transition effect
 * a notice that XXX has gone fullscreen that stays on screen for a couple of seconds (1.5? 3?)
   The mockup had a specific behavior that tried to strike a balance between annoying enough
   to be noticed but not _too_ annoying.
Flags: needinfo?(dveditz)
So this effectively forces me to watch the annoying fullscreen warning every single time? The first thing I did when HTML5 videos became available is disable it via full-screen-api.approval-required;false.
Why not provide a "don't warn me again" checkbox like the norm is for annoying nonsense like this.
Comment 25 has answered my question in comment 10. Remove all the ni?s.
Flags: needinfo?(smooney)
Flags: needinfo?(rlb)
Blocks: 857494
Flags: qe-verify+
Verified on Firefox 42 beta 8 (20151019161651) under Windows 7 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.10.4 and the fullscreen modal dialog looks like: http://i.imgur.com/0vpIGXj.jpg .

I’m not sure if this is the expected design because it does not correspond to the mockups from Comment 11 ( http://people.mozilla.org/~mverdi/projects/fullscreen/ ). 


Xidorn, could you please confirm that this ( http://i.imgur.com/0vpIGXj.jpg ) is the intended design for Firefox 42?
Flags: needinfo?(quanxunzhen)
Yes, I think this is what we are shipping for Firefox 42, because I failed to land bug 1160023 on this version.
Flags: needinfo?(quanxunzhen)
Thanks Xidorn for your prompt answer! 

Based on Comment 28 and Comment 29, I am marking this bug as Verified Fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.