Electron 21+ breaks `sodium_malloc`

[mixmix]

Trying to upgrade to electron@26 , I started seeing an error which I tracked back to

sodium-native/blob/v3.4.1/binding.c#L95

SN_STATUS_THROWS(napi_create_external_buffer(env, size, ptr, NULL, NULL, &buf), "failed to create a n-api buffer")

Some research surfaces this issue https://github.com/electron/electron/issues/35801

TL;DR - the new memory cage in v8 deprecated napi_create_external_buffer :+1:

The electron issue has people discussing different approaches to navigating this breaking change. I don't know enough of this domain (yet) to be able to take this further.

[mixmix]

Electron 20.x.y has reached end-of-support ...

:ballot_box_with_check: Last working version electron@20.3.8

[mixmix]

@mafintosh / @emilbayes I manage the scuttlebutt maintenance fund and would love to contribute funds to this issue being resolved.

Are either of you interested (or can you recommend someone who you'd trust to merge/publish a fix?). I'm interested in version 3 being patched for the moment, but the same fix will be applicable to 4. I imagine this is relevant to Keet too?

[mafintosh]

Dont think its fixable as electron disabled the api. We just dont use the secure mem atm. If I am wrong and it is fixable, then happy to apply.

[mixmix]

@mafintosh I can see these paths forward, have marked this bits which I think you might be able to answer with (:question:).

If any seem of interest to explore, would be up for resourcing that (even if it's a dead end). If (1) is safe and fine, I can take that path instead

1. replace sodium_malloc with Buffer.alloc

• :warning: less secure?
  • can you speak to this << :question:
• :syringe: modules I'd be changing:
  • noise-protocol (emile's)
  • hmac-blake2b (emile's)
  • artefact-store (mine)
  • ssb-private-group-keys (mine)

2. copy buffers into memory cage

This blog describes how to do this: electronjs.org/blog/v8-memory-cage#i-want-to-refactor-a-node-native...

This will copy the data into a newly-allocated memory region that is inside the V8 memory cage. Optionally, N-API can also provide a pointer to the newly-copied data, in case you need to modify or reference it after the fact.

• :ballot_box_with_check: looks doable
• :snail: performance hit?
  • how to check << :question:
  • could check if cage is in place and pivot napi use based on that?
  • napi_no_external_buffers_allowed or NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED (see docs)

3. fancy v8 memory allocation?

End of same blog as in (2) says:

Refactoring to use V8's memory allocator is a little more complicated, because it requires modifying the create_external_resource function to use memory allocated by V8, instead of using malloc. This may be more or less feasible, depending on whether or not you control the definition of create_external_resource. The idea is to first create the buffer using V8, e.g. with napi_create_buffer, and then initialize the resource into the memory that has been allocated by V8. It is important to retain a napi_ref to the Buffer object for the lifetime of the resource, otherwise V8 may garbage-collect the Buffer and potentially result in use-after-free errors.

• :thinking: I don't know enough to comment
  • needs research / feasibility check << :question:

[mafintosh]

1. not up to sodium-native to make that decision, thats up to the consumer. we can expose a flag whether its enabled if we dont already. We simply moved away from secure buffers in our active modules for Hypercore cause its too painful in js unfortunately.
2. might be misunderstanding the link, but if you copy the contents of the buffer the secure memory doesn't really do much for you, security wise.
3. sodium needs to malloc the buffer here, unsure if that works.

[mixmix]

(1) answers my question adequately I think - if Hypercore team is happy with Buffer.alloc for good enough security, then it's likely good enough for my needs too. There are probably easier ways to attack my projects that trying to access Buffers

Appreciate your time, thanks @mafintosh

[mixmix]

Here's the solution I'm using. This is at the start of the main.js file that electron runs:

// WARNING - monkey patch
const na = require('sodium-native')
na.sodium_malloc = function sodium_malloc_monkey_patched (n) {
  return Buffer.alloc(n)
}

:see_no_evil:

[mixmix]

Ah, and you also need

na.sodium_free = function sodium_free_monkey_patched () {}