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

[v10.x backport] Update openssl1.1.1a #26270

Closed
wants to merge 12 commits into from
Closed

[v10.x backport] Update openssl1.1.1a #26270

wants to merge 12 commits into from

Conversation

sam-github
Copy link
Contributor

@sam-github sam-github commented Feb 23, 2019 •
edited
Loading

Backport openssl 1.1.1a related PRs. They depend on each other, so I am including in one PR:

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

@nodejs-github-bot
Copy link
Collaborator

@sam-github sadly an error occured when I tried to trigger a build :(

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. openssl Issues and PRs related to the OpenSSL dependency. v10.x labels Feb 23, 2019
@sam-github sam-github mentioned this pull request Feb 23, 2019
Closed
4 tasks
@sam-github
Copy link
Contributor Author

re: #24979 (comment)

@nodejs/lts @MylesBorins @BethGriggs Please make sure this makes its way into the upcoming 10.x semver-minor release.

@mscdex mscdex mentioned this pull request Feb 26, 2019
Merged
3 tasks
@sam-github sam-github mentioned this pull request Feb 27, 2019
Closed
@sam-github
Copy link
Contributor Author

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

sam-github and others added 12 commits February 28, 2019 13:33
@sam-github
tls: add min/max protocol version options
be6a3a1 
The existing secureProtocol option only allows setting the allowed
protocol to a specific version, or setting it to "all supported
versions". It also used obscure strings based on OpenSSL C API
functions. Directly setting the min or max is easier to use and explain.

Backport-PR-URL: nodejs#24676
PR-URL: nodejs#24405
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
@sam-github
doc: fix REPLACEME for tls min/max protocol option
8e4a622 
Fill in correct pr-url: value in the YAML changelog that was missing
from f512f5e. The stanza was also sorted in the wrong order, most
recent is supposed to be in the beginning of the changes, not the end.

PR-URL: nodejs#24759
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@sam-github
deps: upgrade openssl sources to 1.1.1a
39d09ce 
This updates all sources in deps/openssl/openssl with openssl-1.1.1a.

PR-URL: nodejs#25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Backport-PR-URL: nodejs#25688
@shigeki @sam-github
deps: fix gyp/gypi for openssl-1.1.1
e8b39cf 
Some of defines and cppflags in the build config of OpenSSL-1.1.1 were
moved to new attributes. Gyp and gypi file generations are needed to be
fixed to include them.

PR-URL: nodejs#25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Backport-PR-URL: nodejs#25688
@shigeki @sam-github
deps: fix MacOS and Win build for OpenSSL-1.1.1
af05709 
Because llvm on MacOS does not support AVX-512, asm files need to be limited to
AVX-2 support even when they are generated on Linux.  fake_gcc.pl returns the
fake llvm banner version for MacOS as if the assembler supports upto AVX-2.

For Windows, makefiles for nmake were updated in OpenSSL-1.1.1 and they are
rewritten into GNU makefile format by hand.

PR-URL: nodejs#25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Backport-PR-URL: nodejs#25688
@shigeki @sam-github
deps: add s390 asm rules for OpenSSL-1.1.1
08cf226 
This is a floating patch against OpenSSL-1.1.1 to generate asm files
with Makefile rules.

PR-URL: nodejs#25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Backport-PR-URL: nodejs#25688
@shigeki @sam-github
deps: add only avx2 configs for OpenSSL-1.1.1
0cf96fb 
OpenSSL-1.1.1 has new support of AVX-512 but AVX-2 asm files still need
to be generated for the older assembler support to keep backward
compatibilities.

PR-URL: nodejs#25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Backport-PR-URL: nodejs#25688
@shigeki @sam-github
deps: fix for non GNU assembler in AIX
87ff9f7 
AIX has own assembler not GNU as that does not support --noexecstack.

PR-URL: nodejs#25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Backport-PR-URL: nodejs#25688
@shigeki @sam-github
doc: fix assembler requirement for OpenSSL-1.1.1
eb3470a 
Add new requirements of assembler version for AVX-512 support
in OpenSSL-1.1.1.

PR-URL: nodejs#25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Backport-PR-URL: nodejs#25688
@sam-github
deps: update archs files for OpenSSL-1.1.1a
312dcc3 
`cd deps/openssl/config; make` updates all archs dependant files.

PR-URL: nodejs#25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Backport-PR-URL: nodejs#25688
@shigeki @sam-github
tls: workaround handshakedone in renegotiation
6be596d 
`SSL_CB_HANDSHAKE_START` and `SSL_CB_HANDSHAKE_DONE` are called
sending HelloRequest in OpenSSL-1.1.1.
We need to check whether this is in a renegotiation state or not.

PR-URL: nodejs#25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Backport-PR-URL: nodejs#25688
@sam-github
test: assert on client and server side seperately
d502073 
This gets better coverage of the codes, and is more explicit. It also
works around ordering differences in the errors produced by openssl.
The approach was tested with 1.1.0 and 1.1.1, as well as TLSv1.2 vs
TLSv1.3. OpenSSL 1.1.0 is relevant when node is built against a shared
openssl.

PR-URL: nodejs#25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Backport-PR-URL: nodejs#25688
@sam-github
Copy link
Contributor Author

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

@sam-github
Copy link
Contributor Author

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

the few failures look like the flaky builds, so trying to resume.

@sam-github sam-github mentioned this pull request Mar 1, 2019
Closed
2 tasks
@sam-github
Copy link
Contributor Author

sam-github commented Mar 2, 2019 •
edited by BethGriggs
Loading

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

@sam-github sam-github mentioned this pull request Mar 7, 2019
Merged
@mhdawson
Copy link
Member

@sam-github did the backports land cleanly or where there things up needed to adjust?

@sam-github
Copy link
Contributor Author

sam-github commented Mar 14, 2019 •
edited
Loading

@BethGriggs tls: add min/max had minor textual conflicts, because 3b4159c is on master and not on 10.x, so I had to do some minor textual fixups there. Compare git show f512f5ea138 and git show be6a3a1, you don't have to be a TLS expert. The rest of the changes just cherry-picked clean, IIRC. Maybe I should have done a seperate PR for each backport, but since they are cumulative (EDIT: dependant), I would have had to do it sequentially, and they would have had to be reviewed and merged to v10.x staging before I could do the next. It didn't occur to me this would need re-review.

@nodejs/crypto PTAL

@sam-github sam-github mentioned this pull request Mar 26, 2019
Merged
@nodejs-github-bot
Copy link
Collaborator

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

BethGriggs pushed a commit that referenced this pull request Mar 28, 2019
@sam-github @BethGriggs
tls: add min/max protocol version options
d3ebad2 
The existing secureProtocol option only allows setting the allowed
protocol to a specific version, or setting it to "all supported
versions". It also used obscure strings based on OpenSSL C API
functions. Directly setting the min or max is easier to use and explain.

Backport-PR-URL: #26270
PR-URL: #24405
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
BethGriggs pushed a commit that referenced this pull request Mar 28, 2019
@sam-github @BethGriggs
doc: fix REPLACEME for tls min/max protocol option
ecae727 
Fill in correct pr-url: value in the YAML changelog that was missing
from f512f5e. The stanza was also sorted in the wrong order, most
recent is supposed to be in the beginning of the changes, not the end.

Backport-PR-URL: #26270
PR-URL: #24759
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
BethGriggs pushed a commit that referenced this pull request Mar 28, 2019
@sam-github @BethGriggs
deps: upgrade openssl sources to 1.1.1a
2d396fe 
This updates all sources in deps/openssl/openssl with openssl-1.1.1a.

Backport-PR-URL: #26270
PR-URL: #25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
BethGriggs pushed a commit that referenced this pull request Mar 28, 2019
@shigeki @BethGriggs
deps: add s390 asm rules for OpenSSL-1.1.1
0e7019f 
This is a floating patch against OpenSSL-1.1.1 to generate asm files
with Makefile rules.

Backport-PR-URL: #26270
PR-URL: #25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
BethGriggs pushed a commit that referenced this pull request Mar 28, 2019
@shigeki @BethGriggs
deps: fix gyp/gypi for openssl-1.1.1
70a785c 
Some of defines and cppflags in the build config of OpenSSL-1.1.1 were
moved to new attributes. Gyp and gypi file generations are needed to be
fixed to include them.

Backport-PR-URL: #26270
PR-URL: #25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
BethGriggs pushed a commit that referenced this pull request Mar 28, 2019
@shigeki @BethGriggs
deps: fix MacOS and Win build for OpenSSL-1.1.1
f5369da 
Because llvm on MacOS does not support AVX-512, asm files need to be limited to
AVX-2 support even when they are generated on Linux.  fake_gcc.pl returns the
fake llvm banner version for MacOS as if the assembler supports upto AVX-2.

For Windows, makefiles for nmake were updated in OpenSSL-1.1.1 and they are
rewritten into GNU makefile format by hand.

Backport-PR-URL: #26270
PR-URL: #25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
BethGriggs pushed a commit that referenced this pull request Mar 28, 2019
@shigeki @BethGriggs
deps: add only avx2 configs for OpenSSL-1.1.1
639b1d2 
OpenSSL-1.1.1 has new support of AVX-512 but AVX-2 asm files still need
to be generated for the older assembler support to keep backward
compatibilities.

Backport-PR-URL: #26270
PR-URL: #25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
BethGriggs pushed a commit that referenced this pull request Mar 28, 2019
@shigeki @BethGriggs
doc: fix assembler requirement for OpenSSL-1.1.1
e05eb3e 
Add new requirements of assembler version for AVX-512 support
in OpenSSL-1.1.1.

Backport-PR-URL: #26270
PR-URL: #25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
BethGriggs pushed a commit that referenced this pull request Mar 28, 2019
@shigeki @BethGriggs
deps: fix for non GNU assembler in AIX
56441a0 
AIX has own assembler not GNU as that does not support --noexecstack.

Backport-PR-URL: #26270
PR-URL: #25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
BethGriggs pushed a commit that referenced this pull request Mar 28, 2019
@shigeki @BethGriggs
tls: workaround handshakedone in renegotiation
8dd8033 
`SSL_CB_HANDSHAKE_START` and `SSL_CB_HANDSHAKE_DONE` are called
sending HelloRequest in OpenSSL-1.1.1.
We need to check whether this is in a renegotiation state or not.

Backport-PR-URL: #26270
PR-URL: #25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
BethGriggs pushed a commit that referenced this pull request Mar 28, 2019
@sam-github @BethGriggs
test: assert on client and server side seperately
fe8e07d 
This gets better coverage of the codes, and is more explicit. It also
works around ordering differences in the errors produced by openssl.
The approach was tested with 1.1.0 and 1.1.1, as well as TLSv1.2 vs
TLSv1.3. OpenSSL 1.1.0 is relevant when node is built against a shared
openssl.

Backport-PR-URL: #26270
PR-URL: #25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
BethGriggs pushed a commit that referenced this pull request Mar 28, 2019
@sam-github @BethGriggs
deps: update archs files for OpenSSL-1.1.1a
b0b73fa 
`cd deps/openssl/config; make` updates all archs dependant files.

Backport-PR-URL: #26270
PR-URL: #25381
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
@BethGriggs
Copy link
Member

Landed on v10.x-staging (a92286d...b0b73fa)

@BethGriggs BethGriggs closed this Mar 28, 2019
@sam-github sam-github deleted the update_openssl1.1.1a-v10.x branch March 29, 2019 18:57
BaochengSu added a commit to BaochengSu/node that referenced this pull request Oct 22, 2020
@BaochengSu
OpenSSL 1.1.1 support
c915a50 
Ported from
OpenSUSE:nodejs8-8.17.0-lp152.147.1:openssl_1_1_1.patch

Original commit message:

Backport OpenSSL 1.1.1 support, mostly be disabling TLS 1.3
Upstream commits:

commit 8dd8033
Author: Shigeki Ohtsu <[email protected]>
Date:   Wed Sep 12 17:34:24 2018 +0900

    tls: workaround handshakedone in renegotiation

    `SSL_CB_HANDSHAKE_START` and `SSL_CB_HANDSHAKE_DONE` are called
    sending HelloRequest in OpenSSL-1.1.1.
    We need to check whether this is in a renegotiation state or not.

    Backport-PR-URL: nodejs#26270
    PR-URL: nodejs#25381
    Reviewed-By: Daniel Bevenius <[email protected]>
    Reviewed-By: Shigeki Ohtsu <[email protected]>

commit 161dca7
Author: Sam Roberts <[email protected]>
Date:   Wed Nov 28 14:11:18 2018 -0800

    tls: re-define max supported version as 1.2

    Several secureProtocol strings allow any supported TLS version as the
    maximum, but our maximum supported protocol version is TLSv1.2 even if
    someone configures a build against an OpenSSL that supports TLSv1.3.

    Fixes: nodejs#24658

    PR-URL: nodejs#25024
    Reviewed-By: Richard Lau <[email protected]>
    Reviewed-By: Ben Noordhuis <[email protected]>
    Reviewed-By: Daniel Bevenius <[email protected]>
    Reviewed-By: Colin Ihrig <[email protected]>

Partial port, remain compatible with 1.0.2:

commit 970ce14
Author: Shigeki Ohtsu <[email protected]>
Date:   Wed Mar 14 14:26:55 2018 +0900

    crypto: remove deperecated methods of TLS version

    All version-specific methods were deprecated in OpenSSL 1.1.0 and
    min/max versions explicitly need to be set.
    This still keeps comptatible with JS and OpenSSL-1.0.2 APIs for now.

    crypto, constants: add constant of OpenSSL-1.1.0

    Several constants for OpenSSL-1.1.0 engine were removed and renamed in
    OpenSSL-1.1.0. This added one renamed constant in order to have a
    compatible feature with that of OpenSSL-1.0.2.
    Other missed or new constants in OpenSSL-1.1.0 are not yet added.

    crypto,tls,constants: remove OpenSSL1.0.2 support

    This is semver-majar change so that we need not to have
    compatibilities with older versions.

    Fixes: nodejs#4270
    PR-URL: nodejs#19794
    Reviewed-By: James M Snell <[email protected]>
    Reviewed-By: Rod Vagg <[email protected]>
    Reviewed-By: Michael Dawson <[email protected]>

Signed-off-by: Su Baocheng <[email protected]>
BaochengSu added a commit to BaochengSu/node that referenced this pull request Jul 14, 2022
@BaochengSu
OpenSSL 1.1.1 support
5df9bc7 
Ported from
OpenSUSE:nodejs8-8.17.0-lp152.147.1:openssl_1_1_1.patch

Original commit message:

Backport OpenSSL 1.1.1 support, mostly be disabling TLS 1.3
Upstream commits:

commit 8dd8033
Author: Shigeki Ohtsu <[email protected]>
Date:   Wed Sep 12 17:34:24 2018 +0900

    tls: workaround handshakedone in renegotiation

    `SSL_CB_HANDSHAKE_START` and `SSL_CB_HANDSHAKE_DONE` are called
    sending HelloRequest in OpenSSL-1.1.1.
    We need to check whether this is in a renegotiation state or not.

    Backport-PR-URL: nodejs#26270
    PR-URL: nodejs#25381
    Reviewed-By: Daniel Bevenius <[email protected]>
    Reviewed-By: Shigeki Ohtsu <[email protected]>

commit 161dca7
Author: Sam Roberts <[email protected]>
Date:   Wed Nov 28 14:11:18 2018 -0800

    tls: re-define max supported version as 1.2

    Several secureProtocol strings allow any supported TLS version as the
    maximum, but our maximum supported protocol version is TLSv1.2 even if
    someone configures a build against an OpenSSL that supports TLSv1.3.

    Fixes: nodejs#24658

    PR-URL: nodejs#25024
    Reviewed-By: Richard Lau <[email protected]>
    Reviewed-By: Ben Noordhuis <[email protected]>
    Reviewed-By: Daniel Bevenius <[email protected]>
    Reviewed-By: Colin Ihrig <[email protected]>

Partial port, remain compatible with 1.0.2:

commit 970ce14
Author: Shigeki Ohtsu <[email protected]>
Date:   Wed Mar 14 14:26:55 2018 +0900

    crypto: remove deperecated methods of TLS version

    All version-specific methods were deprecated in OpenSSL 1.1.0 and
    min/max versions explicitly need to be set.
    This still keeps comptatible with JS and OpenSSL-1.0.2 APIs for now.

    crypto, constants: add constant of OpenSSL-1.1.0

    Several constants for OpenSSL-1.1.0 engine were removed and renamed in
    OpenSSL-1.1.0. This added one renamed constant in order to have a
    compatible feature with that of OpenSSL-1.0.2.
    Other missed or new constants in OpenSSL-1.1.0 are not yet added.

    crypto,tls,constants: remove OpenSSL1.0.2 support

    This is semver-majar change so that we need not to have
    compatibilities with older versions.

    Fixes: nodejs#4270
    PR-URL: nodejs#19794
    Reviewed-By: James M Snell <[email protected]>
    Reviewed-By: Rod Vagg <[email protected]>
    Reviewed-By: Michael Dawson <[email protected]>

Signed-off-by: Su Baocheng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@BethGriggs BethGriggs BethGriggs approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. 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.
7 participants
@sam-github @nodejs-github-bot @rvagg @mhdawson @BethGriggs @gireeshpunathil @shigeki