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

crypto.scrypt() #8417

Closed
jorangreef opened this issue Sep 6, 2016 · 7 comments
Closed

crypto.scrypt() #8417

jorangreef opened this issue Sep 6, 2016 · 7 comments
Labels
crypto Issues and PRs related to the crypto subsystem. feature request Issues that request new features to be added to Node.js.

Comments

@jorangreef
Copy link
Contributor

@indutny would there be any support for adding scryptSync() and scrypt() to the crypto module?

There is already support for PBKDF2 but scrypt can offer an order of magnitude or more strength for the same derivation time.

@bnoordhuis
Copy link
Member

The crypto module only exposes what openssl supports and openssl does not support scrypt. Likewise for HKDF, ref #8418.

@bnoordhuis bnoordhuis added crypto Issues and PRs related to the crypto subsystem. feature request Issues that request new features to be added to Node.js. labels Sep 6, 2016
@jorangreef
Copy link
Contributor Author

jorangreef commented Sep 6, 2016

Surely openssl is just the implementation of much of the crypto module? Does it prescribe the interface?

Perhaps adding scrypt is a slippery slope, people might start asking for bcrypt. But PBKDF2 is long in the teeth and scrypt is a decent improvement over PBKDF2 and bcrypt.

@bnoordhuis
Copy link
Member

It restricts what functionality node can expose. We would either have to bundle the scrypt reference implementation or write a sizable chunk of code to wire up the building blocks that openssl does offer.

Taking on more dependencies is not something we do lightly and writing cryptographic code, even the kind that just connects the dots, requires very careful scrutiny. It's an interesting exercise but given the choice I'd rather do neither.

(And yes, it's also a slippery slope.)

@chyzwar
Copy link

chyzwar commented Jun 2, 2017

OpenSSL 1.1.0 added scrypt support.

@gibfahn
Copy link
Member

gibfahn commented Jun 3, 2017

OpenSSL 1.1.0 added scrypt support.

This can probably be reopened once #11828 (or it's successor) lands.

@jorangreef jorangreef changed the title crypto.scryptSync() crypto.scrypt() Sep 7, 2017
@targos
Copy link
Member

targos commented May 22, 2018

Reopening. #20816 is the PR that implements the feature.

@targos targos reopened this May 22, 2018
@jorangreef
Copy link
Contributor Author

Thanks @bnoordhuis !

targos pushed a commit that referenced this issue Jun 13, 2018
Scrypt is a password-based key derivation function that is designed to
be expensive both computationally and memory-wise in order to make
brute-force attacks unrewarding.

OpenSSL has had support for the scrypt algorithm since v1.1.0.  Add a
Node.js API modeled after `crypto.pbkdf2()` and `crypto.pbkdf2Sync()`.

Changes:

* Introduce helpers for copying buffers, collecting openssl errors, etc.

* Add new infrastructure for offloading crypto to a worker thread.

* Add a `AsyncWrap` JS class to simplify pbkdf2(), randomBytes() and
  scrypt().

Fixes: #8417
PR-URL: #20816
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
dna2github pushed a commit to dna2fork/node that referenced this issue Jun 14, 2018
Scrypt is a password-based key derivation function that is designed to
be expensive both computationally and memory-wise in order to make
brute-force attacks unrewarding.

OpenSSL has had support for the scrypt algorithm since v1.1.0.  Add a
Node.js API modeled after `crypto.pbkdf2()` and `crypto.pbkdf2Sync()`.

Changes:

* Introduce helpers for copying buffers, collecting openssl errors, etc.

* Add new infrastructure for offloading crypto to a worker thread.

* Add a `AsyncWrap` JS class to simplify pbkdf2(), randomBytes() and
  scrypt().

Fixes: nodejs#8417
PR-URL: nodejs#20816
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

No branches or pull requests

5 participants