From dbd314b9d649b9c327b0d43ee583bdf6c1329fb6 Mon Sep 17 00:00:00 2001 From: Bogdan Opanchuk Date: Tue, 1 Aug 2023 23:05:33 -0700 Subject: [PATCH] Make array logic fall back to the slice logic Since serde does not support uniform serialization of fixed-sized arrays. --- serdect/README.md | 19 ++-- serdect/src/array.rs | 93 ++++--------------- serdect/src/lib.rs | 15 ++- serdect/src/slice.rs | 175 +++++++++++++++++++++-------------- serdect/tests/bincode.rs | 2 +- serdect/tests/cbor.rs | 2 +- serdect/tests/messagepack.rs | 2 +- 7 files changed, 150 insertions(+), 158 deletions(-) diff --git a/serdect/README.md b/serdect/README.md index 39e6f3b1f..ac933c7b7 100644 --- a/serdect/README.md +++ b/serdect/README.md @@ -29,17 +29,14 @@ While this crate can't ensure that format implementations don't perform other kinds of data-dependent branching on the contents of the serialized data, using a constant-time hex serialization with human-readable formats should help reduce the overall timing variability. -The table below lists the crates `serdect` is tested against. -❌ marks the cases for which the serialization is not constant-size for a given data size (due to the format limitations), and therefore will not be constant-time too. - -| Crate | `array` | `slice` | -|--------------------------------------------------------------------|:--------:|:--------:| -| [`bincode`](https://crates.io/crates/bincode) v1 | ✅ | ✅ | -| [`ciborium`](https://crates.io/crates/ciborium) v0.2 | ❌ | ✅ | -| [`rmp-serde`](https://crates.io/crates/rmp-serde) v1 | ❌ | ✅ | -| [`serde-json-core`](https://crates.io/crates/serde-json-core) v0.5 | ✅ | ✅ | -| [`serde-json`](https://crates.io/crates/serde-json) v1 | ✅ | ✅ | -| [`toml`](https://crates.io/crates/toml) v0.7 | ✅ | ✅ | + +`serdect` is tested against the following crates: +- [`bincode`](https://crates.io/crates/bincode) v1 +- [`ciborium`](https://crates.io/crates/ciborium) v0.2 +- [`rmp-serde`](https://crates.io/crates/rmp-serde) v1 +- [`serde-json-core`](https://crates.io/crates/serde-json-core) v0.5 +- [`serde-json`](https://crates.io/crates/serde-json) v1 +- [`toml`](https://crates.io/crates/toml) v0.7 ## Minimum Supported Rust Version diff --git a/serdect/src/array.rs b/serdect/src/array.rs index 68be6002b..25ee4285b 100644 --- a/serdect/src/array.rs +++ b/serdect/src/array.rs @@ -1,11 +1,20 @@ //! Serialization primitives for arrays. -use core::fmt; +// Unfortunately, we currently cannot assert generically that we are serializing +// a fixed-size byte array. +// See https://github.com/serde-rs/serde/issues/2120 for the discussion. +// Therefore we have to fall back to the slice methods, +// which will add the size information in the binary formats. +// The only difference is that for the arrays we require the size of the data +// to be exactly equal to the size of the buffer during deserialization, +// while for slices the buffer can be larger than the deserialized data. + +use core::marker::PhantomData; -use serde::de::{Error, SeqAccess, Visitor}; -use serde::ser::SerializeTuple; use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use crate::slice; + #[cfg(feature = "zeroize")] use zeroize::Zeroize; @@ -16,17 +25,7 @@ where S: Serializer, T: AsRef<[u8]>, { - if serializer.is_human_readable() { - crate::serialize_hex::<_, _, false>(value, serializer) - } else { - let mut seq = serializer.serialize_tuple(value.as_ref().len())?; - - for byte in value.as_ref() { - seq.serialize_element(byte)?; - } - - seq.end() - } + slice::serialize_hex_lower_or_bin(value, serializer) } /// Serialize the given type as upper case hex when using human-readable @@ -36,17 +35,7 @@ where S: Serializer, T: AsRef<[u8]>, { - if serializer.is_human_readable() { - crate::serialize_hex::<_, _, true>(value, serializer) - } else { - let mut seq = serializer.serialize_tuple(value.as_ref().len())?; - - for byte in value.as_ref() { - seq.serialize_element(byte)?; - } - - seq.end() - } + slice::serialize_hex_upper_or_bin(value, serializer) } /// Deserialize from hex when using human-readable formats or binary if the @@ -57,56 +46,12 @@ where D: Deserializer<'de>, { if deserializer.is_human_readable() { - struct StrVisitor<'b>(&'b mut [u8]); - - impl<'de> Visitor<'de> for StrVisitor<'_> { - type Value = (); - - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(formatter, "a string of length {}", self.0.len() * 2) - } - - fn visit_str(self, v: &str) -> Result - where - E: Error, - { - if v.len() != self.0.len() * 2 { - return Err(Error::invalid_length(v.len(), &self)); - } - - base16ct::mixed::decode(v, self.0).map_err(E::custom)?; - - Ok(()) - } - } - - deserializer.deserialize_str(StrVisitor(buffer)) + deserializer.deserialize_str(slice::StrVisitor::(buffer, PhantomData)) } else { - struct ArrayVisitor<'b>(&'b mut [u8]); - - impl<'de> Visitor<'de> for ArrayVisitor<'_> { - type Value = (); - - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(formatter, "an array of length {}", self.0.len()) - } - - fn visit_seq(self, mut seq: A) -> Result - where - A: SeqAccess<'de>, - { - for (index, byte) in self.0.iter_mut().enumerate() { - *byte = match seq.next_element()? { - Some(byte) => byte, - None => return Err(Error::invalid_length(index, &self)), - }; - } - - Ok(()) - } - } - - deserializer.deserialize_tuple(buffer.len(), ArrayVisitor(buffer)) + deserializer.deserialize_byte_buf(slice::SliceVisitor::( + buffer, + PhantomData, + )) } } diff --git a/serdect/src/lib.rs b/serdect/src/lib.rs index d7d130b1e..b38b2005d 100644 --- a/serdect/src/lib.rs +++ b/serdect/src/lib.rs @@ -49,8 +49,17 @@ //! let data = SecretData([42; 32]); //! //! let serialized = bincode::serialize(&data).unwrap(); -//! // bincode, a binary serialization format, is serialized into bytes. -//! assert_eq!(serialized.as_slice(), [42; 32]); +//! // bincode, a binary serialization format is serialized into bytes. +//! assert_eq!( +//! serialized.as_slice(), +//! [ +//! // Array size. +//! 32, 0, 0, 0, 0, 0, 0, 0, +//! // Actual data. +//! 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, +//! 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, +//! ] +//! ); //! # let deserialized: SecretData = bincode::deserialize(&serialized).unwrap(); //! # assert_eq!(deserialized, data); //! @@ -98,7 +107,7 @@ //! assert_eq!( //! serialized.as_slice(), //! [ -//! // Not fixed-size, so a size will be encoded. +//! // Slice size. //! 32, 0, 0, 0, 0, 0, 0, 0, //! // Actual data. //! 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, diff --git a/serdect/src/slice.rs b/serdect/src/slice.rs index 283c87a53..7e7b3470f 100644 --- a/serdect/src/slice.rs +++ b/serdect/src/slice.rs @@ -1,6 +1,7 @@ //! Serialization primitives for slices. use core::fmt; +use core::marker::PhantomData; use serde::de::{Error, Visitor}; use serde::{Deserializer, Serializer}; @@ -42,84 +43,124 @@ where } } -/// Deserialize from hex when using human-readable formats or binary if the -/// format is binary. Fails if the `buffer` is smaller then the resulting -/// slice. -pub fn deserialize_hex_or_bin<'de, D>(buffer: &mut [u8], deserializer: D) -> Result<&[u8], D::Error> -where - D: Deserializer<'de>, -{ - if deserializer.is_human_readable() { - struct StrVisitor<'b>(&'b mut [u8]); +pub(crate) trait LengthCheck { + fn length_check(buffer_length: usize, data_length: usize) -> bool; + fn expecting( + formatter: &mut fmt::Formatter<'_>, + data_type: &str, + data_length: usize, + ) -> fmt::Result; +} - impl<'de, 'b> Visitor<'de> for StrVisitor<'b> { - type Value = &'b [u8]; +pub(crate) struct ExactLength; - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - formatter, - "a string with a maximum length of {}", - self.0.len() * 2 - ) - } +impl LengthCheck for ExactLength { + fn length_check(buffer_length: usize, data_length: usize) -> bool { + buffer_length == data_length + } + fn expecting( + formatter: &mut fmt::Formatter<'_>, + data_type: &str, + data_length: usize, + ) -> fmt::Result { + write!(formatter, "{} of length {}", data_type, data_length) + } +} - fn visit_str(self, v: &str) -> Result - where - E: Error, - { - // TODO: Map `base16ct::Error::InvalidLength` to `Error::invalid_length`. - base16ct::mixed::decode(v, self.0).map_err(E::custom) - } +struct UpperBound; + +impl LengthCheck for UpperBound { + fn length_check(buffer_length: usize, data_length: usize) -> bool { + buffer_length >= data_length + } + fn expecting( + formatter: &mut fmt::Formatter<'_>, + data_type: &str, + data_length: usize, + ) -> fmt::Result { + write!( + formatter, + "{} with a maximum length of {}", + data_type, data_length + ) + } +} + +pub(crate) struct StrVisitor<'b, T: LengthCheck>(pub &'b mut [u8], pub PhantomData); + +impl<'de, 'b, T: LengthCheck> Visitor<'de> for StrVisitor<'b, T> { + type Value = (); + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + T::expecting(formatter, "a string", self.0.len() * 2) + } + + fn visit_str(self, v: &str) -> Result + where + E: Error, + { + if !T::length_check(self.0.len() * 2, v.len()) { + return Err(Error::invalid_length(v.len(), &self)); } + // TODO: Map `base16ct::Error::InvalidLength` to `Error::invalid_length`. + base16ct::mixed::decode(v, self.0) + .map(|_| ()) + .map_err(E::custom) + } +} - deserializer.deserialize_str(StrVisitor(buffer)) - } else { - struct SliceVisitor<'b>(&'b mut [u8]); +pub(crate) struct SliceVisitor<'b, T: LengthCheck>(pub &'b mut [u8], pub PhantomData); - impl<'de, 'b> Visitor<'de> for SliceVisitor<'b> { - type Value = &'b [u8]; +impl<'de, 'b, T: LengthCheck> Visitor<'de> for SliceVisitor<'b, T> { + type Value = (); - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - formatter, - "a slice with a maximum length of {}", - self.0.len() - ) - } + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + T::expecting(formatter, "an array", self.0.len()) + } - fn visit_bytes(self, v: &[u8]) -> Result - where - E: Error, - { - // Workaround for - // https://github.com/rust-lang/rfcs/blob/b1de05846d9bc5591d753f611ab8ee84a01fa500/text/2094-nll.md#problem-case-3-conditional-control-flow-across-functions - if v.len() <= self.0.len() { - let buffer = &mut self.0[..v.len()]; - buffer.copy_from_slice(v); - return Ok(buffer); - } - - Err(E::invalid_length(v.len(), &self)) - } + fn visit_bytes(self, v: &[u8]) -> Result + where + E: Error, + { + // Workaround for + // https://github.com/rust-lang/rfcs/blob/b1de05846d9bc5591d753f611ab8ee84a01fa500/text/2094-nll.md#problem-case-3-conditional-control-flow-across-functions + if T::length_check(self.0.len(), v.len()) { + let buffer = &mut self.0[..v.len()]; + buffer.copy_from_slice(v); + return Ok(()); + } - #[cfg(feature = "alloc")] - fn visit_byte_buf(self, mut v: Vec) -> Result - where - E: Error, - { - // Workaround for - // https://github.com/rust-lang/rfcs/blob/b1de05846d9bc5591d753f611ab8ee84a01fa500/text/2094-nll.md#problem-case-3-conditional-control-flow-across-functions - if v.len() <= self.0.len() { - let buffer = &mut self.0[..v.len()]; - buffer.swap_with_slice(&mut v); - return Ok(buffer); - } - - Err(E::invalid_length(v.len(), &self)) - } + Err(E::invalid_length(v.len(), &self)) + } + + #[cfg(feature = "alloc")] + fn visit_byte_buf(self, mut v: Vec) -> Result + where + E: Error, + { + // Workaround for + // https://github.com/rust-lang/rfcs/blob/b1de05846d9bc5591d753f611ab8ee84a01fa500/text/2094-nll.md#problem-case-3-conditional-control-flow-across-functions + if T::length_check(self.0.len(), v.len()) { + let buffer = &mut self.0[..v.len()]; + buffer.swap_with_slice(&mut v); + return Ok(()); } - deserializer.deserialize_byte_buf(SliceVisitor(buffer)) + Err(E::invalid_length(v.len(), &self)) + } +} + +/// Deserialize from hex when using human-readable formats or binary if the +/// format is binary. Fails if the `buffer` is smaller then the resulting +/// slice. +pub fn deserialize_hex_or_bin<'de, D>(buffer: &mut [u8], deserializer: D) -> Result<(), D::Error> +where + D: Deserializer<'de>, +{ + if deserializer.is_human_readable() { + deserializer.deserialize_str(StrVisitor::(buffer, PhantomData)) + } else { + deserializer.deserialize_byte_buf(SliceVisitor::(buffer, PhantomData)) } } diff --git a/serdect/tests/bincode.rs b/serdect/tests/bincode.rs index 9b5035051..7f3f37638 100644 --- a/serdect/tests/bincode.rs +++ b/serdect/tests/bincode.rs @@ -14,7 +14,7 @@ const EXAMPLE_BYTES: [u8; 16] = hex!("000102030405060708090A0B0C0D0EFF"); const BINCODE_SLICE: [u8; 24] = hex!("1000000000000000000102030405060708090A0B0C0D0EFF"); /// bincode serialization of [`EXAMPLE_BYTES`] as an array. -const BINCODE_ARRAY: [u8; 16] = EXAMPLE_BYTES; +const BINCODE_ARRAY: [u8; 24] = BINCODE_SLICE; #[test] fn deserialize_slice() { diff --git a/serdect/tests/cbor.rs b/serdect/tests/cbor.rs index 36b26ec84..0ffe8ff73 100644 --- a/serdect/tests/cbor.rs +++ b/serdect/tests/cbor.rs @@ -19,7 +19,7 @@ const CBOR_SLICE: [u8; 17] = hex!("50000102030405060708090A0B0C0D0EFF"); /// CBOR serialization of [`EXAMPLE_BYTES`] as an array. /// (first three bits, `0b100` denote an array, and the last five, `0b10000` denote the length) /// Note the 0x18 marker before 0xFF, denoting that the integers are dynamically sized. -const CBOR_ARRAY: [u8; 18] = hex!("90000102030405060708090A0B0C0D0E18FF"); +const CBOR_ARRAY: [u8; 17] = CBOR_SLICE; #[test] fn deserialize_slice() { diff --git a/serdect/tests/messagepack.rs b/serdect/tests/messagepack.rs index 21abf27cd..7f9a4b9ab 100644 --- a/serdect/tests/messagepack.rs +++ b/serdect/tests/messagepack.rs @@ -15,7 +15,7 @@ const MESSAGEPACK_SLICE: [u8; 18] = hex!("C410000102030405060708090A0B0C0D0EFF") /// messagepack serialization of [`EXAMPLE_BYTES`] as an array. /// Note the 0xCC marker before 0xFF, denoting that the integers are dynamically sized. -const MESSAGEPACK_ARRAY: [u8; 20] = hex!("DC0010000102030405060708090A0B0C0D0ECCFF"); +const MESSAGEPACK_ARRAY: [u8; 18] = MESSAGEPACK_SLICE; #[test] fn deserialize_slice() {