FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

deps: update openssl to OpenSSL 3.0.2 by hassaanp · Pull Request #42356 · nodejs/node · GitHub

/ node Public

deps: update openssl to OpenSSL 3.0.2 - #42356

Closed
hassaanp wants to merge 3 commits into
nodejs:masterfrom
hassaanp:deps/update-openssl-to-openssl1.1.1n
Closed

deps: update openssl to OpenSSL 3.0.2#42356
hassaanp wants to merge 3 commits into
nodejs:masterfrom
hassaanp:deps/update-openssl-to-openssl1.1.1n

Conversation

Copy link
Copy Markdown
Contributor

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 March 16, 2022 04:49
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
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 <midawson@redhat.com>
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

Copy link
Copy Markdown
Contributor Author

@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.

Copy link
Copy Markdown
Member

1.1.1n -> 3.0.2 in the PR title?

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

This comment was marked as outdated.

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

Copy link
Copy Markdown
Contributor

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

aduh95 commented Mar 16, 2022
edited
Loading

Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Member

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 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

RSLGTM

richardlau commented Mar 16, 2022
edited
Loading

Copy link
Copy Markdown
Member

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

RaisinTen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

RSLGTM

This comment was marked as outdated.

This comment was marked as outdated.

mhdawson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
Collaborator

Landed in f4b7f6d...f1b6d87

nodejs-github-bot pushed a commit that referenced this pull request Mar 17, 2022
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: #42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
nodejs-github-bot pushed a commit that referenced this pull request 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 <rlau@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
nodejs-github-bot pushed a commit that referenced this pull request 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 <midawson@redhat.com>

PR-URL: #42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
richardlau pushed a commit that referenced this pull request Mar 17, 2022
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: #42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
richardlau pushed a commit that referenced this pull request 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 <rlau@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
richardlau pushed a commit that referenced this pull request 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 <midawson@redhat.com>

PR-URL: #42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
richardlau mentioned this pull request Mar 17, 2022
xtx1130 pushed a commit to xtx1130/node that referenced this pull request Apr 25, 2022
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: nodejs#42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
xtx1130 pushed a commit to xtx1130/node that referenced this pull request 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 <rlau@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
xtx1130 pushed a commit to xtx1130/node that referenced this pull request 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 <midawson@redhat.com>

PR-URL: nodejs#42356
Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-March/000217.html
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
This file contains hidden or 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
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. 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.

7 participants


Back | FazBrowse Home | New Git URL