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.2m #16691

Closed
wants to merge 7 commits into
base: master
from

Conversation

@shigeki
Contributor

shigeki commented Nov 2, 2017

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.

@shigeki shigeki added the openssl label Nov 2, 2017

@shigeki

This comment has been minimized.

Show comment
Hide comment
@shigeki
Contributor

shigeki commented Nov 2, 2017

@bnoordhuis

LGTM (mostly rubber-stamp.)

@tniessen

Rubberstamp LGTM

@mhdawson

Rubber stamp LGTM

@rvagg

This comment has been minimized.

Show comment
Hide comment
@rvagg

rvagg Nov 2, 2017

Member

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

Member

rvagg commented Nov 2, 2017

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

@MylesBorins

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.

@rvagg

This comment has been minimized.

Show comment
Hide comment
@rvagg

rvagg Nov 2, 2017

Member

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

Member

rvagg commented Nov 2, 2017

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

@cjihrig

cjihrig approved these changes Nov 2, 2017

@MylesBorins

This comment has been minimized.

Show comment
Hide comment
@MylesBorins

MylesBorins Nov 3, 2017

Member

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

  • parallel/test-async-wrap-uncaughtexception
  • #16694

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

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
Member

MylesBorins commented Nov 3, 2017

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

  • parallel/test-async-wrap-uncaughtexception
  • #16694

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

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

shigeki and others added some commits Nov 2, 2017

deps: upgrade openssl sources to 1.0.2m
This replaces all sources of openssl-1.0.2m.tar.gz into
deps/openssl/openssl
deps: fix openssl assembly error on ia32 win32
`x86masm.pl` was mistakenly using .486 instruction set, why `cpuid` (and
perhaps others) are requiring .686 .

Fixes: #589
PR-URL: #1389
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
deps: fix asm build error of openssl in x86_win32
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: #589
PR-URL: #1389
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
openssl: fix keypress requirement in apps on win32
Reapply b910613 .

Fixes: #589
PR-URL: #1389
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
deps: add -no_rand_screen to openssl s_client
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: #1461
PR-URL: #1836
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
deps: copy all openssl header files to include dir
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.
deps: update openssl asm and asm_obsolete files
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

This comment has been minimized.

Show comment
Hide comment
@shigeki

shigeki Nov 3, 2017

Contributor

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

Contributor

shigeki commented Nov 3, 2017

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

@shigeki

This comment has been minimized.

Show comment
Hide comment
@shigeki
Contributor

shigeki commented Nov 3, 2017

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

@rvagg rvagg self-requested a review Nov 3, 2017

@rvagg

rvagg approved these changes Nov 3, 2017

@MylesBorins

This comment has been minimized.

Show comment
Hide comment
@MylesBorins

MylesBorins Nov 3, 2017

Member

One more CI set to rebase against master

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

Member

MylesBorins commented Nov 3, 2017

One more CI set to rebase against master

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

@MylesBorins

This comment has been minimized.

Show comment
Hide comment
@MylesBorins

MylesBorins Nov 3, 2017

Member

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

Member

MylesBorins commented Nov 3, 2017

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

@jasnell

jasnell approved these changes Nov 3, 2017

@MylesBorins

This comment has been minimized.

Show comment
Hide comment
@MylesBorins

MylesBorins Nov 3, 2017

Member

Landed on all relevant staging branches

Member

MylesBorins commented Nov 3, 2017

Landed on all relevant staging branches

This was referenced Nov 3, 2017

@gibfahn gibfahn referenced this pull request Nov 5, 2017

Merged

v8.9.1 proposal #16783

@rvagg

This comment has been minimized.

Show comment
Hide comment
@rvagg

rvagg Nov 6, 2017

Member

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

Member

rvagg commented Nov 6, 2017

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

MylesBorins added a commit that referenced this pull request Nov 6, 2017

2017-11-07, Version 6.12.0 'Boron' (LTS)
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

MylesBorins added a commit that referenced this pull request Nov 6, 2017

2017-11-07, Version 4.8.6 'Argon' (Maintenance)
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

MylesBorins added a commit that referenced this pull request Nov 6, 2017

2017-11-07, Version 4.8.6 'Argon' (Maintenance)
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

@cjihrig cjihrig referenced this pull request Nov 6, 2017

Merged

