Skip to content

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

This updates all sources in deps/openssl/openssl by:
    $ git clone [email protected]: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
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

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

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

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

@RafaelGSS
Copy link
Member Author

Closing in favor of: #43693

@RafaelGSS RafaelGSS closed this Jul 5, 2022
jaanrebane added a commit to jaanrebane/node that referenced this pull request Oct 29, 2025
Some sed lines were previously used to change from C-style #ifdef
to nasm-style %ifdef in x86asm.pl for 32-bit Windows builds,
but this creates problems when the C preprocessor is used before
the assembler to build x86 assembly files.

OpenSSL is using C preprocessor before nasm and uses #ifdef
in this context.

The perl line added to update-openssl.sh will work around the ifdef
issue in a way that enables building for win32 and other x86.

After update-openssl.sh script is run with "regenerate", x86asm.pl
will end up with a modified "endbranch" subroutine that can use
2 types of ifdef (nasm-style %ifdef for win32 and gcc-style #ifdef
for others). Then, after x86asm.pl is run during the node openssl
build process, x86 assembly files may change their ifdef and endif
lines depending on the system they are built for.

Issues that lead to this commit:
  * openssl/openssl#18459
  * nodejs#43603 (comment)
  * nodejs#44822
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.

4 participants