digidem / comapeo-desktop

GNU General Public License v3.0
2 stars 0 forks source link

`sodium-native` doesn't work with our version of Electron #15

Closed achou11 closed 2 months ago

achou11 commented 2 months ago

Using electron@30, we get the following error when attempting to use @mapeo/core in the main process (technically in the utility process):

/Users/andrewchou/GitHub/digidem/comapeo-desktop/node_modules/@mapeo/crypto/lib/key-utils.js:43
  const masterKey = sodium.sodium_malloc(32)
                           ^

Error: failed to create a n-api buffer
    at deriveMasterKeyFromRootKey (/Users/andrewchou/GitHub/digidem/comapeo-desktop/node_modules/@mapeo/crypto/lib/key-utils.js:43:28)
    at new KeyManager (/Users/andrewchou/GitHub/digidem/comapeo-desktop/node_modules/@mapeo/crypto/key-manager.js:41:23)
    at new MapeoManager (file:///Users/andrewchou/GitHub/digidem/comapeo-desktop/node_modules/@mapeo/core/src/mapeo-manager.js:121:24)
    at initializeCore (file:///Users/andrewchou/GitHub/digidem/comapeo-desktop/.vite/build/service/core.js:169:20)
    at file:///Users/andrewchou/GitHub/digidem/comapeo-desktop/.vite/build/service/core.js:75:40
    at ModuleJob.run (node:internal/modules/esm/module_job:222:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:316:24)
    at async node:electron/js2c/utility_init:2:17148
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:138:5)

Node.js v20.14.0

@mapeo/core uses sodium-universal under the hood. Seems related to https://github.com/sodium-friends/sodium-native/issues/185. Haven't tried the suggested solutions there yet.

Replacing sodium-native with sodium-javascript via sodium-universal leads to a different issue (missing method due to incomplete implementation):

/Users/andrewchou/GitHub/digidem/comapeo-desktop/node_modules/@mapeo/crypto/lib/key-utils.js:45
  sodium.crypto_pwhash(
         ^

TypeError: sodium.crypto_pwhash is not a function
gmaclennan commented 2 months ago

I think the monkey patch solution by Mix makes sense to use, well commented so that if/when we get a security review then it's clear why we're doing that.

achou11 commented 2 months ago

applied patch in https://github.com/digidem/comapeo-desktop/tree/mapeo-core-integration and seems to work, so will close for now