From 1859e9c827d792f5238355ec239018af6c87f097 Mon Sep 17 00:00:00 2001 From: Ryan Emery Date: Mon, 14 Oct 2019 09:31:15 -0700 Subject: [PATCH 1/7] fix: eval in portableTimingSafeEqual `eval` was added to help ensure that `portableTimingSafeEqual` is not optimized into a non-constant time function. However, the Content Security Policy `'unsafe-eval'` will flag this. It is better to remove this and risk such an optimization, then to force customers to weaken the Content Security Policy. --- .../src/cryptographic_material.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/modules/material-management/src/cryptographic_material.ts b/modules/material-management/src/cryptographic_material.ts index df6c5978e..841ea98e9 100644 --- a/modules/material-management/src/cryptographic_material.ts +++ b/modules/material-management/src/cryptographic_material.ts @@ -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. + * 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 From be142b9a7c127325c6361ebc28208cd34ac9598e Mon Sep 17 00:00:00 2001 From: seebees Date: Mon, 14 Oct 2019 12:45:04 -0700 Subject: [PATCH 2/7] Update modules/material-management/src/cryptographic_material.ts Co-Authored-By: Matt Bullock --- modules/material-management/src/cryptographic_material.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/material-management/src/cryptographic_material.ts b/modules/material-management/src/cryptographic_material.ts index 841ea98e9..edb3a8032 100644 --- a/modules/material-management/src/cryptographic_material.ts +++ b/modules/material-management/src/cryptographic_material.ts @@ -82,7 +82,7 @@ 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` could prevent the optimization, but this is no grantee. + * Adding `eval` could prevent the optimization, but this is no guarantee. * 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. From f4a6e41c5a3ff34b4f0e3fb1d7af708aea961723 Mon Sep 17 00:00:00 2001 From: seebees Date: Mon, 14 Oct 2019 12:45:10 -0700 Subject: [PATCH 3/7] Update modules/material-management/src/cryptographic_material.ts Co-Authored-By: Matt Bullock --- modules/material-management/src/cryptographic_material.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/material-management/src/cryptographic_material.ts b/modules/material-management/src/cryptographic_material.ts index edb3a8032..05900e974 100644 --- a/modules/material-management/src/cryptographic_material.ts +++ b/modules/material-management/src/cryptographic_material.ts @@ -83,7 +83,7 @@ const timingSafeEqual: (a: Uint8Array, b: Uint8Array) => boolean = (function () function portableTimingSafeEqual (a: Uint8Array, b: Uint8Array) { /* It is *possible* that a runtime could optimize this constant time function. * Adding `eval` could prevent the optimization, but this is no guarantee. - * The eval below is commented out, + * 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, From fb3de3a30540564d65053fb0c5d6a3334c947694 Mon Sep 17 00:00:00 2001 From: seebees Date: Mon, 14 Oct 2019 12:45:21 -0700 Subject: [PATCH 4/7] Update modules/material-management/src/cryptographic_material.ts Co-Authored-By: Matt Bullock --- modules/material-management/src/cryptographic_material.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/material-management/src/cryptographic_material.ts b/modules/material-management/src/cryptographic_material.ts index 05900e974..29311663f 100644 --- a/modules/material-management/src/cryptographic_material.ts +++ b/modules/material-management/src/cryptographic_material.ts @@ -87,7 +87,7 @@ const timingSafeEqual: (a: Uint8Array, b: Uint8Array) => boolean = (function () * 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'`. + * is not worth the cost of making customers allow `'unsafe-eval'`. * If you copy this function for your own use, make sure to educate yourself. * Side channel attacks are pernicious and subtle. */ From 97eb7fb42e3885077106179abaac92cb1e270604 Mon Sep 17 00:00:00 2001 From: seebees Date: Mon, 14 Oct 2019 12:45:28 -0700 Subject: [PATCH 5/7] Update modules/material-management/src/cryptographic_material.ts Co-Authored-By: Matt Bullock --- modules/material-management/src/cryptographic_material.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/material-management/src/cryptographic_material.ts b/modules/material-management/src/cryptographic_material.ts index 29311663f..75cbadc5a 100644 --- a/modules/material-management/src/cryptographic_material.ts +++ b/modules/material-management/src/cryptographic_material.ts @@ -86,7 +86,7 @@ const timingSafeEqual: (a: Uint8Array, b: Uint8Array) => boolean = (function () * 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, + * The value in attempting to ensure that this function is not optimized * is not worth the cost of making customers allow `'unsafe-eval'`. * If you copy this function for your own use, make sure to educate yourself. * Side channel attacks are pernicious and subtle. From c430da0c5af80efb00f1120fa3474c5fccb49865 Mon Sep 17 00:00:00 2001 From: seebees Date: Mon, 14 Oct 2019 12:45:41 -0700 Subject: [PATCH 6/7] Update modules/material-management/src/cryptographic_material.ts Co-Authored-By: Matt Bullock --- modules/material-management/src/cryptographic_material.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/material-management/src/cryptographic_material.ts b/modules/material-management/src/cryptographic_material.ts index 75cbadc5a..05ec3e0f4 100644 --- a/modules/material-management/src/cryptographic_material.ts +++ b/modules/material-management/src/cryptographic_material.ts @@ -92,7 +92,7 @@ const timingSafeEqual: (a: Uint8Array, b: Uint8Array) => boolean = (function () * Side channel attacks are pernicious and subtle. */ // eval('') // eslint-disable-line no-eval - /* Check for early return (Postcondition) UNTESTED: Size is well-know information. + /* Check for early return (Postcondition) UNTESTED: Size is well-know information * and does not leak information about contents. */ if (a.byteLength !== b.byteLength) return false From 42f251ba63399f30bf22cbb8fad5e043d42e872e Mon Sep 17 00:00:00 2001 From: Ryan Emery Date: Mon, 14 Oct 2019 15:13:20 -0700 Subject: [PATCH 7/7] update comment --- modules/material-management/src/cryptographic_material.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/material-management/src/cryptographic_material.ts b/modules/material-management/src/cryptographic_material.ts index 05ec3e0f4..c7f786f50 100644 --- a/modules/material-management/src/cryptographic_material.ts +++ b/modules/material-management/src/cryptographic_material.ts @@ -88,7 +88,8 @@ const timingSafeEqual: (a: Uint8Array, b: Uint8Array) => boolean = (function () * it would fail on this eval. * The value in attempting to ensure that this function is not optimized * is not worth the cost of making customers allow `'unsafe-eval'`. - * If you copy this function for your own use, make sure to educate yourself. + * If you want to copy this function for your own use, + * please review the timing-attack link above. * Side channel attacks are pernicious and subtle. */ // eval('') // eslint-disable-line no-eval