crypto: ensure auth tag set for chacha20-poly1305 #46185

bnoordhuis · 2023-01-12T14:10:32Z

Because OpenSSL v1.x doesn't do that by itself (OpenSSL v3.x does.)

Fixes: #45874

Affects v16.x, not v18.x+.

nodejs-github-bot · 2023-01-12T14:10:37Z

Review requested:

@nodejs/crypto

Because OpenSSL v1.x doesn't do that by itself (OpenSSL v3.x does.) Fixes: nodejs#45874

anonrig · 2023-01-12T15:13:38Z

src/crypto/crypto_cipher.cc

@@ -901,6 +901,14 @@ bool CipherBase::Final(std::unique_ptr<BackingStore>* out) {
  if (kind_ == kDecipher && IsSupportedAuthenticatedMode(ctx_.get()))
    MaybePassAuthTagToOpenSSL();

+  // OpenSSL v1.x doesn't verify the presence of the auth tag so do


Should we add a comment in here to remove this when v16 is not supported?

I believe this comment already serves that purpose. It mentions v1.x which when it's time for a codebase cleanup will get picked up?

Once we fully drop 1.1.1 (including through dynamic linking), we'll have to revisit all version check macros etc. anyway.

nodejs-github-bot · 2023-01-12T17:31:31Z

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

nodejs-github-bot · 2023-01-14T14:16:30Z

Landed in 3803d09

panva · 2023-01-14T15:11:47Z

This PR lands and tests cleanly on v16.x-staging.

Because OpenSSL v1.x doesn't do that by itself (OpenSSL v3.x does.) Fixes: nodejs#45874 PR-URL: nodejs#46185 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Because OpenSSL v1.x doesn't do that by itself (OpenSSL v3.x does.) Fixes: #45874 PR-URL: #46185 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [node](https://github.com/nodejs/node) | stage | minor | `16.19.1-bullseye` -> `16.20.0-bullseye` | --- ### Release Notes <details> <summary>nodejs/node</summary> ### [`v16.20.0`](https://github.com/nodejs/node/releases/tag/v16.20.0): 2023-03-29, Version 16.20.0 'Gallium' (LTS), @BethGriggs [Compare Source](nodejs/node@v16.19.1...v16.20.0) ##### Notable Changes - **deps:** - update undici to 5.20.0 (Node.js GitHub Bot) [#46711](nodejs/node#46711) - update c-ares to 1.19.0 (Michaël Zasso) [#46415](nodejs/node#46415) - upgrade npm to 8.19.4 (npm team) [#46677](nodejs/node#46677) - update corepack to 0.17.0 (Node.js GitHub Bot) [#46842](nodejs/node#46842) - **(SEMVER-MINOR)** **src**: add support for externally shared js builtins (Michael Dawson) [#44376](nodejs/node#44376) ##### Commits - \[[`de6dd67790`](nodejs/node@de6dd67790)] - **crypto**: avoid hang when no algorithm available (Richard Lau) [#46237](nodejs/node#46237) - \[[`4617512788`](nodejs/node@4617512788)] - **crypto**: ensure auth tag set for chacha20-poly1305 (Ben Noordhuis) [#46185](nodejs/node#46185) - \[[`24972164fc`](nodejs/node@24972164fc)] - **deps**: update undici to 5.20.0 (Node.js GitHub Bot) [#46711](nodejs/node#46711) - \[[`85f88c6a8d`](nodejs/node@85f88c6a8d)] - **deps**: V8: cherry-pick [`90be99f`](nodejs/node@90be99fab31c) (Michaël Zasso) [#46646](nodejs/node#46646) - \[[`b4ebe6d47b`](nodejs/node@b4ebe6d47b)] - **deps**: update c-ares to 1.19.0 (Michaël Zasso) [#46415](nodejs/node#46415) - \[[`56cbc7fdda`](nodejs/node@56cbc7fdda)] - **deps**: V8: cherry-pick [`c2792e5`](nodejs/node@c2792e58035f) (Jiawen Geng) [#44961](nodejs/node#44961) - \[[`7af9bdb31e`](nodejs/node@7af9bdb31e)] - **deps**: upgrade npm to 8.19.4 (npm team) [#46677](nodejs/node#46677) - \[[`962a7471b5`](nodejs/node@962a7471b5)] - **deps**: update corepack to 0.17.0 (Node.js GitHub Bot) [#46842](nodejs/node#46842) - \[[`748bc96e35`](nodejs/node@748bc96e35)] - **deps**: update corepack to 0.16.0 (Node.js GitHub Bot) [#46710](nodejs/node#46710) - \[[`a467782499`](nodejs/node@a467782499)] - **deps**: update corepack to 0.15.3 (Node.js GitHub Bot) [#46037](nodejs/node#46037) - \[[`1913b6763d`](nodejs/node@1913b6763d)] - **deps**: update corepack to 0.15.2 (Node.js GitHub Bot) [#45635](nodejs/node#45635) - \[[`809371a15f`](nodejs/node@809371a15f)] - **module**: require.resolve.paths returns null with node schema (MURAKAMI Masahiko) [#45147](nodejs/node#45147) - \[[`086bb2f8d4`](nodejs/node@086bb2f8d4)] - ***Revert*** "**src**: let http2 streams end after session close" (Rich Trott) [#46721](nodejs/node#46721) - \[[`6a01d39120`](nodejs/node@6a01d39120)] - **(SEMVER-MINOR)** **src**: add support for externally shared js builtins (Michael Dawson) [#44376](nodejs/node#44376) - \[[`d081032a60`](nodejs/node@d081032a60)] - **test**: fix test-net-connect-reset-until-connected (Vita Batrla) [#46781](nodejs/node#46781) - \[[`efe1be47ec`](nodejs/node@efe1be47ec)] - **test**: skip test depending on `overlapped-checker` when not available (Antoine du Hamel) [#45015](nodejs/node#45015) - \[[`fc47d58abe`](nodejs/node@fc47d58abe)] - **test**: remove cjs loader from stack traces (Geoffrey Booth) [#44197](nodejs/node#44197) - \[[`cf76d0790d`](nodejs/node@cf76d0790d)] - **test**: fix WPT title when no META title is present (Filip Skokan) [#46804](nodejs/node#46804) - \[[`0d1485b924`](nodejs/node@0d1485b924)] - **test**: fix default WPT titles (Filip Skokan) [#46778](nodejs/node#46778) - \[[`088e9cde3d`](nodejs/node@088e9cde3d)] - **test**: add WPTRunner support for variants and generating WPT reports (Filip Skokan) [#46498](nodejs/node#46498) - \[[`908c4dff44`](nodejs/node@908c4dff44)] - **test**: mark test-crypto-key-objects flaky on Linux (Richard Lau) [#46684](nodejs/node#46684) - \[[`768e56227e`](nodejs/node@768e56227e)] - **tools**: make `utils.SearchFiles` deterministic (Bruno Pitrus) [#44496](nodejs/node#44496) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.walbeck.it/mwalbeck/docker-cyberchef/pulls/187 Co-authored-by: renovate-bot <bot@walbeck.it> Co-committed-by: renovate-bot <bot@walbeck.it>

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [node](https://github.com/nodejs/node) | stage | minor | `16.19.1-bullseye-slim` -> `16.20.0-bullseye-slim` | --- ### Release Notes <details> <summary>nodejs/node</summary> ### [`v16.20.0`](https://github.com/nodejs/node/releases/tag/v16.20.0): 2023-03-29, Version 16.20.0 'Gallium' (LTS), @BethGriggs [Compare Source](nodejs/node@v16.19.1...v16.20.0) ##### Notable Changes - **deps:** - update undici to 5.20.0 (Node.js GitHub Bot) [#46711](nodejs/node#46711) - update c-ares to 1.19.0 (Michaël Zasso) [#46415](nodejs/node#46415) - upgrade npm to 8.19.4 (npm team) [#46677](nodejs/node#46677) - update corepack to 0.17.0 (Node.js GitHub Bot) [#46842](nodejs/node#46842) - **(SEMVER-MINOR)** **src**: add support for externally shared js builtins (Michael Dawson) [#44376](nodejs/node#44376) ##### Commits - \[[`de6dd67790`](nodejs/node@de6dd67790)] - **crypto**: avoid hang when no algorithm available (Richard Lau) [#46237](nodejs/node#46237) - \[[`4617512788`](nodejs/node@4617512788)] - **crypto**: ensure auth tag set for chacha20-poly1305 (Ben Noordhuis) [#46185](nodejs/node#46185) - \[[`24972164fc`](nodejs/node@24972164fc)] - **deps**: update undici to 5.20.0 (Node.js GitHub Bot) [#46711](nodejs/node#46711) - \[[`85f88c6a8d`](nodejs/node@85f88c6a8d)] - **deps**: V8: cherry-pick [`90be99f`](nodejs/node@90be99fab31c) (Michaël Zasso) [#46646](nodejs/node#46646) - \[[`b4ebe6d47b`](nodejs/node@b4ebe6d47b)] - **deps**: update c-ares to 1.19.0 (Michaël Zasso) [#46415](nodejs/node#46415) - \[[`56cbc7fdda`](nodejs/node@56cbc7fdda)] - **deps**: V8: cherry-pick [`c2792e5`](nodejs/node@c2792e58035f) (Jiawen Geng) [#44961](nodejs/node#44961) - \[[`7af9bdb31e`](nodejs/node@7af9bdb31e)] - **deps**: upgrade npm to 8.19.4 (npm team) [#46677](nodejs/node#46677) - \[[`962a7471b5`](nodejs/node@962a7471b5)] - **deps**: update corepack to 0.17.0 (Node.js GitHub Bot) [#46842](nodejs/node#46842) - \[[`748bc96e35`](nodejs/node@748bc96e35)] - **deps**: update corepack to 0.16.0 (Node.js GitHub Bot) [#46710](nodejs/node#46710) - \[[`a467782499`](nodejs/node@a467782499)] - **deps**: update corepack to 0.15.3 (Node.js GitHub Bot) [#46037](nodejs/node#46037) - \[[`1913b6763d`](nodejs/node@1913b6763d)] - **deps**: update corepack to 0.15.2 (Node.js GitHub Bot) [#45635](nodejs/node#45635) - \[[`809371a15f`](nodejs/node@809371a15f)] - **module**: require.resolve.paths returns null with node schema (MURAKAMI Masahiko) [#45147](nodejs/node#45147) - \[[`086bb2f8d4`](nodejs/node@086bb2f8d4)] - ***Revert*** "**src**: let http2 streams end after session close" (Rich Trott) [#46721](nodejs/node#46721) - \[[`6a01d39120`](nodejs/node@6a01d39120)] - **(SEMVER-MINOR)** **src**: add support for externally shared js builtins (Michael Dawson) [#44376](nodejs/node#44376) - \[[`d081032a60`](nodejs/node@d081032a60)] - **test**: fix test-net-connect-reset-until-connected (Vita Batrla) [#46781](nodejs/node#46781) - \[[`efe1be47ec`](nodejs/node@efe1be47ec)] - **test**: skip test depending on `overlapped-checker` when not available (Antoine du Hamel) [#45015](nodejs/node#45015) - \[[`fc47d58abe`](nodejs/node@fc47d58abe)] - **test**: remove cjs loader from stack traces (Geoffrey Booth) [#44197](nodejs/node#44197) - \[[`cf76d0790d`](nodejs/node@cf76d0790d)] - **test**: fix WPT title when no META title is present (Filip Skokan) [#46804](nodejs/node#46804) - \[[`0d1485b924`](nodejs/node@0d1485b924)] - **test**: fix default WPT titles (Filip Skokan) [#46778](nodejs/node#46778) - \[[`088e9cde3d`](nodejs/node@088e9cde3d)] - **test**: add WPTRunner support for variants and generating WPT reports (Filip Skokan) [#46498](nodejs/node#46498) - \[[`908c4dff44`](nodejs/node@908c4dff44)] - **test**: mark test-crypto-key-objects flaky on Linux (Richard Lau) [#46684](nodejs/node#46684) - \[[`768e56227e`](nodejs/node@768e56227e)] - **tools**: make `utils.SearchFiles` deterministic (Bruno Pitrus) [#44496](nodejs/node#44496) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.walbeck.it/mwalbeck/docker-jellyfin-livestream/pulls/243 Co-authored-by: renovate-bot <bot@walbeck.it> Co-committed-by: renovate-bot <bot@walbeck.it>

bnoordhuis requested a review from tniessen January 12, 2023 14:10

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Jan 12, 2023

bnoordhuis mentioned this pull request Jan 12, 2023

chacha20-poly1305 decryption works without setting authTag on decypher in v16 #45874

Closed

crypto: ensure auth tag set for chacha20-poly1305

a2d348d

Because OpenSSL v1.x doesn't do that by itself (OpenSSL v3.x does.) Fixes: nodejs#45874

bnoordhuis force-pushed the fix45874 branch from df8df65 to a2d348d Compare January 12, 2023 14:14

tniessen approved these changes Jan 12, 2023

View reviewed changes

richardlau added the lts-watch-v16.x label Jan 12, 2023

richardlau approved these changes Jan 12, 2023

View reviewed changes

panva approved these changes Jan 12, 2023

View reviewed changes

anonrig approved these changes Jan 12, 2023

View reviewed changes

anonrig reviewed Jan 12, 2023

View reviewed changes

panva added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jan 12, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 12, 2023

This comment was marked as outdated.

Sign in to view

panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 12, 2023

github-actions bot mentioned this pull request Jan 13, 2023

CI Reliability 2023-01-13 nodejs/reliability#482

Open

26 tasks

jasnell approved these changes Jan 13, 2023

View reviewed changes

github-actions bot mentioned this pull request Jan 14, 2023

CI Reliability 2023-01-14 nodejs/reliability#483

Open

19 tasks

bnoordhuis added commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. commit-queue Add this label to land a pull request using GitHub Actions. labels Jan 14, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 14, 2023

nodejs-github-bot merged commit 3803d09 into nodejs:main Jan 14, 2023

This was referenced Jan 15, 2023

CI Reliability 2023-01-15 nodejs/reliability#484

Open

CI Reliability 2023-01-16 nodejs/reliability#485

Open

CI Reliability 2023-01-17 nodejs/reliability#486

Open

github-actions bot mentioned this pull request Jan 18, 2023

CI Reliability 2023-01-18 nodejs/reliability#487

Open

18 tasks

RafaelGSS mentioned this pull request Jan 20, 2023

v19.5.0 proposal #46286

Merged

juanarbol mentioned this pull request Jan 28, 2023

V18.14.0 proposal #46396

Merged

BethGriggs mentioned this pull request Mar 27, 2023

v16.20.0 proposal #47272

Merged

richardlau added backported-to-v16.x and removed lts-watch-v16.x labels Aug 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

crypto: ensure auth tag set for chacha20-poly1305 #46185

crypto: ensure auth tag set for chacha20-poly1305 #46185

bnoordhuis commented Jan 12, 2023

nodejs-github-bot commented Jan 12, 2023

anonrig Jan 12, 2023

panva Jan 12, 2023

tniessen Jan 12, 2023

This comment was marked as outdated.

nodejs-github-bot commented Jan 12, 2023

nodejs-github-bot commented Jan 14, 2023

panva commented Jan 14, 2023

crypto: ensure auth tag set for chacha20-poly1305 #46185

crypto: ensure auth tag set for chacha20-poly1305 #46185

Conversation

bnoordhuis commented Jan 12, 2023

nodejs-github-bot commented Jan 12, 2023

anonrig Jan 12, 2023

Choose a reason for hiding this comment

panva Jan 12, 2023

Choose a reason for hiding this comment

tniessen Jan 12, 2023

Choose a reason for hiding this comment

This comment was marked as outdated.

nodejs-github-bot commented Jan 12, 2023

nodejs-github-bot commented Jan 14, 2023

panva commented Jan 14, 2023