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

Consider zeroize on drop for PublicKey #68

Closed david415 closed 3 years ago

david415 commented 3 years ago

Would you consider adding the zeroize on drop feature for the PublicKey type? This feature implies not auto deriving the Copy trait for PublicKey which might break compatibility for some projects using this crate.

I think this is an important feature and unfortunately it's a very common belief that it's only important to zeroize private key materials. Whereas we would like to achieve the plausibly deniability properties that come from destroying public keys as well.

The context for this request is steeped in the slow crypto train movement: a slow messaging system, similar to agl's Pond in some ways, should have a "panic button" that removes all ciphertext, public and private key materials and so on. People in high risk situations might like to press such a panic button when the secret police show up.

What do you think? I'm more than happy to submit a pull request if this is something you'd merge.

isislovecruft commented 3 years ago

Hi @david415!

Our story so far about Zeroize on Drop for nominally "public" types has been that, should a project need this, they should implement a wrapper type (e.g. struct MyPublicKey(pub(crate) PublicKey), etc.) and implement custom Drop on those. Unfortunately, due to point type conversions internally and other factors within the dalek crates, as well as other use cases, implementing it for everything would likely substantiate an enormous performance regression in a lot of places. I'd be willing to discuss potentially something like a new EphemeralPublicKey type (probably with a better name hopefully) but I'd want to think about how to phrase that to developers so they're more likely to understand the privacy/performance tradeoffs in a way that enable them to make the best decision for their use-case.

isislovecruft commented 3 years ago

I think a thing we'd be more likely to consider, as we've recently done in the curve25519-dalek crate, is to implement Zeroize only, so that yours or someone else's wrapper type would have an easier time deriving Zeroize, and then implementing a custom drop (or deriving one) which utilises it. Would that be a usable compromise for your case @david415?

isislovecruft commented 3 years ago

As of #69 we now have Zeroize for PublicKey, and consumers of this library are able to make a wrapper type which derives it for Drop should they wish to explicitly erase public x25519 keys from memory. Hopefully this is a reasonable compromise for most use-cases where forgetting public keys is important, without degrading performance for other users. If not, or if there are further concerns, please feel free to reopen this issue.