dalek-cryptography / x25519-dalek

X25519 elliptic curve Diffie-Hellman key exchange in pure-Rust, using curve25519-dalek.
BSD 3-Clause "New" or "Revised" License
328 stars 133 forks source link

Feedback/Suggestion #57

Closed mimoo closed 2 years ago

mimoo commented 4 years ago

Hey!

I've been implementing Noise with x25519-dalek and I have a few suggestions:

(happy to take a stab at a PR if you want me to)

mimoo commented 4 years ago

I still like the idea of an ephemeral key API, I think it'd be great to pretty much have it behave like a normal key except that you can't export/serialize it,.

hdevalence commented 4 years ago

Thanks for the feedback! Regarding the first point, consuming the private key ensures that the compiler statically checks that no ephemeral key is used more than once (i.e., is actually ephemeral). Does the updated documentation from #61 (rendered here and here) help the situation?

There's currently no public_key method because the conversion is done by a From impl as PublicKey::from(&secret). Do you think that there is an ergonomic benefit to also providing a public_key? That seems reasonable, as does try_from on slices.

mimoo commented 4 years ago

Thanks for the feedback! Regarding the first point, consuming the private key ensures that the compiler statically checks that no ephemeral key is used more than once (i.e., is actually ephemeral). Does the updated documentation from #61 (rendered here and here) help the situation?

not really, in my case I'm implementing Noise IK and I need to use the ephemeral key twice: https://github.com/libra/libra/blob/master/crypto/crypto/src/noise.rs#L288 so I had to use PrivateKey instead, which makes the key serializable and it would have been nice if it wasn't. I guess if you really want to have a "use only once key" then you'd need another type "semi-ephemeral" (according to your definition of ephemeral). But then it'll start to get confusing :) I was just suggesting that having non-serializable ephemeral keys is more useful than having ephemeral keys that get consumed on first use, at least from my own usage, but I'm just one person so don't jump to conclusion from that :D

There's currently no public_key method because the conversion is done by a From impl as PublicKey::from(&secret). Do you think that there is an ergonomic benefit to also providing a public_key? That seems reasonable, as does try_from on slices.

The From is not clear enough in my opinion. I would usually reserve From implementations for straight forward conversions that you don't need to think about. Here we're doing an import conversion from a private key to a public key so this warrants an explicit function (which I wrote as a wrapper here at the moment: https://github.com/libra/libra/blob/master/crypto/crypto/src/noise.rs#L289)

isislovecruft commented 3 years ago

Hi!

We're super happy to support Noise implementations, but honestly this seems like a fairly niche use case? (Most uses of "ephemeral" keys are not "use this one-time-use key twice".) And even though Noise might define a potential keying arrangement which requires this, that still doesn't necessarily preclude end-users at a higher level using that potential configuration in their protocols. Is there any reason why it would be infeasible to bypass our type-/memory- safety and directly, or indirectly with some sort of hypothetical "use-twice" wrapper struct, use our x25519() function?

mimoo commented 3 years ago

this seems like a fairly niche use case

Really? X3DH will require your ephemeral key to be used several times also, for example. It doesn't seem niche to me but maybe I'm wrong.

isislovecruft commented 2 years ago

This will be in 1.1.0 which I plan to release shortly.