Skip to content

openssl bug when using Node.js 10.x or Node.js 11.x but Node.js 8.x is fine #24964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hsgreen opened this issue Dec 11, 2018 · 23 comments
Closed
Labels
addons Issues and PRs related to native addons. crypto Issues and PRs related to the crypto subsystem. question Issues that look for answers. wontfix Issues that will not be fixed.

Comments

@hsgreen
Copy link

hsgreen commented Dec 11, 2018

I observed a very strange bug when I recently updated from using Node.js 8 to 11 (I also then tried 10.x and it had the same issue as 11.x).

I'm running in a Centos 7 (latest) docker container and install using these instructions.

I have a custom node module that I have built (I rebuild each time I install a new Node.js version), that uses another custom library that I have built that uses OpenSSL. The version of OpenSSL at the system level is standard Centos 7 OpenSSL 1.0.2k-fips. Within this custom library linked in as a shared library from my custom node module it makes the following openssl call:

m_certificate = X509_new();

On all versions of node this call succeeds and m_certificate is not NULL. However, the problem is on Node.js 10.x and 11.x one of the critical members of that object is still set to NULL after that call. This is the cert_info member. When cert_info is NULL it causes subsequent methods to set/update this certificate to seg fault, such as X509_time_adj_ex(X509_get_notBefore(m_certificate), 0, 0, &nowTime.tv_sec) because nearly all these OpenSSL methods/macros blindly call m_certificate->cert_info->....

I thought maybe it was a bug in the system installed OpenSSL, but when I downgraded to use the latest 8.x Node.js version, then the problem went away. It must be something with the OpenSSL configuration or build when using Node.js 10 and 11.

Is there anything in Node.js 10 and 11 that interacts with OpenSSL differently than 8.x or does the pre-built binary distros use a statically linked OpenSSL version that may be different than 8.x and may introduce this odd failure to allocate the cert_info member when calling X509_new()?

@bnoordhuis bnoordhuis added question Issues that look for answers. crypto Issues and PRs related to the crypto subsystem. addons Issues and PRs related to native addons. labels Dec 11, 2018
@bnoordhuis
Copy link
Member

Your issue/question is essentially "can I use two copies of openssl in the same process?" and it's not the first time it's come up, check this repo and nodejs/help.

The answer is 'yes' but you need to be very meticulous about initializing your copy of openssl properly and ensuring that you never mix calls or data structures.

The first thing I'd do is check with a debugger whether that call to X509_new() calls into node or libcrypto.so and double-check if that is the expected location.

@bnoordhuis
Copy link
Member

Is there anything in Node.js 10 and 11 that interacts with OpenSSL differently than 8.x or does the pre-built binary distros use a statically linked OpenSSL version that may be different than 8.x

To answer your specific question: yes, v10.x and v11.x ship openssl 1.1.0j, v8.x openssl v1.0.2q.

@hsgreen
Copy link
Author

hsgreen commented Dec 11, 2018

Thanks that makes sense. I had installed a custom openssl of the same system version built with debug symbols to try and troubleshoot, but it always seemed to go to the Node binary install openssl calls. I'll have to look around to figure out how to build my standalone library that needs openssl using a different openssl version than what node is using and to get them to all play nice. Thanks for confirming and answering!

@hsgreen
Copy link
Author

hsgreen commented Dec 11, 2018

I think this may actually be a more serious design flaw in NodeJS after doing more research and experimentation on this issue.

I found this other thread that is suffering the same problem here.

It seems that the current approach in NodeJS to export the OpenSSL library symbols prevents any native addons from using ANY library on the system that depends on OpenSSL unless the OpenSSL libraries have 100% ABI compatibility.

This would mean I can't make a native addon that uses the system libcurl without risk of segfaults if I upgrade NodeJS to a version beyond the one that has ABI compatibility with the system OpenSSL.

It seems a little ridiculous to expect developers to have to rebuild their custom libraries and all those libraries dependent libraries that use OpenSSL to match the same OpenSSL version used in NodeJS in order to be used as an addon every time you update NodeJS version that includes a new OpenSSL ABI.

Would it be so hard for NodeJS to just use the system provided openssl library and write code like the rest of us that accommodates varying API version differences of OpenSSL across its versions?

@hsgreen hsgreen reopened this Dec 11, 2018
@mscdex
Copy link
Contributor

mscdex commented Dec 12, 2018

Would it be so hard for NodeJS to just use the system provided openssl library and write code like the rest of us that accommodates varying API version differences of OpenSSL across its versions?

You can already build node against a system copy of OpenSSL, provided it's compatible with node's usage of the OpenSSL API.

@bnoordhuis
Copy link
Member

It seems that the current approach in NodeJS to export the OpenSSL library symbols prevents any native addons from using ANY library on the system that depends on OpenSSL unless the OpenSSL libraries have 100% ABI compatibility.

No, you can do that, you just need to be careful with how you link and in what order (and of course the other things I mentioned in my first comment.)

Whether you should is another question. Ask yourself if there are compelling reasons not to link against node's bundled copy of openssl.

Would it be so hard for NodeJS to just use the system provided openssl library and write code like the rest of us that accommodates varying API version differences of OpenSSL across its versions?

We used to do that in the past. There's a reason we moved away from that. :-)

@hsgreen
Copy link
Author

hsgreen commented Dec 12, 2018

No, you can do that, you just need to be careful with how you link and in what order (and of course the other things I mentioned in my first comment.)

Could you point to a reference detailing what you mean by "careful with how you link and in what order"? Due to OpenSSL being a C library the only solutions I've found involve complete remapping of the symbols, which then goes back to essentially requiring a rebuild of all the other system installed libraries we may want in a node addon that happen to use the system OpenSSl library as well.

I even compiled the same version of OpenSSL that the system uses as a static library with debug symbols, linked to my shared library. When running my shared library in a stand alone application it works fine (I can step into the OpenSSL methods etc). When I go and use the shared library in a node addon, it still uses the OpenSSL symbols exposed by the node binary. The interesting thing is even when I'm in gdb and call info symbol SSL_library_init from a break point inside my shared library, gdb says it is a symbol in my shared library. But when I go to step into the OpenSSL methods it jumps over like no debug symbols are available (effectively using the nodejs binary exported symbols for OpenSSL).

I certainly appreciate how most of this is due to the design of OpenSSL, being a C only library, etc. I also realize the pitfalls of using the host system OpenSSL (way out of date, full of security vulnerabilities, etc). I just wonder if you could actually point to the specific method you had in mind that would be a solution to this for having different ABI incompatible versions of OpenSSL in use in this case.

I do realize one could recompile NodeJS to match the system OpenSSL. I'm just trying to find a way that maximizes compatibility between standard system libraries from the base OS maintained repos and the pre-built distributions of nodejs for those systems. One of the things I appreciate about NodeJS is how easy it is to install/distribute and update with the pre-built binary distributions and npm. It would be unfortunate to have to rebuild NodeJS for every minor update that comes out.

Whether you should is another question. Ask yourself if there are compelling reasons not to link against node's bundled copy of openssl.

The compelling reason is it would then require custom builds of a slew of other 3rd party libraries that I can otherwise just use from installation via the system package manager and I have other applications that also use my custom library outside of node on the same system.

We used to do that in the past. There's a reason we moved away from that. :-)

Again if there are "careful" ways to link to make this work, wouldn't there be a "careful" way that the standard NodeJS distribution could be linked to OpenSSL to avoid exposing all the OpenSSL symbols?

@bnoordhuis
Copy link
Member

I regret that I don't have time right now to answer in full. I'll try to come back to this later but a quick comment apropos this:

I even compiled the same version of OpenSSL that the system uses as a static library with debug symbols, linked to my shared library. [..] When I go and use the shared library in a node addon, it still uses the OpenSSL symbols exposed by the node binary.

That sounds impossible; probably something went wrong during the build.

If you linked your addon.node to a static libcrypto.a, then nm addon.node should print no undefined openssl symbols (no symbols marked U or u and with no address.)

@hsgreen
Copy link
Author

hsgreen commented Dec 14, 2018

That sounds impossible; probably something went wrong during the build.

If you linked your addon.node to a static libcrypto.a, then nm addon.node should print no undefined openssl symbols (no symbols marked U or u and with no address.)

I think maybe I was not clear in describing what happened.

  1. Linked a libcrypto.a that I built with debug symbols to my shared library "A"
  2. When I run a test executable that uses shared library "A" through gdb I can successfully step into the openssl methods from libcrypto.a
  3. THEN I linked my shared library "A" as a shared library to my node addon "B".
  4. When I run node in gdb using my addon "B", I can step into my shared library code, but when that code is about to call a method from OpenSSL, gdb shows the symbol as being in my shared library "A", however if I try to "step" into the OpenSSL method in this case it skips over it as if it was using the symbol from the node binary.

Perhaps I need to explicitly link the node addon "B" to the same libcrypto.a file, even though it is already technically inside my shared library "A"?

But even this solution is not optimal because it does not address the use of other 3rd party libraries that my node addon "B" would like to use, that also depend on the system openSSL library, like libcurl.

So regardless, there should be something that can be done to the node build process that prevents it from exporting the OpenSSL symbols that it has statically linked to.

@sam-github
Copy link
Contributor

there should be something that can be done to the node build process that prevents it from exporting the OpenSSL symbols that it has statically linked to.

We could do that, and it would help your use-case, for sure.

But then we would be flooded with bug reports from the node addons that link against those exported symbols.

Node cannot both export its openssl symbols (so that addons can be written against node's statically linked internal openssl with no additional binary deps), and NOT export its symbols (so that addons can be written against arbitrary external openssls).

Node as a project distributes statically linked low-dependency node binaries.

Distributions address the other use-case, they distribute node dynamically linked.

And someone building from source can build in either mode.

Its unfortunate that one build type doesn't satisfy ALL possible use cases, but we haven't found a way.

If you have suggestions other than flipping back and forth between the two incompatible approaches, that would be very interesting. We'd obviously prefer to make this work for everyone.

@hsgreen
Copy link
Author

hsgreen commented Jun 19, 2019

I'm curious to understand the thought process behind those that build add-ons that link relying on the node exported openssl symbols.

Wouldn't they still have to have the exact openssl header files checked out somewhere on their system to get their code to compile? Then they are in a tricky spot of having to constantly monitor what openssl version node was linked with. Versus they build to the openssl they want on their system and let node keep its openssl symbols to itself so it can use whatever version it wants when it is prebuilt. If node gets built from source on a particular deployment then it and the add-ons would all presumably be using the system installed openssl anyhow so no issue with node not exporting openssl symbols.

@richardlau
Copy link
Member

richardlau commented Jun 19, 2019

Wouldn't they still have to have the exact openssl header files checked out somewhere on their system to get their code to compile? Then they are in a tricky spot of having to constantly monitor what openssl version node was linked with.

OpenSSL headers are included in the Node.js headers tarball:

node/tools/install.py

Lines 194 to 198 in 7146ddd

if 'true' == variables.get('node_use_openssl') and \
'false' == variables.get('node_shared_openssl'):
subdir_files('deps/openssl/openssl/include/openssl', 'include/node/openssl/', action)
subdir_files('deps/openssl/config/archs', 'include/node/openssl/archs', action)
subdir_files('deps/openssl/config', 'include/node/openssl', action)

@hsgreen
Copy link
Author

hsgreen commented Jun 19, 2019

I think the biggest argument against bundling the openssl symbols in node is that it precludes any add-on from using any 3rd party library that might use a version of openssl that is different than what node uses. There may be situations where you want to use a proprietary library where you absolutely can't rebuild it to use the node openssl version.

@sam-github
Copy link
Contributor

@hsgreen I agree, and I (and other contributors, I believe) understand that agument, it is the argument I refered to in #24964 (comment), and it is not sufficient to outweigh the use-case for exporting symbols.

If we can enable this use-case, without breaking the other, that would be fabulous. But we aren't going to flip flop back to enabling the use-case you care about and breaking the zero-external dependency use-case. At least, I don't think so. If you seriously think this can get some traction, you could PR a change to not-export the symbols, which would force the change to be approved (or not), and even possibly brought to a TSC vote if consensus cannot be achieved.

I suspect it will not be approved, so just a heads up, I wouldn't want you to waste your time.

/cc @nodejs/crypto

@hsgreen
Copy link
Author

hsgreen commented Jun 19, 2019

The project I had where this was an issue was fine with continuing to use Node 8.x which used the openssl version the target OS had installed so I don't have a lot of motivation to push this. Just trying to provide clarity to those that are following it that everything has been tried from the add-on side to make it work (hiding symbols, static linking, etc etc) and it can't be resolved with node being built with openssl symbols exported.

I'd think that if one is going to the trouble of building with the node openssl symbols (doing the extra steps to pull those node bundled openssl headers) that it would be minimal burden on them to just rebuild node from source and add a compile flag to export the symbols. Then make the default for prebuilt distributions be to not export the openssl symbols.

@sam-github
Copy link
Contributor

Just to clarify:

The project I had where this was an issue was fine with continuing to use Node 8.x which used the openssl version the target OS had installed

Node has never used the target OS's preinstalled openssl in its default builds. Its likely what you are seeing is a change in how symbols were exported.

everything has been tried from the add-on side to make it work ... and it can't be resolved with node being built with openssl symbols exported.

Did anybody try vendoring the libcurl (for example) source into the addon and building it against node's openssl? That seems like it would work, and also create a self-contained addon that didn't fail to npm install if libcurl-dev wasn't pre- installed.

@JCMais
Copy link
Contributor

JCMais commented Jun 20, 2019

Did anybody try vendoring the libcurl (for example) source into the addon and building it against node's openssl? That seems like it would work, and also create a self-contained addon that didn't fail to npm install if libcurl-dev wasn't pre- installed.

This does not work because Node only exports the OpenSSL symbols it uses, which are not always the same ones used by other libs, like libcurl.

@sam-github
Copy link
Contributor

Node doesn't use its exported symbols, they are exported for use by addons. If you post an issue about non-exported symbols, we can export them, we have in the past tried to make sure we export everything we should.

@dtopham75
Copy link

I'm having broadly similar issues to @JCMais.

I have a Node add-on that depends on another shared library (my own), let's call it libabc.so, That in turn dynamically loads (i.e. with dlopen) another shared library (also my own), let's call it libxyz.so, and that is dependent on libcurl, and therefore also on libssl and libcrypto.

I don't get a segv, but it does fail somewhere deep within libcurl, where a call to OpenSSL's SSL_CTX_new fails with a 'library has no ciphers' error.

I have tried various approaches to no avail. I was hopeful that a static build of libcur that references the OpenSSL headers in the node installation would work. But it doesn't. I think the problem is that libcurl aggreagates many different areas of functionality and depends on a lot of other libraries, some of which in turn also depend on OpenSSL. So even if you build a static libcurl that can pick up the OpenSSL symbols exported from node, you still need to reference the libraries libcurl depends on, so it just moves the problem to one of libcurl's dependencies.

I understand the issues around this, both why you don't want node to depend on the system OpenSSL and why you need to export the OpenSSL symbols. But at the moment I just can't see a resolution for this.

(The other more general problem that I have is that my libraries are also used elsewhere in a wider product suite. The Node add-on basically wraps them so that we can expose their functionality as a RESTful interface using Node and Express. It would be preferable not to have to configure them completely differently for use within the Node add-on).

@JCMais
Copy link
Contributor

JCMais commented Jun 27, 2019

@dtopham75 the solution I found was to build a complete version of libcurl statically, including their dependencies, and make sure to always use the same dependencies version than node.js, in case they converge somewhere (for instance, libcurl can be built not only with openssl, but also zlib, brotli, nghttp2, etc, which Node.js also depends on).

The script that does it is here: https://github.com/JCMais/node-libcurl/blob/8eb18a8a1029a46f281801fb9b88d2bb751264de/scripts/ci/build.sh

The addon then uses curl-info to link against libcurl and all their statically built dependencies: https://github.com/JCMais/node-libcurl/blob/8eb18a8a1029a46f281801fb9b88d2bb751264de/binding.gyp#L125-L127

@sam-github
Copy link
Contributor

@alejandroclaro
Copy link

alejandroclaro commented Apr 7, 2020

This conversation seems kind of long and complicated, but my two cents here.

I agree with the comments that it is a problem that node expose OpenSSL symbols and affects any client need to depend on OpenSSL or fork as well. It is a very strong relationship that you are creating with a dependency that is absorbed by node and re-exposed.

I have at least three use cases where this is causing us problems:

  • Support FIPS 140-2 mode operations using OpenSSL 1.0.2 and OM 2.x (yes, it is still possible and it's supported by OpenSSL foundation under Premium support service).
  • Use of dependencies that require another cryptographic module that are OpenSSL forks such as BoringSSL (Example, Electron that needs to be applying patches to node in order to solve the problem).
  • Nodejs addons that depend on OpenSSL or another dependency which in turn depends on OpenSSL. Any difference in the version they are linked with could cause runtime issues.

It would be nice if node just hide the OpenSSL symbols when OpenSSL is linked statically. And let other add-ons depend on the OpenSSL version or fork they need to perform other operations that are not supported by the crypto and tls modules (e.g. XMLSec, CMS/SMIME, etc).

@bnoordhuis
Copy link
Member

This issue was still open because it was forgotten about, not because it's unresolved. I'll go ahead and close it out now.

As Sam mentions, we're unlikely to change the current setup because that would break existing add-ons and close the door on node <-> add-on interop through tls.SecureContext#_external.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addons Issues and PRs related to native addons. crypto Issues and PRs related to the crypto subsystem. question Issues that look for answers. wontfix Issues that will not be fixed.
Projects
None yet
Development

No branches or pull requests

8 participants