Adds Curve448 #87
Adds Curve448 #87
Conversation
|
Would you mind updating the README to show that support for 448 is now in the default resolver? That should help users see that Side note: I looked over the new Curve448 crates you made and I have to say, nice work :) |
Done, let me know if you find anything else.
Thank you, much appreciated :) |
|
Very excited for this! Thanks so much for this effort, and apologies for the delay in initial review. First skim looks good, and I appreciate how similar to the dalek API you've made your Curve448 crate, which makes that review more straightforward. |
There was a problem hiding this comment.
Thanks again for this, and sorry for my lag!
This PR looks good, and I'm generally more curious x448/Ed448-Goldilocks - what state do you consider the library in right now, is it still experimental or would you say you're confident in its correctness? I see a good amount of XXX comments in Ed448-Goldilocks in particular.
|
|
||
| fn set(&mut self, privkey: &[u8]) { | ||
| copy_slices!(privkey, &mut self.privkey); | ||
| self.pubkey = x448::x448_unchecked(self.privkey, x448::X448_BASEPOINT_BYTES); |
There was a problem hiding this comment.
What's the difference between this and the checked version? It looks like from the documentation that checked will assert that low-order points aren't used?
There was a problem hiding this comment.
Yep that’s completely correct. I used the unchecked version for consistency between X25519 and X448
|
Hey Jake, No problem at all. The Montgomery curve arithmetic which X448 relies on was copied from the Dalek-cryptography crate and modified for Curve448. In that regard, I’m quite confident in its correctness, as I’m quite confident that Curve2559 is correct. Test vectors have also been included from the relevant RFCs for X448 and Decaf. The two differences between Curve448 and Curve25519:
The mapping is not strictly needed as it is an optimisation that allows you to use the precomputed base-points on the Edwards curve for Scalar mul. This is an optimisation that I need to add. For the rest of the repo, there are quite a few XXXs that are API tweaks/optimisations that need to be added. If you see an XXX that contradicts this point, please let me know. It shouldn’t matter for X448 as that only relies on Curve448 though. You can also use the |
|
@kevaundray thanks so much for the responses! Thinking it over, in line with the way snow has used feature flags for safety in other places with newer crypto libraries, perhaps we can just merge this behind a feature flag for now as an opt-in and then remove the feature flag (or make it default) after it's had some time to settle and we get more eyes on it. Does that sound reasonable to you? |
|
Yep, that sounds completely reasonable! |
|
any update on this? I think it'd be cool to support Ed448 in the snow library |
This PR:
Renames Ed448 to Curve448.
Adds X448 from crate-crypto : (unchecked API)
Uncomments the code that was ignoring the tests for Curve448.
Notes:
The unchecked API does not check for low order points. This was done to mirror the default X22519 implementation. The check can be added back by removing the
uncheckedkeyword from the functions.There is one unwrap in the
dhmethod whenfrom_bytes_uncheckedis called. This will return None, if you do not pass 56 bytes. This should mirror the try_into().unwrap() method.The underlying EC library for X448 does not implement a fixed base scalar mul, so benchmarks for key generation will become more efficient when this implemented, without changing the API.