w3ctag / design-reviews

W3C specs and API reviews
Creative Commons Zero v1.0 Universal
332 stars 56 forks source link

Sensor APIs #207

Closed anssiko closed 4 years ago

anssiko commented 7 years ago

Dear TAG!

We're requesting a TAG review of:

And the following concrete sensors that extend the Generic Sensor API:

Further resources:

Further details (optional):

You should also know that...

We'd prefer the TAG provide feedback as (please select one):

anssiko commented 6 years ago

(We now have a polyfill too: https://github.com/kenchris/sensor-polyfills)

annevk commented 6 years ago

I'm wondering what you ask the user here. Given the the amount of data these sensors can leak, it seems really hard to come up with a question.

anssiko commented 6 years ago

@annevk, any such user-facing strings are implementation details, but if you're interested in the Chrome's UX, see the public UX document. It should be noted that while the Permissions registry specifies low level permission names for these sensors, implementations are encouraged to coalesce them into appropriate higher level concepts familiar to the user as needed. For example, it'd be rather bad UX to ask the user: "annevankesteren.nl wants to access your gyroscope [Block] [Allow]"

If your question is more broadly about the security and privacy protections in place, it is better answered by the security and privacy assessment linked to from the initial TAG review request and the quite extensive security and privacy section of the Generic Sensor API spec and the equivalent sections in the derived specs for sensor-specific considerations, risks, and mitigation strategies.

Thanks for reviewing this work. We'd warmly welcome further feedback from Mozilla to make sure you'd feel good about implementing these APIs in the future.

dbaron commented 6 years ago

Let me restate @annevk's question another way: How do you ask the user a question that is both concise and accurate enough that the user's consent is meaningful? Will the user understand, for example, that accurate accelerometer, gyroscope, and magnetometer measurements might reveal all the text that the user types while they're being measured, or what sorts of activities the user is doing, and perhaps their location (based on mapping a pattern of movement to paths that exist on a map)?

anssiko commented 6 years ago

@dbaron, we have engineering English strings in place currently (see the above public UX document), but I can't speak on behalf of Chrome UX folks on the language to be used in production. Maybe @raymeskhoury can talk to that.

What I can say is, we're implementing in Chromium sensor indicators that are shown in omnibox, disclosure UI (nonintrusive, can be dismissed with or without user action), origin-specific and global permission settings that provide users ability to control access to sensors.

These are the planned UI improvements. Together with the identified and specified normative mitigation strategies, the group feels this is quite an improvement over the currently widely shipping DeviceOrientationEvent and DeviceMotionEvent specifications.

Thanks @dbaron and @annevk for asking good questions!

raymeskhoury commented 6 years ago

@dbaron - In Chrome the current (not fully implemented) plan is to ship these sensors on by default, such that users will not be asked for consent. The reasoning being that we've attempted to throttle frequency/resolution to levels where the attacks you mention would be infeasible. Our security/privacy teams felt comfortable that the risk was low enough that it would be ok to enable these sensors by default with indicators that allow users to opt-out.

We agree that it would be very hard to present a prompt to the user in an easily understandable way and this was part of our motivation for trying to avoid a prompt.

annevk commented 6 years ago

Thanks, it'd be interesting to hear from the security researchers that found the original flaws if that is indeed sufficient.

I also think that the specification should define the maximum allowed frequency to ensure end user security and interoperability. (There's some text on this, but it's not specific.)

Also, what is the plan for the various legacy APIs in this area? Is the hope that those can be removed somehow? Are there metrics to back that up? Or will we end up with duplicate API surface?

annevk commented 6 years ago

With thresholds on things like accelerometer and gyroscope (which don't seem to mention any throttling by the way; known issue?), how useful are they still for WebVR and such?

anssiko commented 6 years ago

Also, what is the plan for the various legacy APIs in this area? Is the hope that those can be removed somehow? Are there metrics to back that up? Or will we end up with duplicate API surface?

@annevk, the Devices and Sensors Working Group intends to fix remaining known issues with the legacy DeviceOrientation Event spec and finish its progress on the Recommendation track (see the draft Charter). As long as there are multiple shipping implementations, it is worthwhile to try fix the legacy spec.

The endgame would be that all the implementers eventually ship the new API surface, and the legacy APIs can be deprecated for good. Over the transition period, there will be some duplication to give web developers adequate time to migrate to the new APIs. The deprecation of the legacy APIs should be easier eventually, since they're just events that'd stop firing.

raymeskhoury commented 6 years ago

Thanks, it'd be interesting to hear from the security researchers that found the original flaws if that is indeed sufficient.

Feel free to reach out to them. @noncombatant did the auditing on the threshholds for us

I also think that the specification should define the maximum allowed frequency to ensure end user security and interoperability. (There's some text on this, but it's not specific.)

I'll leave that to the spec authors. My guess is that it's a fuzzy line but we (Chrome) deemed the practical risk to be low enough with the thresholds we selected.

With thresholds on things like accelerometer and gyroscope (which don't seem to mention any throttling by the way; known issue?), how useful are they still for WebVR and such?

I believe they still satisfy the vast majority of use cases. We considered exposing higher threshholds behind a permission prompt but this didn't seem necessary and again has problems due to the difficulty of UX. Again, @noncombatant may remember details.

tomrittervg commented 6 years ago

I reviewed the documents from a Security/Privacy perspective.

https://w3c.github.io/sensors/

Section 4.1.4 and 4.1.5 could go into more detail about what they mean. In 4.0 there is mention of tracking across origin.

Cross-Device linking and out-of-band communication is not listed in 4.1.

Section 5.6 does not mention two of the mandatory conditions listed in 4.2: Secure Context and Permission has been granted (or decided not to be required).

Various similar security checks seem to be missing from the algorithms in 8.1 (which contains only the policy-controlled feature check), 8.2 (which has none), 8.3, and pretty much all 8.x items. I'm not sure if these algorithms should have the security checks, but I'm not sure why not especially when one of the checks is present in 8.1.

The timestamp property of a Sensor is a high res timestamp that can be used as the base for various attacks; but this is not mentioned. Sensors that produce regular frequency readings can also be used to build a high-res timestamp.

https://w3c.github.io/ambient-light/

Section 3: I seem to recall there was an attack/trick that used ambient light sensor to detect user heartbeat (when one held the phone with one's body covering or near the sensor.)

It omits mentioning device fingerprinting.

"The generic mitigation strategies are described in the Generic Sensor API [GENERIC-SENSOR]." - should we clarify that these need to be actually enforced.

https://w3c.github.io/accelerometer/

"There are no specific security and privacy considerations beyond those described in the Generic Sensor API [GENERIC-SENSOR]."

Which are so large and numerable this statement could probably be applied to all of the sub documents. But punting on the specific threats relevant to Acceleraometer does not provide feedback to the implementer as to whether to require a permission prompt; nor does it provide specific recommendations to limit sampling frequency or accuracy.

https://w3c.github.io/gyroscope/

"There are no specific security and privacy considerations beyond those described in the Generic Sensor API [GENERIC-SENSOR]."

Same concern.

https://w3c.github.io/magnetometer/

Security and Privacy Considerations does not make any mention about the difference between calibrated and uncalibrated. I'm skeptical that Calibrated sensors actually entirely eliminate outside interferance of nearby objects, but let's assume it does. It seems like uncalibrated could be used to perform Keystroke Monitoring if you happen to be wearing a peice of jewlery that affects the reading (or maybe even not).

Also, exposing the bias values themselves seems concerning and could provide user fingerprinting, cross device linking, and device fingerprinting.

It omits mentioning device fingerprinting.

https://w3c.github.io/magnetometer/

"There are no specific security and privacy considerations beyond those described in the Generic Sensor API [GENERIC-SENSOR]."

Same concern.

tomrittervg commented 6 years ago

Oh, I forgot some.

https://w3c.github.io/proximity/

"There are no specific security and privacy considerations beyond those described in the Generic Sensor API [GENERIC-SENSOR]."

Same concern.

https://github.com/w3c/accelerometer/blob/gh-pages/security-questionnaire.md https://github.com/w3c/gyroscope/blob/gh-pages/security-questionnaire.md https://github.com/w3c/magnetometer/blob/gh-pages/security-questionnaire.md https://github.com/w3c/orientation-sensor/blob/gh-pages/security-questionnaire.md https://github.com/w3c/ambient-light/blob/gh-pages/security-questionnaire.md https://github.com/w3c/proximity/blob/gh-pages/security-questionnaire.md

3.5 Does this specification expose any other data to an origin that it doesn’t currently have access to?

Why do all of these say "No"?

3.12 Does this specification expose temporary identifiers to the web?

It seems like all of the sensors would qualifier as temporary identifiers. Especially things that change less frequently, like proximiity and ambient light.

3.13 Does this specification distinguish between behavior in first-party and third-party contexts?

I thought it did, based https://w3c.github.io/sensors/#losing-focus

alexshalamov commented 6 years ago

@tomrittervg Thanks for the feedback! It would take some time to clarify all concerns that have been reported. We will start looking into it and address raised issues.

anssiko commented 6 years ago

@tomrittervg, thanks for reviewing these specs and submitting feedback. The Device and Sensors WG will consider your Security and Privacy feedback together with feedback received from the W3C's Security IG and Privacy IG when the wide review period ends 2017-12-31.

We're tracking the wide review feedback across groups in https://github.com/w3c/sensors/issues/299

annevk commented 6 years ago

I believe they still satisfy the vast majority of use cases.

When I mentioned throttling for gyroscope and accelerator to folks working on VR and AR at Mozilla they said that would give an unacceptable user experience...

cc @larsbergstrom

martinthomson commented 6 years ago

Another concern re: fingerprinting. The existence of a sensor provides some fingerprinting information.

cvan commented 6 years ago

@annevk: re: perf, it looks like @jsantell's gotten the WebVR Polyfill on Chrome for Android performing acceptably in this PR, per RelativeOrientationSensor

anssiko commented 6 years ago

The Device and Sensors WG responsible for these specs is eagerly waiting for the TAG’s official response. The DAS WG discussed the feedback provided in this issue on its call yesterday and concluded the comments (by mostly other than TAG members) can be addressed by clarifications to the spec prose. However, the group agreed to wait for the TAG’s official response before it starts to make these concrete updates to the specs to address the comments.

On behalf of the DAS WG, I’d like to thank the current contributors in this issue for their insightful feedback.

cynthia commented 6 years ago

We apologize for taking so long to get back to this - the thread on privacy expanded beyond what normally happens in a normal review, so we had to spend some time combing through that so we don't end up saying the same thing again.

Overall, we are happy with the spec as it stands. The bits that are of concern are obviously the privacy bits above - and could be unclear in the minutes, but the sensors (at least in the current spec) being a top level global, singular object could be improved - the implicit implication from what I understand is that this is due to the background of where this comes from, and considering the background was an understandable decision. However, from a extensibility perspective we are not entirely sure this is the best way forward.

An additional remark would be on making these sensors embed friendly components, which allow other hardware API interfaces to drag and drop them into their APIs as they wish, which the current spec doesn't seem to allow due to the limitation noted above. This mostly comes from usecases such as WebVR controllers and expanding gamepad API to accommodate sensors within a gamepad.

Following the game controller use case..

Most of the privacy concerns we had have been covered above, so we won't repeat those remarks - aside from the remark that the concerns discussed above should be addressed. We would like to see if there is a way to avoid downsampling frequency in contexts where this would make the sensor unusable, as @annevk noted above. Putting high frequency polling behind a flag would address some level of the privacy concerns.

Aside from that, apologies for the delayed review, and thank you for bringing this up with us. We look forward to sensors becoming a first class citizen of the web.

anssiko commented 6 years ago

The Device and Sensors WG will now start to address your comments and will ask you to review the proposed changes by notifying this issue.

I'd like to thank @cynthia, the TAG, and other contributors in this issue for their review comments.

cynthia commented 6 years ago

Thanks a lot! We'll leave the issue open and revisit when the proposed changes are in place.

anssiko commented 6 years ago

@cythia, TAG, All:

The DAS WG has produced a summary of the proposed changes made to the specs in scope of this wide review based on the TAG feedback, see:

https://github.com/w3c/sensors/issues/299#issuecomment-369924348

Please review the proposed changes, and let us know in this issue if you have any further questions, concerns, or comments. Hearing nothing we expect TAG to be happy with the group's response but of course appreciate explicit acknowledgement. Thank you all for your review comments!

annevk commented 6 years ago

@anssiko the "official TAG review" included the "privacy bits above", e.g., https://github.com/w3ctag/design-reviews/issues/207#issuecomment-350125076 and various other comments, which do not appear to be addressed.

anssiko commented 6 years ago

@annevk, the DAS WG considered the "Official TAG review" to be https://github.com/w3ctag/design-reviews/issues/207#issuecomment-358006106 and considered other comments under the banners "Other comments in the TAG review issue" and "Security questionnaire comments", unless discussed herein already.

I will bring https://github.com/w3ctag/design-reviews/issues/207#issuecomment-350125076 back to the group for further discussion and come back with a proposed resolution to revisit. Group's response at that time was https://github.com/w3ctag/design-reviews/issues/207#issuecomment-350254873.

cynthia commented 6 years ago

@anssiko Thanks a lot for getting back to us on this - I'll go through the changes and bring this up in a upcoming call.

anssiko commented 6 years ago

I updated https://github.com/w3c/sensors/issues/299#issuecomment-369924348 with proposed changes to address the issue raised by @annevk in https://github.com/w3ctag/design-reviews/issues/207#issuecomment-369926278.

cynthia commented 5 years ago

This is pending my feedback, fell through the cracks - apologies. Will get through this before the end of our Paris F2F. (which is until tomorrow)

cynthia commented 5 years ago

Proposed changes look good. Thanks for following up on this, and apologies for the delay.

anssiko commented 5 years ago

There has been one substantial change since this TAG review took place that is the addition of WebDriver extensions. We'd kindly as TAG review for that delta. Hopefully it is fine to reuse this issue to retain context.

WebDriver API and its extensions enable test automation and are not web-exposed in a normal browsing scenario. The extensions in scope of this review request are defined in the following Automation sections:

Generic Sensor API https://w3c.github.io/sensors/#automation

Accelerometer https://w3c.github.io/accelerometer/#automation

Gyroscope https://w3c.github.io/gyroscope/#automation

Orientation Sensor https://w3c.github.io/orientation-sensor/#automation

We'd like to get TAG feedback by 2019-11-14. Thank you!

Cc @reillyeon @xfq

kenchris commented 4 years ago

I had a look at this and this seems quite sensible and well specified, so no comments on my side.

anssiko commented 4 years ago

Thank you @kenchris for your review.

The CRs have been published so feel free to close this issue.

Announcement: https://www.w3.org/blog/news/archives/8164

kenchris commented 4 years ago

Thanks for flying TAG