Upgrade to OpenSSL-1.0.2m by shigeki · Pull Request #16691 · nodejs/node · GitHub

This upgrades to OpenSSL-1.0.2m . It includes the fix of the moderate severity of CVE-2017-3736 that affects Node in RSA calculations of TLS and crypto modules but the attack is said to be very difficult.

This upgrades have no changes in opensslconf.h in the config dir.

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

Affected core subsystem(s)

deps/openssl

R @nodejs/crypto or others.

CI is running on https://ci.nodejs.org/job/node-test-pull-request/11155/.

LGTM (mostly rubber-stamp.)

Rubberstamp LGTM

Rubber stamp LGTM

needing impact assessment of CVE-2017-3736 so we can decide on whether to rush releases out nodejs/Release#271, @nodejs/crypto please weigh in

LGTM

CI is good... fails are all flakes or infra

My impression of the assessment was that we don't need to rush a release tomorrow but can bundle this with other fixes on tuesday.

running CI again @ rebuilding @ https://ci.nodejs.org/job/node-test-commit/13690/, some odd errors in there, and the flaky test-http2-server-rst-stream should be fixed by 3a977fc now

Ref: nodejs/Release#271 Ref: nodejs-private/security#155 Ref: nodejs/node#16691

linux failures are a known async flake (there is an open Pr to set it to flaky)

parallel/test-async-wrap-uncaughtexception
test: mark as flaky #16694

arm-fanned failures appear infra related, opened [build issue]

weird failures on arm build#969

windows failure is both the same async failure as above and another failure also marked flaky that appears unrelated

sequential/test-inspector-bindings # TODO : Fix flaky test

This replaces all sources of openssl-1.0.2m.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>

Regenerate asm files with Makefile and CC=gcc and ASM=nasm where gcc version was 5.4.0 and nasm version was 2.11.08. Also asm files in asm_obsolete dir to support old compiler and assembler are regenerated without CC and ASM envs.

I've just rebased this with the current master that has a fix of the flaky test.

A new CI is running on https://ci.nodejs.org/job/node-test-pull-request/11160/.

Ref: nodejs/Release#271 Ref: nodejs-private/security#155 Ref: nodejs/node#16691

One more CI set to rebase against master

https://ci.nodejs.org/job/node-test-pull-request/11164/

CI is green aside from windows failures fixed by 3d4d5e0

If there are no complaints I'll land this on master in just over an hour

Landed on all relevant staging branches

Great job @shigeki! Thanks for jumping on this so promptly and giving the thorough impact assessment.

Notable Changes: * assert: - assert.fail() can now take one or two arguments (Rich Trott) #12293 * crypto: - add sign/verify support for RSASSA-PSS (Tobias Nießen) #11705 * deps: - upgrade openssl sources to 1.0.2m (Shigeki Ohtsu) #16691 - upgrade libuv to 1.15.0 (cjihrig) #15745 - upgrade libuv to 1.14.1 (cjihrig) #14866 - upgrade libuv to 1.13.1 (cjihrig) #14117 - upgrade libuv to 1.12.0 (cjihrig) #13306 * fs: - Add support for fs.write/fs.writeSync(fd, buffer, cb) and fs.write/fs.writeSync(fd, buffer, offset, cb) as documented (Andreas Lind) #7856 * inspector: - enable --inspect-brk (Refael Ackermann) #12615 * process: - add --redirect-warnings command line argument (James M Snell) #10116 * src: - allow CLI args in env with NODE_OPTIONS (Sam Roberts) #12028) - --abort-on-uncaught-exception in NODE_OPTIONS (Sam Roberts) #13932 - allow --tls-cipher-list in NODE_OPTIONS (Sam Roberts) #13172 - use SafeGetenv() for NODE_REDIRECT_WARNINGS (Sam Roberts) #12677 * test: - remove common.fail() (Rich Trott) #12293 PR-URL: #16263

Notable Changes: * **crypto**: - update root certificates (Ben Noordhuis) #13279 - update root certificates (Ben Noordhuis) #12402 * **deps**: - add support for more modern versions of INTL (Bruno Pagani) #13040 - upgrade openssl sources to 1.0.2m (Shigeki Ohtsu) #16691 - upgrade openssl sources to 1.0.2l (Daniel Bevenius) #13233 PR-URL: #16500

Notable changes: * CLI: - NODE_OPTIONS now supports the --stack-trace-limit option. nodejs#16495 * deps: - OpenSSL is upgraded to 1.0.2m nodejs#16691 * http: - A 'connect' event handler leak has been fixed. nodejs#16725 - The 103 Early Hints status code is now supported. nodejs#16644 PR-URL: nodejs#16851

Notable Changes: * **crypto**: - update root certificates (Ben Noordhuis) #13279 - update root certificates (Ben Noordhuis) #12402 * **deps**: - add support for more modern versions of INTL (Bruno Pagani) #13040 - upgrade openssl sources to 1.0.2m (Shigeki Ohtsu) #16691 - upgrade openssl sources to 1.0.2l (Daniel Bevenius) #13233 PR-URL: #16500

Notable Changes: * assert: - assert.fail() can now take one or two arguments (Rich Trott) #12293 * crypto: - add sign/verify support for RSASSA-PSS (Tobias Nießen) #11705 * deps: - upgrade openssl sources to 1.0.2m (Shigeki Ohtsu) #16691 - upgrade libuv to 1.15.0 (cjihrig) #15745 - upgrade libuv to 1.14.1 (cjihrig) #14866 - upgrade libuv to 1.13.1 (cjihrig) #14117 - upgrade libuv to 1.12.0 (cjihrig) #13306 * fs: - Add support for fs.write/fs.writeSync(fd, buffer, cb) and fs.write/fs.writeSync(fd, buffer, offset, cb) as documented (Andreas Lind) #7856 * inspector: - enable --inspect-brk (Refael Ackermann) #12615 * process: - add --redirect-warnings command line argument (James M Snell) #10116 * src: - allow CLI args in env with NODE_OPTIONS (Sam Roberts) #12028) - --abort-on-uncaught-exception in NODE_OPTIONS (Sam Roberts) #13932 - allow --tls-cipher-list in NODE_OPTIONS (Sam Roberts) #13172 - use SafeGetenv() for NODE_REDIRECT_WARNINGS (Sam Roberts) #12677 * test: - remove common.fail() (Rich Trott) #12293 PR-URL: #16263

Notable changes: * CLI: - NODE_OPTIONS now supports the --stack-trace-limit option. #16495 * deps: - OpenSSL is upgraded to 1.0.2m #16691 * http: - A 'connect' event handler leak has been fixed. #16725 - The 103 Early Hints status code is now supported. #16644 PR-URL: #16851

Notable Changes: - **openssl**: - upgrade openssl sources to 1.0.2m (Shigeki Ohtsu) [#16691](#16691) - ***Revert*** "**https**: - refactor to use http internals" (Myles Borins) [#16660](#16660) PR-URL: #16783

shigeki added the openssl Issues and PRs related to the OpenSSL dependency. label Nov 2, 2017

bnoordhuis approved these changes Nov 2, 2017

View reviewed changes

richardlau mentioned this pull request Nov 2, 2017

OpenSSL 1.0.2m on 2nd November 2017 nodejs/Release#271

Closed

tniessen approved these changes Nov 2, 2017

View reviewed changes

mhdawson approved these changes Nov 2, 2017

View reviewed changes

MylesBorins approved these changes Nov 2, 2017

View reviewed changes

cjihrig approved these changes Nov 2, 2017

View reviewed changes

rvagg added a commit to nodejs/nodejs.org that referenced this pull request Nov 2, 2017

Blog: update to OpenSSL 1.0.2m announcement

0f1cd94

Ref: nodejs/Release#271 Ref: nodejs-private/security#155 Ref: nodejs/node#16691

rvagg mentioned this pull request Nov 2, 2017

Blog: update to OpenSSL 1.0.2m announcement nodejs/nodejs.org#1443

Merged

MylesBorins mentioned this pull request Nov 3, 2017

weird failures on arm nodejs/build#969

Closed

shigeki and others added 7 commits November 3, 2017 09:18

deps: upgrade openssl sources to 1.0.2m

ff1b8de

This replaces all sources of openssl-1.0.2m.tar.gz into deps/openssl/openssl

deps: copy all openssl header files to include dir

7f94976

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.

openssl: fix keypress requirement in apps on win32

a476006

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

deps: update openssl asm and asm_obsolete files

7506453

Regenerate asm files with Makefile and CC=gcc and ASM=nasm where gcc version was 5.4.0 and nasm version was 2.11.08. Also asm files in asm_obsolete dir to support old compiler and assembler are regenerated without CC and ASM envs.

shigeki force-pushed the upgrade_openssl102m branch from 6fa53c6 to 7506453 Compare November 3, 2017 00:19

rvagg added a commit to nodejs/nodejs.org that referenced this pull request Nov 3, 2017

Blog: update to OpenSSL 1.0.2m announcement (#1443 )

c92475b

Ref: nodejs/Release#271 Ref: nodejs-private/security#155 Ref: nodejs/node#16691

rvagg self-requested a review November 3, 2017 03:34

rvagg approved these changes Nov 3, 2017

View reviewed changes

gireeshpunathil approved these changes Nov 3, 2017

View reviewed changes

MylesBorins added land-on-v4.x labels Nov 3, 2017

This was referenced Nov 3, 2017

v4.8.6 proposal #16500

Merged

v6.12.0 proposal #16263

Merged

gibfahn mentioned this pull request Nov 5, 2017

v8.9.1 proposal #16783

Merged

cjihrig mentioned this pull request Nov 6, 2017

v9.1.0 proposal #16851

Merged

abernix mentioned this pull request Nov 7, 2017

Release 1.5.4 meteor/meteor#9320

Merged

adisbladis mentioned this pull request Nov 10, 2017

Nodejs: Minor updates NixOS/nixpkgs#31494

Closed

8 tasks

Upgrade to OpenSSL-1.0.2m by shigeki · Pull Request #16691 · nodejs/node · GitHub

Uh oh!

Uh oh!

Upgrade to OpenSSL-1.0.2m - #16691

Upgrade to OpenSSL-1.0.2m#16691shigeki wants to merge 7 commits intonodejs:masternodejs/node:masterfrom shigeki:upgrade_openssl102mshigeki/node:upgrade_openssl102mCopy head branch name to clipboard

Conversation

shigeki commented Nov 2, 2017

Checklist

Affected core subsystem(s)

Uh oh!

shigeki commented Nov 2, 2017

Uh oh!

bnoordhuis left a comment

Choose a reason for hiding this comment

Uh oh!

tniessen left a comment

Choose a reason for hiding this comment

Uh oh!

mhdawson left a comment

Choose a reason for hiding this comment

Uh oh!

rvagg commented Nov 2, 2017

Uh oh!

MylesBorins left a comment

Choose a reason for hiding this comment

Uh oh!

rvagg commented Nov 2, 2017

Uh oh!

MylesBorins commented Nov 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shigeki commented Nov 3, 2017

Uh oh!

shigeki commented Nov 3, 2017

Uh oh!

MylesBorins commented Nov 3, 2017

Uh oh!

MylesBorins commented Nov 3, 2017

Uh oh!

MylesBorins commented Nov 3, 2017

Uh oh!

rvagg commented Nov 6, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

Upgrade to OpenSSL-1.0.2m#16691
shigeki wants to merge 7 commits into
nodejs:masterfrom
shigeki:upgrade_openssl102m

MylesBorins commented Nov 3, 2017 •
edited
Loading