Skip to content
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

Allow Buffer as key #33

Closed
webmaster777 opened this issue Oct 20, 2020 · 2 comments
Closed

Allow Buffer as key #33

webmaster777 opened this issue Oct 20, 2020 · 2 comments

Comments

@webmaster777
Copy link

Or any other of the allowed types for the key parameter: <string> | <Buffer> | <TypedArray> | <DataView> | <KeyObject>

Cryptographically secure keys are usually binary encoded, converting them to strings is not always possible.

@natevw
Copy link
Collaborator

natevw commented Nov 27, 2020

Looks like the only real technical blocker here would be to change the assertions from e.g.

if ('string' != typeof secret) throw new TypeError("Secret string must be provided.");

to something like:

if ('undefined' == typeof secret) throw new TypeError("Secret key must be provided.");

This seems relatively harmless to me but I'd want to get at least another set of eyes from downstream maintainers — e.g. @dougwilson — to make sure this wouldn't introduce any risk. (I'm struggling to find a reason not to change this but perhaps there's code out there that, say, for whatever reason, relies on the current assertion. Maybe a reasonable compromise would be to allow this flexibility in a new major release?)

@dougwilson
Copy link

Depending on how .createHmac treats null, you may have to do a check like secret == null (i.e. check for both undefined and null), but otherwise it seems to me the change is harmless and if it were me, I'd just file it under an enhancement (secret can now be anything .createHmac accepts vs just a string), especially since the behavior of a string would be unchanged.

The code is there (imo) as a guard to provide a useful error message. Not sure what .createHmac would do if a user supplied a function as the secret, but if it's a useful error, then probably just letting .createHmac handle it is good enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants