From 9fcc9ca59b8c6dd3508c910f7c0791490b6c8d94 Mon Sep 17 00:00:00 2001
From: Oli Lalonde <olalonde@gmail.com>
Date: Sun, 2 Aug 2020 08:08:39 -0400
Subject: [PATCH] crypto: add randomInt function
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

PR-URL: /~https://github.com/nodejs/node/pull/34600
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
---
 doc/api/crypto.md                   |  39 ++++++
 lib/crypto.js                       |   4 +-
 lib/internal/crypto/random.js       |  80 ++++++++++++-
 test/parallel/test-crypto-random.js | 180 ++++++++++++++++++++++++++++
 4 files changed, 301 insertions(+), 2 deletions(-)

diff --git a/doc/api/crypto.md b/doc/api/crypto.md
index 539ce6fc79de46..f0c7620c10dbcf 100644
--- a/doc/api/crypto.md
+++ b/doc/api/crypto.md
@@ -2801,6 +2801,44 @@ threadpool request. To minimize threadpool task length variation, partition
 large `randomFill` requests when doing so as part of fulfilling a client
 request.
 
+### `crypto.randomInt([min, ]max[, callback])`
+<!-- YAML
+added: REPLACEME
+-->
+
+* `min` {integer} Start of random range (inclusive). **Default**: `0`.
+* `max` {integer} End of random range (exclusive).
+* `callback` {Function} `function(err, n) {}`.
+
+Return a random integer `n` such that `min <= n < max`.  This
+implementation avoids [modulo bias][].
+
+The range (`max - min`) must be less than `2^48`. `min` and `max` must
+be safe integers.
+
+If the `callback` function is not provided, the random integer is
+generated synchronously.
+
+```js
+// Asynchronous
+crypto.randomInt(3, (err, n) => {
+  if (err) throw err;
+  console.log(`Random number chosen from (0, 1, 2): ${n}`);
+});
+```
+
+```js
+// Synchronous
+const n = crypto.randomInt(3);
+console.log(`Random number chosen from (0, 1, 2): ${n}`);
+```
+
+```js
+// With `min` argument
+const n = crypto.randomInt(1, 7);
+console.log(`The dice rolled: ${n}`);
+```
+
 ### `crypto.scrypt(password, salt, keylen[, options], callback)`
 <!-- YAML
 added: v10.5.0
