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

Drop own powerful feature, require active local video source. #83

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rakuco
Copy link
Member

@rakuco rakuco commented Dec 13, 2022

Fixes #79. See w3c/sensors#451 for the Generic Sensors side.

The idea is that, based on the research on potential attacks on the Ambient Light Sensor API, it is important to prompt users before allowing access to illuminance readouts. This was already mandated by the main Generic Sensors spec, as Sensor.start() runs the "Request sensor access" abstract operation.

The challenge with the Ambient Light Sensor API is prompting users in a way that they understand what they are being prompted for; the assumption is that most users are not familiar with what an Ambient Light sensor is.

We have chosen to solve this issue by dropping our "ambient-light-sensor" powerful feature name altogether and integrating with the Media Capture and Streams specification instead: we consider an Ambient Light Sensor to be a 1x1 camera and require there to be at least one local camera source that is not muted or stopped in order for illuminance readouts to be provided.

Per the Media Capture and Streams specification, this is only possible if script has called MediaDevices.getUserMedia() and granted the "camera" permission. This also means the User Agent has at least indicated to the user that a local camera source has started being used.

In other words, an Ambient Light Sensor only provides readings if a local camera source is currently active and being used in the same window as the AmbientLightSensor instance, and when all local camera sources stop we also stop providing readouts and fire an "error" with a NotReadableError exception.


Preview | Diff

@rakuco
Copy link
Member Author

rakuco commented Dec 13, 2022

@reillyeon @anssiko please take a look at this first draft.

The Media Capture references are a bit hand-wavy because the spec itself is not very precise about its definitions of source and device or the way notifications about changes to the [[devicesLiveMap]] slot are supposed to be delivered.

Tying an AmbientLightSensor instance to a specific MediaStreamTrack would allow us to be a bit more precise in how we track the lifetime of the resources we need at the expense of making the API slightly more cumbersome.

I'd appreciate it if you can CC one or more people with more expertise in the Media Capture area to see if this approach makes sense.

@rakuco
Copy link
Member Author

rakuco commented Dec 13, 2022

Also cc @willmorgan

Copy link
Member

@anssiko anssiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM!

I introduced this ALS Media Capture integration proposal (in abstract, based on earlier discussions we had in the WG) at the recent W3C Workshop on Permissions. The proposal spurred active discussion and a separate breakout was created to explore permission grouping. I will report back when the proceedings from that workshop are published. I feel this work should proceed to feed data into the broader discussion.

I consider this integration a low-hanging fruit what comes to permission bundling (to use the workshop terminology). ALS is a "1x1 grayscale camera" that is a proper subset of "camera". Subsetting is arguably not as seamless for some other capabilities.

index.bs Outdated Show resolved Hide resolved
@rakuco
Copy link
Member Author

rakuco commented Dec 14, 2022

cc @lknik too

@lknik
Copy link

lknik commented Dec 15, 2022

So what's the mechanics of "require there to be at least one local video source that is not muted or stopped"? From as user's point of view is the query overtly about the camera?

@rakuco
Copy link
Member Author

rakuco commented Dec 15, 2022

So what's the mechanics of "require there to be at least one local video source that is not muted or stopped"? From as user's point of view is the query overtly about the camera?

From a user's point of view the idea is that this represents a camera, or something akin to it that also causes browsers to show an indicator to users that video capture is happening and that fulfills the {video: true} constraint when it is passed to getUserMedia().

"source" is defined here, "local video" is a term I am not sure about to be honest.

Another way to look at it is that ALS readouts should be provided only when UAs also show users that video capture is happening according to https://w3c.github.io/mediacapture-main/#privacy-indicator-requirements (I am still not sure if this spec should look at the [[devicesLiveMap]] internal slot or anyVideoLive) and the Generic Sensor constraints are also fulfilled (so video capture is happening on the same page as the sensor instance, and the page is visible and same origin-domain with the focused page).

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
rakuco added a commit to rakuco/sensors that referenced this pull request Dec 16, 2022
See the discussion in w3c/ambient-light#83 -- "local video source" could end
up including screen capture sessions.
@rakuco
Copy link
Member Author

rakuco commented Dec 16, 2022

Thanks for the reviews. I'd be glad if you have contacts who work on Media Capture who could help check these changes and suggest the best wording here.

@noamr
Copy link

