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

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 •
edited
Loading

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

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

@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
deps: upgrade openssl sources to 1.0.2h
72f9952 
This replaces all sources of openssl-1.0.2h.tar.gz into
deps/openssl/openssl

PR-URL: nodejs#6551
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
deps: copy all openssl header files to include dir
2dfeb01 
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 <ohtsu@iij.ad.jp>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@indutny
deps: fix openssl assembly error on ia32 win32
dc8dc97 
`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>
deps: fix asm build error of openssl in x86_win32
ff3045e 
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>
openssl: fix keypress requirement in apps on win32
baedfba 
Reapply b910613 .

Fixes: nodejs#589
PR-URL: nodejs#1389
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
deps: add -no_rand_screen to openssl s_client
c450f4a 
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>
deps: update openssl asm and asm_obsolete files
1efd96c 
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 <ohtsu@iij.ad.jp>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@MylesBorins MylesBorins force-pushed the openssl-upgrade-v4 branch from ce23de4 to 1efd96c Compare May 4, 2016 17:09
@MylesBorins MylesBorins merged commit 1efd96c into nodejs:v4.x May 4, 2016
@MylesBorins MylesBorins mentioned this pull request May 4, 2016
Merged
MylesBorins pushed a commit that referenced this pull request May 5, 2016
2016-05-05, Version 4.4.4 'Argon' (LTS)
9cf628e 
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
2016-05-05, Version 4.4.4 'Argon' (LTS)
7c7e50f 
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 <jasnell@gmail.com>
@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:password@aws-postgres.XXX.us-west-2.rds.amazonaws.com: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 notifications@github.com
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 notifications@github.com
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)

@eyalsach eyalsach mentioned this pull request May 16, 2016
Closed
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
crypto Issues and PRs related to the crypto subsystem. 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
@MylesBorins @shigeki @bnoordhuis @eyalsach @indutny @nodejs-github-bot