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

src: use V8 entropy source if RAND_bytes() != 1 by tniessen · Pull Request #44493 · nodejs/node · GitHub

/ node Public

src: use V8 entropy source if RAND_bytes() != 1 - #44493

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
tniessen:src-use-v8-entropy-source-if-openssl-does-not-work
Sep 4, 2022
Merged

src: use V8 entropy source if RAND_bytes() != 1#44493
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
tniessen:src-use-v8-entropy-source-if-openssl-does-not-work

Conversation

tniessen commented Sep 2, 2022

Copy link
Copy Markdown
Member

RAND_bytes() may return 0 to indicate an error, in which case the buffer might not have been filled with random data at all. Instead of ignoring this case, let V8 use its own entropy source. Historically, this used to be a weak source of entropy, but V8 now implements a proper source even on Windows.

And even if V8's own entropy source turns out to be weak, it does not matter much: V8's PRNG itself is not cryptographically secure, so even if it is seeded from a cryptographically secure entropy source, it does not produce cryptographically secure random numbers.

RAND_bytes() may return 0 to indicate an error, in which case the buffer
might not have been filled with random data at all. Instead of ignoring
this case, let V8 use its own entropy source. Historically, this used to
be a weak source of entropy, but V8 now implements a proper source even
on Windows. And even if V8's own entropy source turns out to be weak, it
does not matter much: V8's PRNG itself is not cryptographically secure,
so even if it is seeded from a cryptographically secure entropy source,
it does not produce cryptographically secure random numbers.

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/startup

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Sep 2, 2022
tniessen added openssl Issues and PRs related to the OpenSSL dependency. v8 engine Issues and PRs related to the V8 dependency. and removed lib / src Issues and PRs related to general changes in the lib or src directory. labels Sep 2, 2022
tniessen added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 2, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 2, 2022

This comment was marked as outdated.

Copy link
Copy Markdown
Collaborator

tniessen added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 4, 2022
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 4, 2022
nodejs-github-bot merged commit 7371d33 into nodejs:main Sep 4, 2022

Copy link
Copy Markdown
Collaborator

Landed in 7371d33

Copy link
Copy Markdown
Member

Hi @tniessen, this PR didn't land clearly on v18.x-staging. Would you mind manually backporting it? I know that's little changes, but the cherry-pick was applying other changes when I tried to fix the conflict (https://gist.github.com/RafaelGSS/bde210dddc7a3019e867ba52db037ade) making the make test fail.

Copy link
Copy Markdown
Member

This seems to depend on a crypto (native) method introduced in #35093 and marked as "semver-major"; I will proceed to mark this as a "dont-land-on-v16.x"

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++. needs-ci PRs that need a full CI run. openssl Issues and PRs related to the OpenSSL dependency. v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants


Back | FazBrowse Home | New Git URL