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

Polling enumerateDevices potentially being a fingerprint. #403

Closed
jan-ivar opened this issue Sep 28, 2016 · 11 comments
Closed

Polling enumerateDevices potentially being a fingerprint. #403

jan-ivar opened this issue Sep 28, 2016 · 11 comments
Assignees

Comments

@jan-ivar
Copy link
Member

I raise this separately from #402 (comment) where @foolip discovered this, as the problem is separate, and we may wish to solve one or the other or both (if possible).

This is the same problem as with the devicechange event, except not remedied by #333.

Polling enumerateDevices is also quite expensive, so we want to discourage people from polling it. One way to do that would be to remove any advantage over the devicechange event.

One way to solve this might be to put the same limitations on enumerateDevices as on the event (as I suggest in #402 (comment)), though at the risk of people not trusting enumerateDevices. We could go one step further and have getUserMedia adhere to the old list of devices as well, that way all behavior is in sync so to speak, although such an illusion would shatter quickly once a USB device is removed, and getUserMedia fails with NotFoundError, even though enumerateDevices insists there is a device on the system.

@alvestrand
Copy link
Contributor

So I think this can be closed if we find a satisfactory solution to #402, and we specify that the devicechange event is always fired before enumerateDevices() returns new information, right?
(A consistent - but - strange implementation is that if the set of devices changes, and devicechange is not fired immediately, enumerateDevices() would delay resolving the promise for the appropriate amount of time, and then fire the devicechange event just before resolving the promise.)

@jan-ivar
Copy link
Member Author

jan-ivar commented Oct 5, 2016

They do seem dependent, but I would leave this open until we have it covered.

@jan-ivar
Copy link
Member Author

jan-ivar commented Oct 5, 2016

@alvestrand Not sure how long enumerateDevices() takes today, but if we delay it until the arrival of the event, then that may be detectable. It might be better to return with old info. In any case, the spec isn't likely to specify how fuzzing should be done in detail, so we can leave that to the implementations.

@alvestrand
Copy link
Contributor

@jan-ivar on Chrome Linux, powerful machine with 1 camera -- around 5 ms.
If we fuzz, delay will be detectable. But if it says in the spec that "fuzzing may be applied", then that shouldn't be a surprise to anyone.

@jan-ivar
Copy link
Member Author

jan-ivar commented Oct 6, 2016

I don't think we should time-fuzz enumerateDevices() in normal operation, as that would slow down sites in their camera and mic acquisition (if done properly where they check what the user has first), and if we time-fuzzed only when an event fired, then JS would know an event has fired, and we're back to where we started. If we return old info instead then nothing is observable, so that seems preferable.

@ShijunS
Copy link
Contributor

ShijunS commented Dec 12, 2016

To facilitate the discussion, let's summarize how enumerateDevices() works based on current text.

  • on page load, we create [[storedDeviceList]] and set it to null
  • every time before we fire deviechange event, we set the [[storedDeviceList]] to null
  • when [[storedDeviceList]] is null, enumerateDevices() will actually enumerate the devices and set the[[storedDeviceList]] accordingly
  • when [[storedDeviceList]] is not null, we will resolve enumerateDevices() based on [[storedDeviceList]]

It seems possible to mitigate the perf overhead if apps repeatedly polling enumerateDevices(). It won't give any different results anyway unless the deviechange event has been fired.

One issue as @jan-ivar pointed out is that the [[storedDeviceList]] could be out of sync when a page didn't get deviechange event. The question is whether that is a reasonable compromise. Meanwhile, if issue #414 is resolved with a "MAY", the issue here can potentially be further mitigated in implementations.

@jan-ivar
Copy link
Member Author

#414 is a misunderstanding IMHO (it is a MUST).

@jan-ivar
Copy link
Member Author

@ShijunS Didn't we solve this in #407 ? Waht issue remains?

@aboba
Copy link
Contributor

aboba commented Dec 12, 2016

@jan-ivar PR #407 was closed, not merged.

@aboba
Copy link
Contributor

aboba commented Dec 12, 2016

@jan-ivar @ShijunS Was this fixed in PR #412 ?

@ShijunS
Copy link
Contributor

ShijunS commented Dec 12, 2016

Yup, it is in PR #412. @jan-ivar, I'd agree closing this issue. I don't see anything critical remaining.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants