crypto: add randomFill and randomFillSync

crypto.randomFill and crypto.randomFillSync are similar to
crypto.randomBytes, but allow passing in a buffer as the first
argument. This allows us to reuse buffers to prevent having to
create a new one on every call.

PR-URL: https://github.com/nodejs/node/pull/10209
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

doc/api/crypto.md

@@ -1713,6 +1713,66 @@ This should normally never take longer than a few milliseconds. The only time
 when generating the random bytes may conceivably block for a longer period of
 time is right after boot, when the whole system is still low on entropy.
 
+### crypto.randomFillSync(buf[, offset][, size])
+<!-- YAML
+added: REPLACEME
+-->
+
+* `buf` {Buffer|Uint8Array} Must be supplied.
+* `offset` {number} Defaults to `0`.
+* `size` {number} Defaults to `buf.length - offset`.
+
+Synchronous version of [`crypto.randomFill()`][].
+
+Returns `buf`
+
+```js
+const buf = Buffer.alloc(10);
+console.log(crypto.randomFillSync(buf).toString('hex'));
+
+crypto.randomFillSync(buf, 5);
+console.log(buf.toString('hex'));
+
+// The above is equivalent to the following:
+crypto.randomFillSync(buf, 5, 5);
+console.log(buf.toString('hex'));
+```
+
+### crypto.randomFill(buf[, offset][, size], callback)
+<!-- YAML
+added: REPLACEME
+-->
+
+* `buf` {Buffer|Uint8Array} Must be supplied.
+* `offset` {number} Defaults to `0`.
+* `size` {number} Defaults to `buf.length - offset`.
+* `callback` {Function} `function(err, buf) {}`.
+
+This function is similar to [`crypto.randomBytes()`][] but requires the first
+argument to be a [`Buffer`][] that will be filled. It also
+requires that a callback is passed in.
+
+If the `callback` function is not provided, an error will be thrown.
+
+```js
+const buf = Buffer.alloc(10);
+crypto.randomFill(buf, (err, buf) => {
+  if (err) throw err;
+  console.log(buf.toString('hex'));
+});
+
+crypto.randomFill(buf, 5, (err, buf) => {
+  if (err) throw err;
+  console.log(buf.toString('hex'));
+});
+
+// The above is equivalent to the following:
+crypto.randomFill(buf, 5, 5, (err, buf) => {
+  if (err) throw err;
+  console.log(buf.toString('hex'));
+});
+```
+
 ### crypto.setEngine(engine[, flags])
 <!-- YAML
 added: v0.11.11
@@ -2152,6 +2212,8 @@ the `crypto`, `tls`, and `https` modules and are generally specific to OpenSSL.
 [`crypto.getCurves()`]: #crypto_crypto_getcurves
 [`crypto.getHashes()`]: #crypto_crypto_gethashes
 [`crypto.pbkdf2()`]: #crypto_crypto_pbkdf2_password_salt_iterations_keylen_digest_callback
+[`crypto.randomBytes()`]: #crypto_crypto_randombytes_size_callback
+[`crypto.randomFill()`]: #crypto_crypto_randombytesbuffer_buf_size_offset_cb
 [`decipher.final()`]: #crypto_decipher_final_output_encoding
 [`decipher.update()`]: #crypto_decipher_update_data_input_encoding_output_encoding
 [`diffieHellman.setPublicKey()`]: #crypto_diffiehellman_setpublickey_public_key_encoding

lib/crypto.js

@@ -40,8 +40,10 @@ const setFipsCrypto = binding.setFipsCrypto;
 const timingSafeEqual = binding.timingSafeEqual;
 
 const Buffer = require('buffer').Buffer;
+const kBufferMaxLength = require('buffer').kMaxLength;
 const stream = require('stream');
 const util = require('util');
+const { isUint8Array } = process.binding('util');
 const LazyTransform = require('internal/streams/lazy_transform');
 
 const DH_GENERATOR = 2;
@@ -696,6 +698,74 @@ exports.setEngine = function setEngine(id, flags) {
   return binding.setEngine(id, flags);
 };
 
+const kMaxUint32 = Math.pow(2, 32) - 1;
+
+function randomFillSync(buf, offset = 0, size) {
+  if (!isUint8Array(buf)) {
+    throw new TypeError('"buf" argument must be a Buffer or Uint8Array');
+  }
+
+  assertOffset(offset, buf.length);
+
+  if (size === undefined) size = buf.length - offset;
+
+  assertSize(size, offset, buf.length);
+
+  return binding.randomFill(buf, offset, size);
+}
+exports.randomFillSync = randomFillSync;
+
+function randomFill(buf, offset, size, cb) {
+  if (!isUint8Array(buf)) {
+    throw new TypeError('"buf" argument must be a Buffer or Uint8Array');
+  }
+
+  if (typeof offset === 'function') {
+    cb = offset;
+    offset = 0;
+    size = buf.length;
+  } else if (typeof size === 'function') {
+    cb = size;
+    size = buf.length - offset;
+  } else if (typeof cb !== 'function') {
+    throw new TypeError('"cb" argument must be a function');
+  }
+
+  assertOffset(offset, buf.length);
+  assertSize(size, offset, buf.length);
+
+  return binding.randomFill(buf, offset, size, cb);
+}
+exports.randomFill = randomFill;
+
+function assertOffset(offset, length) {
+  if (typeof offset !== 'number' || offset !== offset) {
+    throw new TypeError('offset must be a number');
+  }
+
+  if (offset > kMaxUint32 || offset < 0) {
+    throw new TypeError('offset must be a uint32');
+  }
+
+  if (offset > kBufferMaxLength || offset > length) {
+    throw new RangeError('offset out of range');
+  }
+}
+
+function assertSize(size, offset, length) {
+  if (typeof size !== 'number' || size !== size) {
+    throw new TypeError('size must be a number');
+  }
+
+  if (size > kMaxUint32 || size < 0) {
+    throw new TypeError('size must be a uint32');
+  }
+
+  if (size + offset > length || size > kBufferMaxLength) {
+    throw new RangeError('buffer too small');
+  }
+}
+
 exports.randomBytes = exports.pseudoRandomBytes = randomBytes;
 
 exports.rng = exports.prng = randomBytes;

src/node_crypto.cc

@@ -5574,11 +5574,18 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
 // Only instantiate within a valid HandleScope.
 class RandomBytesRequest : public AsyncWrap {
  public:
-  RandomBytesRequest(Environment* env, Local<Object> object, size_t size)
+  enum FreeMode { FREE_DATA, DONT_FREE_DATA };
+
+  RandomBytesRequest(Environment* env,
+                     Local<Object> object,
+                     size_t size,
+                     char* data,
+                     FreeMode free_mode)
       : AsyncWrap(env, object, AsyncWrap::PROVIDER_CRYPTO),
         error_(0),
         size_(size),
-        data_(node::Malloc(size)) {
+        data_(data),
+        free_mode_(free_mode) {
     Wrap(object, this);
   }
 
@@ -5599,9 +5606,15 @@ class RandomBytesRequest : public AsyncWrap {
     return data_;
   }
 
+  inline void set_data(char* data) {
+    data_ = data;
+  }
+
   inline void release() {
-    free(data_);
     size_ = 0;
+    if (free_mode_ == FREE_DATA) {
+      free(data_);
+    }
   }
 
   inline void return_memory(char** d, size_t* len) {
@@ -5627,6 +5640,7 @@ class RandomBytesRequest : public AsyncWrap {
   unsigned long error_;  // NOLINT(runtime/int)
   size_t size_;
   char* data_;
+  const FreeMode free_mode_;
 };
 
 
@@ -5665,7 +5679,18 @@ void RandomBytesCheck(RandomBytesRequest* req, Local<Value> argv[2]) {
     size_t size;
     req->return_memory(&data, &size);
     argv[0] = Null(req->env()->isolate());
-    argv[1] = Buffer::New(req->env(), data, size).ToLocalChecked();
+    Local<Value> buffer =
+        req->object()->Get(req->env()->context(),
+                           req->env()->buffer_string()).ToLocalChecked();
+
+    if (buffer->IsUint8Array()) {
+      CHECK_LE(req->size(), Buffer::Length(buffer));
+      char* buf = Buffer::Data(buffer);
+      memcpy(buf, data, req->size());
+      argv[1] = buffer;
+    } else {
+      argv[1] = Buffer::New(req->env(), data, size).ToLocalChecked();
+    }
   }
 }
 
@@ -5684,11 +5709,22 @@ void RandomBytesAfter(uv_work_t* work_req, int status) {
 }
 
 
+void RandomBytesProcessSync(Environment* env,
+                            RandomBytesRequest* req,
+                            Local<Value> argv[2]) {
+  env->PrintSyncTrace();
+  RandomBytesWork(req->work_req());
+  RandomBytesCheck(req, argv);
+  delete req;
+
+  if (!argv[0]->IsNull())
+    env->isolate()->ThrowException(argv[0]);
+}
+
+
 void RandomBytes(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  // maybe allow a buffer to write to? cuts down on object creation
-  // when generating random data in a loop
   if (!args[0]->IsUint32()) {
     return env->ThrowTypeError("size must be a number >= 0");
   }
@@ -5698,7 +5734,13 @@ void RandomBytes(const FunctionCallbackInfo<Value>& args) {
     return env->ThrowRangeError("size is not a valid Smi");
 
   Local<Object> obj = env->NewInternalFieldObject();
-  RandomBytesRequest* req = new RandomBytesRequest(env, obj, size);
+  char* data = node::Malloc(size);
+  RandomBytesRequest* req =
+      new RandomBytesRequest(env,
+                             obj,
+                             size,
+                             data,
+                             RandomBytesRequest::FREE_DATA);
 
   if (args[1]->IsFunction()) {
     obj->Set(env->ondone_string(), args[1]);
@@ -5711,15 +5753,55 @@ void RandomBytes(const FunctionCallbackInfo<Value>& args) {
                   RandomBytesAfter);
     args.GetReturnValue().Set(obj);
   } else {
-    env->PrintSyncTrace();
     Local<Value> argv[2];
-    RandomBytesWork(req->work_req());
-    RandomBytesCheck(req, argv);
-    delete req;
+    RandomBytesProcessSync(env, req, argv);
+    if (argv[0]->IsNull())
+      args.GetReturnValue().Set(argv[1]);
+  }
+}
 
-    if (!argv[0]->IsNull())
-      env->isolate()->ThrowException(argv[0]);
-    else
+
+void RandomBytesBuffer(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+
+  CHECK(args[0]->IsUint8Array());
+  CHECK(args[1]->IsUint32());
+  CHECK(args[2]->IsUint32());
+
+  int64_t offset = args[1]->IntegerValue();
+  int64_t size = args[2]->IntegerValue();
+
+  Local<Object> obj = env->NewInternalFieldObject();
+  obj->Set(env->context(), env->buffer_string(), args[0]).FromJust();
+  char* data = Buffer::Data(args[0]);
+  data += offset;
+
+  RandomBytesRequest* req =
+      new RandomBytesRequest(env,
+                             obj,
+                             size,
+                             data,
+                             RandomBytesRequest::DONT_FREE_DATA);
+  if (args[3]->IsFunction()) {
+    obj->Set(env->context(),
+             FIXED_ONE_BYTE_STRING(args.GetIsolate(), "ondone"),
+             args[3]).FromJust();
+
+    if (env->in_domain()) {
+      obj->Set(env->context(),
+               env->domain_string(),
+               env->domain_array()->Get(0)).FromJust();
+    }
+
+    uv_queue_work(env->event_loop(),
+                  req->work_req(),
+                  RandomBytesWork,
+                  RandomBytesAfter);
+    args.GetReturnValue().Set(obj);
+  } else {
+    Local<Value> argv[2];
+    RandomBytesProcessSync(env, req, argv);
+    if (argv[0]->IsNull())
       args.GetReturnValue().Set(argv[1]);
   }
 }
@@ -6144,6 +6226,7 @@ void InitCrypto(Local<Object> target,
   env->SetMethod(target, "setFipsCrypto", SetFipsCrypto);
   env->SetMethod(target, "PBKDF2", PBKDF2);
   env->SetMethod(target, "randomBytes", RandomBytes);
+  env->SetMethod(target, "randomFill", RandomBytesBuffer);
   env->SetMethod(target, "timingSafeEqual", TimingSafeEqual);
   env->SetMethod(target, "getSSLCiphers", GetSSLCiphers);
   env->SetMethod(target, "getCiphers", GetCiphers);

test/parallel/test-crypto-random.js

@@ -50,6 +50,208 @@ const expectedErrorRegexp = /^TypeError: size must be a number >= 0$/;
   });
 });
 
+{
+  const buf = Buffer.alloc(10);
+  const before = buf.toString('hex');
+  const after = crypto.randomFillSync(buf).toString('hex');
+  assert.notStrictEqual(before, after);
+}
+
+{
+  const buf = new Uint8Array(new Array(10).fill(0));
+  const before = Buffer.from(buf).toString('hex');
+  crypto.randomFillSync(buf);
+  const after = Buffer.from(buf).toString('hex');
+  assert.notStrictEqual(before, after);
+}
+
+{
+  const buf = Buffer.alloc(10);
+  const before = buf.toString('hex');
+  crypto.randomFill(buf, common.mustCall((err, buf) => {
+    assert.ifError(err);
+    const after = buf.toString('hex');
+    assert.notStrictEqual(before, after);
+  }));
+}
+
+{
+  const buf = new Uint8Array(new Array(10).fill(0));
+  const before = Buffer.from(buf).toString('hex');
+  crypto.randomFill(buf, common.mustCall((err, buf) => {
+    assert.ifError(err);
+    const after = Buffer.from(buf).toString('hex');
+    assert.notStrictEqual(before, after);
+  }));
+}
+
+{
+  const buf = Buffer.alloc(10);
+  const before = buf.toString('hex');
+  crypto.randomFillSync(buf, 5, 5);
+  const after = buf.toString('hex');
+  assert.notStrictEqual(before, after);
+  assert.deepStrictEqual(before.slice(0, 5), after.slice(0, 5));
+}
+
+{
+  const buf = new Uint8Array(new Array(10).fill(0));
+  const before = Buffer.from(buf).toString('hex');
+  crypto.randomFillSync(buf, 5, 5);
+  const after = Buffer.from(buf).toString('hex');
+  assert.notStrictEqual(before, after);
+  assert.deepStrictEqual(before.slice(0, 5), after.slice(0, 5));
+}
+
+{
+  const buf = Buffer.alloc(10);
+  const before = buf.toString('hex');
+  crypto.randomFillSync(buf, 5);
+  const after = buf.toString('hex');
+  assert.notStrictEqual(before, after);
+  assert.deepStrictEqual(before.slice(0, 5), after.slice(0, 5));
+}
+
+{
+  const buf = Buffer.alloc(10);
+  const before = buf.toString('hex');
+  crypto.randomFill(buf, 5, 5, common.mustCall((err, buf) => {
+    assert.ifError(err);
+    const after = buf.toString('hex');
+    assert.notStrictEqual(before, after);
+    assert.deepStrictEqual(before.slice(0, 5), after.slice(0, 5));
+  }));
+}
+
+{
+  const buf = new Uint8Array(new Array(10).fill(0));
+  const before = Buffer.from(buf).toString('hex');
+  crypto.randomFill(buf, 5, 5, common.mustCall((err, buf) => {
+    assert.ifError(err);
+    const after = Buffer.from(buf).toString('hex');
+    assert.notStrictEqual(before, after);
+    assert.deepStrictEqual(before.slice(0, 5), after.slice(0, 5));
+  }));
+}
+
+{
+  const bufs = [
+    Buffer.alloc(10),
+    new Uint8Array(new Array(10).fill(0))
+  ];
+
+  for (const buf of bufs) {
+    const len = Buffer.byteLength(buf);
+    assert.strictEqual(len, 10, `Expected byteLength of 10, got ${len}`);
+
+    assert.throws(() => {
+      crypto.randomFillSync(buf, 'test');
+    }, /offset must be a number/);
+
+    assert.throws(() => {
+      crypto.randomFillSync(buf, NaN);
+    }, /offset must be a number/);
+
+    assert.throws(() => {
+      crypto.randomFill(buf, 'test', common.noop);
+    }, /offset must be a number/);
+
+    assert.throws(() => {
+      crypto.randomFill(buf, NaN, common.noop);
+    }, /offset must be a number/);
+
+    const max = require('buffer').kMaxLength + 1;
+
+    assert.throws(() => {
+      crypto.randomFillSync(buf, 11);
+    }, /offset out of range/);
+
+    assert.throws(() => {
+      crypto.randomFillSync(buf, max);
+    }, /offset out of range/);
+
+    assert.throws(() => {
+      crypto.randomFill(buf, 11, common.noop);
+    }, /offset out of range/);
+
+    assert.throws(() => {
+      crypto.randomFill(buf, max, common.noop);
+    }, /offset out of range/);
+
+    assert.throws(() => {
+      crypto.randomFillSync(buf, 0, 'test');
+    }, /size must be a number/);
+
+    assert.throws(() => {
+      crypto.randomFillSync(buf, 0, NaN);
+    }, /size must be a number/);
+
+    assert.throws(() => {
+      crypto.randomFill(buf, 0, 'test', common.noop);
+    }, /size must be a number/);
+
+    assert.throws(() => {
+      crypto.randomFill(buf, 0, NaN, common.noop);
+    }, /size must be a number/);
+
+    {
+      const size = (-1 >>> 0) + 1;
+
+      assert.throws(() => {
+        crypto.randomFillSync(buf, 0, -10);
+      }, /size must be a uint32/);
+
+      assert.throws(() => {
+        crypto.randomFillSync(buf, 0, size);
+      }, /size must be a uint32/);
+
+      assert.throws(() => {
+        crypto.randomFill(buf, 0, -10, common.noop);
+      }, /size must be a uint32/);
+
+      assert.throws(() => {
+        crypto.randomFill(buf, 0, size, common.noop);
+      }, /size must be a uint32/);
+    }
+
+    assert.throws(() => {
+      crypto.randomFillSync(buf, -10);
+    }, /offset must be a uint32/);
+
+    assert.throws(() => {
+      crypto.randomFill(buf, -10, common.noop);
+    }, /offset must be a uint32/);
+
+    assert.throws(() => {
+      crypto.randomFillSync(buf, 1, 10);
+    }, /buffer too small/);
+
+    assert.throws(() => {
+      crypto.randomFill(buf, 1, 10, common.noop);
+    }, /buffer too small/);
+
+    assert.throws(() => {
+      crypto.randomFillSync(buf, 0, 12);
+    }, /buffer too small/);
+
+    assert.throws(() => {
+      crypto.randomFill(buf, 0, 12, common.noop);
+    }, /buffer too small/);
+
+    {
+      // Offset is too big
+      const offset = (-1 >>> 0) + 1;
+      assert.throws(() => {
+        crypto.randomFillSync(buf, offset, 10);
+      }, /offset must be a uint32/);
+
+      assert.throws(() => {
+        crypto.randomFill(buf, offset, 10, common.noop);
+      }, /offset must be a uint32/);
+    }
+  }
+}
+
 // #5126, "FATAL ERROR: v8::Object::SetIndexedPropertiesToExternalArrayData()
 // length exceeds max acceptable value"
 assert.throws(function() {

test/parallel/test-domain-crypto.js

@@ -38,6 +38,8 @@ global.domain = require('domain');
 // should not throw a 'TypeError: undefined is not a function' exception
 crypto.randomBytes(8);
 crypto.randomBytes(8, common.noop);
+const buf = Buffer.alloc(8);
+crypto.randomFillSync(buf);
 crypto.pseudoRandomBytes(8);
 crypto.pseudoRandomBytes(8, common.noop);
 crypto.pbkdf2('password', 'salt', 8, 8, 'sha1', common.noop);