Bug 146531 – Reduce resolution of performance.now

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Alex Christensen]

performance.now has nanosecond resolution right now, which makes timing attacks too precise.  Let's reduce that resolution by flooring the value to make sure we don't get times in the future.

[Alex Christensen]

Created attachment 255963 [details]
Patch

[Alex Christensen]

Created attachment 255974 [details]
Patch

[Alex Christensen]

Created attachment 255975 [details]
Patch

[Alex Christensen]

http://trac.webkit.org/changeset/186208

[Chris Dumez]

This broke http/tests/misc/webtiming-resolution.html:
--- /Volumes/Data/slave/syrah-debug-wk2-tests/build/OpenSource/layout-test-results/http/tests/misc/webtiming-resolution-expected.txt
+++ /Volumes/Data/slave/syrah-debug-wk2-tests/build/OpenSource/layout-test-results/http/tests/misc/webtiming-resolution-actual.txt
@@ -3,5 +3,5 @@
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS ((t1 - t0) / 0.005) % 1 < 1e-10 is true
+FAIL ((t1 - t0) / 0.005) % 1 < 1e-10 should be true. Was false.

[Chris Dumez]

Hmm.. I see you actually introduced the test that is failing.

[Chris Dumez]

(In reply to comment #5)
> This broke http/tests/misc/webtiming-resolution.html:
> ---
> /Volumes/Data/slave/syrah-debug-wk2-tests/build/OpenSource/layout-test-
> results/http/tests/misc/webtiming-resolution-expected.txt
> +++
> /Volumes/Data/slave/syrah-debug-wk2-tests/build/OpenSource/layout-test-
> results/http/tests/misc/webtiming-resolution-actual.txt
> @@ -3,5 +3,5 @@
>  On success, you will see a series of "PASS" messages, followed by "TEST
> COMPLETE".
>  
>  
> -PASS ((t1 - t0) / 0.005) % 1 < 1e-10 is true
> +FAIL ((t1 - t0) / 0.005) % 1 < 1e-10 should be true. Was false.

I can reproduce locally. The left side has the following value:
0.999999999999801

[Chris Dumez]

Reopening due to test failure.

[Chris Dumez]

Created attachment 255989 [details]
Patch

[Chris Dumez]

Comment on attachment 255989 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=255989&action=review

> LayoutTests/http/tests/misc/webtiming-resolution.html:-16
> -shouldBe("((t1 - t0) / 0.005) % 1 < 1e-10", "true");

The previous check did not work for example if (t1 - t0) is 0.00499999999999:
(0.00499999999999 / 0.005) % 1 is 0.999999999998 which is greater than 1e-10.

[Chris Dumez]

Comment on attachment 255989 [details]
Patch

Clearing flags on attachment: 255989

Committed r186216: <http://trac.webkit.org/changeset/186216>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

(In reply to comment #11)
> Comment on attachment 255989 [details]
> Patch
> 
> Clearing flags on attachment: 255989
> 
> Committed r186216: <http://trac.webkit.org/changeset/186216>

This new test fails everywhere:

--- /Volumes/Data/slave/mavericks-release-tests-wk1/build/layout-test-results/http/tests/misc/webtiming-resolution-expected.txt
+++ /Volumes/Data/slave/mavericks-release-tests-wk1/build/layout-test-results/http/tests/misc/webtiming-resolution-actual.txt
@@ -1,7 +1,7 @@
+CONSOLE MESSAGE: line 11: ReferenceError: Can't find variable: performance
 Verifies the minimum resolution is 5 microseconds.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS Math.abs(1 - (t1 - t0) / 0.005) < 1e-10 is true

[Alex Christensen]

More test fixing in http://trac.webkit.org/changeset/186219

[Alex Christensen]

More bot fixing in http://trac.webkit.org/changeset/186222

[Eli Grey (:sephr)]

This change only serves to harm victims of timing attacks even more. For example, it doesn't prevent me from running my navigator.hardwareConcurrency timing attack, it just makes it take longer to complete and wastes additional CPU cycles.

This is not how software should address timing attack vulnerabilities. You harden the vulnerable APIs with NOPs and wait times for completion time normalization.

[Eli Grey (:sephr)]

See this comment from from the relevant spec issue:

https://github.com/tc39/ecmascript_sharedmem/issues/1#issuecomment-175315327