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

Consume user activation #218

Open
wants to merge 14 commits into
base: gh-pages
Choose a base branch
from
Open

Consume user activation #218

wants to merge 14 commits into from

Conversation

marcoscaceres
Copy link
Member

@marcoscaceres marcoscaceres commented Oct 14, 2022

Closes #210

The purpose of this change is to somewhat mitigate the following abuse case, which could be used for hypothetical fingerprinting attack. It could also just generally annoy users if web applications can randomly change orientation while in fullscreen.

await video.requestFullscreen();
// now quickly before user notices, switch all orientations  
for (const orientation of allOrientationsInTheSpec) {
  // Scan the user's supported orientations
  // to add to fingerprint
  const support = {yep: [], nope: []};
  try {
      await screen.orientation.lock(orientation);
      support.yep.push(orientation)
   } catch {
      support.nope.push(orientation)
   }
}

The following tasks have been completed:

  • Modified Web platform tests (link to pull request)

Implementation commitment:


Preview | Diff

@marcoscaceres
Copy link
Member Author

@martinthomson, @saschanaz, we are contemplating doing this in WebKit, so are seeking input from a (potential) second implementation. I'm wondering if you'd be interested in making this change also in Gecko?

It's a potentially breaking change, but I think it's worth it to mitigate some of the above.

As an aside, I've also specified Gecko's mitigations for resisting fingerprinting #215

@saschanaz
Copy link
Member

I don't have any objection, but I'd like to see the pioneer's experience before implementing it in Gecko 🙂

cc @smaug----

@smaug----
Copy link

I am having trouble to understand why one would need transient activation for this. At least I wouldn't implement this before there is some telemetry data about the current usage.

@saschanaz
Copy link
Member

saschanaz commented Oct 24, 2022

Hmm yeah, have we actually observed any abuse or is this just to prevent potential abuse? I do see the point to prevent fingerprinting, but it would be very visible for users, no?

@marcoscaceres
Copy link
Member Author

marcoscaceres commented Oct 25, 2022

@smaug---- wrote:

I am having trouble to understand why one would need transient activation for this.

For example while(true) await screen.orientation.lock(randomOrientaiton()).

I actually accidentally triggered this behavior with a test and it's extremely annoying to break out of on a mobile device:

// Orientation loop!  
screen.orientation.onchange = async () => {
     // the event will fire again ... and again... 
     await screen.orientation.lock(getOppositeOrientation());
}
await screen.orientation.lock(getOppositeOrientation());

Hmm yeah, have we actually observed any abuse or is this just to prevent potential abuse? I do see the point to prevent fingerprinting, but it would be very visible for users, no?

No necessarily. If you make the background black, or if the UA doesn't animate (e.g., reduce animation) when switching orientation, the user might not notice anything.

@marcoscaceres
Copy link
Member Author

marcoscaceres commented Oct 25, 2022

In case you want to see how annoying it is (try it on Android):
https://marcoscaceres.github.io/playground/annoying.html

@smaug----
Copy link

right, and then I wouldn't use that site again. Sites can do many annoying things.
Do we have any telemetry data whether this is actually web compatible?

@saschanaz
Copy link
Member

saschanaz commented Oct 25, 2022

In case you want to see how annoying it is (try it on Android):
https://marcoscaceres.github.io/playground/annoying.html

This somehow throws NotSupportedError on Firefox for Android 🤔

@marcoscaceres
Copy link
Member Author

marcoscaceres commented Oct 27, 2022

@smaug---- wrote:

right, and then I wouldn't use that site again.

That seems really unfair on users and an extremely privileged position to take: users don't always have a choice as to which sites they use. Sometimes there is no alternative (e.g., government websites, specific mail provider, work related, etc.).

We have the opportunity for sites to do the right thing here.

Sites can do many annoying things.

Sure, but that's not a reason to keep the status quo or to make things worse. By that logic, we should have never added SecureContext to Geolocation or gated any API on user activation because "the web does annoying things" 🤷. That seems counter productive.

We should be aiming to make things better when we spot issues (even if it breaks a few sites).

