WIP: secp256k1 DHKEM

[rozbb]

Started working on this and immediately hit a snag I can't figure out. The following fails

cargo check --no-default-features --features "k256,serde_impls"

with the error

error[E0277]: the trait bound `for<'a> k256::PublicKey: Deserialize<'a>` is not satisfied
   --> src/dhkex/ecdh_nistp.rs:141:34
    |
141 |                   type PublicKey = PublicKey;
    |                                    ^^^^^^^^^ the trait `for<'a> Deserialize<'a>` is not implemented for `k256::PublicKey`
...
263 | / nistp_dhkex!(
264 | |     "K-256",
265 | |     DhK256,
266 | |     k256,
...   |
270 | |     0xFF
271 | | );
    | |_- in this macro invocation
    |
    = help: the following other types implement trait `Deserialize<'de>`:
              bool
              char
              isize
              i8
              i16
              i32
              i64
              i128
            and 113 others
note: required by a bound in `DhKeyExchange::PublicKey`
   --> src/dhkex.rs:34:11
    |
27  |     type PublicKey: Clone
    |          --------- required by a bound in this associated type
...
34  |         + for<'a> SerdeDeserialize<'a>;
    |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `DhKeyExchange::PublicKey`
    = note: this error originates in the macro `nistp_dhkex` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `k256::PublicKey: Serialize` is not satisfied
   --> src/dhkex/ecdh_nistp.rs:141:34
    |
141 |                   type PublicKey = PublicKey;
    |                                    ^^^^^^^^^ the trait `Serialize` is not implemented for `k256::PublicKey`
...
263 | / nistp_dhkex!(
264 | |     "K-256",
265 | |     DhK256,
266 | |     k256,
...   |
270 | |     0xFF
271 | | );
    | |_- in this macro invocation
    |
    = help: the following other types implement trait `Serialize`:
              bool
              char
              isize
              i8
              i16
              i32
              i64
              i128
            and 114 others
note: required by a bound in `DhKeyExchange::PublicKey`
   --> src/dhkex.rs:33:11
    |
27  |     type PublicKey: Clone
    |          --------- required by a bound in this associated type
...
33  |         + SerdeSerialize
    |           ^^^^^^^^^^^^^^ required by this bound in `DhKeyExchange::PublicKey`
    = note: this error originates in the macro `nistp_dhkex` (in Nightly builds, run with -Z macro-backtrace for more info)

However, PublicKey does implement these traits.

So it seems like a dependency version issue, right? Nope. The version of serde is consistently v1.0.190 throughout the dep tree (output of cargo tree --no-default-features --features "k256,serde_impls").

Even weirder, replacing the k256 feature with p256 in the cago check makes the problem go away entirely, despite the fact that those crates have the same deps and the same source of serde impls!

Curious if I'm missing anything. Maybe @tarcieri has a quick solution. Otherwise, I'll have to spend more time on this. Also help welcome from @kwantam if you wanna use this as a jumping off point for #50 .

[rozbb]

The pkcs8 is possibly necessary because Serialize is only impld for curves which are AssociatedOid, which is only impld when pkcs8 is enabled.

[tarcieri]

Yep, the serde serializers use SPKI as the serialization format for public keys, which needs the pkcs8 feature.

I suppose it could be better documented.

[DanGould]

Thanks for developing this crate and being so on top of this new addition.

I'm developing some software that uses HPKE and OHTTP in bitcoin contexts where the secp256k1 and bitcoin_hashes crates are common dependencies. Therefore I would prefer to see secp256k1 and bitcoin_hashes crates used for DHKEM(secp256k1, HKDF-SHA256) in order to minimize dependencies in this development. I suppose preference for RustCrypto's k256 follows from the other RustCrypto elliptic-curve deps included in this crate and their shared interface.

Are there other production efforts wishing to use the k256 dependency proposed here? I would love to minimize duplicate work in such security critical software, and still do understand why k256 was the natural choice here.

[tarcieri]

@DanGould the main advantages of k256 are it’s pure Rust and shares a common trait-based API with p256, whereas secp256k1 is an FFI wrapper for a C library.

[rozbb]

@kwantam just rebased on main. Now that serde support is removed, we don't have any errors anymore. This is pretty close to the final product. All it needs is some test vectors (marked via todo!(), which you can see in the failing tests), and some data for kat_tests.rs to work with.

You probably can't push to this branch, so feel free to fork it and make a new PR when you're ready.

[dongcarl]

Should probably also rename the macro:

diff --git a/src/dhkex/ecdh_nist.rs b/src/dhkex/ecdh_nist.rs
index c2702e0..36df025 100644
--- a/src/dhkex/ecdh_nist.rs
+++ b/src/dhkex/ecdh_nist.rs
@@ -1,5 +1,5 @@
 // We define all the NIST P- curve ECDH functionalities in one macro
-macro_rules! nistp_dhkex {
+macro_rules! nist_dhkex {
     (
         $curve_name:expr,
         $dh_name:ident,
@@ -238,7 +238,7 @@ macro_rules! nistp_dhkex {
 use generic_array::typenum;
 
 #[cfg(feature = "p256")]
-nistp_dhkex!(
+nist_dhkex!(
     "P-256",
     DhP256,
     p256,
@@ -249,7 +249,7 @@ nistp_dhkex!(
 );
 
 #[cfg(feature = "p384")]
-nistp_dhkex!(
+nist_dhkex!(
     "P-384",
     DhP384,
     p384,
@@ -260,7 +260,7 @@ nistp_dhkex!(
 );
 
 #[cfg(feature = "k256")]
-nistp_dhkex!(
+nist_dhkex!(
     "K-256",
     DhK256,
     k256,

[DanGould]

I was able to rebase on main, come up with some test vectors, and take Carl's macro naming advice in #59 which appears to pass tests for k256 now.