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

Upgrade to openssl 101t for v0.12 #6553

Closed
wants to merge 7 commits into from
Closed

Upgrade to openssl 101t for v0.12 #6553

wants to merge 7 commits into from

Conversation

shigeki
Copy link
Contributor

@shigeki shigeki commented May 3, 2016

Checklist
  • tests and code linting passes
  • a test and/or benchmark is included
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

tls/crypto

Description of change

openssl sources are upgraded to 1.0.1t and applied floating patches.
One more works was made in this upgrade.

-asm regenerated
asm codes were changed in this upgrade so that asm were regenerated without CC and ASM env.

CI is https://ci.nodejs.org/job/node-test-commit/3167/ but test-tls-server-verify.js was failed on several platforms due to some connection issues between server and spawned processes.
As far as I checked, the failures are not caused by this upgrade and need further investigation.
I'd like to know the test is flaky but I could not check CI results for v0.12 in the past. They seemed to be removed in the CI results.

Shigeki Ohtsu and others added 6 commits May 4, 2016 03:00
deps: upgrade openssl sources to 1.0.1t
b225e36 
This just replaces all sources of openssl-1.0.1t.tar.gz
into deps/openssl/openssl.
deps: copy all openssl header files to include dir
1921fe2 
All symlink files in `deps/openssl/openssl/include/openssl/`
are removed and replaced with real header files to avoid
issues on Windows.
deps: separate sha256/sha512-x86_64.pl for openssl
f27d515 
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.

PR: nodejs#9451
PR-URL: nodejs/node-v0.x-archive#9451
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>

PR: nodejs#25523
PR-URL: nodejs/node-v0.x-archive#25523
Reviewed-By: Julien Gilli <jgilli@fastmail.fm>

PR: nodejs#25654
PR-URL: nodejs/node-v0.x-archive#25654
Reviewed-By: Julien Gilli <jgilli@fastmail.fm>
@indutny
deps: fix openssl assembly error on ia32 win32
99e87fa 
`x86masm.pl` was mistakenly using .486 instruction set, why `cpuid` (and
perhaps others) are requiring .686 .

PR: nodejs#9451
PR-URL: nodejs/node-v0.x-archive#9451
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>

PR: nodejs#25523
PR-URL: nodejs/node-v0.x-archive#25523
Reviewed-By: Julien Gilli <jgilli@fastmail.fm>

PR: nodejs#25654
PR-URL: nodejs/node-v0.x-archive#25654
Reviewed-By: Julien Gilli <jgilli@fastmail.fm>
openssl: fix keypress requirement in apps on win32
352726e 
reapply b910613

PR: nodejs#9451
PR-URL: nodejs/node-v0.x-archive#9451
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>

PR: nodejs#25523
PR-URL: nodejs/node-v0.x-archive#25523
Reviewed-By: Julien Gilli <jgilli@fastmail.fm>

PR: nodejs#25654
PR-URL: nodejs/node-v0.x-archive#25654
Reviewed-By: Julien Gilli <jgilli@fastmail.fm>
deps: update openssl asm files
de861bb 
Regenerate asm files with Makefile without CC and ASM envs.
@shigeki shigeki added the openssl Issues and PRs related to the OpenSSL dependency. label May 3, 2016
@mscdex mscdex added the v0.12 label May 3, 2016
deps: add -no_rand_screen to openssl s_client
bfc50bf 
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.

(cherry picked from commit 9f0f7c38e6df975dd39735d0e9ef968076369c74)

Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node-v0.x-archive#25368
@bnoordhuis
Copy link
Member

Can it be you forgot the -no_rand_screen patch?

@shigeki
Copy link
Contributor Author

shigeki commented May 4, 2016

@bnoordhuis Thanks, I missed it. I should have checked Windows tests more further.

New CI is https://ci.nodejs.org/job/node-test-commit/3187/ . There still exists test-tls-server-verify.js failures. I've tested the current v0.12-staging and it shows the same test failures so they are not related to this PR.

@rvagg
Copy link
Member

rvagg commented May 5, 2016

@nodejs/crypto can we get signoff from someone qualified on this? The test-tls-server-verify.js failures all over the slaves concern me but if they are not new for these changes then I supposed that's OK.

@rvagg
Copy link
Member

rvagg commented May 5, 2016

@shigeki would you mind applying the same to v0.10-staging too please?

@shigeki
Copy link
Contributor Author

shigeki commented May 5, 2016

@rvagg Sure. I've just submitted CI job in https://ci.nodejs.org/job/node-test-commit/3193/. Should I opens a new PR for v0.10?

@shigeki
Copy link
Contributor Author

shigeki commented May 5, 2016

Note that test-tls-server-verify.js is fine in v0.10. The failure is a specific issue in v0.12.

@bnoordhuis
Copy link
Member

LGTM. test-tls-server-verify.js has been failing since at least v0.12.7, possibly longer.

shigeki pushed a commit that referenced this pull request May 5, 2016
deps: upgrade openssl sources to 1.0.1t
08c8ae4 
This just replaces all sources of openssl-1.0.1t.tar.gz
into deps/openssl/openssl.

Fixes: #6458
PR-URL: #6553
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
shigeki pushed a commit that referenced this pull request May 5, 2016
deps: copy all openssl header files to include dir
3938083 
All symlink files in `deps/openssl/openssl/include/openssl/`
are removed and replaced with real header files to avoid
issues on Windows.

Fixes: #6458
PR-URL: #6553
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
shigeki pushed a commit that referenced this pull request May 5, 2016
deps: update openssl asm files
f21705d 
Regenerate asm files with Makefile without CC and ASM envs.

