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 adding support for serde #47

Closed VictorKoenders closed 4 years ago

VictorKoenders commented 4 years ago

It would be nice if the PublicKey would derive serde::Serialize and serde::Deserialize. Serde is the most used serialization library for Rust, and is presumably used a lot in network communication.

Serde support would change the following code:

enum NetworkPacket {
    SendPublicKey([u8; 32])
}

// sender
let key: PublicKey = ...;
let bytes = bincode::serialize(&NetworkPacket::SendPublicKey(key.as_bytes().clone())).unwrap();
// receiver
let buffer: &[u8] = ..;
let packet: NetworkPacket = bincode::deserialize(buffer).unwrap();
match packet {
    NetworkPacket::SendPublicKey(key) => {
        let key: PublicKey = key.into();
    }
}

To:

enum NetworkPacket {
    SendPublicKey(PublicKey)
}

// sender
let key: PublicKey = ...;
let bytes = bincode::serialize(&NetworkPacket::SendPublicKey(key)).unwrap();
// receiver
let buffer: &[u8] = ..;
let packet: NetworkPacket = bincode::deserialize(buffer).unwrap();
match packet {
    NetworkPacket::SendPublicKey(key) => {
        // key is already of type `PublicKey`
    }
}

Which is slightly nicer to work with.

This could also be implemented for StaticSecret, as this is intended to be able to be saved and loaded.

I'd suggest adding a feature called serde, which enables/disables the serde dependency and is disabled by default. When this feature is enabled, PublicKey implements serde::Serialize and serde::Deserialize.

I can implement this if you want me to.

hdevalence commented 4 years ago

I think this would be a great feature, but the serde support in the underlying library will probably change soon, because it was implemented suboptimally/incorrectly: https://github.com/dalek-cryptography/curve25519-dalek/issues/265

I have been very busy with a work project for the last few weeks, but I am hoping to close it out this Friday, so we could possibly revisit this issue next week?

VictorKoenders commented 4 years ago

This is just a minor thing I thought could improve. Don't worry about implementing this if you have things to do in real life, that obviously has priority. I'll look at the curve change and after that's implemented I'll revisit this issue.

hdevalence commented 4 years ago

Closed by #48 :)