crypto: add better scrypt option aliases

Make parameter names available in a human-readable way, for more accessible/self-documenting usage of the `scrypt` functions. This implements a review comment from the original PR that has not been addressed. Refs: #20816 (comment) PR-URL: #21525 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
nodejs · Aug 15, 2018 · 2dc7f17 · 2dc7f17
1 parent c85d00b
commit 2dc7f17
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 14 deletions.
diff --git a/doc/api/crypto.md b/doc/api/crypto.md
@@ -2152,15 +2152,23 @@ request.
 ### crypto.scrypt(password, salt, keylen[, options], callback)
 <!-- YAML
 added: v10.5.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/XXX
+    description: The `cost`, `blockSize` and `parallelization` option names
+                 have been added.
 -->
 * `password` {string|Buffer|TypedArray|DataView}
 * `salt` {string|Buffer|TypedArray|DataView}
 * `keylen` {number}
 * `options` {Object}
-  - `N` {number} CPU/memory cost parameter. Must be a power of two greater
+  - `cost` {number} CPU/memory cost parameter. Must be a power of two greater
     than one. **Default:** `16384`.
-  - `r` {number} Block size parameter. **Default:** `8`.
-  - `p` {number} Parallelization parameter. **Default:** `1`.
+  - `blockSize` {number} Block size parameter. **Default:** `8`.
+  - `parallelization` {number} Parallelization parameter. **Default:** `1`.
+  - `N` {number} Alias for `cost`. Only one of both may be specified.
+  - `r` {number} Alias for `blockSize`. Only one of both may be specified.
+  - `p` {number} Alias for `parallelization`. Only one of both may be specified.
   - `maxmem` {number} Memory upper bound. It is an error when (approximately)
     `128 * N * r > maxmem`. **Default:** `32 * 1024 * 1024`.
 * `callback` {Function}
@@ -2198,15 +2206,23 @@ crypto.scrypt('secret', 'salt', 64, { N: 1024 }, (err, derivedKey) => {
 ### crypto.scryptSync(password, salt, keylen[, options])
 <!-- YAML
 added: v10.5.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/XXX
+    description: The `cost`, `blockSize` and `parallelization` option names
+                 have been added.
 -->
 * `password` {string|Buffer|TypedArray|DataView}
 * `salt` {string|Buffer|TypedArray|DataView}
 * `keylen` {number}
 * `options` {Object}
-  - `N` {number} CPU/memory cost parameter. Must be a power of two greater
+  - `cost` {number} CPU/memory cost parameter. Must be a power of two greater
     than one. **Default:** `16384`.
-  - `r` {number} Block size parameter. **Default:** `8`.
-  - `p` {number} Parallelization parameter. **Default:** `1`.
+  - `blockSize` {number} Block size parameter. **Default:** `8`.
+  - `parallelization` {number} Parallelization parameter. **Default:** `1`.
+  - `N` {number} Alias for `cost`. Only one of both may be specified.
+  - `r` {number} Alias for `blockSize`. Only one of both may be specified.
+  - `p` {number} Alias for `parallelization`. Only one of both may be specified.
   - `maxmem` {number} Memory upper bound. It is an error when (approximately)
     `128 * N * r > maxmem`. **Default:** `32 * 1024 * 1024`.
 * Returns: {Buffer}

diff --git a/lib/internal/crypto/scrypt.js b/lib/internal/crypto/scrypt.js
@@ -80,13 +80,26 @@ function check(password, salt, keylen, options, callback) {
 
   let { N, r, p, maxmem } = defaults;
   if (options && options !== defaults) {
-    if (options.hasOwnProperty('N'))
+    let has_N, has_r, has_p;
+    if (has_N = (options.N !== undefined))
       N = validateInt32(options.N, 'N', 0, INT_MAX);
-    if (options.hasOwnProperty('r'))
+    if (options.cost !== undefined) {
+      if (has_N) throw new ERR_CRYPTO_SCRYPT_INVALID_PARAMETER();
+      N = validateInt32(options.cost, 'cost', 0, INT_MAX);
+    }
+    if (has_r = (options.r !== undefined))
       r = validateInt32(options.r, 'r', 0, INT_MAX);
-    if (options.hasOwnProperty('p'))
+    if (options.blockSize !== undefined) {
+      if (has_r) throw new ERR_CRYPTO_SCRYPT_INVALID_PARAMETER();
+      r = validateInt32(options.blockSize, 'blockSize', 0, INT_MAX);
+    }
+    if (has_p = (options.p !== undefined))
       p = validateInt32(options.p, 'p', 0, INT_MAX);
-    if (options.hasOwnProperty('maxmem'))
+    if (options.parallelization !== undefined) {
+      if (has_p) throw new ERR_CRYPTO_SCRYPT_INVALID_PARAMETER();
+      p = validateInt32(options.parallelization, 'parallelization', 0, INT_MAX);
+    }
+    if (options.maxmem !== undefined)
       maxmem = validateInt32(options.maxmem, 'maxmem', 0, INT_MAX);
     if (N === 0) N = defaults.N;
     if (r === 0) r = defaults.r;

diff --git a/test/parallel/test-crypto-scrypt.js b/test/parallel/test-crypto-scrypt.js
@@ -56,14 +56,50 @@ const good = [
         '7023bdcb3afd7348461c06cd81fd38ebfda8fbba904f8e3ea9b543f6545da1f2' +
         'd5432955613f0fcf62d49705242a9af9e61e85dc0d651e40dfcf017b45575887',
   },
+  {
+    pass: '',
+    salt: '',
+    keylen: 64,
+    cost: 16,
+    parallelization: 1,
+    blockSize: 1,
+    expected:
+        '77d6576238657b203b19ca42c18a0497f16b4844e3074ae8dfdffa3fede21442' +
+        'fcd0069ded0948f8326a753a0fc81f17e8d3e0fb2e0d3628cf35e20c38d18906',
+  },
+  {
+    pass: 'password',
+    salt: 'NaCl',
+    keylen: 64,
+    cost: 1024,
+    parallelization: 16,
+    blockSize: 8,
+    expected:
+        'fdbabe1c9d3472007856e7190d01e9fe7c6ad7cbc8237830e77376634b373162' +
+        '2eaf30d92e22a3886ff109279d9830dac727afb94a83ee6d8360cbdfa2cc0640',
+  },
+  {
+    pass: 'pleaseletmein',
+    salt: 'SodiumChloride',
+    keylen: 64,
+    cost: 16384,
+    parallelization: 1,
+    blockSize: 8,
+    expected:
+        '7023bdcb3afd7348461c06cd81fd38ebfda8fbba904f8e3ea9b543f6545da1f2' +
+        'd5432955613f0fcf62d49705242a9af9e61e85dc0d651e40dfcf017b45575887',
+  },
 ];
 
 // Test vectors that should fail.
 const bad = [
-  { N: 1, p: 1, r: 1 },        // N < 2
-  { N: 3, p: 1, r: 1 },        // Not power of 2.
-  { N: 2 ** 16, p: 1, r: 1 },  // N >= 2**(r*16)
-  { N: 2, p: 2 ** 30, r: 1 },  // p > (2**30-1)/r
+  { N: 1, p: 1, r: 1 },         // N < 2
+  { N: 3, p: 1, r: 1 },         // Not power of 2.
+  { N: 2 ** 16, p: 1, r: 1 },   // N >= 2**(r*16)
+  { N: 2, p: 2 ** 30, r: 1 },   // p > (2**30-1)/r
+  { N: 1, cost: 1 },            // both N and cost
+  { p: 1, parallelization: 1 }, // both p and parallelization
+  { r: 1, blockSize: 1 }        // both r and blocksize
 ];
 
 // Test vectors where 128*N*r exceeds maxmem.