crypto: refactor pbkdf2() and pbkdf2Sync() methods

Use the scrypt() infrastructure to reimplement pbkdf2() in a simpler manner. 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>
nodejs · Jun 13, 2018 · 6262fa4 · 6262fa4
1 parent c9b4592
commit 6262fa4
Show file tree

Hide file tree

Showing 6 changed files with 121 additions and 180 deletions.
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -739,6 +739,12 @@ An invalid [crypto digest algorithm][] was specified.
 A crypto method was used on an object that was in an invalid state. For
 instance, calling [`cipher.getAuthTag()`][] before calling `cipher.final()`.
 
+<a id="ERR_CRYPTO_PBKDF2_ERROR"></a>
+### ERR_CRYPTO_PBKDF2_ERROR
+
+The PBKDF2 algorithm failed for unspecified reasons. OpenSSL does not provide
+more details and therefore neither does Node.js.
+
 <a id="ERR_CRYPTO_SCRYPT_INVALID_PARAMETER"></a>
 ### ERR_CRYPTO_SCRYPT_INVALID_PARAMETER
 

diff --git a/lib/internal/crypto/pbkdf2.js b/lib/internal/crypto/pbkdf2.js
@@ -1,39 +1,61 @@
 'use strict';
 
+const { AsyncWrap, Providers } = process.binding('async_wrap');
+const { Buffer } = require('buffer');
+const { pbkdf2: _pbkdf2 } = process.binding('crypto');
 const {
+  ERR_CRYPTO_INVALID_DIGEST,
+  ERR_CRYPTO_PBKDF2_ERROR,
   ERR_INVALID_ARG_TYPE,
   ERR_INVALID_CALLBACK,
-  ERR_CRYPTO_INVALID_DIGEST,
 } = require('internal/errors').codes;
 const {
   checkIsArrayBufferView,
   checkIsUint,
   getDefaultEncoding,
 } = require('internal/crypto/util');
-const {
-  PBKDF2
-} = process.binding('crypto');
 
 function pbkdf2(password, salt, iterations, keylen, digest, callback) {
   if (typeof digest === 'function') {
     callback = digest;
     digest = undefined;
   }
 
+  ({ password, salt, iterations, keylen, digest } =
+      check(password, salt, iterations, keylen, digest, callback));
+
   if (typeof callback !== 'function')
     throw new ERR_INVALID_CALLBACK();
 
-  return _pbkdf2(password, salt, iterations, keylen, digest, callback);
+  const encoding = getDefaultEncoding();
+  const keybuf = Buffer.alloc(keylen);
+
+  const wrap = new AsyncWrap(Providers.PBKDF2REQUEST);
+  wrap.ondone = (ok) => {  // Retains keybuf while request is in flight.
+    if (!ok) return callback.call(wrap, new ERR_CRYPTO_PBKDF2_ERROR());
+    if (encoding === 'buffer') return callback.call(wrap, null, keybuf);
+    callback.call(wrap, null, keybuf.toString(encoding));
+  };
+
+  handleError(keybuf, password, salt, iterations, digest, wrap);
 }
 
 function pbkdf2Sync(password, salt, iterations, keylen, digest) {
-  return _pbkdf2(password, salt, iterations, keylen, digest);
+  ({ password, salt, iterations, keylen, digest } =
+      check(password, salt, iterations, keylen, digest, pbkdf2Sync));
+  const keybuf = Buffer.alloc(keylen);
+  handleError(keybuf, password, salt, iterations, digest);
+  const encoding = getDefaultEncoding();
+  if (encoding === 'buffer') return keybuf;
+  return keybuf.toString(encoding);
 }
 
-function _pbkdf2(password, salt, iterations, keylen, digest, callback) {
-
-  if (digest !== null && typeof digest !== 'string')
-    throw new ERR_INVALID_ARG_TYPE('digest', ['string', 'null'], digest);
+function check(password, salt, iterations, keylen, digest, callback) {
+  if (typeof digest !== 'string') {
+    if (digest !== null)
+      throw new ERR_INVALID_ARG_TYPE('digest', ['string', 'null'], digest);
+    digest = 'sha1';
+  }
 
   password = checkIsArrayBufferView('password', password);
   salt = checkIsArrayBufferView('salt', salt);
@@ -42,30 +64,17 @@ function _pbkdf2(password, salt, iterations, keylen, digest, callback) {
   iterations = checkIsUint('iterations', iterations, 'a non-negative number');
   keylen = checkIsUint('keylen', keylen);
 
-  const encoding = getDefaultEncoding();
+  return { password, salt, iterations, keylen, digest };
+}
 
-  if (encoding === 'buffer') {
-    const ret = PBKDF2(password, salt, iterations, keylen, digest, callback);
-    if (ret === -1)
-      throw new ERR_CRYPTO_INVALID_DIGEST(digest);
-    return ret;
-  }
+function handleError(keybuf, password, salt, iterations, digest, wrap) {
+  const rc = _pbkdf2(keybuf, password, salt, iterations, digest, wrap);
 
-  // at this point, we need to handle encodings.
-  if (callback) {
-    function next(er, ret) {
-      if (ret)
-        ret = ret.toString(encoding);
-      callback(er, ret);
-    }
-    if (PBKDF2(password, salt, iterations, keylen, digest, next) === -1)
-      throw new ERR_CRYPTO_INVALID_DIGEST(digest);
-  } else {
-    const ret = PBKDF2(password, salt, iterations, keylen, digest);
-    if (ret === -1)
-      throw new ERR_CRYPTO_INVALID_DIGEST(digest);
-    return ret.toString(encoding);
-  }
+  if (rc === -1)
+    throw new ERR_CRYPTO_INVALID_DIGEST(digest);
+
+  if (rc === false)
+    throw new ERR_CRYPTO_PBKDF2_ERROR();
 }
 
 module.exports = {

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -503,6 +503,8 @@ E('ERR_CRYPTO_HASH_FINALIZED', 'Digest already called', Error);
 E('ERR_CRYPTO_HASH_UPDATE_FAILED', 'Hash update failed', Error);
 E('ERR_CRYPTO_INVALID_DIGEST', 'Invalid digest: %s', TypeError);
 E('ERR_CRYPTO_INVALID_STATE', 'Invalid state for operation %s', Error);
+// TODO(bnoordhuis) Decapitalize: s/PBKDF2 Error/PBKDF2 error/
+E('ERR_CRYPTO_PBKDF2_ERROR', 'PBKDF2 Error', Error);
 E('ERR_CRYPTO_SCRYPT_INVALID_PARAMETER', 'Invalid scrypt parameter', Error);
 E('ERR_CRYPTO_SCRYPT_NOT_SUPPORTED', 'Scrypt algorithm not supported', Error);
 // Switch to TypeError. The current implementation does not seem right.

diff --git a/src/env.h b/src/env.h
@@ -244,7 +244,6 @@ struct PackageConfig {
   V(password_string, "password")                                              \
   V(path_string, "path")                                                      \
   V(pending_handle_string, "pendingHandle")                                   \
-  V(pbkdf2_error_string, "PBKDF2 Error")                                      \
   V(pid_string, "pid")                                                        \
   V(pipe_string, "pipe")                                                      \
   V(pipe_target_string, "pipeTarget")                                         \
@@ -337,7 +336,6 @@ struct PackageConfig {
   V(inspector_console_api_object, v8::Object)                                 \
   V(message_port, v8::Object)                                                 \
   V(message_port_constructor_template, v8::FunctionTemplate)                  \
-  V(pbkdf2_constructor_template, v8::ObjectTemplate)                          \
   V(pipe_constructor_template, v8::FunctionTemplate)                          \
   V(performance_entry_callback, v8::Function)                                 \
   V(performance_entry_template, v8::Function)                                 \