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: X25519 and X448 and ECDH #26626

Closed
panva opened this issue Mar 13, 2019 · 9 comments
Closed

crypto: X25519 and X448 and ECDH #26626

panva opened this issue Mar 13, 2019 · 9 comments
Labels
crypto Issues and PRs related to the crypto subsystem. feature request Issues that request new features to be added to Node.js.

Comments

@panva
Copy link
Member

panva commented Mar 13, 2019

Is your feature request related to a problem? Please describe.

Implementing CFRG curves ECDH-ES

Resources:

Describe the solution you'd like

The following WIP on Node already paves the way for EdDSA signatures

👇
To complete the implementation i'd like X25519 and X448 curves/functions to be usable with crypto.createECDH(curveName) to get JOSE ECDH-ES with these OKP keys working.

I understand from this thread (#18770), particularly this comment and the conversation below that it may end up being a separate API and that's fine.

It seems the curves are already somewhat in because the returned error differs when providing nonsense vs. valid crv/function name.

@panva panva changed the title crypto: X25519 and X448 for crypto.ECDH crypto: X25519 and X448 for createECDH Mar 13, 2019
@bnoordhuis bnoordhuis added crypto Issues and PRs related to the crypto subsystem. feature request Issues that request new features to be added to Node.js. labels Mar 13, 2019
@bnoordhuis
Copy link
Member

I don't think it makes sense to bolt X448/X25519 support onto crypto.createECDH() - that API is only expected to work with the curve names from crypto.getCurves().

Supporting them through crypto.generateKeyPair() however might be a good way forward.

It seems the curves are already somewhat in because the returned error differs when providing nonsense vs. valid crv/function name.

The difference is because "X448" and "X25519" are valid NIDs (identifiers.) That doesn't mean they're valid curve names, however.

To wit, crypto.createECDH('AES-128-ECB') and crypto.createECDH('valid') (!) throw the same error.

@panva
Copy link
Member Author

panva commented Mar 13, 2019

@bnoordhuis Ed25519 and Ed448 keys through generateKeyPair is already on the way.
However, be it through createECDH or any other API the X448/X25519 functions are usable in Elliptic Curve Diffie-Hellman. createECDH is where i as a user would expect to look for its support.

The difference is because "X448" and "X25519" are valid NIDs (identifiers.) That doesn't mean they're valid curve names, however.
To wit, crypto.createECDH('AES-128-ECB') and crypto.createECDH('valid') (!) throw the same error.

Thanks for pointing that out.

@bnoordhuis
Copy link
Member

For technical background: crypto.createECDH() currently is a wrapper around openssl's EC_KEY family of functions.

X448 and X25519 on the other hand are implemented through the EVP_PKEY functions - more specifically EVP_PKEY_keygen() - and that's precisely what crypto.generateKeyPair() is a wrapper around.

However, after thinking about it some more, extending crypto.createECDH() could be an option if it was possible to rewrite it using EVP_PKEY functions like EVP_PKEY_derive().

I don't know exactly how feasible that is but my gut feeling is that it's doable.

@panva panva changed the title crypto: X25519 and X448 for createECDH crypto: X25519 and X448 keys and ECDH Mar 19, 2019
@panva panva changed the title crypto: X25519 and X448 keys and ECDH crypto: X25519 and X448 and ECDH Mar 21, 2019
tniessen pushed a commit that referenced this issue Mar 25, 2019
PR-URL: #26774
Refs: #26626
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this issue Mar 28, 2019
PR-URL: #26774
Refs: #26626
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@DuBistKomisch
Copy link

I'm interested in migrating to this API from curve25519-n, it looks like the x25519 key generation has landed, are we just waiting for the equivalent of ECDH#computeSecret?

@panva
Copy link
Member Author

panva commented Aug 6, 2019

I recall @tniessen wanted to take a stab at this a few months back.

@jurelik
Copy link

jurelik commented Sep 19, 2019

Any updates on this issue? If anyone has any information on when we can expect x25519/x448 included in crypto.createECDH(), it would be much appreciated.

@sam-github
Copy link
Contributor

@jurelik I don't see that anyone volunteered to implement this, much less comitted to a timeline.

Are you interested in implementing the feature? We are always looking for more collaborators on crypto support, and there are people who would help get a PR in shape if you needed any help along the way.

@awoie
Copy link

awoie commented Dec 1, 2019

Is anyone working on this?

Decentralized Identity Foundation (DIF) is very interested in using this feature. X25519 became quite popular in our community. We are about to finalize a spec that uses ECDH-ES (X25519) and it would be great to have a node reference implementation: https://github.com/decentralized-identity/did-siop/blob/master/docs/index.html.

@panva
Copy link
Member Author

panva commented Jan 21, 2020

Resolved by crypto.diffieHellman(...) introduced in #31178

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. feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants