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: add randomInt function #34600

Closed
wants to merge 11 commits into from
40 changes: 40 additions & 0 deletions doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -2800,6 +2800,46 @@ 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 (inclusive).
jasnell marked this conversation as resolved.
Show resolved Hide resolved
* `callback` {Function} `function(err, n) {}`.

Return a random integer `n` such that `min <= n <= max`. This
implementation avoids [modulo
bias](https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle#Modulo_bias).
olalonde marked this conversation as resolved.
Show resolved Hide resolved

The cardinality of the range (`max - min + 1`) must be at most `2^48 -
1`. `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, 3): ${n}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a suggestion, why can't we have promisified function also.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yashLadha ... a Promisified version of the crypto APIs is coming. I expect to be able to open the PR in early September (if not sooner)

});
```

```js
// Synchronous
const n = crypto.randomInt(3);
console.log(`Random number chosen from (0, 1, 2, 3): ${n}`);
```

```js
crypto.randomInt(1, 6, (err, n) => {
if (err) throw err;
console.log(`The dice rolled: ${n}`);
});
```

### `crypto.scrypt(password, salt, keylen[, options], callback)`
<!-- YAML
added: v10.5.0
Expand Down
4 changes: 3 additions & 1 deletion lib/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ const {
const {
randomBytes,
randomFill,
randomFillSync
randomFillSync,
randomInt
} = require('internal/crypto/random');
const {
pbkdf2,
Expand Down Expand Up @@ -184,6 +185,7 @@ module.exports = {
randomBytes,
randomFill,
randomFillSync,
randomInt,
scrypt,
scryptSync,
sign: signOneShot,
Expand Down
74 changes: 73 additions & 1 deletion lib/internal/crypto/random.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const {
MathMin,
NumberIsNaN,
NumberIsSafeInteger
} = primordials;

const { AsyncWrap, Providers } = internalBinding('async_wrap');
Expand Down Expand Up @@ -119,6 +120,76 @@ 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 = 281474976710655;
tniessen marked this conversation as resolved.
Show resolved Hide resolved
olalonde marked this conversation as resolved.
Show resolved Hide resolved

// Generates an integer in [min, max] range where both min and max are
// inclusive.
function randomInt(min, max, cb) {
addaleax marked this conversation as resolved.
Show resolved Hide resolved
jasnell marked this conversation as resolved.
Show resolved Hide resolved
// randomInt(max, cb)
// randomInt(max)
if (typeof max === 'function' || typeof max === 'undefined') {
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);
}
addaleax marked this conversation as resolved.
Show resolved Hide resolved
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 + 1;

if (!(range <= RAND_MAX)) {
addaleax marked this conversation as resolved.
Show resolved Hide resolved
throw new ERR_OUT_OF_RANGE('max - min', `< ${RAND_MAX}`, range);
}
jasnell marked this conversation as resolved.
Show resolved Hide resolved

const excess = RAND_MAX % range;
const randLimit = RAND_MAX - excess;

if (isSync) {
// Sync API
while (true) {
jasnell marked this conversation as resolved.
Show resolved Hide resolved
const x = randomBytes(6).readUIntBE(0, 6);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performance suggestion: allocate a buffer once with new FastBuffer(6) and use randomFillSync() to fill it up, that avoids creating potentially many throwaway buffers.

(I recognize that on average we'll get lucky 50% or more on the first try so consider this a take-it-or-leave-it suggestion, not a must.)

// If x > (maxVal - (maxVal % range)), we will get "modulo bias"
if (x > randLimit) {
// Try again
continue;
}
tniessen marked this conversation as resolved.
Show resolved Hide resolved
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;
Expand All @@ -127,5 +198,6 @@ function handleError(ex, buf) {
module.exports = {
randomBytes,
randomFill,
randomFillSync
randomFillSync,
randomInt
};
177 changes: 177 additions & 0 deletions test/parallel/test-crypto-random.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,3 +315,180 @@ assert.throws(
assert.strictEqual(desc.writable, true);
assert.strictEqual(desc.enumerable, false);
});


{
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));
olalonde marked this conversation as resolved.
Show resolved Hide resolved
assert.ok(!randomInts.includes(4));
}
}));
addaleax marked this conversation as resolved.
Show resolved Hide resolved
}
}
{
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));
assert.ok(!randomInts.includes(-7));
}
}));
}
}
{
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));
assert.ok(!randomInts.includes(4));
}
}));
}
}
{
// 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));
assert.ok(!randomInts.includes(4));
}
{
// Synchronous API with min
const randomInts = [];
for (let i = 0; i < 100; i++) {
const n = crypto.randomInt(3, 5);
assert.ok(n >= 3);
assert.ok(n <= 5);
randomInts.push(n);
}

assert.ok(randomInts.includes(3));
assert.ok(randomInts.includes(4));
assert.ok(randomInts.includes(5));
}
{

['10', true, NaN, null, {}, []].forEach((i) => {
assert.throws(() => crypto.randomInt(i, 100, common.mustNotCall()), {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "min" argument must be safe integer.' +
`${common.invalidArgTypeHelper(i)}`,
});

assert.throws(() => crypto.randomInt(0, i, common.mustNotCall()), {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "max" argument must be safe integer.' +
`${common.invalidArgTypeHelper(i)}`,
});

assert.throws(() => crypto.randomInt(i, common.mustNotCall()), {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "max" argument must be safe integer.' +
`${common.invalidArgTypeHelper(i)}`,
});
});

const minInt = Number.MIN_SAFE_INTEGER;
const maxInt = Number.MAX_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 - 5, maxInt + 1, common.mustNotCall()),
{
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "max" argument must be safe integer.' +
`${common.invalidArgTypeHelper(maxInt + 1)}`,
}
);


assert.throws(() => crypto.randomInt(0, 0, common.mustNotCall()), {
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError',
message: 'The value of "max" is out of range. It must be > 0. Received 0'
});

assert.throws(() => crypto.randomInt(0, -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'
});

assert.throws(() => crypto.randomInt(0, common.mustNotCall()), {
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError',
message: 'The value of "max" is out of range. It must be > 0. Received 0'
});

assert.throws(() => crypto.randomInt(2 ** 48, common.mustNotCall()), {
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError',
message: 'The value of "max - min" is out of range. ' +
`It must be <= ${(2 ** 48) - 1}. ` +
'Received 281_474_976_710_657'
});

[1, true, NaN, null, {}, []].forEach((i) => {
assert.throws(
() => crypto.randomInt(1, 2, i),
{
code: 'ERR_INVALID_CALLBACK',
name: 'TypeError',
message: `Callback must be a function. Received ${inspect(i)}`
}
);
});
}