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

[v10.x backport]: Update OpenSSL 1.1.1b #27419

Closed

Conversation

sam-github
Copy link
Contributor

Backport of #26327 via #26951. I will try to backport the rest of TLS1.3 from #26951, but if I don't get it in time, it would stll be a good idea to update to the latest openssl patch release for node.js 10.16.0.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added openssl Issues and PRs related to the OpenSSL dependency. v10.x labels Apr 25, 2019
@sam-github
Copy link
Contributor Author

PR created through mechanical application of the upgrade process in 91f4cac, no further changes required.

@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

Rubber stamp LGTM based on no additional changes being needed.

@nodejs-github-bot
Copy link
Collaborator

@sam-github
Copy link
Contributor Author

Test failure in containerd was due to a worker message port test, which doesn't seem like it is related, so I resumed the tests.

@sam-github
Copy link
Contributor Author

I'm seeing these. Don't think they are related to openssl, so I'm re-trying 10.x-staging.

Is this a known issue?

Stacktrace
uv loop at [0x38cd200] has active handles
out/Release/node[1975]: ../src/debug_utils.cc:320:void node::CheckedUvLoopClose(uv_loop_t*): Assertion `0 && "uv_loop_close() while having open handles"' failed.
 1: 0x8ea8c0 node::Abort() [out/Release/node]
 2: 0x8ea995  [out/Release/node]
 3: 0x8c7832  [out/Release/node]
 4: 0x997cef node::worker::Worker::~Worker() [out/Release/node]
 5: 0x997e91 node::worker::Worker::~Worker() [out/Release/node]
 6: 0x8ce52a node::Environment::RunCleanup() [out/Release/node]
 7: 0x8f514c node::Start(v8::Isolate*, node::IsolateData*, std::vector<std::string, std::allocator<std::string> > const&, std::vector<std::string, std::allocator<std::string> > const&) [out/Release/node]
 8: 0x8f333c node::Start(int, char**) [out/Release/node]
 9: 0x7f17940db3d5 __libc_start_main [/lib64/libc.so.6]
10: 0x8acea5  [out/Release/node]

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@sam-github
Copy link
Contributor Author

sam-github commented Apr 30, 2019

The tests fail in different platforms each time the ci is run, and not in tests that seem connected to openssl (worker tests, for example, don't use crypto). Is the testsuite less stable on 10.x than later platforms?

EDIT: https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu1604_sharedlibs_shared_x64/12553/testReport/junit/(root)/test/parallel_test_worker_messageport_transfer_terminate/ for example, doesn't seem to be openssl related

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

sam-github and others added 6 commits May 10, 2019 14:47
Particularly, ensure that the commit messages are self-explanatory so
that reviewers can understand that the large commits are the result of a
simple repeatable process. This should make them easier to review.

See: nodejs#26327 (comment)

PR-URL: nodejs#26378
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
This updates all sources in deps/openssl/openssl with openssl-1.1.1b.

PR-URL: nodejs#26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Since its not packaged, we don't have to delete it, and the Makefile
and update can become a (tiny) bit simpler.

PR-URL: nodejs#26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
This adds ARM64 Windows support in the OpenSSL build system.

Since OpenSSL's ARM64 Windows support does not have support for ASM--
that is, VC-WIN64-ARM inherits from VC-noCE-common which has no ASM
files--`openssl_no_asm.gypi` is always used for building. This
essentially forces the 'no-asm' Configure flag.

PR-URL: nodejs#26001
Fixes: nodejs#25998
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Signed-off-by: Beth Griggs <[email protected]>
This is a floating patch against OpenSSL-1.1.1 to generate asm files
with Makefile rules.

PR-URL: nodejs#25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Backport-PR-URL: nodejs#25688
`cd deps/openssl/config; make` updates all archs dependant files.

PR-URL: nodejs#26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@sam-github sam-github force-pushed the update-openssl-1.1.1b-v10.x branch from fbdbe8a to 9648b04 Compare May 10, 2019 21:48
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

MylesBorins pushed a commit that referenced this pull request May 16, 2019
Particularly, ensure that the commit messages are self-explanatory so
that reviewers can understand that the large commits are the result of a
simple repeatable process. This should make them easier to review.

See: #26327 (comment)

Backport-PR-URL: #27419
PR-URL: #26378
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
This updates all sources in deps/openssl/openssl with openssl-1.1.1b.

Backport-PR-URL: #27419
PR-URL: #26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
Since its not packaged, we don't have to delete it, and the Makefile
and update can become a (tiny) bit simpler.

Backport-PR-URL: #27419
PR-URL: #26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
This adds ARM64 Windows support in the OpenSSL build system.

Since OpenSSL's ARM64 Windows support does not have support for ASM--
that is, VC-WIN64-ARM inherits from VC-noCE-common which has no ASM
files--`openssl_no_asm.gypi` is always used for building. This
essentially forces the 'no-asm' Configure flag.

Backport-PR-URL: #27419
PR-URL: #26001
Fixes: #25998
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Signed-off-by: Beth Griggs <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
This is a floating patch against OpenSSL-1.1.1 to generate asm files
with Makefile rules.

Backport-PR-URL: #27419
PR-URL: #25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Backport-PR-URL: #25688
MylesBorins pushed a commit that referenced this pull request May 16, 2019
`cd deps/openssl/config; make` updates all archs dependant files.

Backport-PR-URL: #27419
PR-URL: #26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@MylesBorins
Copy link
Contributor

tests lgtm

landed in 0620314...16478de

@sam-github sam-github deleted the update-openssl-1.1.1b-v10.x branch May 16, 2019 16:46
MylesBorins pushed a commit that referenced this pull request May 16, 2019
Particularly, ensure that the commit messages are self-explanatory so
that reviewers can understand that the large commits are the result of a
simple repeatable process. This should make them easier to review.

See: #26327 (comment)

Backport-PR-URL: #27419
PR-URL: #26378
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
This updates all sources in deps/openssl/openssl with openssl-1.1.1b.

Backport-PR-URL: #27419
PR-URL: #26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
Since its not packaged, we don't have to delete it, and the Makefile
and update can become a (tiny) bit simpler.

Backport-PR-URL: #27419
PR-URL: #26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
This adds ARM64 Windows support in the OpenSSL build system.

Since OpenSSL's ARM64 Windows support does not have support for ASM--
that is, VC-WIN64-ARM inherits from VC-noCE-common which has no ASM
files--`openssl_no_asm.gypi` is always used for building. This
essentially forces the 'no-asm' Configure flag.

Backport-PR-URL: #27419
PR-URL: #26001
Fixes: #25998
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Signed-off-by: Beth Griggs <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
This is a floating patch against OpenSSL-1.1.1 to generate asm files
with Makefile rules.

Backport-PR-URL: #27419
PR-URL: #25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Backport-PR-URL: #25688
MylesBorins pushed a commit that referenced this pull request May 16, 2019
`cd deps/openssl/config; make` updates all archs dependant files.

Backport-PR-URL: #27419
PR-URL: #26327
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants