realm / realm-studio

Realm Studio
https://realm.io/products/realm-studio/
Apache License 2.0
305 stars 41 forks source link

Electron doesn't have OpenSSL #163

Closed fealebenpae closed 7 years ago

fealebenpae commented 7 years ago

The electron renderer process doesn't come with OpenSSL linked in, as opposed to the main process (and plain Node.js) which link against OpenSSL statically.

This means that realm-js can't run inside the renderer process on Linux as the OpenSSL symbols it imports are missing.

This is not a problem on macOS, because realm-js doesn't use OpenSSL on macOS, but the system libraries.

A possible workaround is to change the realm-js build scripts to statically link against OpenSSL when building for electron on Linux, but that might break using realm-js in the main process, which already has OpenSSL.

fealebenpae commented 7 years ago

This will also be an issue on Windows for as long as realm-sync uses OpenSSL on Windows.

fealebenpae commented 7 years ago

Background: https://electron.atom.io/blog/2016/08/08/electron-internals-using-node-as-a-library

bmunkholm commented 7 years ago

@bigfish24 Notice this. Currntly blocking Linux support.

bigfish24 commented 7 years ago

@bmunkholm yes @kraenhansen and I spoke he is going to focus on making a release of Realm Studio that only offers RMD browsing so we can release that on Tuesday and claim cross platform support for browser. In addition, because we can auto update, this will allow us to "push" an update to everyone with sync for Windows/Linux platforms once we have sorted this.

fealebenpae commented 7 years ago

I think even plain RMD could be blocked by this as realm-core uses OpenSSL for encryption.

bigfish24 commented 7 years ago

@fealebenpae three assumptions I am making:

  1. We can make a release that works with RMD by Tuesday on Linux and Windows, restricting functionality that requires OpenSSL
  2. We can later update Realm Studio to support OpenSSL and reenable features that are blocked by it on Linux and Windows
  3. The update can be pushed to users that installed the limited initial version

Is that realistic? @astigsen pointed out that maybe 3 is not possible because the update would add native modules and not clear those can be auto updated?

fealebenpae commented 7 years ago

To get Studio to run on Linux even only for RMD might require a special build of core with encryption disabled. But we can easily prepare a docker image that builds core from source without encryption and then builds realm-js for electron on top of that in time for a Tuesday launch.

Studio on Windows is not impacted as realm-core on Windows doesn’t use OpenSSL for encryption so there should be no problems there.

bigfish24 commented 7 years ago

Ok, that's exciting. Any thoughts on 2 and 3 in my assumptions?

fealebenpae commented 7 years ago

No idea on what kind of update strategy electron apps use so I can’t comment on 3.

As for 2... I don’t know enough about electron to be able to say. It’s always possible to link against OpenSSL statically but according to the electron blog post I shared above the renderer process uses BoringSSL, which is based on OpenSSL, and that might cause conflicts if we try to load both in the same process. I have no idea whether this will work, but my hunch is that it might not.

A radical approach is to disable the networking and websocket stacks in realm-sync when building for Node.js and have realm-js provide those using the WebSocket JS API that’s available in both main and renderer processes. Personally I’m in favor of this, though of course someone from the sync team should weigh in.

kraenhansen commented 7 years ago

@bigfish24 to the best of my knowledge the updates contain the node_modules and therefore also any binaries that might have been build, os I think it's safe to assume 3).

simonask commented 7 years ago

A radical approach is to disable the networking and websocket stacks in realm-sync when building for Node.js and have realm-js provide those using the WebSocket JS AP

This is an interesting idea, but unfortunately I think it would be quite hard to get there.

It seems easier to implement encryption and SSL in terms of native Windows crypto.

fealebenpae commented 7 years ago

@simonask I think the bigger problem is Linux.

simonask commented 7 years ago

Alright, I'm not 100% sure I understand the problem, but I will volunteer the following:

  1. If the problem is that Node.js includes OpenSSL, but its symbols are private (invisible), it should not be a problem to simply link our own OpenSSL into our binary.

  2. If the problem is that in certain constellations BoringSSL replaces OpenSSL, it should in theory not be hard to use BoringSSL in our code too - there are other use cases for this (Android), and the API contact surface is not very big.

kspangsege commented 7 years ago

If the problem is that Node.js includes OpenSSL, but its symbols are private (invisible), it should not be a problem to simply link our own OpenSSL into our binary.

Not as long as we do it statically.

simonask commented 7 years ago

What do you mean? If Node.js is statically linked with OpenSSL, but hides its symbols, it should be fine to load a shared library that also links statically with OpenSSL. Right?

kspangsege commented 7 years ago

Yes, if we both link statically, or if they link dynamically, and we link statically, all is supposed to be fine, even if the two versions of OpenSSL are not identical (different flavors or different versions).

kspangsege commented 7 years ago

Of course, this assumes that the Node.js version of OpenSSL is not pushed upon us when we compile the loadable module. If two different versions or flavors of OpenSSL are made available during the same compilation, then everything could go wrong.

kspangsege commented 7 years ago

In other words, each compilation unit, that is a constituent of the loadable module, must only include headers from a single version and flavor of OpenSSL.

Likewise, as the loadable module is linked, it is necessary that the static library of only a single version and flavor of OpenSSL is included. Of course, this is supposed to be the same version and flavor as was used during compilation.

bigfish24 commented 7 years ago

Reposting from Java thread on Slack. They are currently evaluating including BoringSSL statically linked vs. OpenSSL to improve their build process. My understanding is that we could reuse this compatibility, but instead have realm-js just use the library in Electron?

Adam Fish - How does Java handle the SSL stuff? Mulong Chen - currently java is using openssl (called from sync) to handle ssl Adam Fish - and you are considering using BoringSSL? Adam Fish - because that is exactly what we need with Electron Mulong Chen - yes we are ... the biggest problem for openssl is that it is quite difficult to build ... the build system is a mess Mulong Chen - boringssl is quite easy plus i think it is well tested on android devices (chromium is using it already) Adam Fish - interesting, but you are talking about including it with the SDK not linking to it because it is on Android already? Electron already has it available as far as I understand @yavor.georgiev’s comments Mulong Chen - it is NOT on the Android, google's app is static linking to it individually Mulong Chen - it is not a part of NDK right now Mulong Chen - so we are considering to static link to it with realm-java Adam Fish - :thumbsup: ok but I guess if we can make sure sync/core can use it for Android, we could have builds that simply link to the native version of Boring in Electron Mulong Chen - from my testing crypto part used by core is fine, will do some testing of ssl part with sync tomorrow Adam Fish - sweet!

kspangsege commented 7 years ago

My understanding is that we could reuse this compatibility, but instead have realm-js just use the library in Electron?

That sounds right to me.

kraenhansen commented 7 years ago

Update

It seems that we can in fact run Studio using a version Realm JS build with sync enabled and telling the linker to reload libssl:

LD_PRELOAD=/lib/x86_64-linux-gnu/libssl.so.1.0.0 ./realm-studio

The root cause seems to be that realm.node has no information that tells the linker that it needs to load libssl, thus the missing symbols - here is the output of ldd node_modules/realm/compiled/electron-v1.8_linux_x64/realm.node:

linux-vdso.so.1 =>  (0x00007ffd14fcd000)
libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f8caf265000)
libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f8caef5c000)
libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f8caed45000)
libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f8caeb28000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f8cae75e000)
/lib64/ld-linux-x86-64.so.2 (0x000055886edbf000)

☝️ libssl is not on that list.

fealebenpae commented 7 years ago

@kraenhansen that's how Node.js addons work. Because Node.js links statically against libuv, v8, OpenSSL, zlib (and others), addons that use their APIs do not declare linker dependencies against those libraries - the expectation is that the node binary itself will provide their symbols.

kraenhansen commented 7 years ago

But perhaps that assumption does not hold when building Node.js addons to be consumed by Electron. (Which I agree is very unfortunate and annoying).

It feels backwards that a consumer (in this case Realm Studio) of a library (Realm JS) needs to load the libraries dependencies (in this case libssl), which is why using dlopen or LD_PRELOAD can definitely save the day and get a release with sync for Electron on Linux out the door - but is probably not the ideal long-term solution.

simonask commented 7 years ago

Is there a simillar workaround to LD_PRELOAD for Windows?

fealebenpae commented 7 years ago

@kraenhansen that assumption does hold for the electron main process, though. I see this as a limitation in Electron itself - the renderer process should either be fully compatible with native Node.js modules, or not at all.

This isn’t a bug in realm-js that needs to be fixed. You want realm-js to support a whole new platform which comes with implementation and maintenance costs. We have already agreed that the JS team does not have the resources to support Electron right now. Studio will need to maintain workarounds for the time being.

fealebenpae commented 7 years ago

@simonask there are no OpenSSL libraries on Windows for Studio to dynamically load. Studio will need to embed the OpenSSL binaries in its Windows distribution.

kraenhansen commented 7 years ago

Does anyone wants to be assigned to this and "own" it, while I make progress elsewhere? @simonask, @fealebenpae, @morten-krogh, @kspangsege?

kraenhansen commented 7 years ago

I believe we can close this - what do you think @fealebenpae?