Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 2d9c3cc

Browse filesBrowse files
bnoordhuistargos
bnoordhuis
authored and
targos
committed
crypto: refactor randomBytes()
Use the scrypt() infrastructure to reimplement randomBytes() and randomFill() in a simpler manner. PR-URL: #20816 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
1 parent 6262fa4 commit 2d9c3cc
Copy full SHA for 2d9c3cc

File tree

Expand file treeCollapse file tree

4 files changed

+65
-211
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+65
-211
lines changed
Open diff view settings
Collapse file

‎lib/internal/crypto/random.js‎

Copy file name to clipboardExpand all lines: lib/internal/crypto/random.js
+32-10Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
'use strict';
22

3+
const { AsyncWrap, Providers } = process.binding('async_wrap');
4+
const { Buffer } = require('buffer');
5+
const { randomBytes: _randomBytes } = process.binding('crypto');
36
const {
47
ERR_INVALID_ARG_TYPE,
58
ERR_INVALID_CALLBACK,
69
ERR_OUT_OF_RANGE
710
} = require('internal/errors').codes;
811
const { isArrayBufferView } = require('internal/util/types');
9-
const {
10-
randomBytes: _randomBytes,
11-
randomFill: _randomFill
12-
} = process.binding('crypto');
1312

1413
const { kMaxLength } = require('buffer');
1514
const kMaxUint32 = Math.pow(2, 32) - 1;
@@ -27,7 +26,7 @@ function assertOffset(offset, elementSize, length) {
2726
throw new ERR_OUT_OF_RANGE('offset', `>= 0 && <= ${maxLength}`, offset);
2827
}
2928

30-
return offset;
29+
return offset >>> 0; // Convert to uint32.
3130
}
3231

3332
function assertSize(size, elementSize, offset, length) {
@@ -46,14 +45,25 @@ function assertSize(size, elementSize, offset, length) {
4645
throw new ERR_OUT_OF_RANGE('size + offset', `<= ${length}`, size + offset);
4746
}
4847

49-
return size;
48+
return size >>> 0; // Convert to uint32.
5049
}
5150

5251
function randomBytes(size, cb) {
53-
assertSize(size, 1, 0, Infinity);
52+
size = assertSize(size, 1, 0, Infinity);
5453
if (cb !== undefined && typeof cb !== 'function')
5554
throw new ERR_INVALID_CALLBACK();
56-
return _randomBytes(size, cb);
55+
56+
const buf = Buffer.alloc(size);
57+
58+
if (!cb) return handleError(buf, 0, size);
59+
60+
const wrap = new AsyncWrap(Providers.RANDOMBYTESREQUEST);
61+
wrap.ondone = (ex) => { // Retains buf while request is in flight.
62+
if (ex) return cb.call(wrap, ex);
63+
cb.call(wrap, null, buf);
64+
};
65+
66+
_randomBytes(buf, 0, size, wrap);
5767
}
5868

5969
function randomFillSync(buf, offset = 0, size) {
@@ -71,7 +81,7 @@ function randomFillSync(buf, offset = 0, size) {
7181
size = assertSize(size, elementSize, offset, buf.byteLength);
7282
}
7383

74-
return _randomFill(buf, offset, size);
84+
return handleError(buf, offset, size);
7585
}
7686

7787
function randomFill(buf, offset, size, cb) {
@@ -100,7 +110,19 @@ function randomFill(buf, offset, size, cb) {
100110
size = assertSize(size, elementSize, offset, buf.byteLength);
101111
}
102112

103-
return _randomFill(buf, offset, size, cb);
113+
const wrap = new AsyncWrap(Providers.RANDOMBYTESREQUEST);
114+
wrap.ondone = (ex) => { // Retains buf while request is in flight.
115+
if (ex) return cb.call(wrap, ex);
116+
cb.call(wrap, null, buf);
117+
};
118+
119+
_randomBytes(buf, offset, size, wrap);
120+
}
121+
122+
function handleError(buf, offset, size) {
123+
const ex = _randomBytes(buf, offset, size);
124+
if (ex) throw ex;
125+
return buf;
104126
}
105127

106128
module.exports = {
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,6 @@ struct PackageConfig {
344344
V(promise_reject_unhandled_function, v8::Function) \
345345
V(promise_wrap_template, v8::ObjectTemplate) \
346346
V(push_values_to_array_function, v8::Function) \
347-
V(randombytes_constructor_template, v8::ObjectTemplate) \
348347
V(sab_lifetimepartner_constructor_template, v8::FunctionTemplate) \
349348
V(script_context_constructor_template, v8::FunctionTemplate) \
350349
V(script_data_constructor_function, v8::Function) \
Collapse file

‎src/node_crypto.cc‎

Copy file name to clipboardExpand all lines: src/node_crypto.cc
+32-199Lines changed: 32 additions & 199 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,6 @@ using v8::NewStringType;
8282
using v8::Nothing;
8383
using v8::Null;
8484
using v8::Object;
85-
using v8::ObjectTemplate;
8685
using v8::PropertyAttribute;
8786
using v8::ReadOnly;
8887
using v8::Signature;
@@ -4589,208 +4588,50 @@ inline void CopyBuffer(Local<Value> buf, std::vector<char>* vec) {
45894588
}
45904589

45914590

4592-
// Only instantiate within a valid HandleScope.
4593-
class RandomBytesRequest : public AsyncWrap, public ThreadPoolWork {
4594-
public:
4595-
enum FreeMode { FREE_DATA, DONT_FREE_DATA };
4596-
4597-
RandomBytesRequest(Environment* env,
4598-
Local<Object> object,
4599-
size_t size,
4600-
char* data,
4601-
FreeMode free_mode)
4602-
: AsyncWrap(env, object, AsyncWrap::PROVIDER_RANDOMBYTESREQUEST),
4603-
ThreadPoolWork(env),
4604-
error_(0),
4605-
size_(size),
4606-
data_(data),
4607-
free_mode_(free_mode) {
4608-
}
4609-
4610-
inline size_t size() const {
4611-
return size_;
4612-
}
4613-
4614-
inline char* data() const {
4615-
return data_;
4616-
}
4617-
4618-
inline void set_data(char* data) {
4619-
data_ = data;
4620-
}
4591+
struct RandomBytesJob : public CryptoJob {
4592+
unsigned char* data;
4593+
size_t size;
4594+
CryptoErrorVector errors;
4595+
Maybe<int> rc;
46214596

4622-
inline void release() {
4623-
size_ = 0;
4624-
if (free_mode_ == FREE_DATA) {
4625-
free(data_);
4626-
data_ = nullptr;
4627-
}
4628-
}
4597+
inline explicit RandomBytesJob(Environment* env)
4598+
: CryptoJob(env), rc(Nothing<int>()) {}
46294599

4630-
inline void return_memory(char** d, size_t* len) {
4631-
*d = data_;
4632-
data_ = nullptr;
4633-
*len = size_;
4634-
size_ = 0;
4600+
inline void DoThreadPoolWork() override {
4601+
CheckEntropy(); // Ensure that OpenSSL's PRNG is properly seeded.
4602+
rc = Just(RAND_bytes(data, size));
4603+
if (0 == rc.FromJust()) errors.Capture();
46354604
}
46364605

4637-
inline unsigned long error() const { // NOLINT(runtime/int)
4638-
return error_;
4606+
inline void AfterThreadPoolWork() override {
4607+
Local<Value> arg = ToResult();
4608+
async_wrap->MakeCallback(env->ondone_string(), 1, &arg);
46394609
}
46404610

4641-
inline void set_error(unsigned long err) { // NOLINT(runtime/int)
4642-
error_ = err;
4611+
inline Local<Value> ToResult() const {
4612+
if (errors.empty()) return Undefined(env->isolate());
4613+
return errors.ToException(env);
46434614
}
4644-
4645-
size_t self_size() const override { return sizeof(*this); }
4646-
4647-
void DoThreadPoolWork() override;
4648-
void AfterThreadPoolWork(int status) override;
4649-
4650-
private:
4651-
unsigned long error_; // NOLINT(runtime/int)
4652-
size_t size_;
4653-
char* data_;
4654-
const FreeMode free_mode_;
46554615
};
46564616

46574617

4658-
void RandomBytesRequest::DoThreadPoolWork() {
4659-
// Ensure that OpenSSL's PRNG is properly seeded.
4660-
CheckEntropy();
4661-
4662-
const int r = RAND_bytes(reinterpret_cast<unsigned char*>(data_), size_);
4663-
4664-
// RAND_bytes() returns 0 on error.
4665-
if (r == 0) {
4666-
set_error(ERR_get_error()); // NOLINT(runtime/int)
4667-
} else if (r == -1) {
4668-
set_error(static_cast<unsigned long>(-1)); // NOLINT(runtime/int)
4669-
}
4670-
}
4671-
4672-
4673-
// don't call this function without a valid HandleScope
4674-
void RandomBytesCheck(RandomBytesRequest* req, Local<Value> (*argv)[2]) {
4675-
if (req->error()) {
4676-
char errmsg[256] = "Operation not supported";
4677-
4678-
if (req->error() != static_cast<unsigned long>(-1)) // NOLINT(runtime/int)
4679-
ERR_error_string_n(req->error(), errmsg, sizeof errmsg);
4680-
4681-
(*argv)[0] = Exception::Error(OneByteString(req->env()->isolate(), errmsg));
4682-
(*argv)[1] = Null(req->env()->isolate());
4683-
req->release();
4684-
} else {
4685-
char* data = nullptr;
4686-
size_t size;
4687-
req->return_memory(&data, &size);
4688-
(*argv)[0] = Null(req->env()->isolate());
4689-
Local<Value> buffer =
4690-
req->object()->Get(req->env()->context(),
4691-
req->env()->buffer_string()).ToLocalChecked();
4692-
4693-
if (buffer->IsArrayBufferView()) {
4694-
CHECK_LE(req->size(), Buffer::Length(buffer));
4695-
char* buf = Buffer::Data(buffer);
4696-
memcpy(buf, data, req->size());
4697-
(*argv)[1] = buffer;
4698-
} else {
4699-
(*argv)[1] = Buffer::New(req->env(), data, size)
4700-
.ToLocalChecked();
4701-
}
4702-
}
4703-
}
4704-
4705-
4706-
void RandomBytesRequest::AfterThreadPoolWork(int status) {
4707-
std::unique_ptr<RandomBytesRequest> req(this);
4708-
if (status == UV_ECANCELED)
4709-
return;
4710-
CHECK_EQ(status, 0);
4711-
HandleScope handle_scope(env()->isolate());
4712-
Context::Scope context_scope(env()->context());
4713-
Local<Value> argv[2];
4714-
RandomBytesCheck(this, &argv);
4715-
MakeCallback(env()->ondone_string(), arraysize(argv), argv);
4716-
}
4717-
4718-
4719-
void RandomBytesProcessSync(Environment* env,
4720-
std::unique_ptr<RandomBytesRequest> req,
4721-
Local<Value> (*argv)[2]) {
4722-
env->PrintSyncTrace();
4723-
req->DoThreadPoolWork();
4724-
RandomBytesCheck(req.get(), argv);
4725-
4726-
if (!(*argv)[0]->IsNull())
4727-
env->isolate()->ThrowException((*argv)[0]);
4728-
}
4729-
4730-
47314618
void RandomBytes(const FunctionCallbackInfo<Value>& args) {
4619+
CHECK(args[0]->IsArrayBufferView()); // buffer; wrap object retains ref.
4620+
CHECK(args[1]->IsUint32()); // offset
4621+
CHECK(args[2]->IsUint32()); // size
4622+
CHECK(args[3]->IsObject() || args[3]->IsUndefined()); // wrap object
4623+
const uint32_t offset = args[1].As<Uint32>()->Value();
4624+
const uint32_t size = args[2].As<Uint32>()->Value();
4625+
CHECK_GE(offset + size, offset); // Overflow check.
4626+
CHECK_LE(offset + size, Buffer::Length(args[0])); // Bounds check.
47324627
Environment* env = Environment::GetCurrent(args);
4733-
4734-
const int64_t size = args[0]->IntegerValue();
4735-
CHECK(size <= Buffer::kMaxLength);
4736-
4737-
Local<Object> obj = env->randombytes_constructor_template()->
4738-
NewInstance(env->context()).ToLocalChecked();
4739-
char* data = node::Malloc(size);
4740-
std::unique_ptr<RandomBytesRequest> req(
4741-
new RandomBytesRequest(env,
4742-
obj,
4743-
size,
4744-
data,
4745-
RandomBytesRequest::FREE_DATA));
4746-
4747-
if (args[1]->IsFunction()) {
4748-
obj->Set(env->context(), env->ondone_string(), args[1]).FromJust();
4749-
4750-
req.release()->ScheduleWork();
4751-
args.GetReturnValue().Set(obj);
4752-
} else {
4753-
Local<Value> argv[2];
4754-
RandomBytesProcessSync(env, std::move(req), &argv);
4755-
if (argv[0]->IsNull())
4756-
args.GetReturnValue().Set(argv[1]);
4757-
}
4758-
}
4759-
4760-
4761-
void RandomBytesBuffer(const FunctionCallbackInfo<Value>& args) {
4762-
Environment* env = Environment::GetCurrent(args);
4763-
4764-
CHECK(args[0]->IsArrayBufferView());
4765-
CHECK(args[1]->IsUint32());
4766-
CHECK(args[2]->IsUint32());
4767-
4768-
int64_t offset = args[1]->IntegerValue();
4769-
int64_t size = args[2]->IntegerValue();
4770-
4771-
Local<Object> obj = env->randombytes_constructor_template()->
4772-
NewInstance(env->context()).ToLocalChecked();
4773-
obj->Set(env->context(), env->buffer_string(), args[0]).FromJust();
4774-
char* data = Buffer::Data(args[0]);
4775-
data += offset;
4776-
4777-
std::unique_ptr<RandomBytesRequest> req(
4778-
new RandomBytesRequest(env,
4779-
obj,
4780-
size,
4781-
data,
4782-
RandomBytesRequest::DONT_FREE_DATA));
4783-
if (args[3]->IsFunction()) {
4784-
obj->Set(env->context(), env->ondone_string(), args[3]).FromJust();
4785-
4786-
req.release()->ScheduleWork();
4787-
args.GetReturnValue().Set(obj);
4788-
} else {
4789-
Local<Value> argv[2];
4790-
RandomBytesProcessSync(env, std::move(req), &argv);
4791-
if (argv[0]->IsNull())
4792-
args.GetReturnValue().Set(argv[1]);
4793-
}
4628+
std::unique_ptr<RandomBytesJob> job(new RandomBytesJob(env));
4629+
job->data = reinterpret_cast<unsigned char*>(Buffer::Data(args[0])) + offset;
4630+
job->size = size;
4631+
if (args[3]->IsObject()) return RandomBytesJob::Run(std::move(job), args[3]);
4632+
env->PrintSyncTrace();
4633+
job->DoThreadPoolWork();
4634+
args.GetReturnValue().Set(job->ToResult());
47944635
}
47954636

47964637

@@ -5355,7 +5196,6 @@ void Initialize(Local<Object> target,
53555196

53565197
env->SetMethod(target, "pbkdf2", PBKDF2);
53575198
env->SetMethod(target, "randomBytes", RandomBytes);
5358-
env->SetMethod(target, "randomFill", RandomBytesBuffer);
53595199
env->SetMethod(target, "timingSafeEqual", TimingSafeEqual);
53605200
env->SetMethod(target, "getSSLCiphers", GetSSLCiphers);
53615201
env->SetMethod(target, "getCiphers", GetCiphers);
@@ -5380,13 +5220,6 @@ void Initialize(Local<Object> target,
53805220
#ifndef OPENSSL_NO_SCRYPT
53815221
env->SetMethod(target, "scrypt", Scrypt);
53825222
#endif // OPENSSL_NO_SCRYPT
5383-
5384-
Local<FunctionTemplate> rb = FunctionTemplate::New(env->isolate());
5385-
rb->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "RandomBytes"));
5386-
AsyncWrap::AddWrapMethods(env, rb);
5387-
Local<ObjectTemplate> rbt = rb->InstanceTemplate();
5388-
rbt->SetInternalFieldCount(1);
5389-
env->set_randombytes_constructor_template(rbt);
53905223
}
53915224

53925225
} // namespace crypto
Collapse file

‎test/sequential/test-async-wrap-getasyncid.js‎

Copy file name to clipboardExpand all lines: test/sequential/test-async-wrap-getasyncid.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check
120120
crypto.pbkdf2('password', 'salt', 1, 20, 'sha256', mc);
121121

122122
crypto.randomBytes(1, common.mustCall(function rb() {
123-
testInitialized(this, 'RandomBytes');
123+
testInitialized(this, 'AsyncWrap');
124124
}));
125125

126126
if (typeof process.binding('crypto').scrypt === 'function') {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.