v9.1.0 proposal #16851

cjihrig added a commit to cjihrig/node-1 that referenced this pull request Nov 7, 2017

2017-11-07, Version 9.1.0 (Current)
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

cjihrig added a commit to cjihrig/node-1 that referenced this pull request Nov 7, 2017

2017-11-07, Version 9.1.0 (Current)
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

MylesBorins added a commit that referenced this pull request Nov 7, 2017

2017-11-07, Version 4.8.6 'Argon' (Maintenance)
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

MylesBorins added a commit that referenced this pull request Nov 7, 2017

2017-11-07, Version 6.12.0 'Boron' (LTS)
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

cjihrig added a commit that referenced this pull request Nov 7, 2017

2017-11-07, Version 9.1.0 (Current)
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

gibfahn added a commit that referenced this pull request Nov 7, 2017

2017-11-07 Version 8.9.1 'Carbon' (LTS)
- **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

gibfahn added a commit that referenced this pull request Nov 7, 2017

2017-11-07 Version 8.9.1 'Carbon' (LTS)
- **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

gibfahn added a commit that referenced this pull request Nov 7, 2017

2017-11-07, Version 8.9.1 'Carbon' (LTS)
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

@abernix abernix referenced this pull request Nov 7, 2017

Merged

Release 1.5.4 #9320

gibfahn added a commit that referenced this pull request Nov 7, 2017

2017-11-07, Version 8.9.1 'Carbon' (LTS)
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

@adisbladis adisbladis referenced this pull request Nov 10, 2017

Closed

Nodejs: Minor updates #31494

4 of 8 tasks complete

msoechting added a commit to hpicgs/node that referenced this pull request Feb 7, 2018

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

PR-URL: nodejs#16691
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

msoechting added a commit to hpicgs/node that referenced this pull request Feb 7, 2018

deps: copy all openssl header files to include dir
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.

PR-URL: nodejs#16691
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

msoechting added a commit to hpicgs/node that referenced this pull request Feb 7, 2018

deps: update openssl asm and asm_obsolete files
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.

PR-URL: nodejs#16691
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

msoechting added a commit to hpicgs/node that referenced this pull request Feb 7, 2018

2017-11-07, Version 4.8.6 'Argon' (Maintenance)
Notable Changes:

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

PR-URL: nodejs#16500

msoechting added a commit to hpicgs/node that referenced this pull request Feb 7, 2018

2017-11-07, Version 6.12.0 'Boron' (LTS)
Notable Changes:

* assert:
  - assert.fail() can now take one or two arguments (Rich Trott)
    nodejs#12293
* crypto:
  - add sign/verify support for RSASSA-PSS (Tobias Nießen)
    nodejs#11705
* deps:
  - upgrade openssl sources to 1.0.2m (Shigeki Ohtsu)
    nodejs#16691
  - upgrade libuv to 1.15.0 (cjihrig)
    nodejs#15745
  - upgrade libuv to 1.14.1 (cjihrig)
    nodejs#14866
  - upgrade libuv to 1.13.1 (cjihrig)
    nodejs#14117
  - upgrade libuv to 1.12.0 (cjihrig)
    nodejs#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) nodejs#7856
* inspector:
  - enable --inspect-brk (Refael Ackermann)
    nodejs#12615
* process:
  - add --redirect-warnings command line argument (James M Snell)
    nodejs#10116
* src:
  - allow CLI args in env with NODE_OPTIONS (Sam Roberts)
    nodejs#12028)
  - --abort-on-uncaught-exception in NODE_OPTIONS (Sam Roberts)
    nodejs#13932
  - allow --tls-cipher-list in NODE_OPTIONS (Sam Roberts)
    nodejs#13172
  - use SafeGetenv() for NODE_REDIRECT_WARNINGS (Sam Roberts)
    nodejs#12677
* test:
  - remove common.fail() (Rich Trott)
    nodejs#12293

PR-URL: nodejs#16263

msoechting added a commit to hpicgs/node that referenced this pull request Feb 7, 2018

2017-11-07, Version 9.1.0 (Current)
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

msoechting added a commit to hpicgs/node that referenced this pull request Feb 7, 2018

2017-11-07, Version 8.9.1 'Carbon' (LTS)
Notable Changes:

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

PR-URL: nodejs#16783
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment