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 <[email protected]>
Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#42916
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
PR-URL: nodejs#42889
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
PR-URL: nodejs#42862
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Harshitha K P <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Fixes: nodejs#41578

PR-URL: nodejs#42812
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#42893
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Co-authored-by: Luigi Pinca <[email protected]>
Fixes: nodejs#42827

PR-URL: nodejs#42947
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Also affects generateKeySync('hmac', ...)

PR-URL: nodejs#42944
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#42968
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
PR-URL: nodejs#42806
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
PR-URL: nodejs#42747
Refs: https://coverage.nodejs.org/coverage-24adba675179ebba/lib/internal/webstreams/encoding.js.html#L98
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#42683
Refs: nodejs#42440
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]>

PR-URL: nodejs#42934
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=9187
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
`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 <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Refs: nodejs#42701
Refs: nodejs/undici#1346
Refs: nodejs#42939

PR-URL: nodejs#42960
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
PR-URL: nodejs#42937
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
`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 <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Mestery <[email protected]>
Refs: nodejs#11321
Refs: nodejs#17384

PR-URL: nodejs#42989
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
PR-URL: nodejs#42954
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
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 <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
`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 <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Closes: nodejs#42962

PR-URL: nodejs#42963
Fixes: nodejs#42962
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#42952
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Refs: nodejs#11321
Refs: nodejs#17384

PR-URL: nodejs#43001
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#43004
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#43391
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
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 [email protected]: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.