RFC: Plan for updating to Electron >= 21 without compromising extension host stability

[deepak1556]

Copied over from https://github.com/microsoft/vscode-internalbacklog/issues/3548 for visibility.

Tl:dr; For extension authors, to have minimal impact from the V8 sandbox we are making changes to the extension host that will allow your native dependencies to run without having to recompile. No changes needed from the extension side at the moment, but we will document the limitations of the final fix in the release notes.

Background

Electron >= 21 ships with V8 Memory Cage also known as V8 Sandbox. Although, it has the tag sandbox it is completely different from Chromium's renderer sandbox which is more about protecting what system calls or resources a particular process can read/write/execute. From the high level design doc the goal of this sandbox is summarized as

The proposal assumes that an attacker can arbitrarily read and write memory inside a dedicated region of virtual address space - the sandbox - which for example contains all V8 heaps. This ability can be obtained from typical V8 vulnerabilities. To protect other memory in the same process from corruption, and by extension prevent the execution of arbitrary code, all raw pointers in the V8 heaps are “sandboxified”, for example by converting them into offsets relative to the base of the sandbox or into indices into an external pointer table.

The reason for above attack being possible is that V8 heap can point to ArrayBuffer objects whose memory are usually allocated outside the V8 heap as seen in the diagram in the above design doc. With the sandbox, V8 wants to guarantee that even if an attacker can corrupt any data inside the V8 heap, the attacker cannot gain access to memory regions not associated with V8 and the only way this is possible to ensure with the ArrayBuffers is to pre-allocate a virtual address space (currently 1TB) and check that the buffer memory regions are to reside inside this space, so that you can reference these regions by offsets from the base of the virtual address space rather than having raw pointers, these offset pointers are called SandboxedPointers. Memory layout of the V8 sandbox today looks like the following,

Problem Statement

The above sandbox is pretty solid on the Web where V8 manages the allocation of ArrayBuffer memory regions and these will now be internally adjusted to fit the sandbox model without having an end user impact. But on the desktop with Node.js, there is another venue of API available to Native module authors for allocating and managing the buffer regions outside of V8 heap. It is achievable by the following set of API,

1. Nan

Refs https://github.com/nodejs/nan/blob/main/doc/buffers.md#nannewbuffer
Nan::MaybeLocal<v8::Object> Nan::NewBuffer(char* data, uint32_t size)
Nan::MaybeLocal<v8::Object> Nan::NewBuffer(char *data,
                                           size_t length,
                                           Nan::FreeCallback callback,
                                           void *hint)

2. Node-Addon-Api C++ wrapper around the C version of N-API

Refs https://github.com/nodejs/node-addon-api/blob/main/doc/array_buffer.md#new-1
static Napi::ArrayBuffer Napi::ArrayBuffer::New(napi_env env, void* externalData, size_t byteLength)
static Napi::ArrayBuffer Napi::ArrayBuffer::New(napi_env env,
                       void* externalData,
                       size_t byteLength,
                       Finalizer finalizeCallback);
static Napi::ArrayBuffer Napi::ArrayBuffer::New(napi_env env,
                       void* externalData,
                       size_t byteLength,
                       Finalizer finalizeCallback,
                       Hint* finalizeHint);
static Napi::Buffer<T> Napi::Buffer::New(napi_env env, T* data, size_t length);
template <typename Finalizer>
static Napi::Buffer<T> Napi::Buffer::New(napi_env env,
                     T* data,
                     size_t length,
                     Finalizer finalizeCallback);
template <typename Finalizer, typename Hint>
static Napi::Buffer<T> Napi::Buffer::New(napi_env env,
                     T* data,
                     size_t length,
                     Finalizer finalizeCallback,
                     Hint* finalizeHint);

3. N-API

Refs https://nodejs.org/dist/latest-v18.x/docs/api/n-api.html#napi_create_external_arraybuffer
napi_status
napi_create_external_arraybuffer(napi_env env,
                                 void* external_data,
                                 size_t byte_length,
                                 napi_finalize finalize_cb,
                                 void* finalize_hint,
                                 napi_value* result)
napi_status napi_create_external_buffer(napi_env env,
                                        size_t length,
                                        void* data,
                                        napi_finalize finalize_cb,
                                        void* finalize_hint,
                                        napi_value* result)

Consider the following scenario, you have a native addon that is wrapper around a highly performant image compression library. The library that is performing the compression would be allocating its own memory regions for the image data, once compressed to send back this data into V8 so that the addon can expose it to JavaScript without the above API an addon will now have to copy the data from the library's allocated memory region into V8 heap which is going to cost some CPU cycles and additional memory. Instead, with the above API the addon can just get a pointer to the already allocated memory region and pass it to V8, now it is just a pointer operation and V8 is able to read the compressed image data without additional cost.

Keen readers would have already spotted the issue with the above example, given most addons cannot control how memory is allocated from their dependent libraries, it is not possible have the buffer regions to be inside the V8 sandbox leading to FATAL errors when any of the above API eventually calls into V8 at this location

For this reason, Node.js decided to disable the V8 sandbox in order to avoid disrupting the addon community. Additionally, the V8 sandbox will not be offering much security to Node.js since there is already unrestricted access to OS resources from the process. However, for Electron which is aligned with the Chromium security model by default the V8 sandbox was enabled since v21. Now that means, N-API stability is broken with Electron v21 for modules that use any of the above buffer API (**NB:**The ABI stability guarantee of N-API is that if a module is compiled for a particular major version of the binary then it should be fine to run the same module in future major versions of the binary without need for recompilation). Modules are now in requirement to recompile themselves with alternate versions of the buffer API for Electron v21 otherwise they will lead to FATAL error in V8 during execution. The Electron issue linked here is about the stability issue. Tl:dr, from the issue is that N-API will now throw an error when trying to create an external buffer when V8 sandbox is enabled and additionally node-addon-api have released new version of the package that provides a way for native addons to be adopted in Electron with V8 sandbox enabled, refs https://github.com/nodejs/node-addon-api/blob/main/doc/external_buffer.md.

The V8 sandbox feature is controlled by compile time flags, (i-e) you cannot enable/disable the sandbox during execution, you are only allowed to build V8 with either sandbox enabled or disabled. Also, Electron only builds a single version of V8 for both Chromium and Node.js, so sandbox enabled V8 is what gets used by Node.js processes in Electron. This leads to a mixed situation for VSCode, were we have workbench that does not load any Node.js native modules, will have the Chromium sandbox and will also benefit from the V8 sandbox. On the other hand, we have the extension host that is a Node.js process and extensions can load native addons which will violate the V8 sandbox policy unless recompiled. This is what we saw when Electron 22 update was merged and Jupyter extension started crashing the extension host. To share a bit more context here, the Jupyter extension relies on Zeromq@6.0.0-beta.6 native module for message passing, the module uses the following buffer API that leads to the FATAL error. Latest version of the Zeromq module has already adopted the breaking change but the Jupyter extension still needs to bump the module version to avoid the crash.

The bigger problem is, how many other extensions ship with native module that uses the above list of buffer API ? Do we have telemetry on tracking the problematic extensions if they were to crash the extension host due to the sandbox ? Shipping Electron 22 in its current state will lead to destabilized extension host for our users.

Solution

Final answer: Using an allocator shim to reroute heap allocations from Extension host into a partition inside the V8 sandbox

Details of the current solution:

• Partition allocator already shims allocation in the Extension host process, confirmed via https://gist.github.com/deepak1556/b734e972f3871c1e4c5b5ce3aa3b892c#file-output-txt
• Today's allocation in the Extension host are routed to the regular pool of the partition allocator (BRP pool is disabled)
• Extension host is a Node.js process host, (i-e) It primarily runs Node environment which brings in V8
• V8 initializes the configured pool of the partition allocator and creates an array buffer partition that will reside in this pool. The configured pool takes in an existing memory mapping and this address space comes from the V8 sandbox allocator https://source.chromium.org/chromium/chromium/src/+/main:gin/v8_initializer.cc;l=449-472
• Partition allocator supports reconfiguring a partition at runtime, //base/allocator/partition_alloc_support.cc is a good place to look at this.
• feat: add forceAllocationsToV8Sandbox for UtilityProcess API electron/electron#37582 adds patch to allow the reconfiguration code to support configured pool
• Currently there can only be a single configured pool per process, so we cannot create another pool from the V8 sandbox memory region.
• The configured pool has a max size of 16GiB which will now be shared by array buffer partition and other heap allocations shim'd by the partition allocator inside the Extension host process.
• The above will ensure that native module creating external buffers will continue to work without any changes from extension authors

Gotchas of the current solution:

• mmap calls are not shim by the partition allocator, so any module using this as backend for external buffer will still be problematic. From our static analysis, we couldn't find a usage of this syscall but this might be a limitation we would have to document for future extensions.
• 16GiB memory region is now shared by two partitions, so it is possible to hit OOM easier than before. The upper limit is based on the limit imposed the Windows sandbox policy. Maybe we can increase this limit given the process is not sandboxed ? Alternatively, we can add support for multiple configured pools to reside in the process.

Previous iterated solutions

Disable V8 sandbox:

• POC at chore: bump electron@22.11.0 #175843
  • One gotcha is that, the V8 headers shipped by Electron that native modules use to compile has the V8 Sandbox code path enabled as well. So that means, if a native module compiled itself against headers from Electron >= 21 then it will be using incorrect offset calculations for pointer references leading to crash when loaded in Electron runtime that has V8 sandbox disabled. I had to make the following adjustment to the headers during build time in our CI pipeline so that our own builtin native modules are also built for V8 sandbox disabled.
• Document and communicate to extension authors that native modules should not be recompiled with Electron >= 21
  • Native modules built with Nan will always have to recompile for newer versions of Electron because the NODE_MODULE_VERSION number embedded in the binary changes for each major release, Node.js will compare this version number with the number from native module during the load stage to ensure module has been recompiled correctly.
  • If we ship Electron 22 with V8 sandbox disabled and Nan modules built for Electron 19 to be loaded it will fail. A solution for this would be to change the version number in the MS Electron build to be same as Electron 19 but now we are implicitly breaking the guarantee of the version number which is that there is ABI change with V8 headers.
  • For native modules built with N-API as long as they don't recompile then they will continue to work as expected. Quick test was running the Jupyter extension that still had the older Zeromq module and it worked fine in the V8 sandboxed disabled exploration build.
• Offer a transition period for extensions to adopt a newer version of their native module dependency that works with Electron >= 21 or if they are authoring a native module provide guidance on alternative N-API
  • How long should this period be ? (Discussion Topic)
  • How can extensions validate the fix with VSCode during this period ? Should we offer exploration build with V8 sandbox enabled ?
• Enable V8 sandbox as default after the transition period

How to determine impacted extensions

Static analysis:

• Scan all extensions in the marketplace using https://github.com/microsoft/vscode-tools#browse-marketplace-extensions-source-code for any native modules with the problematic buffer API usage
• Most extensions don't directly bundle the native module it might come from a dependency, so which means we will have to scan the dependencies list as well and then search the corresponding native module codebase.
• Results at https://github.com/microsoft/vscode-internalbacklog/issues/3548#issuecomment-1457584512

Telemetry:

• [Dropped idea] Since we have control over the native module load path, we can perform symbol search using dlsym/uv_dlsym for the buffer API similar to what Node.js does to get the module entry point and emit event with the native module filename back to VSCode.
  • Symbol names will be mangled
  • We can only add this side by side with V8 sandbox, otherwise the pointer checks in V8 will not happen. This makes it not so useful if we want to detect in today's stable release

[deepak1556]

• 🏃 Upstream the allocator change to OSS Electron -> feat: add forceAllocationsToV8Sandbox for UtilityProcess API electron/electron#37582
  • Isolate the allocator change only for UtilityProcess since we only care about this issue for Extension host, also reduces the impact of switching allocator pools
• Backport the fix into Electron 22 branch
• Create a test plan item with exploration builds during March 2023 Endgame for sanity testing affected extensions
• Additional test plan item to validate language servers on the impact shared pointer compression cage.

[jasonbu]

so will this feature decrease the virtual address space usage? in my usage situation, after few hours, the virtual address memory will up to about 8GB or 10GB, but those allocated for only about 1GB.
even reload will not help.

[deepak1556]

Closing as we will be shipping E22 update with v1.78

[trailstrider]

Is there anywhere to keep track of all the extensions that have broken due to the E22 update and their fix status (not just those that are open source)?