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-1.1.0h #19794

Closed
wants to merge 11 commits into from
Closed

Conversation

shigeki
Copy link
Contributor

@shigeki shigeki commented Apr 4, 2018

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

This has very big patches due to the source updates from OpenSSL-1.0.2o to 1.1.0h and generated asm files which are not necessary to be reviewed in this PR.

In order to see the differences easily for reviewers, I made two branches in which those changes are removed. Please refer the branch diffs as below in reviewing for it has just several thousands kb diffs.

shigeki/node@raw_upgrade_openssl110h...shigeki:no_archfiles_upgrade_openssl110h

Note that this has a new build requirement of assembler for asm support. Especially nasm is needed to build Windows that is required by OpenSSL. Please refer 9e38498 for details.

CC @nodejs/crypto @nodejs/tsc

The following is the description written in deps/openssl/README.md


This has a new binding scheme in builing OpenSSL-1.1.0 library with
Node.js. OpenSSL-1.1.0 uses a new build system with perl for various
supported platforms. See openssl/Configurations/README and
openssl/Configurations/README.design in the OpenSSL source for
details.

In order to build OpenSSL library without perl in the build of Node.js
for various supported platforms, platform dependent files (e.g. asm
and header files ) are pre-generated and stored into the
config/archs directory.

  • config/Makefile and config/generate_gypi.pl

    Makefile has supported platform list and generates and copies
    platform dependent files (e.g. asm files) into arch directory with
    generate.pl. Platform dependent gypi files also created obtaining
    build information from configdata.pm that is generated with
    Configure in the OpenSSL build system.

    For Windows, Configure generates makefile that is only available to
    nmake command. config/Makefile_VC-WIN32 and
    config/Makefile_VC-WIN64A are made created by hand for the use of
    GNU make. If make rules or targets are changed in the version up of
    OpenSSL, they should be also updated.

  • gyp and gypi files (openssl*.{gyp,gypi})

    openssl.gyp has two targets of openssl and openssl-cli referred
    from node.gyp. They includes asm and no_asm gypi files with arch
    dependent gypi according to its build options and platforms . The
    gyp data which is common with asm and no_asm are stored in
    openssl_common.gypi.

  • header files (config/*.{h,h.tmpl})

    bn_conf.h, dso_conf.h and opensslconf.h are platform dependent
    in the OpenSSL sources. They are replaced with config/*.h.tmpl
    files to include the file in the ../../../config/ and referred to
    each arch files that depends on asm and no-asm option.

Supported architectures for use of ASM

Here is a list of supported architectures for use of ASM in OpenSSL.

--dest-os --dest-cpu OpenSSL target arch CI
aix ppc aix-gcc o
aix ppc64 aix64-gcc o
linux ia32 linux-elf o
linux x32 linux-x32 -
linux x64 linux-x86_64 o
linux arm linux-armv4 o
linux arm64 linux-aarch64 o
linux ppc linux-ppc o
linux ppc64 linux-ppc64 o
linux ppc64 linux-ppc64le o
linux s390 linux32-s390x o
linux s390x linux64-s390x o
mac ia32 darwin-i386-cc -
mac x64 darwin64-x86-cc o
win ia32 VC-WIN32 -
win x64 VC-WIN64A o
solaris ia32 solaris-x86-gcc o
solaris x64 solaris64-x86_64-gcc o
freebsd ia32 BSD-x86 -
freebsd x64 BSD-x86_64 o
openbsd ia32 BSD-x86 -
openbsd x64 BSD-x86_64 -
others others linux-elf -

These are listed in config/Makefile.
Please refer config/opensslconf_asm.h for details.

Upgrading OpenSSL

Please refer config/README.md .

@shigeki shigeki added semver-major PRs that contain breaking changes and should be released in the next major version. openssl Issues and PRs related to the OpenSSL dependency. labels Apr 4, 2018
@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. labels Apr 4, 2018
@shigeki shigeki force-pushed the upgrade_openssl110h branch from cea6e02 to d4ab4a0 Compare April 4, 2018 13:36
doc/api/tls.md Outdated
[SSL_CTX_set_timeout]: https://www.openssl.org/docs/man1.0.2/ssl/SSL_CTX_set_timeout.html
[SSL_METHODS]: https://www.openssl.org/docs/man1.0.2/ssl/ssl.html#DEALING-WITH-PROTOCOL-METHODS
[SSL_CTX_set_timeout]: https://www.openssl.org/docs/man1.1.0/ssl/SSL_CTX_set_timeout.html
[SSL_METHODS]: https://www.openssl.org/docs/man1.1.0/ssl/ssl.html#DEALING-WITH-PROTOCOL-METHODS
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems the new hash needs to be #Dealing-with-Protocol-Methods

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the hash link. I resolved some conflicts but it needs a more fix . Fixed in 9cf8473b2c75f043f81cf82785d43b4e03dda967

BUILDING.md Outdated
@@ -88,6 +88,23 @@ Depending on host platform, the selection of toolchains may vary.

* Visual Studio 2017 or the Build Tools thereof

#### OpenSSL asm support

OpenSSL-1.1.0 requires the following asssember version for use of asm
Copy link
Contributor

Choose a reason for hiding this comment

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

asssember -> assembler

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 07d5ac52ee66ada4e4a716edb0d4880360b122d6.

BUILDING.md Outdated

Otherwise, `--openssl-no-asm` is added with warning in configure.

*Note:* The forthcoming OpenSSL-1.1.1 will require more higher
Copy link
Contributor

Choose a reason for hiding this comment

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

more higher -> higher

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ditto.

@@ -0,0 +1,76 @@
This has a new binding scheme in builing OpenSSL-1.1.0 library with
Copy link
Contributor

Choose a reason for hiding this comment

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

builing -> building

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in cb6aee5a3d8201fb656a8e971b3741d85976a9d0.

@@ -0,0 +1,76 @@
This has a new binding scheme in builing OpenSSL-1.1.0 library with
Node.js. OpenSSL-1.1.0 uses a new build system with perl for various
Copy link
Contributor

Choose a reason for hiding this comment

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

perl -> Perl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ditto.

`openssl/Configurations/README.design` in the OpenSSL source for
details.

In order to build OpenSSL library without perl in the build of Node.js
Copy link
Contributor

Choose a reason for hiding this comment

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

perl -> Perl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ditto.


`openssl.gyp` has two targets of openssl and openssl-cli referred
from `node.gyp`. They includes asm and no_asm gypi files with arch
dependent gypi according to its build options and platforms . The
Copy link
Contributor

Choose a reason for hiding this comment

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

platforms . -> platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

from `node.gyp`. They includes asm and no_asm gypi files with arch
dependent gypi according to its build options and platforms . The
gyp data which is common with asm and no_asm are stored in
openssl_common.gypi.
Copy link
Contributor

Choose a reason for hiding this comment

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

openssl_common.gypi -> `openssl_common.gypi`?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

`bn_conf.h`, `dso_conf.h` and `opensslconf.h` are platform dependent
in the OpenSSL sources. They are replaced with `config/*.h.tmpl`
files to include the file in the `../../../config/` and referred to
each arch files that depends on asm and no-asm option.
Copy link
Contributor

Choose a reason for hiding this comment

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

each arch files -> each arch file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

| linux | arm64 | linux-aarch64 | o |
| linux | ppc | linux-ppc | o |
| linux | ppc64 | linux-ppc64 | o |
| linux | ppc64 | linux-ppc64le | o | (node_byteoder: little)
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not see (node_byteoder: little) in the rendered table. Should it be there? Is it parsed as a hidden comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a note. I fixed it as a footnote.


### Upgrading OpenSSL

Please refer [config/README.md](config/README.md) .
Copy link
Contributor

Choose a reason for hiding this comment

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

) . -> ).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@vsemozhetbyt
Copy link
Contributor

Are there any more docs to review except doc/api/crypto.md, doc/api/tls.md, BUILDING.md, and deps/openssl/README.md?

Should deps/openssl/doc/UPGRADING.md and deps/openssl/config/README.md be reviewed or are they upstream docs?

@shigeki
Copy link
Contributor Author

shigeki commented Apr 4, 2018

@vsemozhetbyt Thanks for fixing my English. Please review deps/openssl/config/README.md . deps/openssl/doc/UPGRADING.md was removed.

@shigeki
Copy link
Contributor Author

shigeki commented Apr 4, 2018

CI of https://ci.nodejs.org/job/node-test-pull-request/14044/ will be fine except ubuntu1604_sharedlibs_openssl102_x64. It is to be fixed in nodejs/build#1210.

@@ -53,7 +53,7 @@ Here is a list of supported architectures for use of ASM in OpenSSL.
| linux | arm64 | linux-aarch64 | o |
| linux | ppc | linux-ppc | o |
| linux | ppc64 | linux-ppc64 | o |
| linux | ppc64 | linux-ppc64le | o | (node_byteoder: little)
| linux | ppc64 | linux-ppc64le | o |(*1)
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Apr 4, 2018

Choose a reason for hiding this comment

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

It is not in the table still:

https://github.com/shigeki/node/blob/cb6aee5a3d8201fb656a8e971b3741d85976a9d0/deps/openssl/README.md#supported-architectures-for-use-of-asm

Maybe it should be placed in a cell or its own column should be added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I missed to view and check markdown. Fixed to include it in the cell as f8cdc0f.

Copy link
Contributor

@vsemozhetbyt vsemozhetbyt left a comment

Choose a reason for hiding this comment

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

Sorry for nits)

## Upgrading OpenSSL-1.1.0

### Requirements
- Linux enviroment (Only CentOS7.1 and Ubuntu16 are tested)
Copy link
Contributor

Choose a reason for hiding this comment

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

enviroment -> environment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

- Linux enviroment (Only CentOS7.1 and Ubuntu16 are tested)
- perl5
- nasm (http://www.nasm.us/) The version of 2.11 or higher is needed.
- GNU as in binutils. The version of 2.26 or higher is needed.
Copy link
Contributor

Choose a reason for hiding this comment

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

as -> `as`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

$ nasm -v
NASM version 2.11.08
```
### 1. Obtain and extract new OpenSS sources.
Copy link
Contributor

Choose a reason for hiding this comment

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

sources. -> sources for consistency with other headings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

```
### 1. Obtain and extract new OpenSS sources.

Get a new source from https://www.openssl.org/source/ and extract
Copy link
Contributor

Choose a reason for hiding this comment

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

Get a new source... and extract them

-> Get a new source... and extract all files?
or
-> Get new source files... and extract them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. My choice is the former.

$ nasm -v
NASM version 2.11.08
```
### 1. Obtain and extract new OpenSS sources.
Copy link
Contributor

Choose a reason for hiding this comment

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

OpenSS -> OpenSSL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

*Note*: On Windows, OpenSSL Configure generate `makefile` that can be
used for `nmake` command. The `make` command in the step 2 above uses
`Makefile_VC-WIN64A` and `Makefile_VC-WIN32` that is manually
created. When sources files or build options are updated in Windows,
Copy link
Contributor

Choose a reason for hiding this comment

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

sources files -> source files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

used for `nmake` command. The `make` command in the step 2 above uses
`Makefile_VC-WIN64A` and `Makefile_VC-WIN32` that is manually
created. When sources files or build options are updated in Windows,
it needs to change the these two Makefiles by hand. If you are not
Copy link
Contributor

Choose a reason for hiding this comment

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

the these -> these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

created. When sources files or build options are updated in Windows,
it needs to change the these two Makefiles by hand. If you are not
sure, please ask @shigeki for details.
### 4. Commits and make test
Copy link
Contributor

Choose a reason for hiding this comment

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

4. -> 5.

Commits -> Commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

sure, please ask @shigeki for details.
### 4. Commits and make test

Updates all architecture dependent files. Do not forget to git add or remove
Copy link
Contributor

Choose a reason for hiding this comment

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

Updates -> Update or This updates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. My choice is the former.

deps: update archs files for OpenSSL-1.1.0
```

Finally, build Node and run test it.
Copy link
Contributor

Choose a reason for hiding this comment

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

run test it -> run tests or test it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. My choice is the former.

Copy link
Member

@jasnell jasnell 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

@rvagg
Copy link
Member

rvagg commented Apr 5, 2018

@shigeki so I take it from nodejs/build#1210 that you're not able to maintain backward compatibility with 1.0.2? As per nodejs/TSC#479 the hope was that we would maintain the ability to still compile against 1.0.2 in the same way that Node 8/9 can compile against 1.1.0 now. The problem is going to be with Linux distros that insist on dynamically compiling OpenSSL against what they ship, so if they don't ship OpenSSL 1.1.0 then they won't be able to ship Node 10. I'm not sure which distros, if any, that is actually going to impact, however.

@rvagg rvagg mentioned this pull request Apr 5, 2018
4 tasks
@shigeki
Copy link
Contributor Author

shigeki commented Apr 5, 2018

@rvagg It can be possible to be compatible between 1.0.2 and 1.1.0 at this moment. But I am pessimistic to maintain it until the EOLS of 1.0.2 at the end of 2019 unless we keep freezing new features of OpenSSL-1.1.x. Node8/9 could do it since we did not add no new crypto/tls features specific to 1.1.0. I'm fearing that we lose a chance to remove the support of 1.0.2 in Node10 in the future.

If it needs a large discussion, I can make back compatibilities with 1.0.2 in this PR and submit a separated PR or issues to remove 1.0.2 support.

@rvagg
Copy link
Member

rvagg commented Apr 5, 2018

If it's not too much work I'd like to see 1.0.2 support maintained @shigeki. I think liberal use of "this feature is not supported by 1.0.2" runtime errors is fine so we can move forward embracing 1.1.x features and just make them not available to builds against 1.0.2, similar to how we've handled FIPS and how others have handled LibreSSL support.

@bnoordhuis @indutny could either of you weigh in on this? Perhaps it's just not worth it and we should make a clean break?

@kapouer are you available to offer an opinion here since you have a foot in the dynamic linking camp? If Node 10 goes out with no ability to compile against 1.0.2 how many yelps are we going to hear?

Copy link
Contributor Author

@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.

@vsemozhetbyt Thanks for reviewing my English. I fixed in 9a62a0c.

Copy link
Contributor

@vsemozhetbyt vsemozhetbyt left a comment

Choose a reason for hiding this comment

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

Docs LGTM)

@shigeki shigeki force-pushed the upgrade_openssl110h branch from 9a62a0c to 53a63ef Compare April 5, 2018 14:30
@jasnell
Copy link
Member

jasnell commented Apr 5, 2018

Btw, if the plan is for this to go in to 10.0.0, it should land no later than April 10th. After that, I'll only pull in tsc approved semver-majors

@jasnell jasnell added this to the 10.0.0 milestone Apr 5, 2018
@rvagg
Copy link
Member

rvagg commented Apr 6, 2018

OK, no response to my 1.0.2 proposition so how about this: let's just land pure 1.1.0 support and ditch 1.0.2 support. It's something that could be added in afterward, during 10.x Current if enough people yelp.

I'll go ahead and pull 1.0.2 out of CI for 10+ and I'll update nodejs/TSC#479 to change the plan and get that in front of the TSC.

@shigeki anything else we need to get this landed?

configure Outdated
o['variables']['node_use_openssl'] = b(not options.without_ssl)
o['variables']['node_shared_openssl'] = b(options.shared_openssl)
o['variables']['openssl_no_asm'] = 1 if options.openssl_no_asm else 0
variables = o['variables'];
Copy link
Member

Choose a reason for hiding this comment

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

semicolon probably should go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Thanks.

@rvagg
Copy link
Member

rvagg commented Apr 6, 2018

a near perfect CI run @ https://ci.nodejs.org/job/node-test-commit/17482/, one failure, a known Raspberry Pi problem unrelated to this 👍 great work @shigeki

@shigeki
Copy link
Contributor Author

shigeki commented Apr 6, 2018

I will wait for anyone's reviews until the next Monday night in JST.

codebytere added a commit that referenced this pull request Apr 30, 2019
Notable changes:

* deps: add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) [#19794](#19794)
* src: add .code and SSL specific error properties (Sam Roberts) [#25093](#25093)
* tls:
  * add --tls-min-v1.2 CLI switch (Sam Roberts) [#26951](#26951)
  * supported shared openssl 1.1.0 (Sam Roberts) [#26951](#26951)
  * revert default max toTLSv1.2 (Sam Roberts) [#26951](#26951)
  * revert change to invalid protocol error type (Sam Roberts) [#26951](#26951)
  * support TLSv1.3 (Sam Roberts) [#26209](#26209)
  * add code for ERR\_TLS\_INVALID\_PROTOCOL\_METHOD (Sam Roberts) [#24729](#24729)

PR-URL: #27314
BaochengSu added a commit to BaochengSu/node that referenced this pull request Oct 22, 2020
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 <ohtsu@ohtsu.org>
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 <daniel.bevenius@gmail.com>
    Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>

commit 161dca7
Author: Sam Roberts <vieuxtech@gmail.com>
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 <riclau@uk.ibm.com>
    Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
    Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
    Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Partial port, remain compatible with 1.0.2:

commit 970ce14
Author: Shigeki Ohtsu <ohtsu@ohtsu.org>
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 <jasnell@gmail.com>
    Reviewed-By: Rod Vagg <rod@vagg.org>
    Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Signed-off-by: Su Baocheng <baocheng.su@siemens.com>
BaochengSu added a commit to BaochengSu/node that referenced this pull request Jul 14, 2022
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 <ohtsu@ohtsu.org>
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 <daniel.bevenius@gmail.com>
    Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>

commit 161dca7
Author: Sam Roberts <vieuxtech@gmail.com>
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 <riclau@uk.ibm.com>
    Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
    Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
    Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Partial port, remain compatible with 1.0.2:

commit 970ce14
Author: Shigeki Ohtsu <ohtsu@ohtsu.org>
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 <jasnell@gmail.com>
    Reviewed-By: Rod Vagg <rod@vagg.org>
    Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Signed-off-by: Su Baocheng <baocheng.su@siemens.com>
tniessen added a commit to tniessen/node that referenced this pull request Aug 20, 2022
This function was introduced in 2684c90
as an internal helper function. The C++ implementation became a no-op in
a57e2f2 when building against OpenSSL
1.1.0 (instead of OpenSSL 1.0.2), and eventually became a no-op in all
supported OpenSSL versions in 970ce14.
Finally, eb20447 removed the only call
site of setFreeListLength (which was already a no-op at that point).

Refs: nodejs#1529
Refs: nodejs#10859
Refs: nodejs#19794
Refs: nodejs#38116
nodejs-github-bot pushed a commit that referenced this pull request Aug 22, 2022
This function was introduced in 2684c90
as an internal helper function. The C++ implementation became a no-op in
a57e2f2 when building against OpenSSL
1.1.0 (instead of OpenSSL 1.0.2), and eventually became a no-op in all
supported OpenSSL versions in 970ce14.
Finally, eb20447 removed the only call
site of setFreeListLength (which was already a no-op at that point).

Refs: #1529
Refs: #10859
Refs: #19794
Refs: #38116
PR-URL: #44300
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
RafaelGSS pushed a commit that referenced this pull request Sep 5, 2022
This function was introduced in 2684c90
as an internal helper function. The C++ implementation became a no-op in
a57e2f2 when building against OpenSSL
1.1.0 (instead of OpenSSL 1.0.2), and eventually became a no-op in all
supported OpenSSL versions in 970ce14.
Finally, eb20447 removed the only call
site of setFreeListLength (which was already a no-op at that point).

Refs: #1529
Refs: #10859
Refs: #19794
Refs: #38116
PR-URL: #44300
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
tniessen added a commit to tniessen/node that referenced this pull request Sep 10, 2022
These constants have not existed since OpenSSL 1.1.0 reached EOL a few
years ago.

Refs: nodejs#19794
nodejs-github-bot pushed a commit that referenced this pull request Sep 12, 2022
These constants have not existed since OpenSSL 1.1.0 reached EOL a few
years ago.

Refs: #19794
PR-URL: #44589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Fyko pushed a commit to Fyko/node that referenced this pull request Sep 15, 2022
This function was introduced in 2684c90
as an internal helper function. The C++ implementation became a no-op in
a57e2f2 when building against OpenSSL
1.1.0 (instead of OpenSSL 1.0.2), and eventually became a no-op in all
supported OpenSSL versions in 970ce14.
Finally, eb20447 removed the only call
site of setFreeListLength (which was already a no-op at that point).

Refs: nodejs#1529
Refs: nodejs#10859
Refs: nodejs#19794
Refs: nodejs#38116
PR-URL: nodejs#44300
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Fyko pushed a commit to Fyko/node that referenced this pull request Sep 15, 2022
These constants have not existed since OpenSSL 1.1.0 reached EOL a few
years ago.

Refs: nodejs#19794
PR-URL: nodejs#44589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Sep 26, 2022
These constants have not existed since OpenSSL 1.1.0 reached EOL a few
years ago.

Refs: #19794
PR-URL: #44589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Sep 26, 2022
These constants have not existed since OpenSSL 1.1.0 reached EOL a few
years ago.

Refs: #19794
PR-URL: #44589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Sep 26, 2022
These constants have not existed since OpenSSL 1.1.0 reached EOL a few
years ago.

Refs: #19794
PR-URL: #44589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
juanarbol pushed a commit that referenced this pull request Oct 3, 2022
This function was introduced in 2684c90
as an internal helper function. The C++ implementation became a no-op in
a57e2f2 when building against OpenSSL
1.1.0 (instead of OpenSSL 1.0.2), and eventually became a no-op in all
supported OpenSSL versions in 970ce14.
Finally, eb20447 removed the only call
site of setFreeListLength (which was already a no-op at that point).

Refs: #1529
Refs: #10859
Refs: #19794
Refs: #38116
PR-URL: #44300
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
juanarbol pushed a commit that referenced this pull request Oct 4, 2022
These constants have not existed since OpenSSL 1.1.0 reached EOL a few
years ago.

Refs: #19794
PR-URL: #44589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
juanarbol pushed a commit that referenced this pull request Oct 4, 2022
These constants have not existed since OpenSSL 1.1.0 reached EOL a few
years ago.

Refs: #19794
PR-URL: #44589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
juanarbol pushed a commit that referenced this pull request Oct 4, 2022
This function was introduced in 2684c90
as an internal helper function. The C++ implementation became a no-op in
a57e2f2 when building against OpenSSL
1.1.0 (instead of OpenSSL 1.0.2), and eventually became a no-op in all
supported OpenSSL versions in 970ce14.
Finally, eb20447 removed the only call
site of setFreeListLength (which was already a no-op at that point).

Refs: #1529
Refs: #10859
Refs: #19794
Refs: #38116
PR-URL: #44300
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
juanarbol pushed a commit that referenced this pull request Oct 4, 2022
These constants have not existed since OpenSSL 1.1.0 reached EOL a few
years ago.

Refs: #19794
PR-URL: #44589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
juanarbol pushed a commit that referenced this pull request Oct 7, 2022
This function was introduced in 2684c90
as an internal helper function. The C++ implementation became a no-op in
a57e2f2 when building against OpenSSL
1.1.0 (instead of OpenSSL 1.0.2), and eventually became a no-op in all
supported OpenSSL versions in 970ce14.
Finally, eb20447 removed the only call
site of setFreeListLength (which was already a no-op at that point).

Refs: #1529
Refs: #10859
Refs: #19794
Refs: #38116
PR-URL: #44300
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
juanarbol pushed a commit that referenced this pull request Oct 7, 2022
These constants have not existed since OpenSSL 1.1.0 reached EOL a few
years ago.

Refs: #19794
PR-URL: #44589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
juanarbol pushed a commit that referenced this pull request Oct 10, 2022
This function was introduced in 2684c90
as an internal helper function. The C++ implementation became a no-op in
a57e2f2 when building against OpenSSL
1.1.0 (instead of OpenSSL 1.0.2), and eventually became a no-op in all
supported OpenSSL versions in 970ce14.
Finally, eb20447 removed the only call
site of setFreeListLength (which was already a no-op at that point).

Refs: #1529
Refs: #10859
Refs: #19794
Refs: #38116
PR-URL: #44300
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
juanarbol pushed a commit that referenced this pull request Oct 10, 2022
These constants have not existed since OpenSSL 1.1.0 reached EOL a few
years ago.

Refs: #19794
PR-URL: #44589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
juanarbol pushed a commit that referenced this pull request Oct 11, 2022
This function was introduced in 2684c90
as an internal helper function. The C++ implementation became a no-op in
a57e2f2 when building against OpenSSL
1.1.0 (instead of OpenSSL 1.0.2), and eventually became a no-op in all
supported OpenSSL versions in 970ce14.
Finally, eb20447 removed the only call
site of setFreeListLength (which was already a no-op at that point).

Refs: #1529
Refs: #10859
Refs: #19794
Refs: #38116
PR-URL: #44300
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
juanarbol pushed a commit that referenced this pull request Oct 11, 2022
These constants have not existed since OpenSSL 1.1.0 reached EOL a few
years ago.

Refs: #19794
PR-URL: #44589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this pull request Jan 3, 2023
This function was introduced in 2684c90
as an internal helper function. The C++ implementation became a no-op in
a57e2f2 when building against OpenSSL
1.1.0 (instead of OpenSSL 1.0.2), and eventually became a no-op in all
supported OpenSSL versions in 970ce14.
Finally, eb20447 removed the only call
site of setFreeListLength (which was already a no-op at that point).

Refs: nodejs/node#1529
Refs: nodejs/node#10859
Refs: nodejs/node#19794
Refs: nodejs/node#38116
PR-URL: nodejs/node#44300
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
guangwong pushed a commit to noslate-project/node that referenced this pull request Jan 3, 2023
These constants have not existed since OpenSSL 1.1.0 reached EOL a few
years ago.

Refs: nodejs/node#19794
PR-URL: nodejs/node#44589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this pull request Jan 3, 2023
This function was introduced in 2684c90
as an internal helper function. The C++ implementation became a no-op in
a57e2f2 when building against OpenSSL
1.1.0 (instead of OpenSSL 1.0.2), and eventually became a no-op in all
supported OpenSSL versions in 970ce14.
Finally, eb20447 removed the only call
site of setFreeListLength (which was already a no-op at that point).

Refs: nodejs/node#1529
Refs: nodejs/node#10859
Refs: nodejs/node#19794
Refs: nodejs/node#38116
PR-URL: nodejs/node#44300
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
guangwong pushed a commit to noslate-project/node that referenced this pull request Jan 3, 2023
These constants have not existed since OpenSSL 1.1.0 reached EOL a few
years ago.

Refs: nodejs/node#19794
PR-URL: nodejs/node#44589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants