ronomon / crypto-async

Fast, reliable cipher, hash and hmac methods executed in Node's threadpool for multi-core throughput.
MIT License
174 stars 15 forks source link

Using with Electron #10

Open apiriouIpso opened 4 years ago

apiriouIpso commented 4 years ago

Electron and crypto-async

I can't get this lib to work with Electron. I made a basic project reproducing my problem here

Bug Description

I can't use the module without recompiling it because, the following code crashes when executed with electron but works perfectly fine when executed on node directly.

const cryptoAsync = require('@ronomon/crypto-async')

try {
  const text = Buffer.from('Hello World')
  const key = Buffer.alloc(16)
  const iv = Buffer.alloc(16)
  console.log('This prints to the console')
  const encryptedText = cryptoAsync.cipher('aes-128-ctr', 0, key, iv, text)
  console.log('But this will not print if the previous call crashed')
  const clearText = cryptoAsync.cipher('aes-128-ctr', 1, key, iv, encryptedText)
  console.log(clearText.toString())
} catch (e) {
  console.error(e)
}

I've tested on both Windows (Windows 10) and Mac OS (El Capitan) and observed two different behaviors (I'm guessing cause MacOS uses the system openssl)

How to reproduce

Causes

As far as I can tell both errors seems to originate from the same line in binding.c in the @ronomon/crypto-async repo :

const EVP_CIPHER* evp_cipher = EVP_get_cipherbyname(algorithm);
if (!evp_cipher) {
  THROW(env, E_ALGORITHM_UNKNOWN);
}

On windows the EVP_get_cipherbyname method crashes, on Mac it returns null

This seems to be the result of the differences with the bundled version of node in electron.

Rebuilding for Electron

I've also tried to rebuild the native module for electron using the following commands

cd node_modules
cd @ronomon/crypto-async
node-gyp rebuild --target=8.1.0 --arch=x64 --target_platform=win32 --dist-url=https://atom.io/download/atom-shell --module_name=@ronomon/crypto-async --module_path=../lib/binding/electron-v8.1-win32-x64

But the build fails cause openssl/err.h and others openssl headers file cannot be found.

It seems that electron now uses boringssl instead of openssl and does not export the same headers/symbols that Node does.

From the different issues I've come accross on github, I can only think of two solutions (let me know If you can think of any other workaround):

jorangreef commented 4 years ago

Thanks @apiriouIpso for the excellent write-up.

Yes, if Electron is no longer bundling or exporting OpenSSL then that's a showstopper.

I am not sure I want to go down the road of bundling OpenSSL statically with @ronomon/crypto-async itself, since the module targets Node.js primarily.

It's a pity that Electron is not keeping step with Node.js.

A third option is to get Node.js to move to BoringSSL, but I think Electron probably needs to sort this out.

apiriouIpso commented 4 years ago

I assume the Node.js team will not move to BoringSSL mostly because of this line that implies that stability will never be guaranted:

Although BoringSSL is an open source project, it is not intended for general use, as OpenSSL is. We don't recommend that third parties depend upon it. Doing so is likely to be frustrating because there are no guarantees of API or ABI stability.

I've seen a few issues on this subject such as this one.

Also I'm guessing the folks working on Electron had no choice but to migrate because Chromium itself switched to BoringSSL as seen on this issue.

Since I only need to be able to use the AES cipher in CTR mode, I'm guessing it'll be faster to make my own native module supporting this one algorithm, which is a shame cause your module seemed perfect for my use case.

Thanks for the answer.

jorangreef commented 4 years ago

Thanks @apiriouIpso , is there otherwise not a one or two liner patch for Electron that can expose OpenSSL? I am assuming OpenSSL is still bundled statically with the Node that ships with Electron?

apiriouIpso commented 4 years ago

Thanks @apiriouIpso , is there otherwise not a one or two liner patch for Electron that can expose OpenSSL? I am assuming OpenSSL is still bundled statically with the Node that ships with Electron?

If I'm not mistaken they are now building their Node.js version with BoringSSL. See https://github.com/electron/electron/issues/11801#issuecomment-437173719

Since BoringSSL is a fork of OpenSSL (and still retains a good compatibility from what I gather) maybe it's possible to expose BoringSSL in a mostly compatible way, but it's uncharted territory for me, and would also need to be updated every once in a while to keep up with Electron/BoringSSL's updates.

alejandroclaro commented 4 years ago

@jorangreef @apiriouIpso another options is to compile Electron enabling BoringSSL compilation flag to add a prefix (a kind of namespace) to the functions, and patches (they already do it to some extent) the code that uses that functions.

This way any other nodejs add-on could depend on OpenSSL without name clashing.

I'm facing a similar issue and maybe we can ask Electron team to do this in order to solve problems with third party add-ons like this one.

jorangreef commented 4 years ago

Thanks @alejandroclaro, if you could take care of it with the Electron team that would be great.

alejandroclaro commented 4 years ago

@jorangreef I just notice that Electron is not exporting BoringSSL symbols. So, I was able to compile an addon which depends on OpenSSL without issues and wrap what I need.

This will work as long as Electron upholds this decision. It doesn't work with NodeJS due to symbol clashing. But in the case of crypto-async maybe a conditional compilation can work. If the module is being used in NodeJS it works as it is currently doing, but if it must be rebuild for electron (e.g. electron-rebuild), then it links to the system OpenSSL dynamically instead.

jorangreef commented 4 years ago

@alejandroclaro, could you provide a sample gyp file to do this?