Skip to content

crypto: improve prime size argument validation #42234

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Conversation

tniessen
Copy link
Member

@tniessen tniessen commented Mar 6, 2022

The current validation in JavaScript is insufficient and also produces an incorrect error message, restricting the size parameter to 32-bit values, whereas the C++ backend restricts the size parameter to the positive range of an int.

This change tightens the validation in JavaScript and adapts the error message accordingly, making the validation in C++ superfluous.

Refs: #42207

The current validation in JavaScript is insufficient and also produces
an incorrect error message, restricting the size parameter to 32-bit
values, whereas the C++ backend restricts the size parameter to the
positive range of an int.

This change tightens the validation in JavaScript and adapts the error
message accordingly, making the validation in C++ superfluous.

Refs: nodejs#42207
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot 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 Mar 6, 2022
@tniessen tniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 6, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 6, 2022
@nodejs-github-bot

This comment was marked as outdated.

@tniessen
Copy link
Member Author

tniessen commented Mar 6, 2022

The test failure on alpine-last-latest-x64 does not seem related, but still interesting: test.parallel/test-crypto-key-objects has status crashed (-6).

@nodejs-github-bot
Copy link
Collaborator

@tniessen tniessen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 7, 2022
@VoltrexKeyva VoltrexKeyva removed the needs-ci PRs that need a full CI run. label Mar 7, 2022
@tniessen tniessen added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 8, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 8, 2022
@nodejs-github-bot nodejs-github-bot merged commit e8697cf into nodejs:master Mar 8, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in e8697cf

@bengl
Copy link
Member

bengl commented Mar 21, 2022

This doesn't land on 17.x, and it looks like it's because it's modifying code added in a semver-major, so I'm adding the dont-land-on-17 label. Please comment if you think it should land.

xtx1130 pushed a commit to xtx1130/node that referenced this pull request Apr 25, 2022
The current validation in JavaScript is insufficient and also produces
an incorrect error message, restricting the size parameter to 32-bit
values, whereas the C++ backend restricts the size parameter to the
positive range of an int.

This change tightens the validation in JavaScript and adapts the error
message accordingly, making the validation in C++ superfluous.

Refs: nodejs#42207

PR-URL: nodejs#42234
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this pull request May 31, 2022
The current validation in JavaScript is insufficient and also produces
an incorrect error message, restricting the size parameter to 32-bit
values, whereas the C++ backend restricts the size parameter to the
positive range of an int.

This change tightens the validation in JavaScript and adapts the error
message accordingly, making the validation in C++ superfluous.

Refs: #42207

PR-URL: #42234
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
The current validation in JavaScript is insufficient and also produces
an incorrect error message, restricting the size parameter to 32-bit
values, whereas the C++ backend restricts the size parameter to the
positive range of an int.

This change tightens the validation in JavaScript and adapts the error
message accordingly, making the validation in C++ superfluous.

Refs: #42207

PR-URL: #42234
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jul 11, 2022
The current validation in JavaScript is insufficient and also produces
an incorrect error message, restricting the size parameter to 32-bit
values, whereas the C++ backend restricts the size parameter to the
positive range of an int.

This change tightens the validation in JavaScript and adapts the error
message accordingly, making the validation in C++ superfluous.

Refs: #42207

PR-URL: #42234
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
The current validation in JavaScript is insufficient and also produces
an incorrect error message, restricting the size parameter to 32-bit
values, whereas the C++ backend restricts the size parameter to the
positive range of an int.

This change tightens the validation in JavaScript and adapts the error
message accordingly, making the validation in C++ superfluous.

Refs: #42207

PR-URL: #42234
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
The current validation in JavaScript is insufficient and also produces
an incorrect error message, restricting the size parameter to 32-bit
values, whereas the C++ backend restricts the size parameter to the
positive range of an int.

This change tightens the validation in JavaScript and adapts the error
message accordingly, making the validation in C++ superfluous.

Refs: nodejs/node#42207

PR-URL: nodejs/node#42234
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants