deps: update to openssl-3.0.1+quic by richardlau · Pull Request #41177 · nodejs/node

richardlau · 2021-12-14T23:39:03Z

Refs: quictls/openssl#69
Refs: https://mta.openssl.org/pipermail/openssl-announce/2021-December/000212.html

Updated as per https://github.com/nodejs/node/blob/master/doc/guides/maintaining-openssl.md. This took longer than expected because it turned out the Dockerfile was missing a Perl module used by deps/openssl/config/generate_headers.pl (which only exists in the Node.js versions where we build with OpenSSL 3).

cc @danielleadams

The `deps/openssl/config/generate_headers.pl` imports `Text::Template` but that Perl module is not present in the default Perl installation in the Dockerfile and needs to be explicitly installed.

nodejs-github-bot · 2021-12-14T23:39:08Z

Review requested:

@nodejs/gyp

danielleadams

LGTM

danielleadams · 2021-12-14T23:48:57Z

Do we need to fast track this? I don't think we are in any hurry to get 17.x out today

richardlau · 2021-12-15T00:23:06Z

😞 We appear to be seeing the same test-crypto-engine failures on macOS as with Node.js 16 and the OpenSSL-1.1.1m+quic update in #41175 (comment).

I'm not seeing similar failures on either release line on Linux (and neither is our Jenkins CI). Anyone on macOS able to take a look?

richardlau · 2021-12-15T00:39:29Z

e.g. https://ci.nodejs.org/job/node-test-commit-osx/nodes=osx1015/43114/testReport/(root)/test/parallel_test_crypto_engine/

node:internal/crypto/util:111 throw new ERR_CRYPTO_ENGINE_UNKNOWN(id); ^Error [ERR_CRYPTO_ENGINE_UNKNOWN]: Engine "/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/libtest_crypto_engine.dylib" was not found at new NodeError (node:internal/errors:371:5) at Object.setEngine (node:internal/crypto/util:111:11) at Object.<anonymous> (/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/test/parallel/test-crypto-engine.js:47:12) at Module._compile (node:internal/modules/cjs/loader:1097:14) at Object.Module._extensions..js (node:internal/modules/cjs/loader:1149:10) at Module.load (node:internal/modules/cjs/loader:975:32) at Function.Module._load (node:internal/modules/cjs/loader:822:12) at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12) at node:internal/main/run_main_module:17:47 { code: 'ERR_CRYPTO_ENGINE_UNKNOWN'}Node.js v18.0.0-pre

Maybe cc @RaisinTen who updated the test_crypto_engine test in #40481.

tniessen · 2021-12-15T03:12:02Z

I don't use macos but my best guess is that this is due to openssl/openssl#17073. We are probably (incorrectly) loading the engine twice and OpenSSL did not report that as an error before as far as I can tell.

mcollina

lgtm

richardlau · 2021-12-15T15:15:27Z

I don't use macos but my best guess is that this is due to openssl/openssl#17073. We are probably (incorrectly) loading the engine twice and OpenSSL did not report that as an error before as far as I can tell.

I think this is correct.

test-macstadium-macos11:osx11 iojs$ ./out/Release/openssl-cli versionOpenSSL 3.0.0+quic 7 sep 2021 (Library: OpenSSL 3.0.0+quic 7 sep 2021)test-macstadium-macos11:osx11 iojs$ ./out/Release/openssl-cli-3.0.1 versionOpenSSL 3.0.1+quic 14 Dec 2021 (Library: OpenSSL 3.0.1+quic 14 Dec 2021)test-macstadium-macos11:osx11 iojs$

OpenSSL 3.0.0, duplicate engines, no error (^D (ctrl+D) to terminate input):

test-macstadium-macos11:osx11 iojs$ ./out/Release/openssl-cli dgst -sha1 -engine /Users/iojs/build/workspace/node-test-commit-osx-arm/nodes/osx11/out/Release/libtest_crypto_engine.dylib -engine /Users/iojs/build/workspace/node-test-commit-osx-arm/nodes/osx11/out/Release/libtest_crypto_engine.dylibEngine "libtest_crypto_engine" set.Engine "libtest_crypto_engine" set.SHA1(stdin)= da39a3ee5e6b4b0d3255bfef95601890afd80709test-macstadium-macos11:osx11 iojs$

OpenSSL 3.0.1 now throws an error for the same command:

