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

crypto: avoid infinite loops in prime generation #37212

Merged
merged 0 commits into from
Feb 9, 2021

Conversation

tniessen
Copy link
Member

@tniessen tniessen commented Feb 3, 2021

It's difficult to find precise conditions that would prevent infinite loops here and that can be checked without a significant performance impact.

Passing parameters that cause an infinite loop within OpenSSL will permanently block the application thread when in sync mode, or permanently disable one thread in the libuv thread pool when in async mode. Especially the latter behavior is hard to debug, and throwing reasonable errors makes that much easier.

These conditions prevent cases that will, with high probability, result in infinite loops within OpenSSL. In cases where OpenSSL does not get stuck in an infinite loop, the parameters that match these conditions would not result in randomly generated primes.

This is a fast best-effort approach that doesn't require additional BIGNUM allocations.

These conditions do not prevent all infinite loops.

@tniessen tniessen added crypto Issues and PRs related to the crypto subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. labels Feb 3, 2021
@tniessen tniessen force-pushed the crypto-prime-avoid-infinite-loops branch from 73aa358 to cd94327 Compare February 3, 2021 17:49
@benjamingr
Copy link
Member

Just write a program that takes another program as input and returns whether or not that program with the given parameter contains an infinite loop. That sounds like a reasonable and easy task :D

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

Good comments!

@tniessen
Copy link
Member Author

tniessen commented Feb 3, 2021

Thank you @benjamingr!

Just write a program that takes another program as input and returns whether or not that program with the given parameter contains an infinite loop. That sounds like a reasonable and easy task :D

Oh, absolutely, just give me a couple of hours to disprove Turing! :P

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

Good catch.

@nodejs-github-bot

This comment has been minimized.

@tniessen tniessen force-pushed the crypto-prime-avoid-infinite-loops branch from a188e82 to d35d516 Compare February 5, 2021 16:37
@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 Feb 5, 2021
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member

Trott commented Feb 9, 2021

Landed in fdd7a87...6e804a9

Trott pushed a commit to tniessen/node that referenced this pull request Feb 9, 2021
PR-URL: nodejs#37212
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@Trott Trott closed this Feb 9, 2021
@Trott Trott force-pushed the crypto-prime-avoid-infinite-loops branch from d35d516 to 6e804a9 Compare February 9, 2021 00:49
@Trott Trott merged commit 6e804a9 into nodejs:master Feb 9, 2021
danielleadams pushed a commit that referenced this pull request Feb 16, 2021
PR-URL: #37212
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this pull request Feb 16, 2021
PR-URL: #37212
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This was referenced Feb 16, 2021
@tniessen tniessen deleted the crypto-prime-avoid-infinite-loops branch February 21, 2021 11:57
@tniessen tniessen removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

None yet

6 participants