chore: minimally replaces keygrip dep#174
chore: minimally replaces keygrip dep#174yowainwright wants to merge 2 commits intopillarjs:masterfrom
Conversation
index.js
Outdated
| var ah = crypto.createHmac('sha256', key).update(sa).digest() | ||
| var bh = crypto.createHmac('sha256', key).update(sb).digest() | ||
|
|
||
| return bufferEqual(ah, bh) && a === b |
There was a problem hiding this comment.
Do we really need a === b at the end when bufferEqual(...) already handled the comparison?
index.js
Outdated
| function constantTimeCompare (a, b) { | ||
| var sa = String(a) | ||
| var sb = String(b) | ||
| var key = crypto.pseudoRandomBytes(32) |
There was a problem hiding this comment.
| var key = crypto.pseudoRandomBytes(32) | |
| var key = crypto.randomBytes(32) |
Maybe use randomBytes instead?
| if (constantTimeCompare(digest, computed)) { | ||
| return i | ||
| } |
There was a problem hiding this comment.
Would it be better to check whether crypto.timingSafeEqual(...) is existing and call it here rather than in bufferEqual?
There was a problem hiding this comment.
The update, as is, preserves expected node compatibility; adapted from keygrip.
Happy to update along with a node compatibility update if desired!
| var http = require('http') | ||
| var https = require('https') | ||
| var Keygrip = require('keygrip') | ||
| var Keygrip = require('..').Keygrip |
There was a problem hiding this comment.
We may need this change in other test files that require keygrip.
|
In addition, we may need an update to the README file about removing/modifying the mentioning of Keygrip. |
yowainwright
force-pushed
the
KOA_ISSUE_1915
branch
from
4b421de to
f999171
Compare
2 participants
Description
This PR is a minimal replacement of keygrip which has been deprecated.
By removing this dep, we fix upstream koa
Thanks!