search

anvilresearch / webcrypto

W3C Web Cryptography API for Node.js
MIT License
82 stars 14 forks source link

Ed25519 Algorithm Support (fixed) #54

Closed thelunararmy closed 7 years ago

thelunararmy commented 7 years ago

Ready for review and merging.

EternalDeiwos commented 7 years ago

Given that we're going a little off-script with this I get that some usage has needed to be guessed.

I think it might be a good idea to structure the usage of EDDSA a little more like ECDSA w.r.t. the namedCurve property, even if we're only supporting ed25519 for now. This would include the correct NotCurrentlySupported error being thrown if someone were to try ed448.

Also, on that note: we should specify in the README that we additionally support K256 (secp256k1) in ECDSA and only ed25519 in EDDSA.

Otherwise looks fantastic; thanks for the work and keen to start playing with it 😁

thelunararmy commented 7 years ago

Implemented most of what you said, the only thing omitted being the CurrentlyNotSupported error. Realistically users shouldnt try ed448 since the new readme specifically only says what is and what isn't supported. See 064f0817b8cf273b1836915a0f8277743822fae7

EternalDeiwos commented 7 years ago

Thanks for taking care of that.

Your table that you added to the README looks like we only support secp256k1 for ECDSA when we also support the NIST curves P-256/384/512.

thelunararmy commented 7 years ago

Derp, completely forgot. Partially since you added the support for all those. Added to README.

EternalDeiwos commented 7 years ago

Looks fantastic. Thank you sir 😁 .