crypto: add generatePrime/checkPrime #36997

jasnell · 2021-01-19T14:30:37Z

APIs for generating and checking pseudo-random primes.

Scratching an itch here. For a set of crypto benchmarks that I've been running I've needed the ability to generate prime test vectors but always had to rely on an external module. Which is silly since openssl has this built in and quite easy to expose.

One of the use cases is to generate primes for diffie hellman with a bit more control over the generation parameters.

crypto.generatePrime(32, {
  safe: true,
  add: 12n,
  rem: 11n,
}, (err, prime) => {
  const dh = crypto.createDiffieHellman(prime);
});

The crypto.checkPrime() does exactly what its name suggest... verifies that an input is a prime within a reasonable margin of error.

Signed-off-by: James M Snell jasnell@gmail.com

nodejs-github-bot · 2021-01-19T16:34:17Z

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

panva · 2021-01-19T17:22:11Z

cc @nodejs/crypto

doc/api/crypto.md

jasnell · 2021-01-22T19:31:08Z

The PR has been updated to split checkPrime() into sync and async variants. The check prime operation can take a while based on the size of the prime candidate and the number of checks so moving that off loop makes the most sense.

nodejs-github-bot · 2021-01-22T20:27:45Z

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

nodejs-github-bot · 2021-01-22T22:48:08Z

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

tniessen

Using ArrayBuffer as the primary representation of prime numbers might be WebCrypto-ish, but it doesn't feel JavaScript-ish or Node.js-ish to me. Both OpenSSL and JavaScript have a BigInt/BIGNUM type, and I don't think our new APIs should steer away from that fact. While static representations of keys typically encode numbers as byte sequences, programmers should have access to types with better semantics. (Sure, higher-level types don't provide memory safety etc., but... it's JavaScript.)

doc/api/crypto.md

src/crypto/crypto_random.cc

nodejs-github-bot · 2021-01-24T17:20:48Z

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

doc/api/crypto.md

jasnell · 2021-01-25T15:29:19Z

Update: I've renamed the function to generatePrime()/generatePrimeSync() to avoid some name bikeshedding debates later on

doc/api/crypto.md

src/crypto/crypto_random.cc

APIs for generating and checking pseudo-random primes Signed-off-by: James M Snell <jasnell@gmail.com>

nodejs-github-bot · 2021-01-25T23:06:21Z

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

tniessen · 2021-01-26T15:03:42Z

LGTM. I think it's great to support BigInt, and I understand the hesitation to use BigInt as the default output format even though I personally believe it would be a better fit. rem and add likely don't need to be in secure memory, but they are short-lived, so it doesn't matter much.

jasnell · 2021-01-26T15:37:39Z

Landed in bb13469

APIs for generating and checking pseudo-random primes Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #36997 Reviewed-By: Tobias Nießen <tniessen@tnie.de>

Notable changes: crypto: * (SEMVER-MINOR) add generatePrime/checkPrime (James M Snell) #36997 * (SEMVER-MINOR) experimental (Ed/X)25519/(Ed/X)448 support (James M Snell) #36879 deps: * upgrade npm to 7.5.0 (Ruy Adorno) #37117 dgram: * (SEMVER-MINOR) support AbortSignal in createSocket (Nitzan Uziely) #37026 doc: * add Zijian Liu to collaborators (ZiJian Liu) #37075 esm: * deprecate legacy main lookup for modules (Guy Bedford) #36918 readline: * (SEMVER-MINOR) add history event and option to set initial history (Mattias Runge-Broberg) #33662 * (SEMVER-MINOR) add support for the AbortController to the question method (Mattias Runge-Broberg) #33676 PR-URL: TODO

Notable changes: crypto: * (SEMVER-MINOR) add generatePrime/checkPrime (James M Snell) #36997 * (SEMVER-MINOR) experimental (Ed/X)25519/(Ed/X)448 support (James M Snell) #36879 deps: * upgrade npm to 7.5.0 (Ruy Adorno) #37117 dgram: * (SEMVER-MINOR) support AbortSignal in createSocket (Nitzan Uziely) #37026 doc: * add Zijian Liu to collaborators (ZiJian Liu) #37075 esm: * deprecate legacy main lookup for modules (Guy Bedford) #36918 readline: * (SEMVER-MINOR) add history event and option to set initial history (Mattias Runge-Broberg) #33662 * (SEMVER-MINOR) add support for the AbortController to the question method (Mattias Runge-Broberg) #33676 PR-URL: #37183

