1442504 - Disable disjoint timer queries to prevent use as a high-precision timer

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect, P1)

Description

[Kelsey Gilbert [:jgilbert]]

Disjoint timer queries offer an incredibly precise timestamp, *and* a timestamp/interval separate from the core performance.now() timer. We need to disable it for the time being, and think hard about how we can mitigate its usefulness for timing attacks before reintroducing it.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: 0002-Bug-1442504-Disable-disjoint-timer-queries.-r-milan.patch

Comment 2

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8955391 [details] [diff] [review]
0002-Bug-1442504-Disable-disjoint-timer-queries.-r-milan.patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Difficult. We don't state why we remove it.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No, we don't talk about why we remove it.

Which older supported branches are affected by this flaw?
Older than 50+

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Trivial backports.

How likely is this patch to cause regressions; how much testing does it need?
This extension isn't guaranteed, so we're just making it go from <90% to 0%.

Comment 3

[Liz Henry (:lizzard)]

Assuming 52 is affected since you mention the issue goes back further than 50.
What security rating would you put on this? 
Are you looking for uplift to 59 ?

Comment 4

[Al Billings [:abillings - ex-MoCo]]

Calling this a sec-high and giving it sec-approval+ for trunk. This needs a beta and release branch nomination if we're going to ship it in 59 as well.

Comment 5

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8955391 [details] [diff] [review]
0002-Bug-1442504-Disable-disjoint-timer-queries.-r-milan.patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: sec-high
Fix Landed on Version: will be 60
Risk to taking this patch (and alternatives if risky): none
String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Approval Request Comment
[Feature/Bug causing the regression]: n/a
[User impact if declined]: sec-high
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: not landed yet
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: 60/59/esr32
[Is the change risky?]: no
[Why is the change risky/not risky?]: Disables a feature.
[String changes made/needed]: none

Comment 6

[Kelsey Gilbert [:jgilbert]]

More background: This extension is only enabled on drivers that support it, which is way way less than 100%. Authors should not be assuming that they have this extension, so removing it shouldn't ruin anyone's day.

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bef3db82d7ca31a188f902e317713c88001938ed

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/bef3db82d7ca

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8955391 [details] [diff] [review]
0002-Bug-1442504-Disable-disjoint-timer-queries.-r-milan.patch

Approved for Fx59rc1 and ESR 52.7.0.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/54d0778bbb9d (FIREFOX_59b_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/aeb9e1be02d2
https://hg.mozilla.org/releases/mozilla-esr52/rev/8954ce68a364

Comment 11

[Daniel Veditz [:dveditz]]

Do we need to release note this removal? Or is no one using it because it's already gone in Chrome?

Comment 12

[Kelsey Gilbert [:jgilbert]]

(In reply to Daniel Veditz [:dveditz] from comment #11)
> Do we need to release note this removal? Or is no one using it because it's
> already gone in Chrome?

It depends how much attention you want to draw to it. We're fine not mentioning it.