search

w3ctag / design-reviews

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

Ambient Light Sensor API #115

Closed torgo closed 7 years ago

torgo commented 8 years ago

Split out from #110, originally from @tobie

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

tobie commented 8 years ago

You'll have a hard time reviewing those separately, imho.

slightlyoff commented 8 years ago

Understood, but I'd like to try.

A few notes for the structure of the spec:

Questions about the semantics (mostly out of ignorance, thanks for being patient with me):

This is a bit first-pass, and would be happy to do a deeper review. Let me know if that'd be useful.

tobie commented 8 years ago

Thanks for these early comments. That's really helpful. I'm ooo this week, let me get back to you early next week.

tobie commented 8 years ago
  • There's only a single example and no linked explainer document that would provide deeper understanding of how the various bits get used together. I think this would be clarifying both for the reader and the authors.

So I tried to provide some of this in the Generic Sensor API, notably in the Background and Concepts sections. Agree this still needs work.

  • The example (singular?) doesn't show how to use the Permission API in conjunction with the Ambient Light API, which seems pretty critical to actual use.

Yes. The Permission API is missing critical pieces for proper integration. @jyasskin is actively working on this as we speak. I'll revisit this and clean it up as soon as his changes have landed.

Questions about the semantics (mostly out of ignorance, thanks for being patient with me):

  • While I understand that the Generic Sensor API returns undefined for start() and stop(), it'd seem that individual sensors should probably return promises from these calls so that the start and stop operations can report errors

I'm waiting for cancelable promises for this (tracked here: https://github.com/w3c/sensors/issues/94).

  • There's no definition for what onstatechange would do. Does that report start/stop state? Are those inspectable in some other way?

Yeah, these are still todos in the Generic Sensor API . Sensor objects have a state attribute which moves from idle -> activating -> active -> idle/errored. statechange events are fired on each transition from one of these states to another.

Maybe some sequence diagrams would help understand whats going on here. I'll give those a try.

  • The use of Constructors looks good!

Thanks.

  • What's the model for multiple clients of a sensor like this? I get that AmbientLightSensor likely doesn't have any difficulty multiplexing the data, there will be some sensors that may want to allow writing values and other sorts of configuration that can be considered "exclusive". How does the generic sensor API handle this?

It doesn't, because the underlying APIs won't let you do this. For example, on Android, if one app requires a sensor to be polled at 100Hz, all apps will get data at this frequency. When another app shows up and asks for a higher frequency, all apps will get the new one.

A similar model is used at the Web level in the Generic Sensor API. Basically, the UA compiles the needs of all Sensor objects and serves everyone of them with the most stringent requirements.

And is "sensor state" meant to be mapped to the hardware sensor or specific to the software reflection that the object in question provides (which might be different than the hardware state)?

The latter. It's not possible to know from the underlying APIs whether a sensor is being used by another app or not (though it's certainly possible to infer this through timing attacks, etc.).

For instance, this isn't particularly illuminating (pardon the pun).

This is in reference to the Sensor object's state (not the underlying HW). That said, when the state is "active," so is the underlying hardware, unless there's an error that hasn't been surfaced to the UA layer yet.

  • Requests for permissions aren't modeled in this API. Is the assumption that navigator.permissions.request() will be used here?

Yes. Through the Generic Sensor API, as soon as @jyasskin's changes have landed.

  • Are these sensors available from within worker contexts? They don't appear to be from the IDL but maybe I'm missing some prose?

They're not.

Shouldn't they be (modulo permissions request issues)?

That's a level 2 feature. Tracked here: https://github.com/w3c/sensors/issues/12

  • Glad to see these restricted to Secure Contexts. Nice work.

Copy/pasting boilerplate from @mikewest's spec made that dead easy. I can't claim any credit.

Has there been any consideration of top-level browsing contexts vs. iframes? (cc: @mikewest)

Yes. Here and there.

  • How does the sensor processing model get defined?

I guess the closest answer to this question is here which is currently curiously missing a step to tell the underlying sensor what to do in a UA-specific way. Need to fix this.

If it's a frequency model, what happens when it's faster than the swap-buffer rate (e.g. requestAnimationFrame timing)?

That's a desired feature, actually, to lower latency (e.g. the Oculus Rift polls the gyroscope at 1KHz for this very reason). Frequency is capped at the HW limits of the sensor (First sub-step of the Observe a Sensor abstract operation.

  • This is a bit first-pass, and would be happy to do a deeper review. Let me know if that'd be useful.

Thanks a bunch. What I'm getting out of this loud and clear is that the processing model needs to be explained much more clearly and upfront in the Generic Sensor spec. I'm happy for you to dig more deeply either right away or wait until I'm done with those edits.

slightlyoff commented 7 years ago

Quick notes from today's F2F meeting:

Thanks

tobie commented 7 years ago

Thanks for the update, @slightlyoff. FYI: https://lists.w3.org/Archives/Public/public-device-apis/2016Jun/0041.html

torgo commented 7 years ago

Picked up again 31-Aug-2016. We agreed to add to agenda for next week - to discuss privacy issues specifically. We can discuss the blog post on this as well https://blog.lukaszolejnik.com/privacy-of-ambient-light-sensors/

dbaron commented 7 years ago

https://github.com/w3ctag/spec-reviews/issues/129 may be related

lknik commented 7 years ago

#13 may also be related.

torgo commented 7 years ago

Taken up at Tokyo F2F. We discussed @lknik's feedback. Question of whether frequency / sensitivity should be specified in the spec, or should be a UA issue. Perhaps shouldn't be limited by the spec because there could be legitimate reasons to use higher resolution data…

tobie commented 7 years ago

@torgo agreed. We need to warn UA about the risks, but leave the implementation details implementation details.

tobie commented 7 years ago

@slightlyoff:

Thanks for explaining the sampling rate model. Does all this stuff need to come back to the main thread? Can it be delegated to workers somehow? It feels like interrupting rendering to handle this sort of input is going to jank VERY badly

This is on my todo list though I would have preferred to delay it to a level 2 of the spec to ship this thing first. I'm not sure what the best strategy to handle permissions is within worked and pointers (if you have any) would be appreciated.

Still not seeing an explainer/examples document in the Github repo (https://github.com/w3c/ambient-light). Should I be looking elsewhere for those?

I guess I was hoping that the Generic Sensor spec itself would provide all the required info. What's missing from it?

Don't think you should wait on Cancelable Promises. It seems like a huge boondoggle at this point (just my opinion).

Yeah. We dropped that.

How's the permissions api stuff going? @jyasskin?

I think we have a better story at this point. might get back to you with more specific questions once I'm actually working directly on this.

lknik commented 7 years ago

I'll be happy to review security/privacy considerations once more details about frequency/etc are known.

tobie commented 7 years ago

@lknik what kind of details are you looking for?

lknik commented 7 years ago

@tobie if I understand correctly sites will be able to use the API in workers, and also specify the frequency. The permission prompt will be standard?

In this case:

tobie commented 7 years ago

@tobie if I understand correctly sites will be able to use the API in workers,

Not sure if that's a level 1 or 2 feature, but it's planned, yes.

and also specify the frequency.

correct.

The permission prompt will be standard?

If by this you mean that all sensors are considered powerful features, then yes.

In this case:

can the frequency be really custom-configured?

Yes.

If so, let's think on addressing this one if we do not want to limit the frequency readout with some lower/upper value (sorry @torgo for not listing this more clearly anywhere)

That's indeed a fingerprinting issue we need to warn about.

also, is a single permission prompt suitable for having the possibility to hop at all available frequencies and change them whenever a site wishes? > Would be nice to establish this one as well

My understanding from @maryammjd's comments during TPAC is that reducing poll frequency doesn't significantly reduce security risks. Given https://github.com/w3c/sensors/issues/100#issuecomment-257355433, I don't think starting/stopping and changing frequency can be effectively used as a communication channel.

I guess I should review considerations and e.g. advise UAs to MUST log past frequencies.

Can you file an issue against the generic sensor API for this and explain the reason to do so? I'm wary of such normative requirements for this, fwiw.

lknik commented 7 years ago

Hi @tobie , thanks!

I filled it here.

maryammjd commented 7 years ago

Hi guys,

I am not sure if I am in the same page as others. If you ask me explicit questions, it would be more effective for me to get involved. In general, reducing the frequency rate hasn't proved effective according to our research on accelerometer and gyroscope. I assume it would be the same for other sensors such as light.

tobie commented 7 years ago

@maryammjd, link to your paper might be useful for the informative reference section of the generic sensor API.

maryammjd commented 7 years ago

Here is the free link: https://arxiv.org/abs/1602.04115

lknik commented 7 years ago

Hey @maryammjd - thanks for your input and a link to the paper, looks good!

Depending on the threat model, you're right. But I'm unsure if we can compare different sensors like that. Also of note is the rate of frequency reduction; additionally, let's keep in mind that light sensor is affected also by the environment.

That said, my tests somewhat differed from platform to platform.

hadleybeeman commented 7 years ago

Hi @tobie and @anssiko — we're just catching up on this in our F2F. It looks like the privacy work continues here, which is great. Given that one of the attacks we've discussed is the information an attacker can get about the user's environment (gather enough data to work out what movie the user is watching, or what the layout is of the building they're in) — it would be useful to expand on this possibility in the Security and Privacy Concerns in the Ambient Light spec.

(Right now, it just says that there are no specific security and privacy considerations beyond those described in the Generic Sensor API. The Generic Sensor API says that each sensor type should be assessed individually. So it feels a bit like these considerations aren't getting a mention in either doc just yet.)

If you're set with that, let us know and we'll close this issue.

tobie commented 7 years ago

Hi @hadleybeeman. Sorry with the delay in getting back to you. Tracking this on our side (https://github.com/w3c/ambient-light/issues/13) so feel free to close this one.

lknik commented 7 years ago

Hey @hadleybeeman - as pointed out by @tobie - tried to distill a version encompassing many possible aspects (i.e. high-level).