Node: Cryptographically secure random integer in range

Created on 25 Sep 2019  路  20Comments  路  Source: nodejs/node

Is your feature request related to a problem? Please describe.

There is no API to obtain a random integer within a range. It's a common enough problem to deal with (e.g. to implement other algorithms like Fisher鈥揧ates shuffle) and easy to get wrong. Most other runtimes/languages implement this in their standard library.

Describe the solution you'd like

// something like:
const randomInt = require('crypto').randomInt

const min = 0
const max = 10
randomInt(min, max, cb)

Describe alternatives you've considered

I'm not sure about the internal implementation but I gave it a stab here: https://github.com/olalonde/crypto-range. http://www.pcg-random.org/posts/bounded-rands.html has a nice overview of different techniques (my implementation is probably not the most efficient).

crypto feature request

Most helpful comment

I think this would make a good addition. But.

  1. There should be support for BigInts
  2. Floating-point support would be nice too
  3. A step parameter 脿 la python's random.randrange() might be nice (but not essential)

All 20 comments

@nodejs/tooling

I think this would make a good addition. But.

  1. There should be support for BigInts
  2. Floating-point support would be nice too
  3. A step parameter 脿 la python's random.randrange() might be nice (but not essential)

There should be support for BigInts

randomBigInt(min, max, cb)?

Floating-point support would be nice too

In a generic "random" library I'd agree but since this is "crypto", I'm not entirely sure. At least, I personally am not sure what it would mean to implement it correctly nor feel very confident to do it (some relevant discussion here).

A step parameter 脿 la python's random.randrange() might be nice (but not essential)

Shouldn't be too hard to add this argument.

randomBigInt(min, max, cb)?

Yes, that could work. Another option is to make randomInt() return a BigInt if its arguments are BigInts, i.e.:

typeof randomInt(0, 42)    // 'number'
typeof randomInt(0n, 42n)  // 'bigint'
typeof randomInt(0n, 42)   // TBD: 'bigint' or TypeError?

Pro: lets callers support numbers and bigints transparently

Con: potential for type confusion?

Floating-point support

The problems around generating good, uniformly distributed random floating-point numbers is precisely why I think it would be good to have in core because I expect most people won't be aware of the subtleties with 1.0 / randomInt(0, 42).

If nothing else, it lets us warn about distribution caveats in the documentation.


An alternative approach is to:

  1. implement crypto.randomBits(n) that generates a buffer with exactly n random bits (not bytes) with uniform distribution, and

  2. add a way to turn that buffer into a BigInt efficiently (possibly with support for big and little endianness?)

Higher-level APIs can then be derived from the above primitives and wouldn't necessarily need to live in core.

Technically even randomBits() doesn't need to live in core because it can be implemented in terms of randomBytes():

function randomBits(n) {
  const t = n >>> 3
  const u = n & 7
  const b = randomBytes(t + !!u)
  if (u > 0) b[t] &= (1 << u) - 1
  return b
}

I'm ignoring callbacks here for the sake of brevity.

Pro: lets callers support numbers and bigints transparently
Con: potential for type confusion?

There was some related discussion on the BigInt proposal. I believe it was ultimately decided not to overload existing math functions to support BigInt. In the spirit of consistency, I would vote for an explicit randomBigInt. That being said... the same document warns against using BigInt for crypto purposes, so maybe it would be better to err on the side of caution and leave it out for now?

If there's a chance it can get merged, I'd be happy to send a PR for randomInt(min, max, [step,], cb) but tbh, I don't really feel confident about my ability to implement other APIs.

@nodejs/crypto This needs your input.

Bump

@bnoordhuis:

Con: potential for type confusion?

Really not a fan of polymorphic signatures, not only because of type confusion but due to performance costs. I'd much rather just keep the variants separate.

I can definitely get behind having these APIs in core. I'm less convinced on the floating point variant but I buy into @bnoordhuis' argument there.

Feature seems reasonable to me. @orlonde, it sounds like a PR would be accepted, go for it.

If we are only considering safe integers (that is, 53 bits or less), would the usual floor(range * random()) with a cryptographically secure version of Math.random() be enough? I am not sure, I feel like it might have security implications, but if it does not, we could simply provide the floating point variant for now.

Another option is to make randomInt() return a BigInt if its arguments are BigInts, i.e.:

This doesn't sound too bad to me. I am not a huge fan of bloating our APIs with functions that only differ in types, but I understand @jasnell's arguments.

i think "random floats" are diverse enough of a problem that we should leave it to userland. we have int in range, float in range, int to float in range (e.g. 0x2440 for 10.0), list goes on.

@jasnell @tniessen If most uses end up looking like y = (typeof x === 'bigint' ? randomBigInt : randomInt)(0, x) then we don't gain much from having separate functions.

That said, I don't really feel strongly either way.

@devsnek Can you elaborate? What diversity are you referring to?

Likewise if uses end doing the type check after the function call. Code will already need to be aware that bigints are being used so a dedicated function is not going to make things more difficult there.

Has there been any movement or progress on this feature?
A cryptographically secure Math.random() would be great.

Attempting a pull request: https://github.com/nodejs/node/pull/34600

That's could be a great functionality!

A basic variant of this feature has landed in #34600, and will be available in future releases of Node.js. For further feature requests (e.g., use cases for random big integers), please open a new issue.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/random

function getRandomInt(min, max) {
  min = Math.ceil(min);
  max = Math.floor(max);
  //The maximum is exclusive and the minimum is inclusive
  return Math.floor(Math.random() * (max - min) + min);
}

@RodolfoSilva Math.random() does not provide cryptographically secure random numbers. It shouldn't be used for anything related to security, cf. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/random. On the opposite, the Node.js crypto module provides cryptographically secure methods.

Thanks @madarche 馃榿.

I hadn't paid attention to this note. My mistake.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

cong88 picture cong88  路  3Comments

vsemozhetbyt picture vsemozhetbyt  路  3Comments

stevenvachon picture stevenvachon  路  3Comments

fanjunzhi picture fanjunzhi  路  3Comments

akdor1154 picture akdor1154  路  3Comments