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

Openssl upgrade v4 #6551

Merged
merged 7 commits into from
May 4, 2016
Merged

Openssl upgrade v4 #6551

merged 7 commits into from
May 4, 2016

Conversation

MylesBorins
Copy link
Contributor

@MylesBorins MylesBorins commented May 3, 2016

Affected core subsystem(s)

tls/crypto

Description of change

openssl sources are upgraded to 1.0.2h and applied floating patches.
One 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.

Fix: #6458
R: @indutny or @bnoordhuis or @shigeki

@nodejs-github-bot nodejs-github-bot added the openssl Issues and PRs related to the OpenSSL dependency. label May 3, 2016
@MylesBorins MylesBorins added v4.x crypto Issues and PRs related to the crypto subsystem. labels May 3, 2016
@MylesBorins
Copy link
Contributor Author

/cc @jasnell

@MylesBorins
Copy link
Contributor Author

@MylesBorins
Copy link
Contributor Author

Ci is green 🎉🎉🎉

@shigeki
Copy link
Contributor

shigeki commented May 4, 2016

LGTM. Thanks.

@bnoordhuis
Copy link
Member

LGTM although strictly speaking this PR is targeting the wrong branch (v4.x instead of v4.x-staging) and it brings back a couple of files that were removed in 1d0e4a9.

@MylesBorins
Copy link
Contributor Author

MylesBorins commented May 4, 2016 via email

Shigeki Ohtsu and others added 7 commits May 4, 2016 10:08
This replaces all sources of openssl-1.0.2h.tar.gz into
deps/openssl/openssl

PR-URL: nodejs#6551
Reviewed-By: Shigeki Ohtsu <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
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#6551
Reviewed-By: Shigeki Ohtsu <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
`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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reapply b910613 .

Fixes: nodejs#589
PR-URL: nodejs#1389
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
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 <[email protected]>
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.

PR-URL: nodejs#6551
Reviewed-By: Shigeki Ohtsu <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@MylesBorins MylesBorins merged commit 1efd96c into nodejs:v4.x May 4, 2016
@MylesBorins MylesBorins mentioned this pull request May 4, 2016
MylesBorins pushed a commit that referenced this pull request May 5, 2016
Notable changes

* deps:
  * update openssl to 1.0.2h. (Shigeki Ohtsu)
    [#6551](#6551)
		- Please see our blog postfor more info on the security
			contents of this release.
			https://nodejs.org/en/blog/vulnerability/openssl-may-2016/
MylesBorins pushed a commit that referenced this pull request May 6, 2016
Notable changes

* deps:
  * update openssl to 1.0.2h. (Shigeki Ohtsu)
    [#6551](#6551)
    - Please see our blog postfor more info on the security
      contents of this release.
      https://nodejs.org/en/blog/vulnerability/openssl-may-2016/

PR-URL: #6583
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins deleted the openssl-upgrade-v4 branch May 12, 2016 18:14
@eyalsach
Copy link

for some reason after this fix I can no longer connect to a postgres hosted on aws.
e.g. using code
var pg = require('pg');
pg.connect(connectionString, function(err, client, done) {

where connectionString can be something like:
postgres://master:[email protected]:5432/db_demo

I get: { error: password authentication failed for user "master"...

(same issue with ssl=true)

is this a bug or a change that I need to handle differently?

@indutny
Copy link
Member

indutny commented May 16, 2016

@eyalsach does the same code on a previous node.js version still work?

@bnoordhuis
Copy link
Member

See #6782 (comment), this should be reported to node-pg first, unless it's been bisected to exactly this commit (and even then it's still possible node-pg needs to tweak something, so reporting it there is a good first step.)

@eyalsach
Copy link

Sure, with v5.10.1 it works fine

On Mon, May 16, 2016 at 8:41 AM, Fedor Indutny [email protected]
wrote:

@eyalsach https://github.com/eyalsach does the same code on a previous
node.js version still work?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#6551 (comment)

@eyalsach
Copy link

Will do, thanks

On Mon, May 16, 2016 at 8:43 AM, Ben Noordhuis [email protected]
wrote:

See #6782 (comment)
#6782 (comment), this
should be reported to node-pg first, unless it's been bisected to exactly
this commit (and even then it's still possible node-pg needs to tweak
something, so reporting it there is a good first step.)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#6551 (comment)

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. openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants