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.

Sign up for GitHub

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

Jump to bottom

deps: update OpenSSL 3.0.8+quic #46570

Closed
wants to merge 3 commits into from
Closed

deps: update OpenSSL 3.0.8+quic #46570

wants to merge 3 commits into from

Conversation

RafaelGSS
Copy link
Member

Updated openssl dep to openssl-3.0.8 using the maintenance guide.

Refs: https://mta.openssl.org/pipermail/openssl-announce/2023-February/000251.html

@RafaelGSS
deps: upgrade openssl sources to quictls/openssl-3.0.8+quic
1fcf403 
This updates all sources in deps/openssl/openssl by:
    $ git clone git@github.com:quictls/openssl.git
    $ cd openssl
    $ git checkout openssl-3.0.8+quic
    $ cd ../node/deps/openssl
    $ rm -rf openssl
    $ cp -R ../../../openssl openssl
    $ rm -rf openssl/.git* openssl/.travis*
    $ git add --all openssl
    $ git commit openssl
@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 Feb 8, 2023
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@richardlau
Copy link
Member

The Windows on ARM64 failure is an upstream OpenSSL bug on all the new OpenSSL releases.

The 32-bit Windows x86 failure is something we hit before -- I think we were floating a patch for it on top of OpenSSL?

@richardlau
Copy link
Member

The 32-bit Windows x86 failure is something we hit before -- I think we were floating a patch for it on top of OpenSSL?

There's a note about 32-bit Windows in https://github.com/nodejs/node/blob/main/doc/contributing/maintaining-openssl.md#2-execute-make-in-depsopensslconfig-directory

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/49483/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/49489/

@richardlau
Copy link
Member

@RafaelGSS Looks like something has gone awry with the config generation for this PR.
https://github.com/nodejs/node/actions/runs/4153362833/jobs/7184952274#step:7:930

    "/Users/runner/work/node/node/deps/openssl/openssl/include/openssl/opensslconf.h:14:11: fatal error: 'openssl/configuration.h' file not found\n" +
    '# include <openssl/configuration.h>\n' +
    '          ^~~~~~~~~~~~~~~~~~~~~~~~~\n' +
    '1 error generated.\n' +

RafaelGSS and others added 2 commits February 13, 2023 11:17
@RafaelGSS
deps: update archs files for quictls/openssl-3.0.8+quic
0e41238 
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 @RafaelGSS
deps: cherry-pick Windows ARM64 fix for openssl
8dc2d94 
Original commit message:

    rsa: add msvc intrinsic for non x64 platforms

    _umul128() is x86_64 (x64) only, while __umulh() works everywhere, but
    doesn't generate optimal code on x64

Refs: openssl/openssl#20244
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/49504/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/49511/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/49516/

@RafaelGSS
Copy link
Member Author

Landed in f369c0a...9f468cc

@RafaelGSS RafaelGSS closed this Feb 16, 2023
@MylesBorins
Copy link
Contributor

This doesn't land cleanly on v19.x, If this is supposed to land can someone please backport

@richardlau richardlau added dont-land-on-v14.x dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. and removed backport-requested-v19.x labels Feb 20, 2023
@richardlau
Copy link
Member

This landed in v19.x via #46573 as part of the recent security releases.

@targos
Copy link
Member

targos commented Mar 13, 2023

d93d771 landed with the wrong PR-URL (it points to issue #46571)

@RafaelGSS RafaelGSS mentioned this pull request Apr 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
dependencies Pull requests that update a dependency file. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. needs-ci PRs that need a full CI run. openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@RafaelGSS @nodejs-github-bot @richardlau @MylesBorins @targos @jasnell