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

[v14.x] deps: update OpenSSL 1.1.1t #46566

Closed
wants to merge 3 commits into from

Conversation

RafaelGSS
Copy link
Member

Updated openssl dep to openssl-1.1.1t using the maintenance guide.

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

This updates all sources in deps/openssl/openssl by:
    $ cd deps/openssl/
    $ rm -rf openssl
    $ tar zxf ~/tmp/openssl-1.1.1t.tar.gz
    $ mv openssl-1.1.1t openssl
    $ 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/include/crypto/bn_conf.h
   $ git add deps/openssl/openssl/include/crypto/dso_conf.h
   $ git add deps/openssl/openssl/include/openssl/opensslconf.h
   $ git commit
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp

@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. v14.x labels Feb 8, 2023
@nodejs-github-bot

This comment was marked as outdated.

@richardlau richardlau added the fast-track PRs that do not need to wait for 48 hours to land. label Feb 8, 2023
@github-actions

This comment was marked as outdated.

@richardlau
Copy link
Member

This has failed to compile for Windows arm64:
https://ci.nodejs.org/job/node-compile-windows/49265/nodes=win-vs2019-arm64/consoleFull (link is restricted while the CI is locked down):

13:17:47 openssl.lib(rsa_sup_mul.obj) : error LNK2019: unresolved external symbol _umul128 referenced in function _mul_add_limb [C:\workspace\node-compile-windows\node\deps\openssl\openssl-cli.vcxproj]
13:17:47 ..\..\out\Release\openssl-cli.exe : fatal error LNK1120: 1 unresolved externals [C:\workspace\node-compile-windows\node\deps\openssl\openssl-cli.vcxproj]

@nodejs/platform-windows-arm

@richardlau
Copy link
Member

This has failed to compile for Windows arm64: https://ci.nodejs.org/job/node-compile-windows/49265/nodes=win-vs2019-arm64/consoleFull (link is restricted while the CI is locked down):

13:17:47 openssl.lib(rsa_sup_mul.obj) : error LNK2019: unresolved external symbol _umul128 referenced in function _mul_add_limb [C:\workspace\node-compile-windows\node\deps\openssl\openssl-cli.vcxproj]
13:17:47 ..\..\out\Release\openssl-cli.exe : fatal error LNK1120: 1 unresolved externals [C:\workspace\node-compile-windows\node\deps\openssl\openssl-cli.vcxproj]

@nodejs/platform-windows-arm

Looks like we've hit openssl/openssl#20234. There's two competing open PRs:

We'll either have to float one of those patches or somehow temporarily stop building on Windows ARM64 as it doesn't sound like OpenSSL will be fixing this any time soon as to them, openssl/openssl#20234 (comment):

VC-WIN64-ARM is an unadopted platform which means any regressions in it won't trigger a release.

@niyas-sait
Copy link
Contributor

Cc @StefanStojanovic for Windows Arm64

@richardlau
Copy link
Member

Checking if cherry-picking the changes from openssl/openssl#20244 fixes the Win ARM64 build.
This PR with richardlau@91b5276: https://ci.nodejs.org/job/node-test-commit/59690/

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

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 10, 2023

@richardlau richardlau removed the fast-track PRs that do not need to wait for 48 hours to land. label Feb 13, 2023
richardlau pushed a commit that referenced this pull request Feb 13, 2023
This updates all sources in deps/openssl/openssl by:
    $ cd deps/openssl/
    $ rm -rf openssl
    $ tar zxf ~/tmp/openssl-1.1.1t.tar.gz
    $ mv openssl-1.1.1t openssl
    $ git add --all openssl
    $ git commit openssl

PR-URL: #46566
Refs: https://mta.openssl.org/pipermail/openssl-announce/2023-February/000251.html
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com>
richardlau pushed a commit that referenced this pull request Feb 13, 2023
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/include/crypto/bn_conf.h
   $ git add deps/openssl/openssl/include/crypto/dso_conf.h
   $ git add deps/openssl/openssl/include/openssl/opensslconf.h
   $ git commit

PR-URL: #46566
Refs: https://mta.openssl.org/pipermail/openssl-announce/2023-February/000251.html
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com>
richardlau added a commit that referenced this pull request Feb 13, 2023
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

PR-URL: #46566
Refs: openssl/openssl#20244
Refs: https://mta.openssl.org/pipermail/openssl-announce/2023-February/000251.html
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com>
@richardlau
Copy link
Member

Landed in 224e93c...9e62215.

@richardlau richardlau closed this Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants