Skip to content
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

deps: update openssl to OpenSSL 3.0.2 #42356

Closed

Conversation

hassaanp
Copy link
Contributor

@hassaanp hassaanp commented Mar 16, 2022

Updated openssl dep to openssl-3.0.2+quic using the maintenance guide.

Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html

hassaanp and others added 3 commits Mar 16, 2022
This updates all sources in deps/openssl/openssl by:
    $ git clone [email protected]: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
Last OpenSSL 3 update changes behaviour back to be
closer to that of OpenSSL 1.1.1. Remove some instances
where we expected different errors from OpenSSL 3 versus
OpenSSL 1.1.1.

Signed-off-by: Michael Dawson <[email protected]>
@nodejs-github-bot nodejs-github-bot added dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. openssl Issues and PRs related to the OpenSSL dependency. labels Mar 16, 2022
@hassaanp
Copy link
Contributor Author

hassaanp commented Mar 16, 2022

@mhdawson @richardlau
As per your suggestions, I have opened a PR against master and also cherry-picked the fix for the tests.
I hope this looks good.

@RaisinTen
Copy link
Member

RaisinTen commented Mar 16, 2022

1.1.1n -> 3.0.2 in the PR title?

@hassaanp hassaanp changed the title deps: update openssl to OpenSSL 1.1.1n deps: update openssl to OpenSSL 3.0.2 Mar 16, 2022
@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 16, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 16, 2022
@nodejs-github-bot

This comment was marked as outdated.

@richardlau richardlau added fast-track PRs that do not need to wait for 48 hours to land. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Mar 16, 2022
@github-actions
Copy link

github-actions bot commented Mar 16, 2022

Fast-track has been requested by @richardlau. Please 👍 to approve.

@aduh95
Copy link
Contributor

aduh95 commented Mar 16, 2022

All commits should be self-contained, meaning every commit should pass all
tests. This makes it much easier when bisecting to find a breaking change.

Looking at d37dceb, it looks like tests would not be passing for e06c733 and/or 3361921. Should we add a first commit to disable the failing test (before the OpenSSL update) and re-enable it in a follow up commit?

@aduh95 aduh95 added dont-land-on-v12.x dont-land-on-v14.x PRs that should not land on the v14.x-staging branch and should not be released in v14.x. dont-land-on-v16.x PRs that should not land on the v16.x-staging branch and should not be released in v16.x. labels Mar 16, 2022
@richardlau
Copy link
Member

richardlau commented Mar 16, 2022

All commits should be self-contained, meaning every commit should pass all
tests. This makes it much easier when bisecting to find a breaking change.

Looking at d37dceb, it looks like tests would not be passing for e06c733 and/or 3361921. Should we add a first commit to disable the failing test (before the OpenSSL update) and re-enable it in a follow up commit?

This is a tricky one for OpenSSL because as long as I can remember we've always split the OpenSSL updates into two commits, one to update the sources and a second to regen the config files, and the first commit isn't buildable without the second. I've no preference either way regarding the test in the third commit.

aduh95
aduh95 approved these changes Mar 16, 2022
Copy link
Contributor

@aduh95 aduh95 left a comment

RSLGTM

@richardlau
Copy link
Member

richardlau commented Mar 16, 2022

I'm currently refreshing the CI machines to update the sharedlibs containers to OpenSSL 3.0.2 to fix the failing linked-openssl300 build.

Copy link
Member

@RaisinTen RaisinTen left a comment

RSLGTM

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

Copy link
Member

@mhdawson mhdawson left a comment

LGTM

@mhdawson
Copy link
Member

mhdawson commented Mar 16, 2022

This is a tricky one for OpenSSL because as long as I can remember we've always split the OpenSSL updates into two commits, one to update the sources and a second to regen the config files, and the first commit isn't buildable without the second. I've no preference either way regarding the test in the third commit.

I think we might want to adjust the text saying all commits must pass all tests, for something like an OpenSSL update I see value in them being separate in terms of understanding/being able to recreate. A PR to update the text would be a good place to have that discussion and see if we want to update the OpenSSL patch generation process but that should not be part of getting the security releases out.

@nodejs-github-bot
Copy link
Contributor

nodejs-github-bot commented Mar 16, 2022

@nodejs-github-bot
Copy link
Contributor

nodejs-github-bot commented Mar 16, 2022

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 17, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 17, 2022
@nodejs-github-bot
Copy link
Contributor

nodejs-github-bot commented Mar 17, 2022

Landed in f4b7f6d...f1b6d87

nodejs-github-bot pushed a commit that referenced this issue Mar 17, 2022
This updates all sources in deps/openssl/openssl by:
    $ git clone [email protected]: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: #42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this issue Mar 17, 2022
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: #42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
nodejs-github-bot pushed a commit that referenced this issue Mar 17, 2022
Last OpenSSL 3 update changes behaviour back to be
closer to that of OpenSSL 1.1.1. Remove some instances
where we expected different errors from OpenSSL 3 versus
OpenSSL 1.1.1.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
richardlau pushed a commit that referenced this issue Mar 17, 2022
This updates all sources in deps/openssl/openssl by:
    $ git clone [email protected]: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: #42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
richardlau pushed a commit that referenced this issue Mar 17, 2022
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: #42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
richardlau pushed a commit that referenced this issue Mar 17, 2022
Last OpenSSL 3 update changes behaviour back to be
closer to that of OpenSSL 1.1.1. Remove some instances
where we expected different errors from OpenSSL 3 versus
OpenSSL 1.1.1.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@richardlau richardlau mentioned this pull request Mar 17, 2022
xtx1130 pushed a commit to xtx1130/node that referenced this issue Apr 25, 2022
This updates all sources in deps/openssl/openssl by:
    $ git clone [email protected]: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: nodejs#42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
xtx1130 pushed a commit to xtx1130/node that referenced this issue Apr 25, 2022
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: nodejs#42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
xtx1130 pushed a commit to xtx1130/node that referenced this issue Apr 25, 2022
Last OpenSSL 3 update changes behaviour back to be
closer to that of OpenSSL 1.1.1. Remove some instances
where we expected different errors from OpenSSL 3 versus
OpenSSL 1.1.1.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. dependencies Pull requests that update a dependency file. dont-land-on-v14.x PRs that should not land on the v14.x-staging branch and should not be released in v14.x. dont-land-on-v16.x PRs that should not land on the v16.x-staging branch and should not be released in v16.x. fast-track PRs that do not need to wait for 48 hours to land. needs-ci PRs that need a full CI run. openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants