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

"Rate-limiting change notifications" section is confusing #291

Closed
rakuco opened this issue Sep 6, 2024 · 4 comments · Fixed by #302
Closed

"Rate-limiting change notifications" section is confusing #291

rakuco opened this issue Sep 6, 2024 · 4 comments · Fixed by #302
Labels
bug Something is wrong or under specified

Comments

@rakuco
Copy link
Member

rakuco commented Sep 6, 2024

Based on its contents, https://w3c.github.io/compute-pressure/#rate-limiting-change-notifications seems to have been added before https://w3c.github.io/compute-pressure/#dfn-may-receive-data, as the former says "inactive windows" should receive updates every 10 seconds (and does not take workers into consideration) whereas the latter's focus checks should prevent other windows from receiving updates at all.

Furthermore, there is nothing in the data collection algorithms that prevents an observer's sampling period from being < 1 second, although the only existing implementation of this API currently implements this limit at the pressure source side by polling the platform every second. From the text, it's not clear if this should be done at the pressure source level for all windows and workers or per-window/worker.

(It's also important to keep in mind that implementing any rate-limiting per-context needs an Automation-related note, as at least in web-platform-tests we need to be able to fetch and process data more than 1 per second to test the rate obfuscation algorithms without taking too long to run the tests)

@arskama arskama added the bug Something is wrong or under specified label Sep 17, 2024
@arskama arskama mentioned this issue Sep 18, 2024
29 tasks
@kenchris
Copy link
Contributor

What about something like:

11.2.2 Rate-limiting change notifications

By rate-limiting the delivery of the pressure state information we remove the attacker's ability to observe the precise time when a value transitions between two states.

More precisely, once the pressure observer is activated, it will be called once with initial values, and then is called when the values change. The subsequent calls will be rate-limited. When the callback is called, the most recent value is reported.
The specification will recommend a rate limit of at most one update per second, though this is [=implementation-defined=]. We will also recommend that the call timings are jittered across contexts such as workers and main thread.

These measures benefit the user's privacy, by reducing the risk of identifying a device across multiple origins. The rate-limiting also benefits the user's security, by making it difficult to use this API for timing attacks. Last, rate-limiting change callbacks places an upper bound on the performance overhead of this API.

The rate limiting should be disabled during automation and for testing purposes, for reliable and quickly passing tests.
Rate limiting can be implemented in the user agent, but it might also be possible to simply change the polling/sampling rate of the underlying hardware counters, if not accessed via a higher level framework.

@arskama
Copy link
Contributor

arskama commented Sep 26, 2024

I m ok with the wording, but @rakuco has definitely an opinion on that one.

@rakuco
Copy link
Member Author

rakuco commented Sep 26, 2024

The specification will recommend a rate limit of at most one update per second, though this is [=implementation-defined=]. We will also recommend that the call timings are jittered across contexts such as workers and main thread.

  • Why "this spec will recommend" instead of making this normative?
  • These requirements are always easier to follow and have an easier to measure impact when they are baked into the algorithms:
    • Are we talking about limits on the platform side or when delivering an update to each frame/worker?
    • Does there need to be any validation of the sampleInterval member passed to PressureObserver.observe()?
  • Similarly, is it enough for the spec to recommend jitter or should it be a normative requirement? This spec has not required jittering values before; does it mean it was open to security issues being mitigated now or is this something completely new?

The rate limiting should be disabled during automation and for testing purposes, for reliable and quickly passing tests.

We should avoid having separate paths for automation and non-automation. If the items above are integrated into the algorithms, you could allow setting them to some specific value via WebDriver, for example, or just write any tests in WPT in a way that does not depend on those values. In the worst case, defining some lower intervals for the automation case is better than not having any limiting altogether.

@kenchris
Copy link
Contributor

It basically sound like you want us to remove the section which I believe was added before we took over the project.

It is just additional ideas that could be applied for further privacy mitigations if some browsers believe that would be more important than more precise values. I generally don't think we need any of these ideas in the algorithms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong or under specified
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants