Node: Support BoringSSL as alternative to OpenSSL

[hashseed]

BoringSSL is a fork of OpenSSL maintained by the Chromium project. It shares a lot of the same API.

There are use cases where it it is preferable to use BoringSSL over OpenSSL:

• Using Node.js as part of Electron, as Electron uses Chromium.
• Building Node.js with GN, since BoringSSL already comes with well-maintained GN files.

Currently Node.js does not link against BoringSSL because it uses some APIs that BoringSSL does not implement. One example is SSL_CTX_set_ciphersuites. BoringSSL supports SSL_CTX_set_cipher_list though, so it would be as simple as an #ifdef to make this work.

I'm trying to gauge the interest in this work. I would be willing to invest the time and create necessary PRs.

[Trott]

Conversation on this from 2015-2016: https://github.com/nodejs/node/issues/1860

@nodejs/crypto

[ryzokuken]

@nornagon handles the majority of the work on the Electron fork of Node.js in this direction, so his opinion will also be very insightful.

Essentially, IMO, it shouldn't be a lot of work to make this happen as pointed out by @hashseed, especially since this could easily be done by checking how Electron's fork handles these edge cases and wrapping that behavior in a series of #ifdefs.

However, I am not sure that I am completely onboard with the idea since that'll increase the overhead of adding features and functionalities to the crypto API, even though that overhead will be as minimal as it possibly could be.
/cc @tniessen for opinions on this.

Essentially, it's a bit much to expect the crypto team to walk on eggshells since everyone will have to cross-check functionality to make sure if both libraries are on the same page regarding the behavior in question and if not, handle the said edge case gracefully.

[hashseed]

My intention is to not maintain a fork of Node, but rather be able to pull upstream Node as direct dependency.

I'm sure there is some interest in interop from electron as well. If something that works with OpenSSL but breaks with BoringSSL in an unobvious way, that would be a potential security issue for electron.

[refack]

What always tuned _me_ off was:

¯\_(ツ)_/¯

[ChALkeR]

But Google does not recommend using it:

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.

There are no API or ABI stability guarantees for BoringSSL, that essentially means that compilation with it can not be _supported_ for Node.js. It could be done in a "technically works, but not supported and no guarantees" kinda way, but that'll tend to break, I think? Also, that would place all the maintenance burden on the @nodejs/crypto team, because BoringSSL doesn't do that.

[ryzokuken]

@ChALkeR very true. I doubt there's someone from @nodejs/crypto who would not agree to that.

[sam-github]

I don't have any objection in principal to tweeking node's crypto/tls code so it compiles with boringssl. I don't see us making any comittment to support it, so it might continuously break, and those who care would have to PR fixes.

Whether any specific change is worth the overhead of accepting would depend on that change and how intrusive it is.

WRT specifically to set_ciphersuites, I'm about to PR deeper dependence on it. The current call is just a backwards compatability fix (see https://github.com/openssl/openssl/pull/7759#issuecomment-457697535 for some background). It could be ifdefd based on its existence, but soon a larger change that will depend on using set_ciphersuites to support TS1.3 ciphers is going to be PRed (by me :-), and it is possible that boringssl support would be fairly involved after that change, and might even effect the js code.

But, all this is just speculation. I suggest you PR the actual changes that would make consuming node easier for you, and see what comments it gets.

[hashseed]

Thanks for the comments! Currently BoringSSL support is not a top priority for me, but I'll consider this at some point.

[nornagon]

FWIW, in Electron we are patching Node and BoringSSL (& working with upstream boringssl) to provide this support already. See e.g. https://boringssl-review.googlesource.com/c/boringssl/+/33984, https://boringssl-review.googlesource.com/c/boringssl/+/33984, https://boringssl-review.googlesource.com/c/boringssl/+/32525.

[hashseed]

Interesting. So next time I try linking against BoringSSL it may just work :)