Do we have any telemetry data whether this is actually web compatible?

No, but given the API is gated already on fullscreen, which requires transient activation, it should be broadly compatible.

I don't think I'd ship this in webkit without this mitigation, for the reasons I already gave.

I'm inviting other browser vendors to make what are pretty reasonable changes (along with #230).

@marcoscaceres
Copy link
Member Author

@saschanaz wrote:

This somehow throws NotSupportedError on Firefox for Android

I spotted that too. That's it's already not interoperable across browsers may be our saving grace 😇.

@marcoscaceres
Copy link
Member Author

Oh! so no joke... It looks like locking might be disabled by default on Firefox for Android 🤯.

See:
https://searchfox.org/mozilla-central/source/modules/libpref/init/StaticPrefList.yaml#3362

And see below for where the NotAllowedError is coming from a few lines below:
https://searchfox.org/mozilla-central/source/dom/base/ScreenOrientation.cpp#334

@saschanaz, can you check if dom.screenorientation.allow-lock is also false for you too?

@marcoscaceres
Copy link
Member Author

Ok, enabling it also starts rejecting with AboutErrors, which is kinda what we want.

I think it's rejecting the in Gecko because calling .lock() again inside the event handler is causing the AbortError to be returned (as the first promise hasn't settled).

@saschanaz
Copy link
Member

Oh, https://bugzilla.mozilla.org/show_bug.cgi?id=1767449 tracks that, thanks. Didn't know that!

Given that Fullscreen API does not consume the activation, I think this should have a telemetry so that we can know what websites will break when screen locking consumes it.

I think it's rejecting the in Gecko because calling .lock() again inside the event handler is causing the AbortError to be returned (as the first promise hasn't settled).

Confirmed. Can we spec this instead? 😂

@marcoscaceres
Copy link
Member Author

As an aside, I updated all the WPT tests a few days ago. I think this is the relevant test:
https://wpt.live/screen-orientation/lock-unlock-check.html

@marcoscaceres marcoscaceres changed the title Consume transient activation Consume user activation Nov 1, 2022
@marcoscaceres
Copy link
Member Author

@smaug----, @saschanaz, here is another case where it might make sense to consume the user activation: whatwg/html#8490

Would appreciate your input.

@marcoscaceres
Copy link
Member Author

just fyi, WebKit doesn't have a means to collect telemetry, but I still feel pretty strongly that the API should require and consume user activation. There is no valid use case for that I can think of for allowing a site to continually switch screen orientations without a gesture.

@michaelwasserman
Copy link
Member

Would this break entering fullscreen and locking orientation with one activation if requestFullscreen consumes the gesture per whatwg/fullscreen#153 ?

@LJWatson
Copy link

...I actually accidentally triggered this behavior with a test and it's extremely annoying to break out of on a mobile

Something not everyone will be able to do. People with physical disabilities, particularly those that affect upper body movement and mobility, may have their device attached (to their wheelchair for example) in a fixed position, and so turning their device to match the orientation is not an option.

@michaelwasserman
Copy link
Member

Thanks for your work here, @marcoscaceres. We should absolutely prevent abuse akin to your compelling example.

The most apparent risk is breaking uses of Element.requestFullscreen() and Screen.Orientation.lock() from one transient user activation. Long term, whatwg/fullscreen#186 seems nice, but we should avoid breaking existing sites. My best naive idea is an internal slot to permit locking orientation without transient user activation shortly after a fullscreen request (akin to the slot discussed in whatwg/html#8490).

I'm not aware of other use cases for locking orientation without consuming a transient user activation (e.g. auto-advancing media queue, games, mapping). Still, attempting to measure potential impact seems like a prerequisite to making a potentially disruptive change. Collecting new Chromium metrics beyond the basic use counter might help.

@marcoscaceres
Copy link
Member Author

Resolution: Let's just spec potential mitigation and not do this (and suggest rate limiting)... and maybe push whatwg/fullscreen#186

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

Successfully merging this pull request may close these issues.

Require a gesture?
5 participants