noamr commented Dec 17, 2022

Some drive-by comments (I'm not a reviewer to either of these specs, but I ran into it at the W3C permissions workshop):

  • Usually I read "editorial" as changing wording without changing meaning/behavior. This seems like a big change in behavior. Are we using the word differently in different specs?
  • The use-cases in the spec contain some entries that this would not work with - smart home, lighting at the work place... Seems like this is about changing the purpose of the ALS API to be more of an API that supplements video captures (the 3rd and 4th use cases). So I would expect that this change modifies the use cases, and focuses on them in the examples.
  • I don't think we should treat the ALS as a 1x1 camera. Because:
    1. It feels wrong (to me, though others at the permissions workshop indicated a similar sentiment).
    2. Do users know where that 1x1 camera is on their phone/laptop? I don't. I can cover or turn my camera away but I have no idea how to physically do that with my ambient light sensor. So if I start a video stream and then physically cover my camera for part of it, the stream would still include ambient light readings, which is probably not what I intended.
    3. Purpose: cameras measure what you show them, ALS is ambient by definition, meaning it is meant to capture light without user involvement.

Suggestion: If the idea here is that you'd get an ambient-light stream together with your video stream (which is kind of how I understand this change, with the only example provided), perhaps a more direct way to do it is that AL would be an additional MediaStreamTrack rather than a bespoke JS API? I think this still has some privacy/permission issues, but at least it would be more purpose-driven than capability-driven.

@alvestrand
Copy link

Seems somewhat scary - this (treating the light sensor as an 1x1 camera) would mean that the red "I am capturing you" light would light up on all tabs that access the ambient light sensor, which is usually a fairly strong indication of privacy exposure. We risk numbing the user to the red light indicator "because it's so frequently on".

@alvestrand
Copy link

We do have a number of features (IP address exposure, device enumeration) that have their permissions tied back to the camera permission, without being a camera. Copying that kind of access is a possibility.

@noamr
Copy link

noamr commented Dec 19, 2022

We do have a number of features (IP address exposure, device enumeration) that have their permissions tied back to the camera permission, without being a camera. Copying that kind of access is a possibility.

Device enumeration is tied to the camera only when you try to enumerate the camera, no? https://w3c.github.io/mediacapture-main/#camera-information-can-be-exposed

Which API is "IP address exposure"?

@rakuco
Copy link
Member Author

rakuco commented Dec 19, 2022

Seems somewhat scary - this (treating the light sensor as an 1x1 camera) would mean that the red "I am capturing you" light would light up on all tabs that access the ambient light sensor, which is usually a fairly strong indication of privacy exposure. We risk numbing the user to the red light indicator "because it's so frequently on".

"Treating the light sensor as a 1x1 camera" was used in the sense of "we want the privacy requirements to be as strict as if an actual camera was being used".

However, this spec does not request permission, but only requires it. In other words, it cannot turn on the red "I am capturing you" light on its own -- attempting to start the sensor without having camera capture already happening will result in an error. In this example, the red indicator would light up on multiple tabs if and only if all these tabs are also capturing video.

@rakuco rakuco changed the title Editorial: Drop own powerful feature, require active local video source. Drop own powerful feature, require active local video source. Dec 19, 2022
@rakuco
Copy link
Member Author

rakuco commented Dec 19, 2022

Hey @noamr, thanks for the comments. The more feedback we get about this change, the better.

Some drive-by comments (I'm not a reviewer to either of these specs, but I ran into it at the W3C permissions workshop):

  • Usually I read "editorial" as changing wording without changing meaning/behavior. This seems like a big change in behavior. Are we using the word differently in different specs?

My bad. Looking at https://www.w3.org/2021/Process-20211102/#correction-classes again it does look like "editorial" does not apply here. I've updated the PR title and the relevant commit messages.

[...]

Suggestion: If the idea here is that you'd get an ambient-light stream together with your video stream (which is kind of how I understand this change, with the only example provided), perhaps a more direct way to do it is that AL would be an additional MediaStreamTrack rather than a bespoke JS API? I think this still has some privacy/permission issues, but at least it would be more purpose-driven than capability-driven.

The idea of integrating ALS into getUserMedia() was discussed in #64 (comment) and #79 (comment).

At the very least, this would require much bigger changes in the (current) implementation, as //services/device/generic_sensor and the Media Capture stack in Chromium do not communicate at all. There's also the question of whether data should be provided as a stream or only when a certain threshold is reached. Finally, if we define a new MediaStreamTrack type for ALS and require developers to do something like getUserMedia({video: true, als: true}), don't you end up in the same situation where you need to figure out a way to prompt users for access and explain to them what an ambient light sensor is?

You raise some interesting points about why an ALS shouldn't be treated like a camera, but I'd like to understand if going down the getUserMedia() route really makes sense or not.

rakuco added a commit to rakuco/sensors that referenced this pull request Jan 17, 2023
See the discussion in w3c/ambient-light#83 -- "local video source" could end
up including screen capture sessions.
rakuco added a commit to rakuco/sensors that referenced this pull request Jan 17, 2023
See the discussion in w3c/ambient-light#83 -- "local video source" could end
up including screen capture sessions.
@lknik
Copy link

lknik commented Jan 18, 2023

So what's the mechanics of "require there to be at least one local video source that is not muted or stopped"? From as user's point of view is the query overtly about the camera?

From a user's point of view the idea is that this represents a camera, or something akin to it that also causes browsers to show an indicator to users that video capture is happening and that fulfills the {video: true} constraint when it is passed to getUserMedia().

OK. In other words - the webpage is capturing the video, anyway, is that right? There may be a some constraints as 'where the camera is pointing and whether it's the same direction as ALS', but it's probably not necessary to go in such details.

@rakuco
Copy link
Member Author

rakuco commented Jan 18, 2023

OK. In other words - the webpage is capturing the video, anyway, is that right?

Correct. Illuminance data is reported if and only if the web page is capturing video from a camera.

Fixes w3c#79.

The idea is that, based on the research on potential attacks on the Ambient
Light Sensor API, it is important to prompt users before allowing access to
illuminance readouts. This was already mandated by the main Generic Sensors
spec, as `Sensor.start()` runs the "Request sensor access" abstract
operation.

The challenge with the Ambient Light Sensor API is prompting users in a way
that they understand what they are being prompted for; the assumption is
that most users are not familiar with what an Ambient Light sensor is.

We have chosen to solve this issue by dropping our "ambient-light-sensor"
powerful feature name altogether and integrating with the Media Capture and
Streams specification instead: we consider an Ambient Light Sensor to be a
1x1 camera and require there to be at least one local video source that is
not muted or stopped in order for illuminance readouts to be provided.

Per the Media Capture and Streams specification, this is only possible if
script has called `MediaDevices.getUserMedia()` and granted the "camera"
permission. This also means the User Agent has at least indicated to the
user that a local video source has started being used.

In other words, an Ambient Light Sensor only provides readings if a local
video source (such as a camera) is currently active and being used in the
same window as the AmbientLightSensor instance, and when all local video
sources stop we also stop providing readouts and fire an "error" with a
NotReadableError exception.

The Use Cases section had to be shortened, as some items described there no
longer make much sense when a camera is required.
The last example was removed because it showed a use case that was deleted
in the previous commit, as it no longer seems very relevant with the new
media capture requirements.
rakuco and others added 2 commits January 18, 2023 17:21
Co-authored-by: Reilly Grant <reillyeon@users.noreply.github.com>
This avoids ambiguity, including the fact that a screen capture session
could qualify as a "local video source".

We really want to tie this API's permission model to the presence of an
active camera specifically.
@rakuco
Copy link
Member Author

rakuco commented Jan 18, 2023

I've pushed a few changes to the Use Cases and Examples sections to remove the bits that do not make sense with the new camera requirement. The Use Cases list got quite small, so I'd appreciate it if people (like @willmorgan for example :-) could help think of a few more ideas to add there.

This addresses most of @noamr's points. IMO the remaining ones are "users do not know where their ambient light sensor is located" and "expose illuminance as a new MediaStreamTrack". I feel like doing the latter wouldn't help with the former. I couldn't find any examples of new MediaStreamTrack types that are not video or audio (unless I misunderstood the suggestion), and it is still unclear how to deal with prompting users if we do that.

@anssiko
Copy link
Member

anssiko commented Feb 8, 2023

I will report back when the proceedings from that workshop are published.

Published: https://www.w3.org/Privacy/permissions-ws-2022/report

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.

Add camera permission requirement to spec?
6 participants