@@ -3573,6 +3611,7 @@ See the [list of SSL OP Flags][] for details.
 [NIST SP 800-131A]: https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar1.pdf
 [NIST SP 800-132]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-132.pdf
 [NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
+[modulo bias]: https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle#Modulo_bias
 [Nonce-Disrespecting Adversaries]: /~https://github.com/nonce-disrespect/nonce-disrespect
 [OpenSSL's SPKAC implementation]: https://www.openssl.org/docs/man1.1.0/apps/openssl-spkac.html
 [RFC 1421]: https://www.rfc-editor.org/rfc/rfc1421.txt
diff --git a/lib/crypto.js b/lib/crypto.js
index e4b9d479a50671..b2bcc4d0a44592 100644
--- a/lib/crypto.js
+++ b/lib/crypto.js
@@ -52,7 +52,8 @@ const {
 const {
   randomBytes,
   randomFill,
-  randomFillSync
+  randomFillSync,
+  randomInt
 } = require('internal/crypto/random');
 const {
   pbkdf2,
@@ -184,6 +185,7 @@ module.exports = {
   randomBytes,
   randomFill,
   randomFillSync,
+  randomInt,
   scrypt,
   scryptSync,
   sign: signOneShot,
diff --git a/lib/internal/crypto/random.js b/lib/internal/crypto/random.js
index 801c0eb8789fcd..561586472fd835 100644
--- a/lib/internal/crypto/random.js
+++ b/lib/internal/crypto/random.js
@@ -3,6 +3,7 @@
 const {
   MathMin,
   NumberIsNaN,
+  NumberIsSafeInteger
 } = primordials;
 
 const { AsyncWrap, Providers } = internalBinding('async_wrap');
@@ -119,6 +120,82 @@ function randomFill(buf, offset, size, cb) {
   _randomBytes(buf, offset, size, wrap);
 }
 
+// Largest integer we can read from a buffer.
+// e.g.: Buffer.from("ff".repeat(6), "hex").readUIntBE(0, 6);
+const RAND_MAX = 0xFFFF_FFFF_FFFF;
+
+// Generates an integer in [min, max) range where min is inclusive and max is
+// exclusive.
+function randomInt(min, max, cb) {
+  // Detect optional min syntax
+  // randomInt(max)
+  // randomInt(max, cb)
+  const minNotSpecified = typeof max === 'undefined' ||
+    typeof max === 'function';
+
+  if (minNotSpecified) {
+    cb = max;
+    max = min;
+    min = 0;
+  }
+
+  const isSync = typeof cb === 'undefined';
+  if (!isSync && typeof cb !== 'function') {
+    throw new ERR_INVALID_CALLBACK(cb);
+  }
+  if (!NumberIsSafeInteger(min)) {
+    throw new ERR_INVALID_ARG_TYPE('min', 'safe integer', min);
+  }
+  if (!NumberIsSafeInteger(max)) {
+    throw new ERR_INVALID_ARG_TYPE('max', 'safe integer', max);
+  }
+  if (!(max >= min)) {
+    throw new ERR_OUT_OF_RANGE('max', `>= ${min}`, max);
+  }
+
+  // First we generate a random int between [0..range)
+  const range = max - min;
+
+  if (!(range <= RAND_MAX)) {
+    throw new ERR_OUT_OF_RANGE(`max${minNotSpecified ? '' : ' - min'}`,
+                               `<= ${RAND_MAX}`, range);
+  }
+
+  const excess = RAND_MAX % range;
+  const randLimit = RAND_MAX - excess;
+
+  if (isSync) {
+    // Sync API
+    while (true) {
+      const x = randomBytes(6).readUIntBE(0, 6);
+      // If x > (maxVal - (maxVal % range)), we will get "modulo bias"
+      if (x > randLimit) {
+        // Try again
+        continue;
+      }
+      const n = (x % range) + min;
+      return n;
+    }
+  } else {
+    // Async API
+    const pickAttempt = () => {
+      randomBytes(6, (err, bytes) => {
+        if (err) return cb(err);
+        const x = bytes.readUIntBE(0, 6);
+        // If x > (maxVal - (maxVal % range)), we will get "modulo bias"
+        if (x > randLimit) {
+          // Try again
+          return pickAttempt();
+        }
+        const n = (x % range) + min;
+        cb(null, n);
+      });
+    };
+
+    pickAttempt();
+  }
+}
+
 function handleError(ex, buf) {
   if (ex) throw ex;
   return buf;
@@ -127,5 +204,6 @@ function handleError(ex, buf) {
 module.exports = {
   randomBytes,
   randomFill,
-  randomFillSync
+  randomFillSync,
+  randomInt
 };
diff --git a/test/parallel/test-crypto-random.js b/test/parallel/test-crypto-random.js
index 8a34f0ca223810..67b63c42b52814 100644
--- a/test/parallel/test-crypto-random.js
+++ b/test/parallel/test-crypto-random.js
@@ -315,3 +315,183 @@ assert.throws(
   assert.strictEqual(desc.writable, true);
   assert.strictEqual(desc.enumerable, false);
 });
+
+
+{
+  // Asynchronous API
+  const randomInts = [];
+  for (let i = 0; i < 100; i++) {
+    crypto.randomInt(3, common.mustCall((err, n) => {
+      assert.ifError(err);
+      assert.ok(n >= 0);
+      assert.ok(n < 3);
+      randomInts.push(n);
+      if (randomInts.length === 100) {
+        assert.ok(!randomInts.includes(-1));
+        assert.ok(randomInts.includes(0));
+        assert.ok(randomInts.includes(1));
+        assert.ok(randomInts.includes(2));
+        assert.ok(!randomInts.includes(3));
+      }
+    }));
+  }
+}
+{
+  // Synchronous API
+  const randomInts = [];
+  for (let i = 0; i < 100; i++) {
+    const n = crypto.randomInt(3);
+    assert.ok(n >= 0);
+    assert.ok(n < 3);
+    randomInts.push(n);
+  }
+
+  assert.ok(!randomInts.includes(-1));
+  assert.ok(randomInts.includes(0));
+  assert.ok(randomInts.includes(1));
+  assert.ok(randomInts.includes(2));
+  assert.ok(!randomInts.includes(3));
+}
+{
+  // Positive range
+  const randomInts = [];
+  for (let i = 0; i < 100; i++) {
+    crypto.randomInt(1, 3, common.mustCall((err, n) => {
+      assert.ifError(err);
+      assert.ok(n >= 1);
+      assert.ok(n < 3);
+      randomInts.push(n);
+      if (randomInts.length === 100) {
+        assert.ok(!randomInts.includes(0));
+        assert.ok(randomInts.includes(1));
+        assert.ok(randomInts.includes(2));
+        assert.ok(!randomInts.includes(3));
+      }
+    }));
+  }
+}
+{
+  // Negative range
+  const randomInts = [];
+  for (let i = 0; i < 100; i++) {
+    crypto.randomInt(-10, -8, common.mustCall((err, n) => {
+      assert.ifError(err);
+      assert.ok(n >= -10);
+      assert.ok(n < -8);
+      randomInts.push(n);
+      if (randomInts.length === 100) {
+        assert.ok(!randomInts.includes(-11));
+        assert.ok(randomInts.includes(-10));
+        assert.ok(randomInts.includes(-9));
+        assert.ok(!randomInts.includes(-8));
+      }
+    }));
+  }
+}
+{
+
+  ['10', true, NaN, null, {}, []].forEach((i) => {
+    const invalidMinError = {
+      code: 'ERR_INVALID_ARG_TYPE',
+      name: 'TypeError',
+      message: 'The "min" argument must be safe integer.' +
+               `${common.invalidArgTypeHelper(i)}`,
+    };
+    const invalidMaxError = {
+      code: 'ERR_INVALID_ARG_TYPE',
+      name: 'TypeError',
+      message: 'The "max" argument must be safe integer.' +
+               `${common.invalidArgTypeHelper(i)}`,
+    };
+
+    assert.throws(
+      () => crypto.randomInt(i, 100),
+      invalidMinError
+    );
+    assert.throws(
+      () => crypto.randomInt(i, 100, common.mustNotCall()),
+      invalidMinError
+    );
+    assert.throws(
+      () => crypto.randomInt(i),
+      invalidMaxError
+    );
+    assert.throws(
+      () => crypto.randomInt(i, common.mustNotCall()),
+      invalidMaxError
+    );
+    assert.throws(
+      () => crypto.randomInt(0, i, common.mustNotCall()),
+      invalidMaxError
+    );
+    assert.throws(
+      () => crypto.randomInt(0, i),
+      invalidMaxError
+    );
+  });
+
+  const maxInt = Number.MAX_SAFE_INTEGER;
+  const minInt = Number.MIN_SAFE_INTEGER;
+
+  crypto.randomInt(minInt, minInt + 5, common.mustCall());
+  crypto.randomInt(maxInt - 5, maxInt, common.mustCall());
+
+  assert.throws(
+    () => crypto.randomInt(minInt - 1, minInt + 5, common.mustNotCall()),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      name: 'TypeError',
+      message: 'The "min" argument must be safe integer.' +
+      `${common.invalidArgTypeHelper(minInt - 1)}`,
+    }
+  );
+
+  assert.throws(
+    () => crypto.randomInt(maxInt + 1, common.mustNotCall()),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      name: 'TypeError',
+      message: 'The "max" argument must be safe integer.' +
+      `${common.invalidArgTypeHelper(maxInt + 1)}`,
+    }
+  );
+
+  crypto.randomInt(0, common.mustCall());
+  crypto.randomInt(0, 0, common.mustCall());
+  assert.throws(() => crypto.randomInt(-1, common.mustNotCall()), {
+    code: 'ERR_OUT_OF_RANGE',
+    name: 'RangeError',
+    message: 'The value of "max" is out of range. It must be >= 0. Received -1'
+  });
+
+  const MAX_RANGE = 0xFFFF_FFFF_FFFF;
+  crypto.randomInt(MAX_RANGE, common.mustCall());
+  crypto.randomInt(1, MAX_RANGE + 1, common.mustCall());
+  assert.throws(
+    () => crypto.randomInt(1, MAX_RANGE + 2, common.mustNotCall()),
+    {
+      code: 'ERR_OUT_OF_RANGE',
+      name: 'RangeError',
+      message: 'The value of "max - min" is out of range. ' +
+               `It must be <= ${MAX_RANGE}. ` +
+               'Received 281_474_976_710_656'
+    }
+  );
+
+  assert.throws(() => crypto.randomInt(MAX_RANGE + 1, common.mustNotCall()), {
+    code: 'ERR_OUT_OF_RANGE',
+    name: 'RangeError',
+    message: 'The value of "max" is out of range. ' +
+             `It must be <= ${MAX_RANGE}. ` +
+             'Received 281_474_976_710_656'
+  });
+
+  [true, NaN, null, {}, [], 10].forEach((i) => {
+    const cbError = {
+      code: 'ERR_INVALID_CALLBACK',
+      name: 'TypeError',
+      message: `Callback must be a function. Received ${inspect(i)}`
+    };
+    assert.throws(() => crypto.randomInt(0, 1, i), cbError);
+  });
+}