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.4+quic #43603

Closed
wants to merge 4 commits into from
Closed

deps: update OpenSSL 3.0.4+quic #43603

wants to merge 4 commits into from

Conversation

RafaelGSS
Copy link
Member

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

Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-June/000228.html

In this PR I had to run to clean up all .s files

clean-s:
	find archs \( -name \*.s \) -exec rm "{}" \;

Note: if that's works, I'll update the maintaining-openssl document

RafaelGSS added 2 commits June 28, 2022 14:56
@RafaelGSS
deps: upgrade openssl sources to quictls/openssl-3.0.4
9e5048a 
This updates all sources in deps/openssl/openssl by:
    $ git clone git@github.com:quictls/openssl.git
    $ cd openssl
    $ git checkout openssl-3.0.4+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
@RafaelGSS
deps: update archs files for quictls/openssl-3.0.4
edb21b5 
After an OpenSSL source update, all the config files need to be
regenerated and committed by:
    $ make -C deps/openssl/config clean-s
    $ make -C deps/openssl/config
    $ git add deps/openssl/config/archs
    $ git add deps/openssl/openssl
    $ 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. labels Jun 28, 2022
@mscdex
Copy link
Contributor

mscdex commented Jun 28, 2022

I think we should wait for 3.0.5 instead as there was a vulnerability introduced in 3.0.4.

@RafaelGSS RafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 29, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 29, 2022
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@richardlau
Copy link
Member

32-bit Windows is failing to compile 😞.
https://ci.nodejs.org/job/node-compile-windows/45715/nodes=win-vs2019-x86/console

19:52:56   config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm:772: error: label or instruction expected at start of line
19:52:56   config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm:775: error: label or instruction expected at start of line
19:52:56   config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm:780: error: label or instruction expected at start of line
19:52:56   config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm:783: error: label or instruction expected at start of line
19:52:56   config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm:787: error: label or instruction expected at start of line
19:52:56   config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm:790: error: label or instruction expected at start of line
19:52:56   config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm:794: error: label or instruction expected at start of line
19:52:56   config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm:797: error: label or instruction expected at start of line
19:52:56   config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm:802: error: label or instruction expected at start of line
19:52:56   config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm:805: error: label or instruction expected at start of line
19:52:56   config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm:810: error: label or instruction expected at start of line
19:52:56   config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm:813: error: label or instruction expected at start of line
19:52:56   brotli_bit_stream.c
19:52:56   config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm:817: error: label or instruction expected at start of line
19:52:56   config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm:820: error: label or instruction expected at start of line
19:52:56 C:\workspace\node-compile-windows\node\deps\openssl\openssl.targets(28,5): error MSB3721: The command "call nasm.exe "-f win32" "-o" "..\..\out\Release\obj\openssl\bf-586.obj" "config\archs\VC-WIN32\asm\crypto\bf\bf-586.asm"" exited with code 1. [C:\workspace\node-compile-windows\node\deps\openssl\openssl.vcxproj]

@richardlau
Copy link
Member

The Win32 error is probably openssl/openssl#18459 😞

@RafaelGSS
Copy link
Member Author

The Win32 error is probably openssl/openssl#18459 disappointed

You right. Also, likely we'll need to update our openssl/config/Makefile to support .s and .S and remove them just on duplication. What do you think?

This was referenced Jun 30, 2022
Closed
Closed
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@richardlau
Copy link
Member

@RafaelGSS we'll need to regenerate the OpenSSL config after 3b4fa48 for it to have any effect.

@RafaelGSS
deps: update archs files for quictls/openssl-3.0.4
15ca0dc 
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
@nodejs-github-bot
Copy link
Collaborator

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

@RafaelGSS
Copy link
Member Author

Closing in favor of: #43693

@RafaelGSS RafaelGSS closed this Jul 5, 2022
@github-actions github-actions bot mentioned this pull request Jul 8, 2022
Open
16 tasks
@richardlau richardlau mentioned this pull request Nov 3, 2022
Closed
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

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@RafaelGSS @nodejs-github-bot @mscdex @richardlau