Skip to content

Commit a430ef5

Browse files
committed
src: prepare for v8 sandboxing
PR-URL: #58376 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
1 parent 83052ff commit a430ef5

7 files changed

+81
-7
lines changed

src/crypto/crypto_dh.cc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ using ncrypto::DHPointer;
2222
using ncrypto::EVPKeyCtxPointer;
2323
using ncrypto::EVPKeyPointer;
2424
using v8::ArrayBuffer;
25+
using v8::BackingStoreInitializationMode;
26+
using v8::BackingStoreOnFailureMode;
2527
using v8::ConstructorBehavior;
2628
using v8::Context;
2729
using v8::DontDelete;
@@ -58,6 +60,20 @@ MaybeLocal<Value> DataPointerToBuffer(Environment* env, DataPointer&& data) {
5860
struct Flag {
5961
bool secure;
6062
};
63+
#ifdef V8_ENABLE_SANDBOX
64+
auto backing = ArrayBuffer::NewBackingStore(
65+
env->isolate(),
66+
data.size(),
67+
BackingStoreInitializationMode::kUninitialized,
68+
BackingStoreOnFailureMode::kReturnNull);
69+
if (!backing) {
70+
THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
71+
return MaybeLocal<Value>();
72+
}
73+
if (data.size() > 0) {
74+
memcpy(backing->Data(), data.get(), data.size());
75+
}
76+
#else
6177
auto backing = ArrayBuffer::NewBackingStore(
6278
data.get(),
6379
data.size(),
@@ -67,6 +83,7 @@ MaybeLocal<Value> DataPointerToBuffer(Environment* env, DataPointer&& data) {
6783
},
6884
new Flag{data.isSecure()});
6985
data.release();
86+
#endif // V8_ENABLE_SANDBOX
7087

7188
auto ab = ArrayBuffer::New(env->isolate(), std::move(backing));
7289
return Buffer::New(env, ab, 0, ab->ByteLength()).FromMaybe(Local<Value>());

src/crypto/crypto_util.cc

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ using ncrypto::EnginePointer;
3434
using ncrypto::SSLPointer;
3535
using v8::ArrayBuffer;
3636
using v8::BackingStore;
37+
using v8::BackingStoreInitializationMode;
38+
using v8::BackingStoreOnFailureMode;
3739
using v8::BigInt;
3840
using v8::Context;
3941
using v8::EscapableHandleScope;
@@ -339,16 +341,37 @@ ByteSource& ByteSource::operator=(ByteSource&& other) noexcept {
339341
return *this;
340342
}
341343

342-
std::unique_ptr<BackingStore> ByteSource::ReleaseToBackingStore() {
344+
std::unique_ptr<BackingStore> ByteSource::ReleaseToBackingStore(
345+
Environment* env) {
343346
// It's ok for allocated_data_ to be nullptr but
344347
// only if size_ is zero.
345348
CHECK_IMPLIES(size_ > 0, allocated_data_ != nullptr);
349+
#ifdef V8_ENABLE_SANDBOX
350+
// If the v8 sandbox is enabled, then all array buffers must be allocated
351+
// via the isolate. External buffers are not allowed. So, instead of wrapping
352+
// the allocated data we'll copy it instead.
353+
354+
// TODO(@jasnell): It would be nice to use an abstracted utility to do this
355+
// branch instead of duplicating the V8_ENABLE_SANDBOX check each time.
356+
std::unique_ptr<BackingStore> ptr = ArrayBuffer::NewBackingStore(
357+
env->isolate(),
358+
size(),
359+
BackingStoreInitializationMode::kUninitialized,
360+
BackingStoreOnFailureMode::kReturnNull);
361+
if (!ptr) {
362+
THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
363+
return nullptr;
364+
}
365+
memcpy(ptr->Data(), allocated_data_, size());
366+
OPENSSL_clear_free(allocated_data_, size_);
367+
#else
346368
std::unique_ptr<BackingStore> ptr = ArrayBuffer::NewBackingStore(
347369
allocated_data_,
348370
size(),
349371
[](void* data, size_t length, void* deleter_data) {
350372
OPENSSL_clear_free(deleter_data, length);
351373
}, allocated_data_);
374+
#endif // V8_ENABLE_SANDBOX
352375
CHECK(ptr);
353376
allocated_data_ = nullptr;
354377
data_ = nullptr;
@@ -357,7 +380,7 @@ std::unique_ptr<BackingStore> ByteSource::ReleaseToBackingStore() {
357380
}
358381

359382
Local<ArrayBuffer> ByteSource::ToArrayBuffer(Environment* env) {
360-
std::unique_ptr<BackingStore> store = ReleaseToBackingStore();
383+
std::unique_ptr<BackingStore> store = ReleaseToBackingStore(env);
361384
return ArrayBuffer::New(env->isolate(), std::move(store));
362385
}
363386

@@ -648,8 +671,19 @@ namespace {
648671
// using OPENSSL_malloc. However, if the secure heap is
649672
// initialized, SecureBuffer will automatically use it.
650673
void SecureBuffer(const FunctionCallbackInfo<Value>& args) {
651-
CHECK(args[0]->IsUint32());
652674
Environment* env = Environment::GetCurrent(args);
675+
#ifdef V8_ENABLE_SANDBOX
676+
// The v8 sandbox is enabled, so we cannot use the secure heap because
677+
// the sandbox requires that all array buffers be allocated via the isolate.
678+
// That is fundamentally incompatible with the secure heap which allocates
679+
// in openssl's secure heap area. Instead we'll just throw an error here.
680+
//
681+
// That said, we really shouldn't get here in the first place since the
682+
// option to enable the secure heap is only available when the sandbox
683+
// is disabled.
684+
UNREACHABLE();
685+
#else
686+
CHECK(args[0]->IsUint32());
653687
uint32_t len = args[0].As<Uint32>()->Value();
654688

655689
auto data = DataPointer::SecureAlloc(len);
@@ -676,6 +710,7 @@ void SecureBuffer(const FunctionCallbackInfo<Value>& args) {
676710

677711
Local<ArrayBuffer> buffer = ArrayBuffer::New(env->isolate(), store);
678712
args.GetReturnValue().Set(Uint8Array::New(buffer, 0, len));
713+
#endif // V8_ENABLE_SANDBOX
679714
}
680715

681716
void SecureHeapUsed(const FunctionCallbackInfo<Value>& args) {

src/crypto/crypto_util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ class ByteSource final {
185185
// Creates a v8::BackingStore that takes over responsibility for
186186
// any allocated data. The ByteSource will be reset with size = 0
187187
// after being called.
188-
std::unique_ptr<v8::BackingStore> ReleaseToBackingStore();
188+
std::unique_ptr<v8::BackingStore> ReleaseToBackingStore(Environment* env);
189189

190190
v8::Local<v8::ArrayBuffer> ToArrayBuffer(Environment* env);
191191

src/crypto/crypto_x509.cc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ using v8::Array;
2929
using v8::ArrayBuffer;
3030
using v8::ArrayBufferView;
3131
using v8::BackingStoreInitializationMode;
32+
using v8::BackingStoreOnFailureMode;
3233
using v8::Boolean;
3334
using v8::Context;
3435
using v8::Date;
@@ -131,13 +132,29 @@ MaybeLocal<Value> ToBuffer(Environment* env, BIOPointer* bio) {
131132
if (bio == nullptr || !*bio) [[unlikely]]
132133
return {};
133134
BUF_MEM* mem = *bio;
135+
#ifdef V8_ENABLE_SANDBOX
136+
// If the v8 sandbox is enabled, then all array buffers must be allocated
137+
// via the isolate. External buffers are not allowed. So, instead of wrapping
138+
// the BIOPointer we'll copy it instead.
139+
auto backing = ArrayBuffer::NewBackingStore(
140+
env->isolate(),
141+
mem->length,
142+
BackingStoreInitializationMode::kUninitialized,
143+
BackingStoreOnFailureMode::kReturnNull);
144+
if (!backing) {
145+
THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
146+
return MaybeLocal<Value>();
147+
}
148+
memcpy(backing->Data(), mem->data, mem->length);
149+
#else
134150
auto backing = ArrayBuffer::NewBackingStore(
135151
mem->data,
136152
mem->length,
137153
[](void*, size_t, void* data) {
138154
BIOPointer free_me(static_cast<BIO*>(data));
139155
},
140156
bio->release());
157+
#endif // V8_ENABLE_SANDBOX
141158
auto ab = ArrayBuffer::New(env->isolate(), std::move(backing));
142159
Local<Value> ret;
143160
if (!Buffer::New(env, ab, 0, ab->ByteLength()).ToLocal(&ret)) return {};

src/js_native_api_v8.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ napi_status NewExternalString(napi_env env,
114114
CHECK_NEW_STRING_ARGS(env, str, length, result);
115115

116116
napi_status status;
117-
#if defined(V8_ENABLE_SANDBOX)
117+
#ifdef V8_ENABLE_SANDBOX
118118
status = create_api(env, str, length, result);
119119
if (status == napi_ok) {
120120
if (copied != nullptr) {

src/node_api.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,9 +1056,9 @@ napi_create_external_buffer(napi_env env,
10561056
NAPI_PREAMBLE(env);
10571057
CHECK_ARG(env, result);
10581058

1059-
#if defined(V8_ENABLE_SANDBOX)
1059+
#ifdef V8_ENABLE_SANDBOX
10601060
return napi_set_last_error(env, napi_no_external_buffers_allowed);
1061-
#endif
1061+
#endif // V8_ENABLE_SANDBOX
10621062

10631063
v8::Isolate* isolate = env->isolate;
10641064

src/node_options.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ void PerProcessOptions::CheckOptions(std::vector<std::string>* errors,
8181
}
8282

8383
// Any value less than 2 disables use of the secure heap.
84+
#ifndef V8_ENABLE_SANDBOX
85+
// The secure heap is not supported when V8_ENABLE_SANDBOX is enabled.
8486
if (secure_heap >= 2) {
8587
if ((secure_heap & (secure_heap - 1)) != 0)
8688
errors->push_back("--secure-heap must be a power of 2");
@@ -93,6 +95,7 @@ void PerProcessOptions::CheckOptions(std::vector<std::string>* errors,
9395
if ((secure_heap_min & (secure_heap_min - 1)) != 0)
9496
errors->push_back("--secure-heap-min must be a power of 2");
9597
}
98+
#endif // V8_ENABLE_SANDBOX
9699
#endif // HAVE_OPENSSL
97100

98101
if (use_largepages != "off" &&
@@ -1172,6 +1175,7 @@ PerProcessOptionsParser::PerProcessOptionsParser(
11721175
"force FIPS crypto (cannot be disabled)",
11731176
&PerProcessOptions::force_fips_crypto,
11741177
kAllowedInEnvvar);
1178+
#ifndef V8_ENABLE_SANDBOX
11751179
AddOption("--secure-heap",
11761180
"total size of the OpenSSL secure heap",
11771181
&PerProcessOptions::secure_heap,
@@ -1180,6 +1184,7 @@ PerProcessOptionsParser::PerProcessOptionsParser(
11801184
"minimum allocation size from the OpenSSL secure heap",
11811185
&PerProcessOptions::secure_heap_min,
11821186
kAllowedInEnvvar);
1187+
#endif // V8_ENABLE_SANDBOX
11831188
#endif // HAVE_OPENSSL
11841189
#if OPENSSL_VERSION_MAJOR >= 3
11851190
AddOption("--openssl-legacy-provider",

0 commit comments

Comments
 (0)