Added support for P-384

[OtaK]

Hi! I added support for p384.

There is a breaking change to be mindful of:

• moved the p256 implementation from the root of ecdh_nistp to ecdh_nistp::p256

But basically:

• Added support for P384 DHKEX - I did refrain from getting deps up to date, but I must admit that it was tempting 😅 (Edit: I did after all, seeing there's literally no breakage from it)
• Updated p256 to 0.13 (& added p384 0.13)
• Massively refactored the NIST P-XXX key rejection sampling process to make it generic over the key length. This prepares the groundwork for when the p521 crate will be ready for consumption.
• Small cleanup on how features were structured. Implicit features based on dependencies name is a code smell in general (the specific dependency you're using becomes an API rather than an implementation detail). This changes nothing on the users' standpoint.
• Removed the serde_derive dev-dependency in favor of enabling the "derive" feature on serde

Feel free to ask for any change!

NB: I also have a PR pending for X448 that depends on this PR.

[rozbb]

Thanks for this! It'll take a little bit for me to get through this, so bear with me 🙏

[OtaK]

I'll fix the CI issues on my side - very weird because locally there's no issue and I've been using my fork without issues (and everything passes test vectors and KATs)

I'll get to it in the next few days

[OtaK]

This should now be much better - also rebased on top of the master branch

[rozbb]

This looks great! One ask: could you possibly factor out the impls in p256.rs and p384.rs to a single macro? It looks like it's nearly entirely duplicated code, so it'd be best as a macro. Lmk if that's a pain and I can help out

[OtaK]

This looks great! One ask: could you possibly factor out the impls in p256.rs and p384.rs to a single macro? It looks like it's nearly entirely duplicated code, so it'd be best as a macro. Lmk if that's a pain and I can help out

Oops I entirely didn't see your comment (dammit github notifications!) sorry!
I thought about it, if you want I can do it no issues, but seems you're already on it :D

Let me know if you need anything!

(On top of that I'll add x448 support next - with the help of the ed448-goldilocks crate -, as mentioned before, and it seems that honestly we could also macro-implement it as well. It's roughly the same thing as x25519 but with 56-byte keys)

[rozbb]

No worries, already done! Take a look at the changes I made and lmk if they look good. Once I figure out this faliing test, I'll merge.

Re goldilocks: I'm not sure the crate is mature enough to expose to HPKE users. It appears to be barely maintained.

[OtaK]

No worries, already done! Take a look at the changes I made and lmk if they look good. Once I figure out this faliing test, I'll merge.

Looks good, but I have one gripe: https://github.com/rozbb/rust-hpke/pull/41/files#diff-ca6f6f9186921533454b89e427f1a1e994ec9073010cd16555e79d043cae98a3R186

It seems you reverted to the previous derive_keypair algorithm and not the one I introduced: rozbb/rust-hpke@e9bdd3a (#41)

This was one of the main points of my change beyond the p384 support; as the p521 crate is very close to being usable for HPKE purposes, having a generic algorithm over the length of the key is preparing a lot of things to make things easier later.

Re goldilocks: I'm not sure the crate is mature enough to expose to HPKE users. It appears to be barely maintained.

The latest release is 10 days ago. It's slowly maintained but it seems to be. And sadly, it's pretty much the only ed448 implementation in the whole Rust ecosystem :(

[rozbb]

I refactored it to explicitly take in a bitmask. I think this is more appropriate because it's explicit. The pubkey size has nothing inherently to do with the bitmask, so it doesn't make much sense to make one a function of the other.

Re goldilocks: the last commit before then was Dec. 2022. And before then Feb. 2022. If it becomes actively maintained in the near future then I'll reconsider, but for now it does not seem sufficiently supported for the intended users of this crate.

[OtaK]

I refactored it to explicitly take in a bitmask. I think this is more appropriate because it's explicit. The pubkey size has nothing inherently to do with the bitmask, so it doesn't make much sense to make one a function of the other.

You're right. I took a look at the spec and I have no idea why I was rejection sampling key_length_as_bits times instead of 256 times as indicated in the spec. My bad!

Re goldilocks: the last commit before then was Dec. 2022. And before then Feb. 2022. If it becomes actively maintained in the near future then I'll reconsider, but for now it does not seem sufficiently supported for the intended users of this crate.

Gotcha. Just so you know, my implementation lives here: https://github.com/OtaK/rust-hpke/blob/28bda9e3a148c46528c00ed745008ab01c500f0a/src/dhkex/x448.rs

It's still untested as of now, but I'll get back to it when needed.

[rozbb]

Ah I see the confusion now.

Thanks for the impl! If/when the crate matures I'll happily pull it in.

Oh and I think I deleted all the P curve test vectors! Will fix later today