Replace OpenSSL 1.1.1 with OpenSSL 3.0.0

[danbev]

This issue is a question to the community about when we should replace OpenSSL 1.1.1 which is statically linked with Node.js (the version in the deps directory) with OpenSSL 3.0.0.

#38512 is currently a draft pull request and will replace the OpenSSL version that is currently in the deps directory and when performing a normal build OpenSSL 3.0+quic will be statically linked to the Node.js executable. We will still be able to dynamically link to OpenSSL 1.1.1 and we have a CI job which dynamically links to OpenSSL 1.1.1 which is run for every pull request to make sure that we maintain backward compatibility.

The question is when does the community think that we should make this switch to OpenSSL 3.0+quic?

[targos]

What are the implications for Node.js users?

[richardlau]

OpenSSL 1.1.1 will be supported until 2023-09-11 (LTS) [1]. This is before the expected End-of-Life for Node.js 16 (2024-04-30, [2]) so we will need to make a call at some point as to what we do there (either switch or bring forward the Node.js 16 End-of-Life date to align with end of support for OpenSSL 1.1.1). To avoid the same problem with Node.js 18 we should switch before next April.

From what I've seen from the work @danbev has been doing and refreshing the OpenSSL 3.0.0 alpha/final builds in the CI it looks like we did have to change a number of tests to account for error message differences -- I have no idea if that's representative of real world use.

I would prefer we switched to OpenSSL 3.0.0 for Node.js 17 -- a semver-major would be the natural point to make the switch.

[1] https://www.openssl.org/policies/releasestrat.html
[2] https://github.com/nodejs/Release#release-schedule

[mhdawson]

Given the EOL dates I don't think we have a choice for 18.x and I'd favor doing it in 17 unless @danbev who has done the work to get Node.js working on OpenSSL 3.0 feels it is too early.

From the 3.0 release announcement: https://www.openssl.org/blog/blog/2021/09/07/OpenSSL3.Final/

Please download OpenSSL 3.0 from here and upgrade your applications to work with it. OpenSSL 3.0 is a major release and not fully backwards compatible with the previous release. Most applications that worked with OpenSSL 1.1.1 will still work unchanged and will simply need to be recompiled (although you may see numerous compilation warnings about using deprecated APIs). Some applications may need to make changes to compile and work correctly, and many applications will need to be changed to avoid the deprecations warnings. We have put together a migration guide to describe the major differences in OpenSSL 3.0 compared to previous releases.

Scanning through the migration guide I think the main impact to Node.js users would be that some smaller key sizes/algorithms would now not be supported/return an error. For example:

Enforce a minimum DH modulus size of 512 bits
Smaller sizes now result in an error.

[danbev]

I'm in favor of getting this into Node.js 17.

I agree that this could impact Node.js users using smaller key sizes like @mhdawson mentioned above.

[danbev]

Any objections to changing the PR to be a non-draft pull request?

[tniessen]

Scanning through the migration guide I think the main impact to Node.js users would be that some smaller key sizes/algorithms would now not be supported/return an error. For example:

Enforce a minimum DH modulus size of 512 bits
Smaller sizes now result in an error.

The DH change should not be an issue for most applications (512 DH bits are considered weaker than 256 ECDH bits). However, there are PBKDF2 restrictions that would likely affect users (but those restrictions are disabled by default).

Node.js currently does not support SM2 keys because #37066 did not attract any interest. This is not a problem with OpenSSL 1.1.1, but OpenSSL 3 appears to load EC keys with the SM2 curve as SM2 keys:

EC EVP_PKEYs with the SM2 curve have been reworked to automatically become EVP_PKEY_SM2 rather than EVP_PKEY_EC.

Lastly, according to the migration guide, we might need to rework some crypto internals to avoid some performance losses (e.g., key derivation).

Overall, I am in favor of upgrading to OpenSSL 3 for Node.js 17 if possible, as long as we keep testing against OpenSSL 1.1.1.

[mhdawson]

+1 on the continuing to test against OpenSSL 1.1.1, I think that might need to be through a dynamic link. @danbev can you clarify on that front.

[mhdawson]

From the TSC meeting today (minutes - nodejs/TSC#1095) there were no objections/concerns voiced from those who were present (12 TSC members). I think that means we should be able to proceed.

[danbev]

+1 on the continuing to test against OpenSSL 1.1.1, I think that might need to be through a dynamic link. @danbev can you clarify on that front.

There will still be a CI job that dynamically links with OpenSSL 1.1.1 to make sure that we are backwards compatible 👍

[nettybun]

Will it be possible to statically compile OpenSSL 3.0.0 FIPS into versions of Node prior to 17? I don't know if there's an ABI issue that prevents this? #38512 has too many changes for GitHub to load so I can't see if it's only OpenSSL code or also involves many changes to Node itself.

I read https://github.com/nodejs/node/blob/master/BUILDING.md#building-nodejs-with-fips-compliant-openssl that says it's not possible to statically link 3.0.0 to "current version of Node.js" but I'd like to have a static FIPS-compliant binary of the latest LTS Node 14 - will that be possible?

Thanks everyone for amazing work pushing this through for Node 17.

[danbev]

Will it be possible to statically compile OpenSSL 3.0.0 FIPS into versions of Node prior to 17?

I think the current plan is to include OpenSSL 3.0.0 starting from 17 and not backporting.

#38512 has too many changes for GitHub to load so I can't see if it's only OpenSSL code or also involves many changes to Node itself.

There only two commits that involve src and other issues we found have been applied to upstream OpenSSL.

src diff

$ git show --patch master..origin/openssl-3.0-statically-linked src

commit 87a4b3b332022eda0f7b95c84aaf245e3edb1b3d
Author: Daniel Bevenius <[email protected]>
Date:   Fri Jul 30 11:02:52 2021 +0200

    src,tools: include new quic.h header
    
    This commit updates the node_metadata.{cc,h} to include openssl/quic.h
    which was recently added in quictls/openssl. Previously this was part of
    the generated openssl/crypto.h but has been moved out in
    Commit 5517e642fc2a531666c909aae0180e9d258d539e ("QUIC: Don't muck with
    FIPS checksums")

diff --git a/src/node_metadata.cc b/src/node_metadata.cc
index 46d9be0dfc..d64236d9d8 100644
--- a/src/node_metadata.cc
+++ b/src/node_metadata.cc
@@ -11,6 +11,9 @@
 
 #if HAVE_OPENSSL
 #include <openssl/opensslv.h>
+#if NODE_OPENSSL_HAS_QUIC
+#include <openssl/quic.h>
+#endif
 #endif  // HAVE_OPENSSL
 
 #ifdef OPENSSL_INFO_QUIC
diff --git a/src/node_metadata.h b/src/node_metadata.h
index 4486d5af2c..b7cacae4c3 100644
--- a/src/node_metadata.h
+++ b/src/node_metadata.h
@@ -8,6 +8,9 @@
 
 #if HAVE_OPENSSL
 #include <openssl/crypto.h>
+#if NODE_OPENSSL_HAS_QUIC
+#include <openssl/quic.h>
+#endif
 #endif  // HAVE_OPENSSL
 
 namespace node {

commit cb723bf7cc5a4e1760736609e257824129349b8e
Author: Daniel Bevenius <[email protected]>
Date:   Wed May 26 13:51:01 2021 +0200

    src: suppress compilation warning in inspector_socket.cc
    
    When building/linking against OpenSSL 3.0.0alpha17 the following
    compilation error occurs:
    
    In file included from ../src/inspector_socket.cc:7:
    ../src/inspector_socket.cc: In function
    ‘void node::inspector::{anonymous}::generate_accept_string(
        const string&, char (*)[28])’:
    ../src/inspector_socket.cc:150:8: error:
    second operand of conditional expression has no effect
    [-Werror=unused-value]
      150 |  reinterpret_cast<unsigned char*>(hash));
          |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    ../deps/openssl/openssl/include/openssl/sha.h:57:57: note:
    in definition of macro ‘SHA1’
       57 |  (EVP_Q_digest(NULL, "SHA1", NULL, d, n, md, NULL) ? md : NULL)
          |                                                      ^~
    ../src/inspector_socket.cc:150:47: error:
    third operand of conditional expression has no effect
    [-Werror=unused-value]
      150 |        reinterpret_cast<unsigned char*>(hash));
          |                                               ^
    
    This commit suppresses this warning for OpenSSL 3.0.

diff --git a/src/inspector_socket.cc b/src/inspector_socket.cc
index cacff747d0..1650c3fe01 100644
--- a/src/inspector_socket.cc
+++ b/src/inspector_socket.cc
@@ -146,8 +146,8 @@ static void generate_accept_string(const std::string& client_key,
   static const char ws_magic[] = "258EAFA5-E914-47DA-95CA-C5AB0DC85B11";
   std::string input(client_key + ws_magic);
   char hash[SHA_DIGEST_LENGTH];
-  SHA1(reinterpret_cast<const unsigned char*>(&input[0]), input.size(),
-       reinterpret_cast<unsigned char*>(hash));
+  USE(SHA1(reinterpret_cast<const unsigned char*>(&input[0]), input.size(),
+       reinterpret_cast<unsigned char*>(hash)));
   node::base64_encode(hash, sizeof(hash), *buffer, sizeof(*buffer));
 }

I'd like to have a static FIPS-compliant binary of the latest LTS Node 14 - will that be possible?

I can't say if that is something that will be done as I think that the plan is use OpenSSL 3.0 from 17 onwards. But to backport we would need a few commit that have been applied to master in addition to the #38512 to do that.

[tniessen]

Node.js currently does not support SM2 keys because #37066 did not attract any interest. This is not a problem with OpenSSL 1.1.1, but OpenSSL 3 appears to load EC keys with the SM2 curve as SM2 keys:

EC EVP_PKEYs with the SM2 curve have been reworked to automatically become EVP_PKEY_SM2 rather than EVP_PKEY_EC.

Confirmed that this leads to slight inconsistencies, see #38512 (comment).

[mhdawson]

@tniessen, @danbev looks like the related PR landed and comments were addressed there, closing. Please let me know if you think that was not the right thing to do.

[johndevedu]

@danbev you mentioned that dynamically linking OpenSSL 1.1.1 will still be available. Do you expect this backward compatibility to be maintained for the entirety of Node 18 cycle?

[danbev]

Do you expect this backward compatibility to be maintained for the entirety of Node 18 cycle?

That is my understanding, and as mentioned in one of the comments above there is still a CI job that builds and tests Node.js linking to OpenSSL 1.1.1.

[richardlau]

dynamically linking OpenSSL 1.1.1 will still be available. Do you expect this backward compatibility to be maintained for the entirety of Node 18 cycle?

I expect compatibility to be maintained while OpenSSL 1.1.1 is maintained. The OpenSSL project plan to support 1.1.1 until 2023-09-11 [1] which is before the end of Node.js 18's lifecycle (2024-04-30).

[1] https://www.openssl.org/policies/releasestrat.html