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

deps: upgrade OpenSSL to version 1.0.2l #13233

Closed
wants to merge 9 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented May 26, 2017

This pull request contains commits to upgrade OpenSSL to version 1.0.2.l

This includes commits following the instructions in Upgrading OpenSSL, and also reverts f439065 as mentioned in #13161.

Fixes: #13161

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

deps, crypto

@nodejs-github-bot nodejs-github-bot added the openssl Issues and PRs related to the OpenSSL dependency. label May 26, 2017
@@ -235,7 +235,7 @@ extern "C" {
even newer MIPS CPU's, but at the moment one size fits all for
optimization options. Older Sparc's work better with only UNROLL, but
there's no way to tell at compile time what it is you're running on */

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these changes necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not sure and thought it would be better to be safe than sorry and get feedback on the PR first. Let me know and I'll remove that commit. Thanks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OpenSSL Configure generates this whitespace but it is always removed with git am --whitespace=fix. So we can left it as it will be removed.

@mscdex
Copy link
Contributor

mscdex commented May 26, 2017

/cc @nodejs/crypto

@danbev
Copy link
Contributor Author

danbev commented May 26, 2017

@danbev
Copy link
Contributor Author

danbev commented May 26, 2017

test/osx failure looks unrelated

console output:

not ok 457 parallel/test-http-1.0-keep-alive
  ---
  duration_ms: 6.478
  severity: crashed
  stack: |-
    oh no!
    exit code: CRASHED (Signal: 9)
  ...
not ok 458 parallel/test-http-abort-before-end
  ---
  duration_ms: 6.475
  severity: crashed
  stack: |-
    oh no!
    exit code: CRASHED (Signal: 9)
  ...

@shigeki
Copy link
Contributor

shigeki commented May 26, 2017

It seems asm files are not updated. Did you run https://github.com/nodejs/node/blob/master/deps/openssl/doc/UPGRADING.md#6-asm-files-for-openssl?

@danbev
Copy link
Contributor Author

danbev commented May 26, 2017

It seems asm files are not updated. Did you run

I actually did not run them, will do that now. For some reason I though there was no need to have them updated but looking closer I realise there are changes to them. About the versions of gcc and nasm, do these have to be exact matches or are later version also alright?

@danbev
Copy link
Contributor Author

danbev commented May 26, 2017

Regenerate asm files with Makefile and CC=gcc and ASM=gcc where gcc-4.8.4.

@shigeki Should the commit message be ASM=nasm here instead or is the above correct?

@danbev
Copy link
Contributor Author

danbev commented May 27, 2017

@shigeki
Copy link
Contributor

shigeki commented May 29, 2017

Regenerate asm files with Makefile and CC=gcc and ASM=gcc where gcc-4.8.4.
@shigeki Should the commit message be ASM=nasm here instead or is the above correct?

@danbev ASM=nasm is the right one. I mistook the word in that commit message.
I checked the asm updates of 8561e192173b4df66f941c07a0cc0a4748eea1a9 and found that something wrong in checking nasm version with ASM env. Could you give me an output of nasm -v?

@danbev
Copy link
Contributor Author

danbev commented May 29, 2017

ASM=nasm is the right one. I mistook the word in that commit message.

Great, I used that in the commit message and also added the version of nasm being used. If you think this looks good I can update step 6.3.

Could you give me an output of nasm -v?

$ nasm -v
NASM version 2.11.06 compiled on Nov  4 2014

@shigeki
Copy link
Contributor

shigeki commented May 29, 2017

@danbev Sorry, I should have ask you about your CC environment not ASM. It seems that your commit of updating asm files in 8561e192173b4df66f941c07a0cc0a4748eea1a9 failed to check the avx version of https://github.com/nodejs/node/blob/master/deps/openssl/openssl/crypto/aes/asm/aesni-mb-x86_64.pl#L51-L54. I requires that gas version above 2.22, which is $avx == 2 on Linux to regenerate asm files.

This is my output gas version.

$ $CC -Wa,-v -c -o /dev/null -x assembler /dev/null
GNU assembler version 2.26.1 (x86_64-linux-gnu) using BFD version (GNU Binutils for Ubuntu) 2.26.1

What is yours?

@shigeki
Copy link
Contributor

shigeki commented May 29, 2017

@danbev shigeki@aaa09eb is my asm updates on upgrading 1.0.2l. If it have any difference with yours, there is something wrong between your env and mine.

@danbev
Copy link
Contributor Author

danbev commented May 29, 2017

What is yours?

$ $CC -Wa,-v -c -o /dev/null -x assembler /dev/null
Apple LLVM version 8.0.0 (clang-800.0.42.1)
Target: x86_64-apple-darwin15.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
 "/Library/Developer/CommandLineTools/usr/bin/clang" -cc1as -triple x86_64-apple-macosx10.11.0 -filetype obj -main-file-name null -target-cpu core2 -fdebug-compilation-dir /Users/danielbevenius/work/nodejs/node/deps/openssl -dwarf-debug-producer Apple LLVM version 8.0.0 (clang-800.0.42.1) -dwarf-version=2 -mrelocation-model pic -o /dev/null /dev/null

My $CC version is:

$ $CC --version
gcc-4.8 (Homebrew gcc48 4.8.3) 4.8.3
Copyright (C) 2013 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Sorry if I misunderstood the docs, the example does show using linux but does this have to be done on Linux?

@shigeki
Copy link
Contributor

shigeki commented May 29, 2017

I've got it. The upgrading doc is only for Linux. I tried to run on MacOS but it failed. I did not investigate it further. You can fix it to work it on MacOS or re-update it on Linux.

@danbev
Copy link
Contributor Author

danbev commented May 29, 2017

I've got it. The upgrading doc is only for Linux. I tried to run on MacOS but it failed. I did not investigate it further. You can fix it to work it on MacOS or re-update it on Linux.

I'll take a look and see if I can get this working on MacOS, otherwise I'll re-update it on Linux. Thanks

@danbev
Copy link
Contributor Author

danbev commented May 29, 2017

@shigeki I built this on Linux as I was not able to get it work properly on MacOS. Could you take a look, I'm a little concerned that I have a bunch or deletes in asm_obsolete that were not in your example commit. Did I miss something?

I was wondering about these steps:

ohtsu@ubuntu:~/github/node/deps/openssl/asm$ cd ../asm_obsolete/
ohtsu@ubuntu:~/github/node/deps/openssl/asm_obsolete$ unset CC
ohtsu@ubuntu:~/github/node/deps/openssl/asm_obsolete$ unset ASM
ohtsu@ubuntu:~/github/node/deps/openssl/asm_obsolete$ make clean
find . -iname '*.asm' -exec rm "{}" \;
find . -iname '*.s' -exec rm "{}" \;
find . -iname '*.S' -exec rm "{}" \;
ohtsu@ubuntu:~/github/node/deps/openssl$ git status

We do a make clean but it is not followed by a make, is that correct? I did not do a make as I'm following the doc but wanted to double check that I did not miss a command here. Thanks

@shigeki
Copy link
Contributor

shigeki commented May 30, 2017

@danbev You are right. It was missed in the doc that

ohtsu@ubuntu:~/github/node/deps/openssl/asm_obsolete$ make

after make clean in order to generate asm files in case of no CC and ASM envs.

Could you update asm files in the asm_obsolete dir and also add the commit to fix UPGRADING.md?

@danbev
Copy link
Contributor Author

danbev commented May 30, 2017

Could you update asm files in the asm_obsolete dir and also add the commit to fix UPGRADING.md?

@shigeki I've updated now and also rebased, plus added an additional commit for UPGRADING.md. Let me know what you think, thanks!

@danbev
Copy link
Contributor Author

danbev commented May 30, 2017

Copy link
Contributor

@shigeki shigeki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything is fine. Thanks for your work.
I will land this later to see if anyone puts another approvals.

@danbev
Copy link
Contributor Author

danbev commented May 31, 2017

@shigeki Thanks for your help and your patience on this!

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubber stamp LGTM.

@shigeki
Copy link
Contributor

shigeki commented Jun 1, 2017

@danbev I've just found one issue in this PR.
You added 12f61e94f8c396d1aeda73aaac65d2e10b954416 to reverted the floating patch but the revert fix was applied after replacing openssl sources so that it reverts original openssl source fix.
It is needed to revert the floating patch before replacing openssl sources or just leave it without reverting. I think the latter is simple. Please remove 12f61e94f8c396d1aeda73aaac65d2e10b954416.

danbev added 2 commits June 1, 2017 13:42
This replaces all sources of openssl-1.0.2l.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.
gibfahn pushed a commit that referenced this pull request Jun 20, 2017
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.

Fixes: #13161
PR-URL: #13233
Backport-PR-URL: #13695
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
gibfahn pushed a commit that referenced this pull request Jun 20, 2017
Added the missing make command in steps 6.3 when building
asm_obsolete.

Also updated the commit message to include the version nasm in
addition to the gcc version.

Fixes: #13161
PR-URL: #13233
Backport-PR-URL: #13695
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2017
This replaces all sources of openssl-1.0.2l.tar.gz into
deps/openssl/openssl

Fixes: #13161
PR-URL: #13233
Backport-PR-URL: #13695
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2017
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: #13161
PR-URL: #13233
Backport-PR-URL: #13695
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2017
Regenerate config files for supported platforms with Makefile.

Fixes: #13161
PR-URL: #13233
Backport-PR-URL: #13695
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2017
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.

Fixes: #13161
PR-URL: #13233
Backport-PR-URL: #13695
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2017
Added the missing make command in steps 6.3 when building
asm_obsolete.

Also updated the commit message to include the version nasm in
addition to the gcc version.

Fixes: #13161
PR-URL: #13233
Backport-PR-URL: #13695
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@MylesBorins MylesBorins mentioned this pull request Jul 18, 2017
MylesBorins pushed a commit that referenced this pull request Sep 19, 2017
This replaces all sources of openssl-1.0.2l.tar.gz into
deps/openssl/openssl

Fixes: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Sep 19, 2017
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: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Sep 19, 2017
Regenerate config files for supported platforms with Makefile.

Fixes: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Sep 19, 2017
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.

Fixes: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Sep 19, 2017
Added the missing make command in steps 6.3 when building
asm_obsolete.

Also updated the commit message to include the version nasm in
addition to the gcc version.

Fixes: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@MylesBorins MylesBorins mentioned this pull request Sep 20, 2017
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
This replaces all sources of openssl-1.0.2l.tar.gz into
deps/openssl/openssl

Fixes: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
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: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
Regenerate config files for supported platforms with Makefile.

Fixes: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
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.

Fixes: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
Added the missing make command in steps 6.3 when building
asm_obsolete.

Also updated the commit message to include the version nasm in
addition to the gcc version.

Fixes: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@MylesBorins MylesBorins mentioned this pull request Oct 25, 2017
MylesBorins added a commit that referenced this pull request Nov 6, 2017
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
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
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request May 5, 2024
This replaces all sources of openssl-1.0.2l.tar.gz into
deps/openssl/openssl

Fixes: nodejs/node#13161
PR-URL: nodejs/node#13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request May 5, 2024
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/node#13161
PR-URL: nodejs/node#13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request May 5, 2024
Regenerate config files for supported platforms with Makefile.

Fixes: nodejs/node#13161
PR-URL: nodejs/node#13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request May 5, 2024
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.

Fixes: nodejs/node#13161
PR-URL: nodejs/node#13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request May 5, 2024
Added the missing make command in steps 6.3 when building
asm_obsolete.

Also updated the commit message to include the version nasm in
addition to the gcc version.

Fixes: nodejs/node#13161
PR-URL: nodejs/node#13233
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
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.

7 participants