This test had two problems: * The first argument was a number in both cases, which is what caused the (expected) ERR_INVALID_ARG_TYPE error -- the validity of the 'checks' option was not actually verified at all. Thus, the first argument should be valid for this particular test. * The function returned by common.mustNotCall() was passed to assert.throws() as a third argument instead of being passed to checkPrime() as a third argument. (Isn't JavaScript great?) This again led to the (expected) ERR_INVALID_ARG_TYPE error, but again, the validity of the 'checks' option was not verified. Fix both issues by ensuring that all arguments except the 'checks' option are valid. Refs: nodejs#36997

This test had two problems: * The first argument was a number in both cases, which is what caused the (expected) ERR_INVALID_ARG_TYPE error -- the validity of the 'checks' option was not actually verified at all. Thus, the first argument should be valid for this particular test. * The function returned by common.mustNotCall() was passed to assert.throws() as a third argument instead of being passed to checkPrime() as a third argument. (Isn't JavaScript great?) This again led to the (expected) ERR_INVALID_ARG_TYPE error, but again, the validity of the 'checks' option was not verified. Fix both issues by ensuring that all arguments except the 'checks' option are valid. Refs: #36997 PR-URL: #47139 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

jasnell added crypto Issues and PRs related to the crypto subsystem. semver-minor PRs that contain new features and should be released in the next minor version. labels Jan 19, 2021

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. labels Jan 19, 2021

This comment has been minimized.

Sign in to view

targos reviewed Jan 19, 2021

View reviewed changes

doc/api/crypto.md Outdated Show resolved Hide resolved

jasnell force-pushed the random-prime branch from cda64b6 to 6b49bc9 Compare January 22, 2021 19:29

This comment has been minimized.

Sign in to view

jasnell added the review wanted PRs that need reviews. label Jan 22, 2021

tniessen reviewed Jan 23, 2021

View reviewed changes

doc/api/crypto.md Outdated Show resolved Hide resolved

tniessen requested changes Jan 23, 2021

View reviewed changes

doc/api/crypto.md Outdated Show resolved Hide resolved

doc/api/crypto.md Outdated Show resolved Hide resolved

src/crypto/crypto_random.cc Outdated Show resolved Hide resolved

src/crypto/crypto_random.cc Show resolved Hide resolved

jasnell force-pushed the random-prime branch 2 times, most recently from 688eeaa to f16510f Compare January 23, 2021 19:29

jasnell requested a review from tniessen January 23, 2021 19:54

jasnell force-pushed the random-prime branch from f16510f to 0e13f36 Compare January 23, 2021 21:14

This comment has been minimized.

Sign in to view

jasnell force-pushed the random-prime branch from 0e13f36 to a21eb2f Compare January 24, 2021 17:16

jasnell requested a review from targos January 24, 2021 17:22

jasnell commented Jan 25, 2021

View reviewed changes

doc/api/crypto.md Outdated Show resolved Hide resolved

jasnell commented Jan 25, 2021

View reviewed changes

doc/api/crypto.md Outdated Show resolved Hide resolved

jasnell force-pushed the random-prime branch 2 times, most recently from 17e8dd6 to 6ce845a Compare January 25, 2021 15:28

jasnell changed the title ~~crypto: add randomPrime/randomPrimeSync/checkPrime~~ crypto: add generatePrime/checkPrime Jan 25, 2021

tniessen reviewed Jan 25, 2021

View reviewed changes

crypto: add generatePrime/checkPrime

00f5546

APIs for generating and checking pseudo-random primes Signed-off-by: James M Snell <jasnell@gmail.com>

jasnell force-pushed the random-prime branch from cc70379 to 00f5546 Compare January 25, 2021 23:05

jasnell requested a review from tniessen January 25, 2021 23:06

tniessen approved these changes Jan 26, 2021

View reviewed changes

jasnell removed the review wanted PRs that need reviews. label Jan 26, 2021

jasnell closed this Jan 26, 2021

jasnell added a commit that referenced this pull request Jan 26, 2021

crypto: add generatePrime/checkPrime

bb13469

APIs for generating and checking pseudo-random primes Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #36997 Reviewed-By: Tobias Nießen <tniessen@tnie.de>

tniessen mentioned this pull request Feb 1, 2021

src: simplify calls to BN_bin2bn in prime gen #37169

Closed

targos pushed a commit that referenced this pull request Feb 2, 2021

crypto: add generatePrime/checkPrime

110063d

APIs for generating and checking pseudo-random primes Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #36997 Reviewed-By: Tobias Nießen <tniessen@tnie.de>

targos mentioned this pull request Feb 2, 2021

v15.8.0 release proposal #37183

Merged

targos added dont-land-on-v12.x labels Aug 8, 2021

tniessen mentioned this pull request Mar 17, 2023

test: fix 'checks' validation test for checkPrime #47139

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

crypto: add generatePrime/checkPrime #36997

crypto: add generatePrime/checkPrime #36997

jasnell commented Jan 19, 2021 •

edited

This comment has been minimized.

nodejs-github-bot commented Jan 19, 2021 •

edited by jasnell

panva commented Jan 19, 2021

jasnell commented Jan 22, 2021

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Jan 22, 2021

nodejs-github-bot commented Jan 22, 2021 •

edited by jasnell

tniessen left a comment

This comment has been minimized.

nodejs-github-bot commented Jan 24, 2021 •

edited by jasnell

jasnell commented Jan 25, 2021

nodejs-github-bot commented Jan 25, 2021 •

edited by jasnell

tniessen commented Jan 26, 2021

jasnell commented Jan 26, 2021

crypto: add generatePrime/checkPrime #36997

crypto: add generatePrime/checkPrime #36997

Conversation

jasnell commented Jan 19, 2021 • edited

This comment has been minimized.

nodejs-github-bot commented Jan 19, 2021 • edited by jasnell

panva commented Jan 19, 2021

jasnell commented Jan 22, 2021

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Jan 22, 2021

nodejs-github-bot commented Jan 22, 2021 • edited by jasnell

tniessen left a comment

Choose a reason for hiding this comment

This comment has been minimized.

nodejs-github-bot commented Jan 24, 2021 • edited by jasnell

jasnell commented Jan 25, 2021

nodejs-github-bot commented Jan 25, 2021 • edited by jasnell

tniessen commented Jan 26, 2021

jasnell commented Jan 26, 2021

jasnell commented Jan 19, 2021 •

edited

nodejs-github-bot commented Jan 19, 2021 •

edited by jasnell

nodejs-github-bot commented Jan 22, 2021 •

edited by jasnell

nodejs-github-bot commented Jan 24, 2021 •

edited by jasnell

nodejs-github-bot commented Jan 25, 2021 •

edited by jasnell