Support dynamically linking with OpenSSL 3.0 #37669
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nodejs-github-bot
added
c++
jasnell
approved these changes
Mar 9, 2021
addaleax
reviewed
Mar 9, 2021
danbev
force-pushed
the
dynamically-link-openssl-3.0
branch
from
28f2c9b to
518e678
Compare
danbev
force-pushed
the
dynamically-link-openssl-3.0
branch
from
518e678 to
7da5860
Compare
Currently there are three compilation errors generated for
crypto_hkdf.cc:
./src/crypto/crypto_hkdf.cc:
In static member function ‘static bool
node::crypto::HKDFTraits::DeriveBits(node::Environment*,
const node::crypto::HKDFConfig&, node::crypto::ByteSource*)’:
../src/crypto/crypto_hkdf.cc:113:24: error:
invalid conversion from ‘const char*’ to ‘const unsigned char*’
[-fpermissive]
113 | params.salt.get(),
| ~~~~~~~~~~~~~~~^~
| |
| const char*
In file included from ../src/crypto/crypto_util.h:18,
from ../src/crypto/crypto_keys.h:6,
from ../src/crypto/crypto_hkdf.h:6,
from ../src/crypto/crypto_hkdf.cc:1:
/openssl_build_master/include/openssl/kdf.h:130:54: note:
initializing argument 2 of ‘int EVP_PKEY_CTX_set1_hkdf_salt(
EVP_PKEY_CTX*, const unsigned char*, int)’
130 | const unsigned char *salt, int saltlen);
| ~~~~~~~~~~~~~~~~~~~~~^~~~
This commit adds the OPENSSL_API_COMPAT macro and sets it to version 1.0.0 of OpenSSL when linking with a shared OpenSSL library. The motivation for this is that when linking against OpenSSL 3.x there are a lot of deprecation warnings and this allows them to be avoided. When we later upgrade the code base to 3.x this value can then be updated.
This commit adds a constant to identify if the version of OpenSSl is 3 or above. The motivation for this is it allows for checking this value in tests to make sure that they work with OpenSSL 3.x, and also with earlier versions.
This commit fixes a number of test failures reported when using OpenSSL 3.0, for example: Error: error:0500007E:Diffie-Hellman routines::modulus too small Check have been added for OpenSSL 3 and use the larger sizes only for OpenSSL 3.0 as these sizes seem to cause timeouts when using OpenSSL 1.1.1.
This commit adds a macro check for OpenSSL 3 and used EVP_default_properties_is_fips_enabled instead of FIPS_mode which has been removed in OpenSSL 3.
This commit aquires the Mutex in ManagedEVPPKey::operator= to avoid multiple threads updating the underlying EVP_PKEY in OpenSSL 3.0. There are additional changes to the code to avoid dead locks, making sure to release the lock before aquiring a new lock. Refs: nodejs@79d44baae2
This commit add const to EC_KEY, DSA, RSA pointer to avoid compilation errors when linking against OpenSSL 3.0. Refs: openssl/openssl@7bc0fdd
danbev
force-pushed
the
dynamically-link-openssl-3.0
branch
from
7da5860 to
6b8c504
Compare
|
Re-run of failing node-test-linux-linked-debug ✔️ |
danbev
added a commit
that referenced
this pull request
Mar 16, 2021
This commit enables node to dynamically link against OpenSSL 3.0. The motivation for opening this PR even though OpenSSL 3.0 has not been released yet is to allow a nightly CI job to be created. This will allow us stay on top of changes required for OpenSSL 3.0, and also to make sure that changes to node crypto do not cause issues when linking to OpenSSL 3.0. PR-URL: #37669 Refs: #29817 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
|
Landed in 640fe94. |
danielleadams
pushed a commit
that referenced
this pull request
Mar 16, 2021
This commit enables node to dynamically link against OpenSSL 3.0. The motivation for opening this PR even though OpenSSL 3.0 has not been released yet is to allow a nightly CI job to be created. This will allow us stay on top of changes required for OpenSSL 3.0, and also to make sure that changes to node crypto do not cause issues when linking to OpenSSL 3.0. PR-URL: #37669 Refs: #29817 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
Merged
danielleadams
pushed a commit
that referenced
this pull request
Mar 16, 2021
This commit enables node to dynamically link against OpenSSL 3.0. The motivation for opening this PR even though OpenSSL 3.0 has not been released yet is to allow a nightly CI job to be created. This will allow us stay on top of changes required for OpenSSL 3.0, and also to make sure that changes to node crypto do not cause issues when linking to OpenSSL 3.0. PR-URL: #37669 Refs: #29817 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
codebytere
added a commit
to electron/electron
that referenced
this pull request
May 20, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
May 31, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
May 31, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
Jun 2, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
Jun 2, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
Jun 2, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
Jun 8, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
Jun 8, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
Jun 9, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
Jun 9, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
Jun 10, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
Jun 10, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enables node to dynamically link against OpenSSL 3.0.
The motivation for opening this PR even though OpenSSL 3.0 has not been released yet is to
allow a nightly CI job to be created. This will allow us stay on top of changes required for
OpenSSL 3.0, and also to make sure that changes to node crypto do not cause issues when
linking to OpenSSL 3.0.
While most changes have been in tests, there have been updates to src/crypto as well.
Refs: #29817
Steps to build and link
During development we have been building OpenSSL upstream and linking against that and used the following commands.
Configure and build OpenSSL
We specify an
prefixwhere this build will be installed into. This path will later be used when configuring Node's build:Configure and build Node.js
The following will configure Node to link against OpenSSl 3.0 that was configured
and built in the previous step.