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

Preparation for upgrading to the forthcoming openssl-1.0.1m #1186

Closed
shigeki opened this issue Mar 18, 2015 · 18 comments
Closed

Preparation for upgrading to the forthcoming openssl-1.0.1m #1186

shigeki opened this issue Mar 18, 2015 · 18 comments
Labels
crypto Issues and PRs related to the crypto subsystem. tls Issues and PRs related to the tls subsystem.

Comments

@shigeki
Copy link
Contributor

shigeki commented Mar 18, 2015

OpenSSL-1.0.1m is to be released on March 19 which fixes vulnerabilities of high severity. For quick release of iojs, I've just made a branch of upgrading it to the current HEAD of OpenSSL_1_0_1-stable branch in https://github.com/shigeki/io.js/tree/WIP_OpenSSL_1_0_1m .

After extracting all sources of the current HEAD of OpenSSL_1_0_1-stable in 879c4c3 , the following 5 commits are needed.

14a7f46 deps: replace all headers in openssl
f46274e deps: separate sha256/sha512-x86_64.pl for openssl
3d8b231 deps: fix openssl assembly error on ia32 win32
206b3f0 deps: remove vpaesni-x86_64.asm in x64-win32-masm
1efcfd5 openssl: fix keypress requirement in apps on win32

The forthcoming release would not be so much changed from the current HEAD so these commits can easily be cherry-picked.
I also checked asm files are not changed with asm/Makefile. CI results of https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/322/ are fine with known test failures except the one of test-signal-unregister on Ubuntu10.01 but I think it's not related with this upgrade.

@bnoordhuis @indutny Could you please review the above five commits for preparation?

@shigeki shigeki added tls Issues and PRs related to the tls subsystem. crypto Issues and PRs related to the crypto subsystem. labels Mar 18, 2015
@shigeki shigeki self-assigned this Mar 18, 2015
@Fishrock123
Copy link
Contributor

I don't see that test failure anywhere else though..

@bnoordhuis
Copy link
Member

14a7f46 is c4b9be7 redux, I think? What about f46274e? Was sha256-x86_64.pl added by @indutny in 508a6c2?

LGTM though. No comments, just some questions.

@indutny
Copy link
Member

indutny commented Mar 18, 2015

Yeah, I think I already did sha256-x86_64.pl thing. Otherwise LGTM

@indutny
Copy link
Member

indutny commented Mar 18, 2015

Thanks for you work!

@rvagg
Copy link
Member

rvagg commented Mar 18, 2015

Agreed in TC meeting today that we'll hold up 1.6.0 for this and that we should release asap after this is merged.

@shigeki
Copy link
Contributor Author

shigeki commented Mar 19, 2015

Thanks for reviews and test check. The release will be between 11:00-15:00 GMT today. I will work it as soon as it is available.

@rvagg
Copy link
Member

rvagg commented Mar 19, 2015

that window starts at 10pm for me, so I may be up, otherwise it'll have to wait for @chrisdickinson to wake up and finish off a release

@shigeki
Copy link
Contributor Author

shigeki commented Mar 19, 2015

I will write down my progress here. If not, I give it over to @bnoordhuis or @indutny .

@jbergstroem
Copy link
Member

While looking through this I found deps/openssl/asm/x64-win32-masm/x86_64cpuid.asm.orig. Guessing this file was checked in by mistake (commit 5edbb53)?

@shigeki
Copy link
Contributor Author

shigeki commented Mar 19, 2015

Good catch! I missed to find it. I will update the commits to remove it.

@shigeki
Copy link
Contributor Author

shigeki commented Mar 19, 2015

Added in b2a214b. Also reverting 8b2363d was deleted and it was force pushed to my branch.

@rvagg
Copy link
Member

rvagg commented Mar 19, 2015

tick tick tick

looks like the details are all available here: https://security-tracker.debian.org/tracker/source-package/openssl and the only one for which a meaningful diff isn't available is CVE-2015-0291 which sounds like it's a 1.0.2 DoS thing which wouldn't apply to us anyway.

https://twitter.com/ramosbugs/status/577935589397278720

@shigeki
Copy link
Contributor Author

shigeki commented Mar 19, 2015

Year, I found that. I feel revealed to know it.

@shigeki
Copy link
Contributor Author

shigeki commented Mar 19, 2015

It's released now. I work it from now on.

@shigeki
Copy link
Contributor Author

shigeki commented Mar 19, 2015

@Fishrock123
Copy link
Contributor

CI's happy.

@shigeki
Copy link
Contributor Author

shigeki commented Mar 19, 2015

@Fishrock123 Yes, I will submit a PR soon.

shigeki pushed a commit that referenced this issue Mar 19, 2015
All sources are just extracted from tarball into deps/openssl/openssl.

change all openssl/include/openssl/*.h to include resolved symbolic
links and openssl/crypto/opensslconf.h to refer config/opensslconf.h

sha256-x86_64.pl does not exist in the origin openssl distribution. It
was copied from sha512-x86_64.pl and both sha256/sha512 scripts were
modified so as to generates only one asm file specified as its key
hash length.

`x86masm.pl` was mistakenly using .486 instruction set, why `cpuid`
(and perhaps others) are requiring .686.

removed vpaesni-x86_64.asm in x64-win32-masm - it is no longer used.

Fixes: #1186
PR-URL: #1206
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@bnoordhuis
Copy link
Member

Closing, #1206 landed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

No branches or pull requests

6 participants