dalek-cryptography / curve25519-dalek

A pure-Rust implementation of group operations on Ristretto and Curve25519
Other
853 stars 422 forks source link

{Signing,Verifying}KeyVisitor: visit_borrowed_bytes -> visit_bytes #602

Closed awesterb closed 8 months ago

awesterb commented 8 months ago

This is a slight improvement to the deserialisation code for SigningKey and VerifyingKey.

The default implementations of visit_borrowed_bytes and visit_byte_buf forward to visit_bytes, but only visit_borrowed_bytes was implemented for the {Signing,Verifying}KeyVisitor. This means that {Signing,Verifying}Keys can only be deserialised from borrowed byte arrays, but not from owned or transient byte arrays. Example:

let bytes = base16ct::lower::decode_vec(
    "66b1419fae979516fb3807dda1b05026b2570a7ab2190254e524af4f0934ddd2",
)
.unwrap();

let d = serde::de::value::BorrowedBytesDeserializer::<serde::de::value::Error>::new(&bytes);
ed25519_dalek::VerifyingKey::deserialize(d).unwrap(); // deserialising from a borrowed byte array works

let d = serde::de::value::BytesDeserializer::<serde::de::value::Error>::new(&bytes);
ed25519_dalek::VerifyingKey::deserialize(d).unwrap_err(); // but not from a transient byte array

Implementing visit_bytes instead of visit_borrowed_bytes enables deserialisation from all three byte array types.

rozbb commented 8 months ago

Thanks! This makes sense. The only reason we'd implement visit_borrowed_bytes separately is if we had a zero-copy deserialization routine for these keys. But all of them copy, so there's no special case here.

robjtede commented 6 months ago

Worth mentioning that this same issue was just filed over on the old repo (https://github.com/dalek-cryptography/ed25519-dalek/issues/315) and this fix hasn't been released yet.