FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

crypto: fix propagation of "memory limit exceeded" by tniessen · Pull Request #53300 · nodejs/node · GitHub

/ node Public

crypto: fix propagation of "memory limit exceeded" - #53300

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
tniessen:crypto-scrypt-memory
Jun 5, 2024
Merged

crypto: fix propagation of "memory limit exceeded"#53300
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
tniessen:crypto-scrypt-memory

Conversation

tniessen commented Jun 3, 2024

Copy link
Copy Markdown
Member

When we throw ERR_CRYPTO_INVALID_SCRYPT_PARAMS after a call to EVP_PBE_scrypt, check if OpenSSL reported an error and if so, append the OpenSSL error message to the default generic error message. In particular, this catches cases when maxmem is not sufficient, which otherwise is difficult to identify because our documentation only provides an approximation of the required maxmem value.

Fixes: #53291

When we throw ERR_CRYPTO_INVALID_SCRYPT_PARAMS after a call to
EVP_PBE_scrypt, check if OpenSSL reported an error and if so, append the
OpenSSL error message to the default generic error message. In
particular, this catches cases when `maxmem` is not sufficient, which
otherwise is difficult to identify because our documentation only
provides an approximation of the required `maxmem` value.

Fixes: nodejs#53291
tniessen added crypto Issues and PRs related to the crypto subsystem. openssl Issues and PRs related to the OpenSSL dependency. labels Jun 3, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Jun 3, 2024

panva left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

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 outstanding review comments, and a CI started. labels Jun 3, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 3, 2024

This comment was marked as outdated.

Copy link
Copy Markdown
Collaborator

panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 5, 2024
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 5, 2024
nodejs-github-bot merged commit 0281e2c into nodejs:main Jun 5, 2024

Copy link
Copy Markdown
Collaborator

Landed in 0281e2c

RafaelGSS pushed a commit that referenced this pull request Jun 7, 2024
When we throw ERR_CRYPTO_INVALID_SCRYPT_PARAMS after a call to
EVP_PBE_scrypt, check if OpenSSL reported an error and if so, append the
OpenSSL error message to the default generic error message. In
particular, this catches cases when `maxmem` is not sufficient, which
otherwise is difficult to identify because our documentation only
provides an approximation of the required `maxmem` value.

Fixes: #53291
PR-URL: #53300
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
RafaelGSS mentioned this pull request Jun 7, 2024
soophoo pushed a commit to soophoo/node that referenced this pull request Jun 20, 2024
When we throw ERR_CRYPTO_INVALID_SCRYPT_PARAMS after a call to
EVP_PBE_scrypt, check if OpenSSL reported an error and if so, append the
OpenSSL error message to the default generic error message. In
particular, this catches cases when `maxmem` is not sufficient, which
otherwise is difficult to identify because our documentation only
provides an approximation of the required `maxmem` value.

Fixes: nodejs#53291
PR-URL: nodejs#53300
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
When we throw ERR_CRYPTO_INVALID_SCRYPT_PARAMS after a call to
EVP_PBE_scrypt, check if OpenSSL reported an error and if so, append the
OpenSSL error message to the default generic error message. In
particular, this catches cases when `maxmem` is not sufficient, which
otherwise is difficult to identify because our documentation only
provides an approximation of the required `maxmem` value.

Fixes: #53291
PR-URL: #53300
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Jul 25, 2024
codebytere added a commit to electron/electron that referenced this pull request Jul 26, 2024
jkleinsc pushed a commit to electron/electron that referenced this pull request Jul 26, 2024
* chore: bump node in DEPS to v20.16.0

* test: skip unstable shadow realm gc tests

nodejs/node#52855

* test: extend env for `test-node-output-errors`

nodejs/node#53535

* src: fix typo in env.cc

nodejs/node#53418

* src: reset `process.versions` during pre-execution

nodejs/node#53444

* chore: fixup patch indices

* src,permission: --allow-wasi & prevent WASI exec

nodejs/node#53124

* tls: use SSL_get_peer_tmp_key

nodejs/node#53366

* deps: update c-ares to 1.29.0

nodejs/node#53155

* src: account for OpenSSL unexpected version

* crypto: fix propagation of "memory limit exceeded"

nodejs/node#53300

* process: add process.getBuiltinModule(id)

nodejs/node#52762

* windows 32bit: config change callback needs to be stdcall

c-ares/c-ares@8f265c9

* fix: building with UNICODE

c-ares/c-ares#802

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Co-authored-by: Keeley Hammond <khammond@slack-corp.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. 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. openssl Issues and PRs related to the OpenSSL dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Invalid scrypt params" error message is unnecessarily generic when exceeding maxmem

4 participants


Back | FazBrowse Home | New Git URL