Fixes: #6458
PR-URL: #6553
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
shigeki pushed a commit that referenced this pull request May 5, 2016
deps: upgrade openssl sources to 1.0.1t
61ccc27 
This just replaces all sources of openssl-1.0.1t.tar.gz
into deps/openssl/openssl.

Fixes: #6458
PR-URL: #6553
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
shigeki pushed a commit that referenced this pull request May 5, 2016
deps: copy all openssl header files to include dir
11eefef 
All symlink files in `deps/openssl/openssl/include/openssl/`
are removed and replaced with real header files to avoid
issues on Windows.

Fixes: #6458
PR-URL: #6553
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
shigeki pushed a commit that referenced this pull request May 5, 2016
deps: update openssl asm files
5d78366 
Regenerate asm files with Makefile without CC and ASM envs.

Fixes: #6458
PR-URL: #6553
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@shigeki
Copy link
Contributor Author

shigeki commented May 5, 2016

Thanks. Landed in v0.12-staging for 08c8ae4, 3938083, 1aecc66, 02b6a6b, f5a961a, f21705d and 2b63396
and v0.10-staging for 61ccc27, 11eefef, 8df4b09, 2bc2427, aa02438, 5d78366 and 7c22f19

@rvagg Please go for new release v0.12 and v0.10.

@shigeki shigeki closed this May 5, 2016
@elicwhite
Copy link

Are we expecting a new release for 0.12 to contain this OpenSSL fix? The 4, 5, and 6 lines got new releases today but I don't see a cut of this for 0.12

@rvagg rvagg mentioned this pull request May 6, 2016
Merged
rvagg added a commit that referenced this pull request May 6, 2016
@rvagg
2016-05-06 Version 0.10.45 (Maintenance) Release
1b2bc79 
Notable changes:

* npm: Correct erroneous version number in v2.15.1 code
  (Forrest L Norvell) #5987
* openssl: Upgrade to v1.0.1t, addressing security vulnerabilities
  (Shigeki Ohtsu) #6553
  - Fixes CVE-2016-2107 "Padding oracle in AES-NI CBC MAC check"
  - Fixes CVE-2016-2105 "EVP_EncodeUpdate overflow"
  - See https://nodejs.org/en/blog/vulnerability/openssl-may-2016/ for
    full details
rvagg added a commit that referenced this pull request May 6, 2016
@rvagg
2016-05-06 Version 0.12.14 (Maintenance) Release
a7376c9 
Notable changes:

* npm: Correct erroneous version number in v2.15.1 code
  (Forrest L Norvell) #5988
* openssl: Upgrade to v1.0.1t, addressing security vulnerabilities
  (Shigeki Ohtsu) #6553
  - Fixes CVE-2016-2107 "Padding oracle in AES-NI CBC MAC check"
  - Fixes CVE-2016-2105 "EVP_EncodeUpdate overflow"
  - See https://nodejs.org/en/blog/vulnerability/openssl-may-2016/
    for full details
@rvagg rvagg mentioned this pull request May 6, 2016
Merged
rvagg added a commit that referenced this pull request May 6, 2016
@rvagg
2016-05-06 Version 0.10.45 (Maintenance) Release
1ec421e 
Notable changes:

* npm: Correct erroneous version number in v2.15.1 code
  (Forrest L Norvell) #5987
* openssl: Upgrade to v1.0.1t, addressing security vulnerabilities
  (Shigeki Ohtsu) #6553
  - Fixes CVE-2016-2107 "Padding oracle in AES-NI CBC MAC check"
  - Fixes CVE-2016-2105 "EVP_EncodeUpdate overflow"
  - See https://nodejs.org/en/blog/vulnerability/openssl-may-2016/ for
    full details
rvagg added a commit that referenced this pull request May 6, 2016
@rvagg
2016-05-06 Version 0.12.14 (Maintenance) Release
98b534f 
Notable changes:

* npm: Correct erroneous version number in v2.15.1 code
  (Forrest L Norvell) #5988
* openssl: Upgrade to v1.0.1t, addressing security vulnerabilities
  (Shigeki Ohtsu) #6553
  - Fixes CVE-2016-2107 "Padding oracle in AES-NI CBC MAC check"
  - Fixes CVE-2016-2105 "EVP_EncodeUpdate overflow"
  - See https://nodejs.org/en/blog/vulnerability/openssl-may-2016/
    for full details
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
@jBarz
deps: upgrade openssl sources to 1.0.1t
8963a0e 
This just replaces all sources of openssl-1.0.1t.tar.gz
into deps/openssl/openssl.

Fixes: nodejs/node#6458
PR-URL: nodejs/node#6553
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
@jBarz
deps: copy all openssl header files to include dir
cc78d29 
All symlink files in `deps/openssl/openssl/include/openssl/`
are removed and replaced with real header files to avoid
issues on Windows.

Fixes: nodejs/node#6458
PR-URL: nodejs/node#6553
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
@jBarz
deps: update openssl asm files
0cd286d 
Regenerate asm files with Makefile without CC and ASM envs.

Fixes: nodejs/node#6458
PR-URL: nodejs/node#6553
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
@rvagg @jBarz
2016-05-06 Version 0.12.14 (Maintenance) Release
b845197 
Notable changes:

* npm: Correct erroneous version number in v2.15.1 code
  (Forrest L Norvell) nodejs/node#5988
* openssl: Upgrade to v1.0.1t, addressing security vulnerabilities
  (Shigeki Ohtsu) nodejs/node#6553
  - Fixes CVE-2016-2107 "Padding oracle in AES-NI CBC MAC check"
  - Fixes CVE-2016-2105 "EVP_EncodeUpdate overflow"
  - See https://nodejs.org/en/blog/vulnerability/openssl-may-2016/
    for full details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
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.
6 participants
@shigeki @bnoordhuis @rvagg @elicwhite @mscdex @indutny