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 102h for master #6550

Closed
wants to merge 8 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.2h and applied floating patches.
Two more works was made in this upgrade.

  • asm regenerated

asm codes were changed in this upgrade so that asm and asm_obsolete were regenerated. openssl headers were unchanged so that config/ are not regenerated.

  • ALPN test fixes

openssl/openssl@af2db04 changed some ALPN behaviors. The tests when ALPN has no selection
should be fixed because openssl was changed NPN callback to be invoked in this case.

Fix: #6458
R: @indutny or @bnoordhuis

Shigeki Ohtsu and others added 8 commits May 3, 2016 23:48
This replaces all sources of openssl-1.0.2h.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=gcc where
gcc-4.8.4. Also asm files in asm_obsolete dir to support old compiler
and assembler are regenerated without CC and ASM envs.
openssl/openssl@af2db04
changed some ALPN behaviors. The tests when ALPN has no selection
should be fixed because openssl was changed NPN callback to be invoked
in this case.
@nodejs-github-bot nodejs-github-bot added the openssl Issues and PRs related to the OpenSSL dependency. label May 3, 2016
@shigeki
Copy link
Contributor Author

shigeki commented May 3, 2016

@MylesBorins
Copy link
Contributor

@shigeki should this backport cleanly to v4 - > v6 or will it require a custom backport?

@shigeki
Copy link
Contributor Author

shigeki commented May 3, 2016

@thealphanerd v4 does not support ALPN so that 3f4e596 cannot be backported. I think others would be fine.

@MylesBorins
Copy link
Contributor

I'll make a PR to backport this to v5 and v4

@MylesBorins
Copy link
Contributor

LGMT btw

@indutny
Copy link
Member

indutny commented May 3, 2016

LGTM, verified sources.

@indutny
Copy link
Member

indutny commented May 3, 2016

cc @bnoordhuis ;)

@bnoordhuis
Copy link
Member

LGTM

Aside: I think I've brought this up before but the way we do upgrades now (basically: nuke deps/openssl, unpack tarball, reapply patches) makes the history very noisy. Is there a reason we don't simply apply the diff between 1.0.2g and 1.0.2h onto our tree?

@shigeki
Copy link
Contributor Author

shigeki commented May 4, 2016

@bnoordhuis I agree that the history gets noisy but I made upgrade procedure for two reasons.

  • it shows clear process to upgrade from new sources. Other people can understand what is applied to deps/openssl from recent git log and find what especially floating patches are needed.
  • openssl build system sometimes are changed after upgrade with adding/removing files and generating some files dynamically with perl scripts. I think we can have more robust procedure of upgrading from official sources every time.

As you know, upgrading openssl is somehow tricky work. I would like make it to be more easier for others who are not familiar with openssl can just follow instructed steps.

shigeki pushed a commit that referenced this pull request May 4, 2016
This replaces all sources of openssl-1.0.2h.tar.gz into
deps/openssl/openssl

Fixes: #6458
PR-URL: #6550
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
shigeki pushed a commit that referenced this pull request May 4, 2016
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.

Fixes: #6458
PR-URL: #6550
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
shigeki pushed a commit that referenced this pull request May 4, 2016
Regenerate asm files with Makefile and CC=gcc and ASM=gcc where
gcc-4.8.4. Also asm files in asm_obsolete dir to support old compiler
and assembler are regenerated without CC and ASM envs.

Fixes: #6458
PR-URL: #6550
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
shigeki pushed a commit that referenced this pull request May 4, 2016
openssl/openssl@af2db04
changed some ALPN behaviors. The tests when ALPN has no selection
should be fixed because openssl was changed NPN callback to be invoked
in this case.

Fixes: #6458
PR-URL: #6550
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@shigeki
Copy link
Contributor Author

shigeki commented May 4, 2016

Thanks everyone. Landed in 59c8e46, 63f090f, 70acd47, 4e4a4e1, f136f72, e673a93, 70ae031 and be480b1.

@shigeki shigeki closed this May 4, 2016
@Fishrock123
Copy link
Contributor

Awesome. Will pull into the v6 proposal momentarily.

Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 4, 2016
This replaces all sources of openssl-1.0.2h.tar.gz into
deps/openssl/openssl

Fixes: nodejs#6458
PR-URL: nodejs#6550
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 4, 2016
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.

Fixes: nodejs#6458
PR-URL: nodejs#6550
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 4, 2016
Regenerate asm files with Makefile and CC=gcc and ASM=gcc where
gcc-4.8.4. Also asm files in asm_obsolete dir to support old compiler
and assembler are regenerated without CC and ASM envs.

Fixes: nodejs#6458
PR-URL: nodejs#6550
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 4, 2016
openssl/openssl@af2db04
changed some ALPN behaviors. The tests when ALPN has no selection
should be fixed because openssl was changed NPN callback to be invoked
in this case.

Fixes: nodejs#6458
PR-URL: nodejs#6550
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Fishrock123 added a commit that referenced this pull request May 5, 2016
* assert: `deep{Strict}Equal()` now works correctly with circular
references. (Rich Trott) #6432
* debugger: Arrays are now formatted correctly in the debugger repl.
(cjihrig) #6448
* deps: Upgrade OpenSSL sources to 1.0.2h (Shigeki Ohtsu)
#6550
* net: Introduced a `Socket#connecting` property. (Fedor Indutny)
#6404
  - Previously this information was only available as the undocumented,
internal `_connecting` property.
* process: Introduced `process.cpuUsage()`. (Patrick Mueller)
#6157
* stream: `Writable#setDefaultEncoding()` now returns `this`.
(Alexander Makarenko) #5040
* util: Two new additions to `util.inspect()`:
  - Added a `maxArrayLength` option to truncate the formatting of
Arrays. (James M Snell) #6334
    - This is set to `100` by default.
  - Added a `showProxy` option for formatting proxy intercepting
handlers. (James M Snell) #6465
    - Inspecting proxies is non-trivial and as such this is off by
default.

PR-URL: #6557
Fishrock123 added a commit that referenced this pull request May 5, 2016
* assert: `deep{Strict}Equal()` now works correctly with circular
references. (Rich Trott) #6432
* debugger: Arrays are now formatted correctly in the debugger repl.
(cjihrig) #6448
* deps: Upgrade OpenSSL sources to 1.0.2h (Shigeki Ohtsu)
#6550
  - Please see our blog post for more info on the security contents of this release:
  - https://nodejs.org/en/blog/vulnerability/openssl-may-2016/
* net: Introduced a `Socket#connecting` property. (Fedor Indutny)
#6404
  - Previously this information was only available as the undocumented,
internal `_connecting` property.
* process: Introduced `process.cpuUsage()`. (Patrick Mueller)
#6157
* stream: `Writable#setDefaultEncoding()` now returns `this`.
(Alexander Makarenko) #5040
* util: Two new additions to `util.inspect()`:
  - Added a `maxArrayLength` option to truncate the formatting of
Arrays. (James M Snell) #6334
    - This is set to `100` by default.
  - Added a `showProxy` option for formatting proxy intercepting
handlers. (James M Snell) #6465
    - Inspecting proxies is non-trivial and as such this is off by
default.

PR-URL: #6557
Fishrock123 added a commit that referenced this pull request May 5, 2016
* assert: `deep{Strict}Equal()` now works correctly with circular
references. (Rich Trott) #6432
* debugger: Arrays are now formatted correctly in the debugger repl.
(cjihrig) #6448
* deps: Upgrade OpenSSL sources to 1.0.2h (Shigeki Ohtsu)
#6550
- Please see our blog post for more info on the security contents of
this release:
- https://nodejs.org/en/blog/vulnerability/openssl-may-2016/
* net: Introduced a `Socket#connecting` property. (Fedor Indutny)
#6404
- Previously this information was only available as the undocumented,
internal `_connecting` property.
* process: Introduced `process.cpuUsage()`. (Patrick Mueller)
#6157
* stream: `Writable#setDefaultEncoding()` now returns `this`.
(Alexander Makarenko) #5040
* util: Two new additions to `util.inspect()`:
- Added a `maxArrayLength` option to truncate the formatting of
Arrays. (James M Snell) #6334
- This is set to `100` by default.
- Added a `showProxy` option for formatting proxy intercepting
handlers. (James M Snell) #6465
- Inspecting proxies is non-trivial and as such this is off by
default.

PR-URL: #6557
@dan-mcdonald
Copy link

@shigeki This change appears to have added two files:

  • deps/openssl/openssl/Makefile
  • deps/openssl/openssl/Makefile.bak

Was that intentional? I think these files are automatically generated.

MylesBorins pushed a commit that referenced this pull request Jan 19, 2017
cherry-pick 65030c7 from v6-staging.

openssl/openssl@af2db04
changed some ALPN behaviors. The tests when ALPN has no selection
should be fixed because openssl was changed NPN callback to be invoked
in this case.

Fixes: #6458
PR-URL: #6550
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
cherry-pick 65030c7 from v6-staging.

openssl/openssl@af2db04
changed some ALPN behaviors. The tests when ALPN has no selection
should be fixed because openssl was changed NPN callback to be invoked
in this case.

Fixes: #6458
PR-URL: #6550
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2017
MylesBorins pushed a commit that referenced this pull request Feb 1, 2017
cherry-pick 65030c7 from v6-staging.

openssl/openssl@af2db04
changed some ALPN behaviors. The tests when ALPN has no selection
should be fixed because openssl was changed NPN callback to be invoked
in this case.

Fixes: #6458
PR-URL: #6550
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenSSL upgrades: May 3rd
7 participants