-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add a note regarding HRT precision #57
Conversation
@toddreifsteck let me know if this covers the conclusion from 2/22 and I'll do the CR update after that. |
index.html
Outdated
@@ -266,7 +266,7 @@ <h3>The <dfn>DOMHighResTimeStamp</dfn> Type</h3> | |||
typedef double DOMHighResTimeStamp; | |||
</pre> | |||
<p>A <a>DOMHighResTimeStamp</a> SHOULD represent a time in milliseconds | |||
accurate to 5 microseconds - see <a href="#sec-privacy-security"></a>.</p> | |||
accurate to some microseconds - see <a href="#clock-resolution"></a> for additional considerations.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'accurate enough to allow measurement and inaccurate enough to prevent attack'
index.html
Outdated
@@ -350,8 +350,11 @@ <h3>Clock resolution</h3> | |||
this new API an attacker may be able to obtain high-resolution estimates | |||
through repeat execution and statistical analysis. To ensure that the new | |||
API does not significantly improve the accuracy or speed of such attacks, | |||
the recommended minimum resolution of the <a>Performance</a> interface | |||
the recommended minimum resolution of the <a>DOMHighResTimeStamp</a> type | |||
should be set to 5 microseconds.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change from 'should be set to 5 microseconds' to 'should be inaccurate enough to prevent attack'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the warning needs to be closer to words that I've filled in. Please feel free to update with your own words as I spent 2 minutes on this.
@igrigorik Please double-check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One nitpick wording suggestion, feel free to ignore if you think current text already captures the state correctly. That aside, lgtm!
index.html
Outdated
the recommended minimum resolution of the <a>DOMHighResTimeStamp</a> type | ||
should be inaccurate enough to prevent attacks.</p> | ||
<div class="issue" data-number="56"> | ||
The recommended minimum resolution of the <a>DOMHighResTimeStamp</a> used to be 5 microseconds. It is however an ongoing issue and has significantly increased recently. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wordsmithing the last sentence a bit...
"Due to recent developments this may need to increase significantly, but the working group has not yet reached consensus on what the new recommend minimum value should be." ?
FWIW, Edge is pushing updates to clamp on 100us + 100us jitter for all explicit clocks. We still aren't certain if this is the right number, but at least it leaves Chrome and Edge in alignment. |
100us+100us is superior to both the Firefox/Safari approach (2ms and 1ms respectively) from the gametime point of view explained in: #56 (comment) That said, it isn't perfect -- ideally this should be solved within a year with some kind of a permissions API (or confirmed hardware-level Spectre/Meltdown fixes in chips) -- it should not be a permanent humandkind thing to standardize 100us/100us -- but it is an acceptable interim solution unlike 1ms+. |
Temporary warning while #56 is being sorted out.
Preview | Diff