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

Upgrade to openssl-1.0.2p in Node-v8 #22320

Closed
wants to merge 7 commits into from

Conversation

shigeki
Copy link
Contributor

@shigeki shigeki commented Aug 14, 2018

This upgrades OpenSSL-1.0.2p. It has no updates of asm files.

test-tls-passphrase.js was failed due to the changes of openssl/openssl@18026c0. This PR includes the fix of the test in 0ec9c32cf9bb99d8ff0effc15de6afb319ebce16.

Fixes: #22187

@nodejs/security-wg

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

@shigeki shigeki added test Issues and PRs related to the tests. openssl Issues and PRs related to the OpenSSL dependency. labels Aug 14, 2018
@nodejs-github-bot nodejs-github-bot added openssl Issues and PRs related to the OpenSSL dependency. v8.x labels Aug 14, 2018
@shigeki
Copy link
Contributor Author

shigeki commented Aug 14, 2018

node-test-linux-linked-openssl102 would be failed in test-tls-passphrase because the CI server still has openssl-1.0.2o. Others test failures seems to be not related to this PR.

@shigeki shigeki mentioned this pull request Aug 15, 2018
4 tasks
@rvagg
Copy link
Member

rvagg commented Aug 15, 2018

@shigeki I was trying to pull this on to v8.x cause we're not releasing everything that's on v8.x-staging but there's conflicts in ecs_ossl.c (and maybe others) because of the patches we were going to float. I would prefer to ditch those now that we have the final version.

Currently I'm messing around with squashing them in to the 1.0.2p commit you added here but I'm not confident that we have precisely the right source in the end. Are you around and can you do this against v8.x instead?

@shigeki shigeki changed the base branch from v8.x-staging to v8.x August 15, 2018 03:35
@shigeki shigeki changed the base branch from v8.x to v8.x-staging August 15, 2018 03:35
@shigeki
Copy link
Contributor Author

shigeki commented Aug 15, 2018

@rvagg Okay, I will change this against v8.x branch.

shigeki and others added 7 commits August 15, 2018 12:38
This replaces all sources of openssl-1.0.2p.tar.gz into
deps/openssl/openssl
All symlink files in `deps/openssl/openssl/include/openssl/`
are removed and replaced with real header files to avoid
issues on Windows. Two files of opensslconf.h in crypto and
include dir are replaced to refer config/opensslconf.h.
`x86masm.pl` was mistakenly using .486 instruction set, why `cpuid` (and
perhaps others) are requiring .686 .

Fixes: nodejs#589
PR-URL: nodejs#1389
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
See
https://mta.openssl.org/pipermail/openssl-dev/2015-February/000651.html

iojs needs to stop using masm and move to nasm or yasm on Win32.

Fixes: nodejs#589
PR-URL: nodejs#1389
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reapply b910613 .

Fixes: nodejs#589
PR-URL: nodejs#1389
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
In openssl s_client on Windows, RAND_screen() is invoked to initialize
random state but it takes several seconds in each connection.
This added -no_rand_screen to openssl s_client on Windows to skip
RAND_screen() and gets a better performance in the unit test of
test-tls-server-verify.
Do not enable this except to use in the unit test.

Fixes: nodejs#1461
PR-URL: nodejs#1836
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
After upgradeing OpenSSL-1.0.2p, test-tls-passphrase.js was failed
due to change of error messages.

Ref: openssl/openssl@18026c0
@shigeki shigeki force-pushed the upgrade_oepnssl102p branch from 0ec9c32 to 127eff6 Compare August 15, 2018 03:41
@shigeki shigeki changed the base branch from v8.x-staging to v8.x August 15, 2018 03:41
@shigeki
Copy link
Contributor Author

shigeki commented Aug 15, 2018

@rvagg Done. Could you try CI?

@shigeki
Copy link
Contributor Author

shigeki commented Aug 15, 2018

I found that test-tls-server-verify.js was failed because 0c9760d is missed in v8.x branch. Others seem to be good.

@rvagg
Copy link
Member

rvagg commented Aug 15, 2018

Yep, we have 0c9760d included in this release already so it's all sorted. CI is good for this and the other commits queued up for release. The only red is on v6 for a GYP addon compile failure on Windows for vcbt2015 which I don't believe is anything to do with our release. Thanks @shigeki!

@rvagg
Copy link
Member

rvagg commented Aug 16, 2018

landed in 6.x and 8.x and published in 6.14.4, 8.11.4, thanks @shigeki!

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. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants