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: update OpenSSL 3.0.5+quic #43693

Closed
wants to merge 352 commits into from
Closed

Conversation

RafaelGSS
Copy link
Member

Updated openssl dep to openssl-3.0.5+quic using the maintenance guide.

Refs: https://mta.openssl.org/pipermail/openssl-announce/2022-July/000232.html

LiviaMedeiros and others added 30 commits May 10, 2022 09:13
This adds the following:
- `fs.read(fd, buffer[, options], callback)`.
- `filehandle.read(buffer[, options])`.

PR-URL: nodejs#42768
Refs: nodejs#42601
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs#42916
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
When Buffer::New passes in existing data,
it cannot be garbage collected in js synchronous execution.

Fixes: nodejs#40828
Refs: nodejs#38300

PR-URL: nodejs#42695
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
PR-URL: nodejs#42889
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
PR-URL: nodejs#42862
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Harshitha K P <harshitha014@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Fixes: nodejs#41578

PR-URL: nodejs#42812
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#42893
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
Fixes: nodejs#42827

PR-URL: nodejs#42947
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Also affects generateKeySync('hmac', ...)

PR-URL: nodejs#42944
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Refs: nodejs#39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs#42852
Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#42968
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: nodejs#42867
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#42823
Refs: https://coverage.nodejs.org/coverage-3a6b975981092213/lib/internal/webstreams/readablestream.js.html#L421
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
PR-URL: nodejs#42806
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
PR-URL: nodejs#42747
Refs: https://coverage.nodejs.org/coverage-24adba675179ebba/lib/internal/webstreams/encoding.js.html#L98
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#42683
Refs: nodejs#42440
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
The linked issue, https://bugs.chromium.org/p/v8/issues/detail?id=6593,
is marked as "Fixed", so I think we can revert this now.

This reverts commit 08a9c4a.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs#42934
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=9187
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
`HandleWrap.isRefed()` was renamed to `hasRef()`. However, the filename
of related TCs has not been reflected.

Refs: nodejs@f31a5ec34a

PR-URL: nodejs#42754
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Refs: nodejs#42701
Refs: nodejs/undici#1346
Refs: nodejs#42939

PR-URL: nodejs#42960
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: nodejs#42937
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
`icu_locales` is generated by joining values from `set` data structure.
However, `set` doesn't guarantee an order, so the result of
`icu_locales` is not determined. For example, the result value could be
'en,root' or 'root,en'. This fix makes it deterministic.

The main reason of this fix is to restore the reproducibility of the
build because the value of `icu_locales` is embedded into `node` binary.

PR-URL: nodejs#42865
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Refs: nodejs#11321
Refs: nodejs#17384

PR-URL: nodejs#42989
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
PR-URL: nodejs#42954
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Node.js unofficially supports a shared library variant where the
main node executable is a thin wrapper around node.dll/libnode.so.
The key benefit of this is to support embedding Node.js in other
applications.

Since Node.js 12 there have been a number of issues preventing the
shared library build from working correctly, primarily on Windows:

* A number of functions used executables such as `mksnapshot` are
    not exported from `libnode.dll` using a `NODE_EXTERN` attribute
* A dependency on the `Winmm` system library is missing
* Incorrect defines on executable targets leads to `node.exe`
    claiming to export a number of functions that are actually in
    `libnode.dll`
* Because `node.exe` attempts to export symbols, `node.lib` gets
    generated causing native extensions to try to link against
    `node.exe` not `libnode.dll`.
* Similarly, because `node.dll` was renamed to `libnode.dll`,
    native extensions don't know to look for `libnode.lib` rather
    than `node.lib`.
* On macOS an RPATH is added to find `libnode.dylib` relative to
    `node` in the same folder. This works fine from the
    `out/Release` folder but not from an installed prefix, where
    `node` will be in `bin/` and `libnode.dylib` will be in `lib/`.
* Similarly on Linux, no RPATH is added so LD_LIBRARY_PATH needs
    setting correctly for `bin/node` to find `lib/libnode.so`.

For the `libnode.lib` vs `node.lib` issue there are two possible
options:

1. Ensure `node.lib` from `node.exe` does not get generated, and
    instead copy `libnode.lib` to `node.lib`. This means addons
    compiled when referencing the correct `node.lib` file will
    correctly depend on `libnode.dll`. The down side is that
    native addons compiled with stock Node.js will still try to
    resolve symbols against node.exe rather than libnode.dll.
2. After building `libnode.dll`, dump the exports using `dumpbin`,
    and process this to generate a `node.def` file to be linked into
    `node.exe` with the `/DEF:node.def` flag. The export entries
    in `node.def` will all read
    ```
    my_symbol=libnode.my_symbol
    ```
    so that `node.exe` will redirect all exported symbols back to
    `libnode.dll`. This has the benefit that addons compiled with
    stock Node.js will load correctly into `node.exe` from a shared
    library build, but means that every embedding executable also
    needs to perform this same trick.

I went with the first option as it is the cleaner of the two
solutions in my opinion. Projects wishing to generate a shared
library variant of Node.js can now, for example,
```
.\vcbuild dll package vs
```
to generate a full node installation including `libnode.dll`,
`Release\node.lib`, and all the necessary headers. Native addons
can then be built against the shared library build easily by
specifying the correct `nodedir` option.

For example
```
>npx node-gyp configure --nodedir
   C:\Users\User\node\Release\node-v18.0.0-win-x64
...
>npx node-gyp build
...
>dumpbin /dependents build\Release\binding.node
Microsoft (R) COFF/PE Dumper Version 14.29.30136.0
Copyright (C) Microsoft Corporation.  All rights reserved.

Dump of file build\Release\binding.node

File Type: DLL

  Image has the following dependencies:

    KERNEL32.dll
    libnode.dll
    VCRUNTIME140.dll
    api-ms-win-crt-string-l1-1-0.dll
    api-ms-win-crt-stdio-l1-1-0.dll
    api-ms-win-crt-runtime-l1-1-0.dll
...
```

PR-URL: nodejs#41850
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
PR-URL: nodejs#42909
Fixes: nodejs#40484
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
`OnStreamAfterReqFinished` uses `v8::Object::Has` to check if it needs
to call `oncomplete`. `v8::Object::Has` needs to execute Javascript.
However when worker threads are involved, `OnStreamAfterReqFinished` may
be called after the worker thread termination has begun via
`worker.terminate()`. This makes `v8::Object::Has` return `Nothing`,
which triggers an assert.

This diff fixes the issue by simply defaulting us to `false` in the case
where `Nothing` is returned. This is sound because we can't execute
`oncomplete` anyway as the isolate is terminating.

Fixes: nodejs#38418

PR-URL: nodejs#42874
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Closes: nodejs#42962

PR-URL: nodejs#42963
Fixes: nodejs#42962
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#42952
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Refs: nodejs#11321
Refs: nodejs#17384

PR-URL: nodejs#43001
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#43004
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
aduh95 and others added 6 commits June 15, 2022 21:28
| The string method             | looks up the property |
| ----------------------------- | --------------------- |
| `String.prototype.match`      | `Symbol.match`        |
| `String.prototype.matchAll`   | `Symbol.matchAll`     |
| `String.prototype.replace`    | `Symbol.replace`      |
| `String.prototype.replaceAll` | `Symbol.replace`      |
| `String.prototype.search`     | `Symbol.search`       |
| `String.prototype.split`      | `Symbol.split`        |

Functions that lookup the `exec` property on the prototype chain:

* `RegExp.prototype[Symbol.match]`
* `RegExp.prototype[Symbol.matchAll]`
* `RegExp.prototype[Symbol.replace]`
* `RegExp.prototype[Symbol.search]`
* `RegExp.prototype[Symbol.split]`
* `RegExp.prototype.test`

`RegExp.prototype[Symbol.replace]` and `RegExp.prototype[Symbol.split]`
are still allowed for a lack of a better solution.

PR-URL: nodejs#43393
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#43391
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Notable changes:

* crypto:
  * remove Node.js-specific webcrypto extensions (Filip Skokan) nodejs#43310
  * add CFRG curves to Web Crypto API (Filip Skokan) nodejs#42507
* dns:
  * accept `'IPv4'` and `'IPv6'` for `family` (Antoine du Hamel) nodejs#43054
* report:
  * add more heap infos in process report (theanarkh) nodejs#43116

PR-URL: nodejs#43385
This updates all sources in deps/openssl/openssl by:
    $ git clone git@github.com:quictls/openssl.git
    $ cd openssl
    $ git checkout openssl-3.0.5+quic
    $ cd ../node/deps/openssl
    $ rm -rf openssl
    $ cp -R ../../../openssl openssl
    $ rm -rf openssl/.git* openssl/.travis*
    $ git add --all openssl
    $ git commit openssl
After an OpenSSL source update, all the config files need to be
regenerated and committed by:
    $ find archs \( -name \*.s \) -exec rm "{}" \;
    Edit deps/openssl/openssl/crypto/perlasm/x86asm.pl changing #ifdef
    to %ifdef to make it compatible to nasm on windows 32.
    $ make -C deps/openssl/config
    $ git add deps/openssl/config/archs
    $ git add deps/openssl/openssl
    $ git commit
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp

@nodejs-github-bot nodejs-github-bot added dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. openssl Issues and PRs related to the OpenSSL dependency. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch. labels Jul 5, 2022
@RafaelGSS
Copy link
Member Author

@richardlau This PR contains both needed operations:

  • Cleanup .s files
  • Apply fix for Windows 32 NASM

See the 4056193 commit description

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

RSLGTM

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

danielleadams pushed a commit that referenced this pull request Jul 7, 2022
This is a security release.

Notable changes:

* (SEMVER-MAJOR) src,deps,build,test: add OpenSSL config appname (Daniel Bevenius) #43124
* (SEMVER-MAJOR) src,doc,test: add --openssl-shared-config option (Daniel Bevenius) #43124
* update archs files for quictls/openssl-3.0.5+quic (RafaelGSS) #43693
* upgrade openssl sources to quictls/openssl-3.0.5+quic (RafaelGSS) #43693

PR-URL: nodejs-private/node-private#329
@RafaelGSS RafaelGSS changed the base branch from v18.x-staging to main July 7, 2022 16:47
@RafaelGSS
Copy link
Member Author

Landed in: 05d8a5f and 91fe38e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. openssl Issues and PRs related to the OpenSSL dependency. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.