test-macstadium-macos11:osx11 iojs$ ./out/Release/openssl-cli-3.0.1 dgst -sha1 -engine /Users/iojs/build/workspace/node-test-commit-osx-arm/nodes/osx11/out/Release/libtest_crypto_engine.dylib -engine /Users/iojs/build/workspace/node-test-commit-osx-arm/nodes/osx11/out/Release/libtest_crypto_engine.dylibEngine "libtest_crypto_engine" set.Invalid engine "/Users/iojs/build/workspace/node-test-commit-osx-arm/nodes/osx11/out/Release/libtest_crypto_engine.dylib"403DB30401000000:error:1300006D:engine routines:dynamic_load:init failed:../deps/openssl/openssl/crypto/engine/eng_dyn.c:494:403DB30401000000:error:13000074:engine routines:ENGINE_by_id:no such engine:../deps/openssl/openssl/crypto/engine/eng_list.c:430:id=/Users/iojs/build/workspace/node-test-commit-osx-arm/nodes/osx11/out/Release/libtest_crypto_engine.dylib403DB30401000000:error:1300006D:engine routines:dynamic_load:init failed:../deps/openssl/openssl/crypto/engine/eng_dyn.c:494:SHA1(stdin)= da39a3ee5e6b4b0d3255bfef95601890afd80709test-macstadium-macos11:osx11 iojs$

It looks like it only objects with absolute paths. If the id is used for the second "duplicate" engine it still works:

test-macstadium-macos11:osx11 iojs$ ./out/Release/openssl-cli-3.0.1 dgst -sha1 -engine /Users/iojs/build/workspace/node-test-commit-osx-arm/nodes/osx11/out/Release/libtest_crypto_engine.dylib -engine libtest_crypto_engineEngine "libtest_crypto_engine" set.Engine "libtest_crypto_engine" set.SHA1(stdin)= da39a3ee5e6b4b0d3255bfef95601890afd80709test-macstadium-macos11:osx11 iojs$

tniessen · 2021-12-15T15:57:12Z

We should fix this eventually. The fact that this only fails on macOS probably means that we are are not doing things properly on other platforms either.

Could you add a TODO comment in the test file and/or in the C++ source code?

Newer versions of OpenSSL now throws an error if an engine is loaded twice by its absolute path (a second load by its id appears to be okay).

This updates all sources in deps/openssl/openssl by: $ git clone git@github.com:quictls/openssl.git $ cd openssl $ cd ../node/deps/openssl $ rm -rf openssl $ cp -R ../openssl openssl $ rm -rf openssl/.git* openssl/.travis* $ git add --all openssl $ git commit openssl

After an OpenSSL source update, all the config files need to be regenerated and committed by: $ make -C deps/openssl/config $ git add deps/openssl/config/archs $ git add deps/openssl/openssl $ git commit

richardlau · 2021-12-15T16:21:29Z

@tniessen I added a comment to the test (3a840bf). PTAL.

tniessen

Thanks!

nodejs-github-bot · 2021-12-15T17:44:42Z

CI: https://ci.nodejs.org/job/node-test-pull-request/41504/ (✔️ 🎉 )

The `deps/openssl/config/generate_headers.pl` imports `Text::Template` but that Perl module is not present in the default Perl installation in the Dockerfile and needs to be explicitly installed. PR-URL: #41177 Refs: quictls/openssl#69 Refs: https://mta.openssl.org/pipermail/openssl-announce/2021-December/000212.html Reviewed-By: Danielle Adams <adamzdanielle@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>

Newer versions of OpenSSL now throws an error if an engine is loaded twice by its absolute path (a second load by its id appears to be okay). PR-URL: #41177 Refs: quictls/openssl#69 Refs: https://mta.openssl.org/pipermail/openssl-announce/2021-December/000212.html Reviewed-By: Danielle Adams <adamzdanielle@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>

This updates all sources in deps/openssl/openssl by: $ git clone git@github.com:quictls/openssl.git $ cd openssl $ cd ../node/deps/openssl $ rm -rf openssl $ cp -R ../openssl openssl $ rm -rf openssl/.git* openssl/.travis* $ git add --all openssl $ git commit openssl PR-URL: #41177 Refs: quictls/openssl#69 Refs: https://mta.openssl.org/pipermail/openssl-announce/2021-December/000212.html Reviewed-By: Danielle Adams <adamzdanielle@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>

After an OpenSSL source update, all the config files need to be regenerated and committed by: $ make -C deps/openssl/config $ git add deps/openssl/config/archs $ git add deps/openssl/openssl $ git commit PR-URL: #41177 Refs: quictls/openssl#69 Refs: https://mta.openssl.org/pipermail/openssl-announce/2021-December/000212.html Reviewed-By: Danielle Adams <adamzdanielle@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>

The `deps/openssl/config/generate_headers.pl` imports `Text::Template` but that Perl module is not present in the default Perl installation in the Dockerfile and needs to be explicitly installed. PR-URL: #41177 Refs: quictls/openssl#69 Refs: https://mta.openssl.org/pipermail/openssl-announce/2021-December/000212.html Reviewed-By: Danielle Adams <adamzdanielle@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>

Newer versions of OpenSSL now throws an error if an engine is loaded twice by its absolute path (a second load by its id appears to be okay). PR-URL: #41177 Refs: quictls/openssl#69 Refs: https://mta.openssl.org/pipermail/openssl-announce/2021-December/000212.html Reviewed-By: Danielle Adams <adamzdanielle@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>

This updates all sources in deps/openssl/openssl by: $ git clone git@github.com:quictls/openssl.git $ cd openssl $ cd ../node/deps/openssl $ rm -rf openssl $ cp -R ../openssl openssl $ rm -rf openssl/.git* openssl/.travis* $ git add --all openssl $ git commit openssl PR-URL: #41177 Refs: quictls/openssl#69 Refs: https://mta.openssl.org/pipermail/openssl-announce/2021-December/000212.html Reviewed-By: Danielle Adams <adamzdanielle@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>

After an OpenSSL source update, all the config files need to be regenerated and committed by: $ make -C deps/openssl/config $ git add deps/openssl/config/archs $ git add deps/openssl/openssl $ git commit PR-URL: #41177 Refs: quictls/openssl#69 Refs: https://mta.openssl.org/pipermail/openssl-announce/2021-December/000212.html Reviewed-By: Danielle Adams <adamzdanielle@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>

@bnb

OpenSSL-3.0.1: OpenSSL-3.0.1 contains a fix for CVE-2021-4044: Invalid handling of X509_verify_cert() internal errors in libssl (Moderate). This is a vulnerability in OpenSSL that may be exploited through Node.js. More information can be read here: https://www.openssl.org/news/secadv/20211214.txt. Contributed by Richard Lau #41177 Other Notable Changes: * lib: * make AbortSignal cloneable/transferable (James M Snell) #41050 * deps: * upgrade npm to 8.3.0 (npm team) #41127 * doc: * add @bnb as a collaborator (Tierney Cyren) #41100 * process: * add `getActiveResourcesInfo()` (Darshan Sen) #40813 * timers: * add experimental scheduler api (James M Snell) #40909 PR-URL: #41167

The `deps/openssl/config/generate_headers.pl` imports `Text::Template` but that Perl module is not present in the default Perl installation in the Dockerfile and needs to be explicitly installed. PR-URL: #41177 Refs: quictls/openssl#69 Refs: https://mta.openssl.org/pipermail/openssl-announce/2021-December/000212.html Reviewed-By: Danielle Adams <adamzdanielle@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>

Newer versions of OpenSSL now throws an error if an engine is loaded twice by its absolute path (a second load by its id appears to be okay). PR-URL: #41177 Refs: quictls/openssl#69 Refs: https://mta.openssl.org/pipermail/openssl-announce/2021-December/000212.html Reviewed-By: Danielle Adams <adamzdanielle@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>

This updates all sources in deps/openssl/openssl by: $ git clone git@github.com:quictls/openssl.git $ cd openssl $ cd ../node/deps/openssl $ rm -rf openssl $ cp -R ../openssl openssl $ rm -rf openssl/.git* openssl/.travis* $ git add --all openssl $ git commit openssl PR-URL: #41177 Refs: quictls/openssl#69 Refs: https://mta.openssl.org/pipermail/openssl-announce/2021-December/000212.html Reviewed-By: Danielle Adams <adamzdanielle@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>

After an OpenSSL source update, all the config files need to be regenerated and committed by: $ make -C deps/openssl/config $ git add deps/openssl/config/archs $ git add deps/openssl/openssl $ git commit PR-URL: #41177 Refs: quictls/openssl#69 Refs: https://mta.openssl.org/pipermail/openssl-announce/2021-December/000212.html Reviewed-By: Danielle Adams <adamzdanielle@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>

@bnb

OpenSSL-3.0.1: OpenSSL-3.0.1 contains a fix for CVE-2021-4044: Invalid handling of X509_verify_cert() internal errors in libssl (Moderate). This is a vulnerability in OpenSSL that may be exploited through Node.js. More information can be read here: https://www.openssl.org/news/secadv/20211214.txt. Contributed by Richard Lau #41177 Other Notable Changes: * lib: * make AbortSignal cloneable/transferable (James M Snell) #41050 * deps: * upgrade npm to 8.3.0 (npm team) #41127 * doc: * add @bnb as a collaborator (Tierney Cyren) #41100 * process: * add `getActiveResourcesInfo()` (Darshan Sen) #40813 * timers: * add experimental scheduler api (James M Snell) #40909 PR-URL: #41167

@bnb

OpenSSL-3.0.1: OpenSSL-3.0.1 contains a fix for CVE-2021-4044: Invalid handling of X509_verify_cert() internal errors in libssl (Moderate). This is a vulnerability in OpenSSL that may be exploited through Node.js. More information can be read here: https://www.openssl.org/news/secadv/20211214.txt. Contributed by Richard Lau #41177 Other Notable Changes: * lib: * make AbortSignal cloneable/transferable (James M Snell) #41050 * deps: * upgrade npm to 8.3.0 (npm team) #41127 * doc: * add @bnb as a collaborator (Tierney Cyren) #41100 * process: * add `getActiveResourcesInfo()` (Darshan Sen) #40813 * timers: * add experimental scheduler api (James M Snell) #40909 PR-URL: #41167

build: update openssl config generator Dockerfile
119842f
The `deps/openssl/config/generate_headers.pl` imports `Text::Template` but that Perl module is not present in the default Perl installation in the Dockerfile and needs to be explicitly installed.

richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 14, 2021

nodejs-github-bot added needs-ci PRs that need a full CI run. openssl Issues and PRs related to the OpenSSL dependency. labels Dec 14, 2021

danielleadams approved these changes Dec 14, 2021
View reviewed changes

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 14, 2021

This comment has been minimized.
Sign in to view

cjihrig approved these changes Dec 14, 2021
View reviewed changes

mcollina approved these changes Dec 15, 2021
View reviewed changes

richardlau force-pushed the openssl3.0.1+quic branch from 429ee17 to 3af0b32Compare December 15, 2021 15:23

richardlau added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Dec 15, 2021

This comment has been minimized.
Sign in to view

richardlau added 3 commits December 15, 2021 11:16

test: do not load absolute path crypto engines twice
3a840bf
Newer versions of OpenSSL now throws an error if an engine is loaded twice by its absolute path (a second load by its id appears to be okay).

deps: update archs files for quictls/openssl-3.0.1+quic
d9275b9
After an OpenSSL source update, all the config files need to be regenerated and committed by: $ make -C deps/openssl/config $ git add deps/openssl/config/archs $ git add deps/openssl/openssl $ git commit

richardlau force-pushed the openssl3.0.1+quic branch from 3af0b32 to d9275b9Compare December 15, 2021 16:20

This comment has been minimized.
Sign in to view

tniessen approved these changes Dec 15, 2021
View reviewed changes

richardlau added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Dec 15, 2021

danielleadams mentioned this pull request Dec 16, 2021
v17.3.0 release proposal #41167
Merged

github-actionsBot mentioned this pull request Dec 17, 2021
CI Reliability 2021-12-17 nodejs/reliability#147
Open
22 tasks

richardlau deleted the openssl3.0.1+quic branch December 17, 2021 00:38

This was referenced Dec 18, 2021
CI Reliability 2021-12-18 nodejs/reliability#148
Open
CI Reliability 2021-12-19 nodejs/reliability#149
Open
CI Reliability 2021-12-20 nodejs/reliability#150
Open
CI Reliability 2021-12-21 nodejs/reliability#151
Open

richardlau mentioned this pull request Apr 26, 2022
OpenSSL update instructions - feedback wanted #42395
Closed

Uh oh!

Uh oh!

Conversation

richardlau commented Dec 14, 2021

Uh oh!

nodejs-github-bot commented Dec 14, 2021

Uh oh!

danielleadams left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

danielleadams commented Dec 14, 2021

Uh oh!

richardlau commented Dec 15, 2021

Uh oh!

richardlau commented Dec 15, 2021

Uh oh!

tniessen commented Dec 15, 2021

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

richardlau commented Dec 15, 2021

Uh oh!

This comment has been minimized.

tniessen commented Dec 15, 2021

Uh oh!

richardlau commented Dec 15, 2021

Uh oh!

This comment has been minimized.

tniessen left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Dec 15, 2021• edited by richardlau Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

nodejs-github-bot commented Dec 15, 2021•
edited by richardlau
Loading