Skip to content

Conversation

Trott
Copy link
Member

@Trott Trott commented Dec 3, 2015

This is #1020 rebased against master and with the nits fixed up.

From original PR:

ECDH.generatePublicKey can get the public key using the curve
and private key. This allows usage of the crypto module where
a developer imports a private key, and then generates a public
key without needing it stored.

Removed the generated_ boolean from the ECDH class, and stopped
checking for it with getPrivateKey() and getPublicKey(). This
allows you to import public and private keys without having to
generate first, which would just rewrite the generated keys
regardless.

An error message was changed to accurately reflect what the error
was.

mbullington and others added 3 commits December 2, 2015 13:26
ECDH.generatePublicKey can get the public key using the curve
and private key. This allows usage of the crypto module where
a developer imports a private key, and then generates a public
key without needing it stored.

Removed the generated_ boolean from the ECDH class, and stopped
checking for it with getPrivateKey() and getPublicKey(). This
allows you to import public and private keys without having to
generate first, which would just rewrite the generated keys
regardless.

An error message was changed to accurately reflect what the error
was.
Change the function name of generatePublicKey to be 
generatePublicKey instead of generateKeys.
@Trott Trott 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 Dec 3, 2015
@mscdex
Copy link
Contributor

mscdex commented Dec 3, 2015

Would it be possible to implicitly cache the generated keys to avoid repeated calculations?

@jasnell
Copy link
Member

jasnell commented Dec 3, 2015

@nodejs/crypto

@Trott
Copy link
Member Author

Trott commented Dec 4, 2015

@mscdex I want to make sure I understand what you're asking. You're talking about basically the same thing as the conversation starting at https://github.com/nodejs/node/pull/1020/files#r25941472 was about, right? It's about how to make sure we don't needlessly go through the work of generating the public key from the private key more than once?

@mscdex
Copy link
Contributor

mscdex commented Dec 4, 2015

@Trott Pretty much.

@mruddy
Copy link

mruddy commented Dec 4, 2015

You guys should check what we've got ready for #3511 to make sure you're not repeating effort.

@Trott
Copy link
Member Author

Trott commented Dec 5, 2015

@mruddy It does seem that #3511 handles more-or-less all the same issues as this one. Let's see if that one can get landed and then maybe this and #1020 can be closed.

@mruddy
Copy link

mruddy commented Dec 5, 2015

@Trott Yep, agreed, exactly what I was thinking.

@bnoordhuis
Copy link
Member

FWIW, I just landed #3511. Should this be closed?

@mruddy
Copy link

mruddy commented Dec 7, 2015

@Trott @bnoordhuis Yep, with the landing of #3511, I believe that this, #4124, and #1020 can now be closed.

@bnoordhuis
Copy link
Member

Okay, closing. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants