Skip to content

fix: eval in portableTimingSafeEqual #227

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Oct 15, 2019
Merged

fix: eval in portableTimingSafeEqual #227

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1859e9c
fix: eval in portableTimingSafeEqual
seebees Oct 14, 2019
be142b9
Update modules/material-management/src/cryptographic_material.ts
seebees Oct 14, 2019
f4a6e41
Update modules/material-management/src/cryptographic_material.ts
seebees Oct 14, 2019
fb3de3a
Update modules/material-management/src/cryptographic_material.ts
seebees Oct 14, 2019
97eb7fb
Update modules/material-management/src/cryptographic_material.ts
seebees Oct 14, 2019
c430da0
Update modules/material-management/src/cryptographic_material.ts
seebees Oct 14, 2019
42f251b
update comment
seebees Oct 14, 2019
7e919b1
Merge branch 'master' into remove_eval
mattsb42-aws Oct 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions modules/material-management/src/cryptographic_material.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,19 @@ const timingSafeEqual: (a: Uint8Array, b: Uint8Array) => boolean = (function ()
/* https://codahale.com/a-lesson-in-timing-attacks/ */
function portableTimingSafeEqual (a: Uint8Array, b: Uint8Array) {
/* It is *possible* that a runtime could optimize this constant time function.
* Adding `eval` should prevent the optimization, but this is no grantee.
* If you copy this function for your own use, make sure to educate yourself.
* Side channel attacks are pernicious and subtle.
*/
eval('') // eslint-disable-line no-eval
* Adding `eval` could prevent the optimization, but this is no grantee.
* The eval below is commented out,
* because if a browser is using a Content Security Policy with `'unsafe-eval'`
* it would fail on this eval.
* The value in attempting to ensure this function is not optimized,
* is not worth the cost of making customers to alow `'unsafe-eval'`.
* If you copy this function for your own use, make sure to educate yourself.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a reference for such a person to read up on? Just saying "educate yourself" comes across as dismissive, and I know that was not your intent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point. https://codahale.com/a-lesson-in-timing-attacks/ is linked above. Perhaps adding please? I was hoping that next sentence might mitigate some of this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "please review the above link before use"?

* Side channel attacks are pernicious and subtle.
*/
// eval('') // eslint-disable-line no-eval
/* Check for early return (Postcondition) UNTESTED: Size is well-know information.
* and does not leak information about contents.
*/
* and does not leak information about contents.
*/
if (a.byteLength !== b.byteLength) return false

let diff = 0
Expand Down