diff --git a/Cargo.toml b/Cargo.toml index 7ea750c..e3eb9a4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,12 @@ rand = "0.8" rand_core = "0.6" serde_test = "1.0" bitcoin_hashes = "0.10" +bincode = "1.3.3" + +# cbor does not build on WASM, we use it in a single trivial test (an example of when +# fixed-width-serde breaks down). Just run the test when on an x86_64 machine. +[target.'cfg(target_arch = "x86_64")'.dependencies] +cbor = "0.4.1" [target.wasm32-unknown-unknown.dev-dependencies] wasm-bindgen-test = "0.3" diff --git a/src/key.rs b/src/key.rs index 8d883aa..4e09c5f 100644 --- a/src/key.rs +++ b/src/key.rs @@ -25,6 +25,9 @@ use crate::Error::{self, InvalidPublicKey, InvalidPublicKeySum, InvalidSecretKey use crate::ffi::{self, CPtr, impl_array_newtype}; use crate::ffi::types::c_uint; +#[cfg(feature = "serde")] +use serde::ser::SerializeTuple; + #[cfg(feature = "global-context")] use crate::{Message, ecdsa, SECP256K1}; #[cfg(all(feature = "global-context", feature = "rand-std"))] @@ -33,6 +36,12 @@ use crate::Scalar; /// Secret 256-bit key used as `x` in an ECDSA signature. /// +/// # Serde support +/// +/// Implements de/serialization with the `serde` feature enabled. We treat the byte value as a tuple +/// of 32 `u8`s for non-human-readable formats. This representation is optimal for for some formats +/// (e.g. [`bincode`]) however other formats may be less optimal (e.g. [`cbor`]). +/// /// # Examples /// /// Basic usage: @@ -45,6 +54,8 @@ use crate::Scalar; /// let secret_key = SecretKey::new(&mut rand::thread_rng()); /// # } /// ``` +/// [`bincode`]: https://docs.rs/bincode +/// [`cbor`]: https://docs.rs/cbor pub struct SecretKey([u8; constants::SECRET_KEY_SIZE]); impl_array_newtype!(SecretKey, u8, constants::SECRET_KEY_SIZE); impl_display_secret!(SecretKey); @@ -68,6 +79,12 @@ pub const ONE_KEY: SecretKey = SecretKey([0, 0, 0, 0, 0, 0, 0, 0, /// A Secp256k1 public key, used for verification of signatures. /// +/// # Serde support +/// +/// Implements de/serialization with the `serde` feature enabled. We treat the byte value as a tuple +/// of 33 `u8`s for non-human-readable formats. This representation is optimal for for some formats +/// (e.g. [`bincode`]) however other formats may be less optimal (e.g. [`cbor`]). +/// /// # Examples /// /// Basic usage: @@ -81,6 +98,8 @@ pub const ONE_KEY: SecretKey = SecretKey([0, 0, 0, 0, 0, 0, 0, 0, /// let public_key = PublicKey::from_secret_key(&secp, &secret_key); /// # } /// ``` +/// [`bincode`]: https://docs.rs/bincode +/// [`cbor`]: https://docs.rs/cbor #[derive(Copy, Clone, PartialEq, Eq, Debug, Hash)] #[repr(transparent)] pub struct PublicKey(ffi::PublicKey); @@ -315,7 +334,11 @@ impl serde::Serialize for SecretKey { let mut buf = [0u8; constants::SECRET_KEY_SIZE * 2]; s.serialize_str(crate::to_hex(&self.0, &mut buf).expect("fixed-size hex serialization")) } else { - s.serialize_bytes(&self[..]) + let mut tuple = s.serialize_tuple(constants::SECRET_KEY_SIZE)?; + for byte in self.0.iter() { + tuple.serialize_element(byte)?; + } + tuple.end() } } } @@ -329,10 +352,11 @@ impl<'de> serde::Deserialize<'de> for SecretKey { "a hex string representing 32 byte SecretKey" )) } else { - d.deserialize_bytes(super::serde_util::BytesVisitor::new( + let visitor = super::serde_util::Tuple32Visitor::new( "raw 32 bytes SecretKey", SecretKey::from_slice - )) + ); + d.deserialize_tuple(constants::SECRET_KEY_SIZE, visitor) } } } @@ -649,7 +673,11 @@ impl serde::Serialize for PublicKey { if s.is_human_readable() { s.collect_str(self) } else { - s.serialize_bytes(&self.serialize()) + let mut tuple = s.serialize_tuple(constants::PUBLIC_KEY_SIZE)?; + for byte in self.serialize().iter() { // Serialize in compressed form. + tuple.serialize_element(&byte)?; + } + tuple.end() } } } @@ -663,10 +691,11 @@ impl<'de> serde::Deserialize<'de> for PublicKey { "an ASCII hex string representing a public key" )) } else { - d.deserialize_bytes(super::serde_util::BytesVisitor::new( - "a bytestring representing a public key", + let visitor = super::serde_util::Tuple33Visitor::new( + "33 bytes compressed public key", PublicKey::from_slice - )) + ); + d.deserialize_tuple(constants::PUBLIC_KEY_SIZE, visitor) } } } @@ -687,13 +716,10 @@ impl Ord for PublicKey { /// /// # Serde support /// -/// [`Serialize`] and [`Deserialize`] are not implemented for this type, even with the `serde` -/// feature active. This is due to security considerations, see the [`serde_keypair`] documentation -/// for details. -/// -/// If the `serde` and `global-context` features are active `KeyPair`s can be serialized and -/// deserialized by annotating them with `#[serde(with = "secp256k1::serde_keypair")]` -/// inside structs or enums for which [`Serialize`] and [`Deserialize`] are being derived. +/// Implements de/serialization with the `serde` and_`global-context` features enabled. Serializes +/// the secret bytes only. We treat the byte value as a tuple of 32 `u8`s for non-human-readable +/// formats. This representation is optimal for for some formats (e.g. [`bincode`]) however other +/// formats may be less optimal (e.g. [`cbor`]). For human-readable formats we use a hex string. /// /// # Examples /// @@ -708,8 +734,8 @@ impl Ord for PublicKey { /// let key_pair = KeyPair::from_secret_key(&secp, &secret_key); /// # } /// ``` -/// [`Deserialize`]: serde::Deserialize -/// [`Serialize`]: serde::Serialize +/// [`bincode`]: https://docs.rs/bincode +/// [`cbor`]: https://docs.rs/cbor #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct KeyPair(ffi::KeyPair); impl_display_secret!(KeyPair); @@ -966,7 +992,11 @@ impl serde::Serialize for KeyPair { s.serialize_str(crate::to_hex(&self.secret_bytes(), &mut buf) .expect("fixed-size hex serialization")) } else { - s.serialize_bytes(&self.0[..]) + let mut tuple = s.serialize_tuple(constants::SECRET_KEY_SIZE)?; + for byte in self.secret_bytes().iter() { + tuple.serialize_element(&byte)?; + } + tuple.end() } } } @@ -980,19 +1010,26 @@ impl<'de> serde::Deserialize<'de> for KeyPair { "a hex string representing 32 byte KeyPair" )) } else { - d.deserialize_bytes(super::serde_util::BytesVisitor::new( + let visitor = super::serde_util::Tuple32Visitor::new( "raw 32 bytes KeyPair", |data| unsafe { let ctx = Secp256k1::from_raw_all(ffi::secp256k1_context_no_precomp as *mut ffi::Context); KeyPair::from_seckey_slice(&ctx, data) } - )) + ); + d.deserialize_tuple(constants::SECRET_KEY_SIZE, visitor) } } } /// An x-only public key, used for verification of Schnorr signatures and serialized according to BIP-340. /// +/// # Serde support +/// +/// Implements de/serialization with the `serde` feature enabled. We treat the byte value as a tuple +/// of 32 `u8`s for non-human-readable formats. This representation is optimal for for some formats +/// (e.g. [`bincode`]) however other formats may be less optimal (e.g. [`cbor`]). +/// /// # Examples /// /// Basic usage: @@ -1006,6 +1043,8 @@ impl<'de> serde::Deserialize<'de> for KeyPair { /// let xonly = XOnlyPublicKey::from_keypair(&key_pair); /// # } /// ``` +/// [`bincode`]: https://docs.rs/bincode +/// [`cbor`]: https://docs.rs/cbor #[derive(Copy, Clone, PartialEq, Eq, Debug, PartialOrd, Ord, Hash)] pub struct XOnlyPublicKey(ffi::XOnlyPublicKey); @@ -1429,7 +1468,11 @@ impl serde::Serialize for XOnlyPublicKey { if s.is_human_readable() { s.collect_str(self) } else { - s.serialize_bytes(&self.serialize()) + let mut tuple = s.serialize_tuple(constants::SCHNORR_PUBLIC_KEY_SIZE)?; + for byte in self.serialize().iter() { + tuple.serialize_element(&byte)?; + } + tuple.end() } } } @@ -1443,10 +1486,11 @@ impl<'de> serde::Deserialize<'de> for XOnlyPublicKey { "a hex string representing 32 byte schnorr public key" )) } else { - d.deserialize_bytes(super::serde_util::BytesVisitor::new( + let visitor = super::serde_util::Tuple32Visitor::new( "raw 32 bytes schnorr public key", XOnlyPublicKey::from_slice - )) + ); + d.deserialize_tuple(constants::SCHNORR_PUBLIC_KEY_SIZE, visitor) } } } @@ -1492,6 +1536,8 @@ pub mod serde_keypair { #[cfg(test)] #[allow(unused_imports)] mod test { + use super::*; + use core::str::FromStr; #[cfg(any(feature = "alloc", feature = "std"))] @@ -1949,6 +1995,7 @@ mod test { static SK_STR: &'static str = "\ 01010101010101010001020304050607ffff0000ffff00006363636363636363\ "; + #[cfg(fuzzing)] static PK_BYTES: [u8; 33] = [ 0x02, 0x18, 0x84, 0x57, 0x81, 0xf6, 0x31, 0xc4, 0x8f, @@ -1971,22 +2018,32 @@ mod test { #[cfg(fuzzing)] let pk = PublicKey::from_slice(&PK_BYTES).expect("pk"); - assert_tokens(&sk.compact(), &[Token::BorrowedBytes(&SK_BYTES[..])]); - assert_tokens(&sk.compact(), &[Token::Bytes(&SK_BYTES)]); - assert_tokens(&sk.compact(), &[Token::ByteBuf(&SK_BYTES)]); + assert_tokens(&sk.compact(), &[ + Token::Tuple{ len: 32 }, + Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1), + Token::U8(0), Token::U8(1), Token::U8(2), Token::U8(3), Token::U8(4), Token::U8(5), Token::U8(6), Token::U8(7), + Token::U8(0xff), Token::U8(0xff), Token::U8(0), Token::U8(0), Token::U8(0xff), Token::U8(0xff), Token::U8(0), Token::U8(0), + Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99), + Token::TupleEnd + ]); assert_tokens(&sk.readable(), &[Token::BorrowedStr(SK_STR)]); assert_tokens(&sk.readable(), &[Token::Str(SK_STR)]); assert_tokens(&sk.readable(), &[Token::String(SK_STR)]); - assert_tokens(&pk.compact(), &[Token::BorrowedBytes(&PK_BYTES[..])]); - assert_tokens(&pk.compact(), &[Token::Bytes(&PK_BYTES)]); - assert_tokens(&pk.compact(), &[Token::ByteBuf(&PK_BYTES)]); + assert_tokens(&pk.compact(), &[ + Token::Tuple{ len: 33 }, + Token::U8(0x02), + Token::U8(0x18), Token::U8(0x84), Token::U8(0x57), Token::U8(0x81), Token::U8(0xf6), Token::U8(0x31), Token::U8(0xc4), Token::U8(0x8f), + Token::U8(0x1c), Token::U8(0x97), Token::U8(0x09), Token::U8(0xe2), Token::U8(0x30), Token::U8(0x92), Token::U8(0x06), Token::U8(0x7d), + Token::U8(0x06), Token::U8(0x83), Token::U8(0x7f), Token::U8(0x30), Token::U8(0xaa), Token::U8(0x0c), Token::U8(0xd0), Token::U8(0x54), + Token::U8(0x4a), Token::U8(0xc8), Token::U8(0x87), Token::U8(0xfe), Token::U8(0x91), Token::U8(0xdd), Token::U8(0xd1), Token::U8(0x66), + Token::TupleEnd + ]); assert_tokens(&pk.readable(), &[Token::BorrowedStr(PK_STR)]); assert_tokens(&pk.readable(), &[Token::Str(PK_STR)]); assert_tokens(&pk.readable(), &[Token::String(PK_STR)]); - } #[test] @@ -2066,10 +2123,14 @@ mod test { "; let sk = KeyPairWrapper(KeyPair::from_seckey_slice(&crate::SECP256K1, &SK_BYTES).unwrap()); - - assert_tokens(&sk.compact(), &[Token::BorrowedBytes(&SK_BYTES[..])]); - assert_tokens(&sk.compact(), &[Token::Bytes(&SK_BYTES)]); - assert_tokens(&sk.compact(), &[Token::ByteBuf(&SK_BYTES)]); + assert_tokens(&sk.compact(), &[ + Token::Tuple{ len: 32 }, + Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1), + Token::U8(0), Token::U8(1), Token::U8(2), Token::U8(3), Token::U8(4), Token::U8(5), Token::U8(6), Token::U8(7), + Token::U8(0xff), Token::U8(0xff), Token::U8(0), Token::U8(0), Token::U8(0xff), Token::U8(0xff), Token::U8(0), Token::U8(0), + Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99), + Token::TupleEnd + ]); assert_tokens(&sk.readable(), &[Token::BorrowedStr(SK_STR)]); assert_tokens(&sk.readable(), &[Token::Str(SK_STR)]); @@ -2221,4 +2282,38 @@ mod test { assert_eq!(got, want) } + + #[test] + #[cfg(not(fuzzing))] + #[cfg(all(feature = "global-context", feature = "serde"))] + fn test_serde_x_only_pubkey() { + use serde_test::{Configure, Token, assert_tokens}; + + static SK_BYTES: [u8; 32] = [ + 1, 1, 1, 1, 1, 1, 1, 1, + 0, 1, 2, 3, 4, 5, 6, 7, + 0xff, 0xff, 0, 0, 0xff, 0xff, 0, 0, + 99, 99, 99, 99, 99, 99, 99, 99 + ]; + + static PK_STR: &'static str = "\ + 18845781f631c48f1c9709e23092067d06837f30aa0cd0544ac887fe91ddd166\ + "; + + let kp = KeyPair::from_seckey_slice(&crate::SECP256K1, &SK_BYTES).unwrap(); + let (pk, _parity) = XOnlyPublicKey::from_keypair(&kp); + + assert_tokens(&pk.compact(), &[ + Token::Tuple{ len: 32 }, + Token::U8(0x18), Token::U8(0x84), Token::U8(0x57), Token::U8(0x81), Token::U8(0xf6), Token::U8(0x31), Token::U8(0xc4), Token::U8(0x8f), + Token::U8(0x1c), Token::U8(0x97), Token::U8(0x09), Token::U8(0xe2), Token::U8(0x30), Token::U8(0x92), Token::U8(0x06), Token::U8(0x7d), + Token::U8(0x06), Token::U8(0x83), Token::U8(0x7f), Token::U8(0x30), Token::U8(0xaa), Token::U8(0x0c), Token::U8(0xd0), Token::U8(0x54), + Token::U8(0x4a), Token::U8(0xc8), Token::U8(0x87), Token::U8(0xfe), Token::U8(0x91), Token::U8(0xdd), Token::U8(0xd1), Token::U8(0x66), + Token::TupleEnd + ]); + + assert_tokens(&pk.readable(), &[Token::BorrowedStr(PK_STR)]); + assert_tokens(&pk.readable(), &[Token::Str(PK_STR)]); + assert_tokens(&pk.readable(), &[Token::String(PK_STR)]); + } } diff --git a/src/schnorr.rs b/src/schnorr.rs index 5061568..cefaee4 100644 --- a/src/schnorr.rs +++ b/src/schnorr.rs @@ -569,9 +569,14 @@ mod tests { assert_tokens(&sig.readable(), &[Token::Str(SIG_STR)]); assert_tokens(&sig.readable(), &[Token::String(SIG_STR)]); - assert_tokens(&pk.compact(), &[Token::BorrowedBytes(&PK_BYTES[..])]); - assert_tokens(&pk.compact(), &[Token::Bytes(&PK_BYTES[..])]); - assert_tokens(&pk.compact(), &[Token::ByteBuf(&PK_BYTES[..])]); + assert_tokens(&pk.compact(), &[ + Token::Tuple{ len: 32 }, + Token::U8(24), Token::U8(132), Token::U8(87), Token::U8(129), Token::U8(246), Token::U8(49), Token::U8(196), Token::U8(143), + Token::U8(28), Token::U8(151), Token::U8(9), Token::U8(226), Token::U8(48), Token::U8(146), Token::U8(6), Token::U8(125), + Token::U8(6), Token::U8(131), Token::U8(127), Token::U8(48), Token::U8(170), Token::U8(12), Token::U8(208), Token::U8(84), + Token::U8(74), Token::U8(200), Token::U8(135), Token::U8(254), Token::U8(145), Token::U8(221), Token::U8(209), Token::U8(102), + Token::TupleEnd + ]); assert_tokens(&pk.readable(), &[Token::BorrowedStr(PK_STR)]); assert_tokens(&pk.readable(), &[Token::Str(PK_STR)]); diff --git a/src/serde_util.rs b/src/serde_util.rs index bc815bc..a439080 100644 --- a/src/serde_util.rs +++ b/src/serde_util.rs @@ -67,3 +67,56 @@ where (self.parse_fn)(v).map_err(E::custom) } } + +macro_rules! impl_tuple_visitor { + ($thing:ident, $len:expr) => { + pub(crate) struct $thing { + expectation: &'static str, + parse_fn: F, + } + + impl $thing + where + F: FnOnce(&[u8]) -> Result, + E: fmt::Display, + { + pub fn new(expectation: &'static str, parse_fn: F) -> Self { + $thing { + expectation, + parse_fn, + } + } + } + + impl<'de, F, T, E> de::Visitor<'de> for $thing + where + F: FnOnce(&[u8]) -> Result, + E: fmt::Display, + { + type Value = T; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str(self.expectation) + } + + fn visit_seq(self, mut seq: V) -> Result + where + V: de::SeqAccess<'de>, + { + let mut bytes = [0u8; $len]; + + for (i, byte) in bytes.iter_mut().enumerate() { + if let Some(value) = seq.next_element()? { + *byte = value; + } else { + return Err(de::Error::invalid_length(i, &self)); + } + } + (self.parse_fn)(&bytes).map_err(de::Error::custom) + } + } + } +} + +impl_tuple_visitor!(Tuple32Visitor, 32); +impl_tuple_visitor!(Tuple33Visitor, 33); diff --git a/tests/serde.rs b/tests/serde.rs new file mode 100644 index 0000000..f0938f8 --- /dev/null +++ b/tests/serde.rs @@ -0,0 +1,87 @@ +#![cfg(feature = "serde")] + +extern crate bincode; +#[cfg(target_arch = "x86_64")] +extern crate cbor; +extern crate secp256k1; + +use secp256k1::{PublicKey, SecretKey, XOnlyPublicKey}; +#[cfg(feature = "global-context")] +use secp256k1::{Secp256k1, KeyPair}; + +// Arbitrary key data. + +static SK_BYTES: [u8; 32] = [ + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x16, 0x17, 0x18, 0x19, 0x20, 0x21, 0x22, 0x23, + 0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x30, 0x31, + 0x0f, 0x10, 0x1f, 0xa0, 0xa9, 0xaa, 0xaf, 0xff, +]; + +static PK_BYTES: [u8; 33] = [ + 0x02, + 0x18, 0x84, 0x57, 0x81, 0xf6, 0x31, 0xc4, 0x8f, + 0x1c, 0x97, 0x09, 0xe2, 0x30, 0x92, 0x06, 0x7d, + 0x06, 0x83, 0x7f, 0x30, 0xaa, 0x0c, 0xd0, 0x54, + 0x4a, 0xc8, 0x87, 0xfe, 0x91, 0xdd, 0xd1, 0x66, +]; + +static XONLY_PK_BYTES: [u8; 32] = [ + 0x18, 0x84, 0x57, 0x81, 0xf6, 0x31, 0xc4, 0x8f, + 0x1c, 0x97, 0x09, 0xe2, 0x30, 0x92, 0x06, 0x7d, + 0x06, 0x83, 0x7f, 0x30, 0xaa, 0x0c, 0xd0, 0x54, + 0x4a, 0xc8, 0x87, 0xfe, 0x91, 0xdd, 0xd1, 0x66, +]; + +fn secret_key() -> SecretKey { + SecretKey::from_slice(&SK_BYTES).expect("failed to create sk from slice") +} + +// Our current serde serialization implementation is only guaranteed to be fixed +// width for bincode. https://docs.rs/bincode/latest/bincode/index.html +#[test] +fn bincode_secret_key() { + let sk = secret_key(); + let ser = bincode::serialize(&sk).unwrap(); + + assert_eq!(ser, SK_BYTES); +} + +#[test] +fn bincode_public_key() { + let pk = PublicKey::from_slice(&PK_BYTES).expect("failed to create pk from slice"); + let ser = bincode::serialize(&pk).unwrap(); + + assert_eq!(ser, &PK_BYTES as &[u8]) +} + +#[test] +#[cfg(feature = "global-context")] +fn bincode_key_pair() { + let secp = Secp256k1::new(); + let kp = KeyPair::from_seckey_slice(&secp, &SK_BYTES).expect("failed to create keypair"); + let ser = bincode::serialize(&kp).unwrap(); + + assert_eq!(ser, SK_BYTES); +} + +#[test] +fn bincode_x_only_public_key() { + let pk = XOnlyPublicKey::from_slice(&XONLY_PK_BYTES).expect("failed to create xonly pk from slice"); + let ser = bincode::serialize(&pk).unwrap(); + + assert_eq!(ser, XONLY_PK_BYTES); +} + +// cbor adds an additional byte of metadata to certain byte values (byte_value < 24). +#[test] +#[cfg(target_arch = "x86_64")] +fn cbor() { + let sk = secret_key(); + + let mut e = cbor::Encoder::from_memory(); + e.encode(sk.as_ref()).unwrap(); + + // 52 because there are 22 bytes in the key for which cbor adds metadata. + assert_eq!(e.as_bytes().len(), 52); +}