From 50ab92c013939b0946256338fcdc7101dc5bfcf9 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Wed, 16 Feb 2022 16:30:47 +0000 Subject: [PATCH] Use fixed width serde impls for secret/public key Currently we serialize `SecretKey` and `PublicKey` using the `BytesVisitor`, this causes the serialized data to contain additional metadata encoding the length (an extra 8 bytes). This extra data is unnecessary since we know in advance the length of these two types (32 and 33 bytes respectively). Implement a sequence based visitor that encodes the keys as fixed width data. --- src/key.rs | 43 +++++++++++++++++++++++++++++++++---------- src/serde_util.rs | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 10 deletions(-) diff --git a/src/key.rs b/src/key.rs index 7f77d4115..cc98a09eb 100644 --- a/src/key.rs +++ b/src/key.rs @@ -28,6 +28,9 @@ use Verification; use constants; use ffi::{self, CPtr}; +#[cfg(feature = "serde")] +use serde::ser::SerializeTuple; + #[cfg(feature = "global-context")] use {Message, ecdsa, SECP256K1}; #[cfg(all(feature = "global-context", feature = "rand-std"))] @@ -302,7 +305,11 @@ impl ::serde::Serialize for SecretKey { let mut buf = [0u8; 64]; s.serialize_str(::to_hex(&self.0, &mut buf).expect("fixed-size hex serialization")) } else { - s.serialize_bytes(&self[..]) + let mut seq = s.serialize_tuple(constants::SECRET_KEY_SIZE)?; + for byte in self.0.iter() { + seq.serialize_element(byte)?; + } + seq.end() } } } @@ -316,7 +323,7 @@ impl<'de> ::serde::Deserialize<'de> for SecretKey { "a hex string representing 32 byte SecretKey" )) } else { - d.deserialize_bytes(super::serde_util::BytesVisitor::new( + d.deserialize_seq(super::serde_util::SeqVisitor::new( "raw 32 bytes SecretKey", SecretKey::from_slice )) @@ -618,7 +625,11 @@ impl ::serde::Serialize for PublicKey { if s.is_human_readable() { s.collect_str(self) } else { - s.serialize_bytes(&self.serialize()) + let mut seq = s.serialize_tuple(constants::PUBLIC_KEY_SIZE)?; + for byte in self.serialize() { + seq.serialize_element(&byte)?; + } + seq.end() } } } @@ -632,7 +643,7 @@ impl<'de> ::serde::Deserialize<'de> for PublicKey { "an ASCII hex string representing a public key" )) } else { - d.deserialize_bytes(super::serde_util::BytesVisitor::new( + d.deserialize_seq(super::serde_util::SeqVisitor::new( "a bytestring representing a public key", PublicKey::from_slice )) @@ -1911,6 +1922,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, @@ -1933,17 +1945,28 @@ 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)]); diff --git a/src/serde_util.rs b/src/serde_util.rs index bc815bc5e..55fc49583 100644 --- a/src/serde_util.rs +++ b/src/serde_util.rs @@ -67,3 +67,44 @@ where (self.parse_fn)(v).map_err(E::custom) } } + +pub struct SeqVisitor { + expectation: &'static str, + parse_fn: F, +} + +impl SeqVisitor +where + F: FnOnce(&[u8]) -> Result, + Err: fmt::Display, +{ + pub fn new(expectation: &'static str, parse_fn: F) -> Self { + SeqVisitor { + expectation, + parse_fn, + } + } +} + +impl<'de, F, T, Err> de::Visitor<'de> for SeqVisitor +where + F: FnOnce(&[u8]) -> Result, + Err: 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 v = Vec::new(); + while let Some(value) = seq.next_element()? { + v.push(value) + } + (self.parse_fn)(&v).map_err(de::Error::custom) + } +}