From e5e45c04ed7720b25b88e6d87a51714725c90395 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Wed, 2 Feb 2022 19:50:43 -0500 Subject: [PATCH 01/25] convert blake2b::Digest to const generics This is still a WIP. The basic method was to define an inner type that behaves the way we want it to, e.g. with special Drop impls, etc. Then for each newtype we want to create, we create a small wrapper over the inner type, like `struct Digest(Blake2bArray)`, and implement two small traits, essentially From and AsRef, to convert to that inner type. In another module, we provide more complex blanket implementations for any types that implement From and AsRef for our inner type. This allow us to, for example, auto-implement `len`, `from_slice`, etc for the newtypes without exposing the inner type (which could be hazardous for inner types like the planned `PrivateArray` type. --- src/hazardous/base.rs | 62 ++++++++++++++++++++++++++++ src/hazardous/hash/blake2/blake2b.rs | 36 ++++++++++++---- src/hazardous/kdf/argon2i.rs | 1 + src/hazardous/mod.rs | 3 ++ src/high_level/kex.rs | 1 + tests/hash/mod.rs | 1 + 6 files changed, 96 insertions(+), 8 deletions(-) create mode 100644 src/hazardous/base.rs diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs new file mode 100644 index 00000000..71696e54 --- /dev/null +++ b/src/hazardous/base.rs @@ -0,0 +1,62 @@ +use crate::errors::UnknownCryptoError; + +pub trait Public: Sized { + fn from_slice(byte: &[u8]) -> Result; + fn as_ref(&self) -> &[u8]; + + fn len(&self) -> usize { + self.as_ref().len() + } + + fn is_empty(&self) -> bool { + self.len() == 0 + } +} + +pub trait OrionDeref { + type Target; + fn deref(&self) -> &Self::Target; +} + +#[derive(Debug, Clone, PartialEq)] +pub struct PublicArray { + value: [u8; MAX], + original_length: usize, +} + +impl Public for PublicArray { + fn from_slice(slice: &[u8]) -> Result { + let slice_len = slice.len(); + + if !(MIN..=MAX).contains(&slice_len) { + return Err(UnknownCryptoError); + } + + let mut value = [0u8; MAX]; + value[..slice_len].copy_from_slice(slice); + + Ok(Self { + value, + original_length: slice_len, + }) + } + + fn as_ref(&self) -> &[u8] { + self.value.get(..self.original_length).unwrap() + } +} + +impl Public for T +where + T: OrionDeref>, + T: From>, +{ + fn from_slice(bytes: &[u8]) -> Result { + let pub_array = PublicArray::::from_slice(bytes)?; + Ok(Self::from(pub_array)) + } + + fn as_ref(&self) -> &[u8] { + self.deref().as_ref() + } +} diff --git a/src/hazardous/hash/blake2/blake2b.rs b/src/hazardous/hash/blake2/blake2b.rs index 99a0db3c..64e955f7 100644 --- a/src/hazardous/hash/blake2/blake2b.rs +++ b/src/hazardous/hash/blake2/blake2b.rs @@ -62,17 +62,37 @@ //! [`mac::blake2b`]: crate::hazardous::mac::blake2b use crate::errors::UnknownCryptoError; +use crate::hazardous::base::{OrionDeref, Public, PublicArray}; use crate::hazardous::hash::blake2::blake2b_core; use crate::hazardous::hash::blake2::blake2b_core::BLAKE2B_OUTSIZE; -construct_public! { - /// A type to represent the `Digest` that BLAKE2b returns. - /// - /// # Errors: - /// An error will be returned if: - /// - `slice` is empty. - /// - `slice` is greater than 64 bytes. - (Digest, test_digest, 1, BLAKE2B_OUTSIZE) +// construct_public! { +// /// A type to represent the `Digest` that BLAKE2b returns. +// /// +// /// # Errors: +// /// An error will be returned if: +// /// - `slice` is empty. +// /// - `slice` is greater than 64 bytes. +// (Digest, test_digest, 1, BLAKE2B_OUTSIZE) +// } + +type Blake2bArray = PublicArray<1, BLAKE2B_OUTSIZE>; + +#[derive(Debug, Clone, PartialEq)] +pub struct Digest(Blake2bArray); + +impl OrionDeref for Digest { + type Target = Blake2bArray; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl From for Digest { + fn from(a: Blake2bArray) -> Self { + Self(a) + } } #[derive(Debug, Clone)] diff --git a/src/hazardous/kdf/argon2i.rs b/src/hazardous/kdf/argon2i.rs index e29657d3..1e73a771 100644 --- a/src/hazardous/kdf/argon2i.rs +++ b/src/hazardous/kdf/argon2i.rs @@ -93,6 +93,7 @@ //! [`zeroize` crate]: https://crates.io/crates/zeroize use crate::errors::UnknownCryptoError; +use crate::hazardous::base::Public; use crate::hazardous::hash::blake2::blake2b::Blake2b; use crate::hazardous::hash::blake2::blake2b_core::BLAKE2B_OUTSIZE; use crate::util; diff --git a/src/hazardous/mod.rs b/src/hazardous/mod.rs index 4c1715f0..4b8da2e6 100644 --- a/src/hazardous/mod.rs +++ b/src/hazardous/mod.rs @@ -44,3 +44,6 @@ pub mod stream; /// Elliptic-Curve Cryptography. pub mod ecc; + +/// TODO: This probably belongs somewhere else. +pub mod base; diff --git a/src/high_level/kex.rs b/src/high_level/kex.rs index c7c0e8d9..30bfeb84 100644 --- a/src/high_level/kex.rs +++ b/src/high_level/kex.rs @@ -89,6 +89,7 @@ pub use crate::hazardous::ecc::x25519::PrivateKey; pub use crate::hazardous::ecc::x25519::PublicKey; use crate::errors::UnknownCryptoError; +use crate::hazardous::base::Public; use crate::hazardous::ecc::x25519; use crate::hazardous::hash::blake2::blake2b::{Blake2b, Digest}; use core::convert::TryFrom; diff --git a/tests/hash/mod.rs b/tests/hash/mod.rs index 64685890..fe7b2b02 100644 --- a/tests/hash/mod.rs +++ b/tests/hash/mod.rs @@ -5,6 +5,7 @@ pub mod sha384_nist_cavp; pub mod sha512_nist_cavp; use crate::TestCaseReader; +use orion::hazardous::base::Public; use orion::hazardous::hash::{blake2, sha2::sha256, sha2::sha384, sha2::sha512}; use orion::hazardous::mac; From fb394f4cbf1ef204fb1114586b681b6e804c1a81 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Thu, 3 Feb 2022 12:48:19 -0500 Subject: [PATCH 02/25] simplified and documented const generics impls This commit introduces the Wrapper type and a generic bound on Public, which is now Public. This allowed us to get past some of the various errors regarding unconstrained type parameters in our blanket implementations for Public. --- src/hazardous/base.rs | 142 ++++++++++++++++++++++++--- src/hazardous/hash/blake2/blake2b.rs | 30 +++--- 2 files changed, 141 insertions(+), 31 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index 71696e54..8d147ff7 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -1,30 +1,83 @@ use crate::errors::UnknownCryptoError; -pub trait Public: Sized { - fn from_slice(byte: &[u8]) -> Result; +/// This trait holds most of the behavior of types whose data are +/// meant to be public. This is what users are expected to import +/// in order to work with various Orion types that represent +/// non-secret data. +pub trait Public: Sized { + /// Construct from a given byte slice. + /// + /// ## Errors + /// `UnknownCryptoError` will be returned if: + /// - `slice` is empty + /// - TODO: figure out how to express max length in the docs + fn from_slice(slice: &[u8]) -> Result; + + /// Return a byte slice representing the underlying data. fn as_ref(&self) -> &[u8]; + /// Get the length of the underlying data in bytes. fn len(&self) -> usize { self.as_ref().len() } + /// Check if the length of the underlying data is 0. fn is_empty(&self) -> bool { self.len() == 0 } } -pub trait OrionDeref { - type Target; - fn deref(&self) -> &Self::Target; +/// This is a trait used to express the fact that a type can be interpreted +/// as or converted from another type. It's used primarily to let us +/// reinterpret simple wrappers over [`PublicArray`][0] as the `PublicArray` +/// itself. +/// +/// [0]: crate::hazardous::base::PublicArray +pub trait Wrapper { + /// This allows us to require that `Self` can return a reference to + /// the underlying `T`. It's functionally equivalent to `AsRef` + fn data(&self) -> &T; + + /// This allows us to require that `Self` can be constructed from + /// its underlying type without possibility of failure. It's + /// functionally equivalent to `From`. + fn from(data: T) -> Self; } +// TODO: Do we want to redefine Wrapper as: +// `trait Wrapper: AsRef + From {}` ? It seems equivalent. If +// we're going to use a macro to generate these `Wrapper` impls anyway, we +// might as well just use the standard traits (?). + +/// `PublicArray` is a convenient type for storing public bytes in an array. +/// It implements [`Public`](crate::hazardous::base::Public), so creating +/// a newtype around it that also implements `Public` is fairly simple. +/// +/// ```rust +/// use orion::hazardous::base::{Public, PublicArray, Wrapper}; +/// +/// // Create a type that must be exactly 32 bytes long (32..=32). +/// type ShaArray = PublicArray<32, 32>; +/// struct ShaDigest(ShaArray); +/// +/// // Implement Wrapper (only has to be imported for newtype creation). +/// // This is the block we may want to have a macro derive for us. +/// impl Wrapper for ShaDigest { +/// fn data(&self) -> &ShaArray { &self.0 } +/// fn from(data: ShaArray) -> Self { Self(data) } +/// } +/// +/// // Thanks to an auto-impl, `ShaDigest` now implements `Public`. +/// let digest = ShaDigest::from_slice(&[42; 32]); +/// assert!(digest.is_ok()); +/// ``` #[derive(Debug, Clone, PartialEq)] pub struct PublicArray { value: [u8; MAX], original_length: usize, } -impl Public for PublicArray { +impl Public for PublicArray { fn from_slice(slice: &[u8]) -> Result { let slice_len = slice.len(); @@ -46,17 +99,82 @@ impl Public for PublicArray { } } -impl Public for T +/// Anything that can be converted to/from a `PublicArray` will +/// implement `Public` thanks to this auto implementation. The +/// ability to be converted to/from a PublicArray is expressed +/// using the `PublicData` trait bound. +impl Public> for T +where + T: Wrapper>, +{ + fn from_slice(bytes: &[u8]) -> Result { + let a = PublicArray::from_slice(bytes)?; + Ok(Self::from(a)) + } + + fn as_ref(&self) -> &[u8] { + self.data().as_ref() + } +} + +/// `PublicVec` is a convenient type for storing public bytes in an `Vec`. +/// It implements [`Public`](crate::hazardous::base::Public), so creating +/// a newtype around it that also implements `Public` is fairly simple. +/// +/// ```rust +/// use orion::hazardous::base::{Public, PublicVec, Wrapper}; +/// +/// // Maybe you want your public key to be variable-sized. +/// struct PublicKey(PublicVec); +/// +/// // Implement Wrapper (only has to be imported for newtype creation). +/// // This is the block we may want to have a macro derive for us. +/// impl Wrapper for PublicKey { +/// fn data(&self) -> &PublicVec { &self.0 } +/// fn from(data: PublicVec) -> Self { Self(data) } +/// } +/// +/// // Thanks to an auto-impl, `PublicKey` now implements `Public`. +/// let digest = PublicKey::from_slice(&[42; 32]); +/// assert!(digest.is_ok()); +/// ``` +#[derive(Debug, Clone, PartialEq)] +pub struct PublicVec { + value: Vec, + original_length: usize, +} + +/// Anything that can be converted to/from a `PublicVec` will +/// implement `Public` thanks to this auto implementation. The +/// ability to be converted to/from a PublicAVecis expressed +/// using the `PublicDynamic` trait bound. +impl Public for PublicVec { + fn from_slice(slice: &[u8]) -> Result { + Ok(Self { + value: Vec::from(slice), + original_length: slice.len(), + }) + } + + fn as_ref(&self) -> &[u8] { + self.value.get(..self.original_length).unwrap() + } +} + +/// Anything that can be converted to/from a `PublicVec` will +/// implement `Public` thanks to this auto implementation. The +/// ability to be converted to/from a PublicArray is expressed +/// using the `Wrapper` trait bound. +impl Public for T where - T: OrionDeref>, - T: From>, + T: Wrapper, { fn from_slice(bytes: &[u8]) -> Result { - let pub_array = PublicArray::::from_slice(bytes)?; - Ok(Self::from(pub_array)) + let a = PublicVec::from_slice(bytes)?; + Ok(Self::from(a)) } fn as_ref(&self) -> &[u8] { - self.deref().as_ref() + self.data().as_ref() } } diff --git a/src/hazardous/hash/blake2/blake2b.rs b/src/hazardous/hash/blake2/blake2b.rs index 64e955f7..5efbe3a8 100644 --- a/src/hazardous/hash/blake2/blake2b.rs +++ b/src/hazardous/hash/blake2/blake2b.rs @@ -62,36 +62,28 @@ //! [`mac::blake2b`]: crate::hazardous::mac::blake2b use crate::errors::UnknownCryptoError; -use crate::hazardous::base::{OrionDeref, Public, PublicArray}; +use crate::hazardous::base::{Public, PublicArray, Wrapper}; use crate::hazardous::hash::blake2::blake2b_core; use crate::hazardous::hash::blake2::blake2b_core::BLAKE2B_OUTSIZE; -// construct_public! { -// /// A type to represent the `Digest` that BLAKE2b returns. -// /// -// /// # Errors: -// /// An error will be returned if: -// /// - `slice` is empty. -// /// - `slice` is greater than 64 bytes. -// (Digest, test_digest, 1, BLAKE2B_OUTSIZE) -// } - type Blake2bArray = PublicArray<1, BLAKE2B_OUTSIZE>; +/// A type to represent the `Digest` that BLAKE2b returns. +/// +/// # Errors: +/// An error will be returned if: +/// - `slice` is empty. +/// - `slice` is greater than 64 bytes. #[derive(Debug, Clone, PartialEq)] pub struct Digest(Blake2bArray); -impl OrionDeref for Digest { - type Target = Blake2bArray; - - fn deref(&self) -> &Self::Target { +impl Wrapper for Digest { + fn data(&self) -> &Blake2bArray { &self.0 } -} -impl From for Digest { - fn from(a: Blake2bArray) -> Self { - Self(a) + fn from(data: Blake2bArray) -> Self { + Self(data) } } From 7c71df1bf8c54c6d2270a654977a973e626a0b91 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Thu, 3 Feb 2022 12:54:28 -0500 Subject: [PATCH 03/25] remove duplicate docs on PublicVec --- src/hazardous/base.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index 8d147ff7..fc2d32dc 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -144,10 +144,6 @@ pub struct PublicVec { original_length: usize, } -/// Anything that can be converted to/from a `PublicVec` will -/// implement `Public` thanks to this auto implementation. The -/// ability to be converted to/from a PublicAVecis expressed -/// using the `PublicDynamic` trait bound. impl Public for PublicVec { fn from_slice(slice: &[u8]) -> Result { Ok(Self { From d950d04f5d7d0877605a7f5f32c682c0723cba6b Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Fri, 4 Feb 2022 01:33:55 -0500 Subject: [PATCH 04/25] move to marker types and traits --- src/hazardous/base.rs | 262 +++++++++++++-------------- src/hazardous/hash/blake2/blake2b.rs | 23 +-- src/hazardous/kdf/argon2i.rs | 1 - src/high_level/kex.rs | 1 - 4 files changed, 130 insertions(+), 157 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index fc2d32dc..341e27b2 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -1,176 +1,156 @@ use crate::errors::UnknownCryptoError; - -/// This trait holds most of the behavior of types whose data are -/// meant to be public. This is what users are expected to import -/// in order to work with various Orion types that represent -/// non-secret data. -pub trait Public: Sized { - /// Construct from a given byte slice. - /// - /// ## Errors - /// `UnknownCryptoError` will be returned if: - /// - `slice` is empty - /// - TODO: figure out how to express max length in the docs - fn from_slice(slice: &[u8]) -> Result; - - /// Return a byte slice representing the underlying data. - fn as_ref(&self) -> &[u8]; - - /// Get the length of the underlying data in bytes. - fn len(&self) -> usize { - self.as_ref().len() - } - - /// Check if the length of the underlying data is 0. - fn is_empty(&self) -> bool { - self.len() == 0 - } -} - -/// This is a trait used to express the fact that a type can be interpreted -/// as or converted from another type. It's used primarily to let us -/// reinterpret simple wrappers over [`PublicArray`][0] as the `PublicArray` -/// itself. -/// -/// [0]: crate::hazardous::base::PublicArray -pub trait Wrapper { - /// This allows us to require that `Self` can return a reference to - /// the underlying `T`. It's functionally equivalent to `AsRef` - fn data(&self) -> &T; - - /// This allows us to require that `Self` can be constructed from - /// its underlying type without possibility of failure. It's - /// functionally equivalent to `From`. - fn from(data: T) -> Self; +use std::{convert::TryFrom, fmt, marker::PhantomData}; + +/// Marker trait for when a type contains some sensitive information. +pub trait Secret {} + +/// Marker trait for when a type contains only non-sensitive information. +/// Be careful if implementing this trait on your own. It cannot +/// cause memory unsafety, and so is not marked `unsafe`. Implementing +/// it can, however, lead to data types containing sensitive data ending +/// up with APIs meant only for types containing only non-sensitive data. +pub trait Public {} + +/// A small trait containing static information about the minimum and +/// maximum size (in bytes) of a type containing data. +pub trait Bounded { + /// The largest number of bytes this type should be allowed to hold. + const MIN: Option = None; + + /// The smallest number of bytes this type should be allowed to hold. + const MAX: Option = None; } -// TODO: Do we want to redefine Wrapper as: -// `trait Wrapper: AsRef + From {}` ? It seems equivalent. If -// we're going to use a macro to generate these `Wrapper` impls anyway, we -// might as well just use the standard traits (?). - -/// `PublicArray` is a convenient type for storing public bytes in an array. -/// It implements [`Public`](crate::hazardous::base::Public), so creating -/// a newtype around it that also implements `Public` is fairly simple. -/// -/// ```rust -/// use orion::hazardous::base::{Public, PublicArray, Wrapper}; -/// -/// // Create a type that must be exactly 32 bytes long (32..=32). -/// type ShaArray = PublicArray<32, 32>; -/// struct ShaDigest(ShaArray); -/// -/// // Implement Wrapper (only has to be imported for newtype creation). -/// // This is the block we may want to have a macro derive for us. -/// impl Wrapper for ShaDigest { -/// fn data(&self) -> &ShaArray { &self.0 } -/// fn from(data: ShaArray) -> Self { Self(data) } -/// } -/// -/// // Thanks to an auto-impl, `ShaDigest` now implements `Public`. -/// let digest = ShaDigest::from_slice(&[42; 32]); -/// assert!(digest.is_ok()); -/// ``` -#[derive(Debug, Clone, PartialEq)] -pub struct PublicArray { - value: [u8; MAX], - original_length: usize, +/// A generic holder for types that are basically just a bag of bytes +/// with extra semantic meaning and restriction on top. We parameterize +/// over the byte storage with parameter `B`. We parameterize over the +/// API-level semantics of the type with phantom type `K`. +#[derive(Clone)] +pub struct Data { + bytes: B, + phantom: PhantomData, } -impl Public for PublicArray { - fn from_slice(slice: &[u8]) -> Result { - let slice_len = slice.len(); - - if !(MIN..=MAX).contains(&slice_len) { +impl<'a, B, K> Data +where + B: TryFrom<&'a [u8], Error = UnknownCryptoError>, + K: Bounded, +{ + /// TODO + pub fn from_slice(slice: &'a [u8]) -> Result { + let min = K::MIN.unwrap_or(0); + let max = K::MAX.unwrap_or(usize::MAX); + if slice.len() < min || slice.len() > max { return Err(UnknownCryptoError); } - let mut value = [0u8; MAX]; - value[..slice_len].copy_from_slice(slice); - Ok(Self { - value, - original_length: slice_len, + bytes: B::try_from(slice)?, + phantom: PhantomData, }) } +} +impl<'a, B, K> Data +where + B: AsRef<[u8]>, +{ + /// Get the length of the contained byte slice. + pub fn len(&self) -> usize { + self.bytes.as_ref().len() + } + + /// Check if the contained byte slice is empty. + pub fn is_empty(&self) -> bool { + self.bytes.as_ref().is_empty() + } +} + +impl<'a, B, K> AsRef<[u8]> for Data +where + B: AsRef<[u8]>, + K: Public, +{ + /// Get a reference to the underlying byte slice. fn as_ref(&self) -> &[u8] { - self.value.get(..self.original_length).unwrap() + self.bytes.as_ref() } } -/// Anything that can be converted to/from a `PublicArray` will -/// implement `Public` thanks to this auto implementation. The -/// ability to be converted to/from a PublicArray is expressed -/// using the `PublicData` trait bound. -impl Public> for T +impl<'a, B, K> Data where - T: Wrapper>, + B: AsRef<[u8]>, + K: Secret, { - fn from_slice(bytes: &[u8]) -> Result { - let a = PublicArray::from_slice(bytes)?; - Ok(Self::from(a)) + /// TODO: Grab docs for `unprotected_as_bytes` and insert here. + pub fn unprotected_as_bytes(&self) -> &[u8] { + self.bytes.as_ref() } +} - fn as_ref(&self) -> &[u8] { - self.data().as_ref() +// We implement this manually to skip over the PhantomData. +impl PartialEq for Data +where + B: PartialEq, +{ + fn eq(&self, other: &Self) -> bool { + self.bytes.eq(&other.bytes) + } +} + +// We implement this manually to skip over the PhantomData. +impl fmt::Debug for Data +where + B: fmt::Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.bytes.fmt(f) } } -/// `PublicVec` is a convenient type for storing public bytes in an `Vec`. -/// It implements [`Public`](crate::hazardous::base::Public), so creating -/// a newtype around it that also implements `Public` is fairly simple. -/// -/// ```rust -/// use orion::hazardous::base::{Public, PublicVec, Wrapper}; -/// -/// // Maybe you want your public key to be variable-sized. -/// struct PublicKey(PublicVec); -/// -/// // Implement Wrapper (only has to be imported for newtype creation). -/// // This is the block we may want to have a macro derive for us. -/// impl Wrapper for PublicKey { -/// fn data(&self) -> &PublicVec { &self.0 } -/// fn from(data: PublicVec) -> Self { Self(data) } -/// } -/// -/// // Thanks to an auto-impl, `PublicKey` now implements `Public`. -/// let digest = PublicKey::from_slice(&[42; 32]); -/// assert!(digest.is_ok()); -/// ``` -#[derive(Debug, Clone, PartialEq)] -pub struct PublicVec { - value: Vec, - original_length: usize, +/// A convenient type for holding data with a static upper bound on +/// its size. The bytes are held with a static array. +#[derive(Clone, Debug)] +pub struct StaticData { + bytes: [u8; MAX], + len: usize, } -impl Public for PublicVec { - fn from_slice(slice: &[u8]) -> Result { +impl TryFrom<&[u8]> for StaticData { + type Error = UnknownCryptoError; + + fn try_from(slice: &[u8]) -> Result { + if slice.len() > MAX { + return Err(UnknownCryptoError); + } + + let mut bytes = [0u8; MAX]; + + // PANIC: This is ok because we just checked that the length + // was less than MAX above. Violating that condition is the + // only thing that would cause this to panic. + bytes + .get_mut(0..slice.len()) + .unwrap() + .copy_from_slice(slice); + Ok(Self { - value: Vec::from(slice), - original_length: slice.len(), + bytes, + len: slice.len(), }) } +} +impl AsRef<[u8]> for StaticData { fn as_ref(&self) -> &[u8] { - self.value.get(..self.original_length).unwrap() + // PANIC: This unwrap is ok because the type's len is checked at + // construction time to be less than MAX. + self.bytes.get(..self.len).unwrap() } } -/// Anything that can be converted to/from a `PublicVec` will -/// implement `Public` thanks to this auto implementation. The -/// ability to be converted to/from a PublicArray is expressed -/// using the `Wrapper` trait bound. -impl Public for T -where - T: Wrapper, -{ - fn from_slice(bytes: &[u8]) -> Result { - let a = PublicVec::from_slice(bytes)?; - Ok(Self::from(a)) - } - - fn as_ref(&self) -> &[u8] { - self.data().as_ref() +impl PartialEq for StaticData { + fn eq(&self, other: &Self) -> bool { + self.bytes.get(..self.len).eq(&other.bytes.get(..other.len)) } } diff --git a/src/hazardous/hash/blake2/blake2b.rs b/src/hazardous/hash/blake2/blake2b.rs index 5efbe3a8..1f1e3b1b 100644 --- a/src/hazardous/hash/blake2/blake2b.rs +++ b/src/hazardous/hash/blake2/blake2b.rs @@ -62,29 +62,24 @@ //! [`mac::blake2b`]: crate::hazardous::mac::blake2b use crate::errors::UnknownCryptoError; -use crate::hazardous::base::{Public, PublicArray, Wrapper}; +use crate::hazardous::base::{Bounded, Data, Public, StaticData}; use crate::hazardous::hash::blake2::blake2b_core; use crate::hazardous::hash::blake2::blake2b_core::BLAKE2B_OUTSIZE; -type Blake2bArray = PublicArray<1, BLAKE2B_OUTSIZE>; - /// A type to represent the `Digest` that BLAKE2b returns. /// /// # Errors: /// An error will be returned if: /// - `slice` is empty. /// - `slice` is greater than 64 bytes. -#[derive(Debug, Clone, PartialEq)] -pub struct Digest(Blake2bArray); - -impl Wrapper for Digest { - fn data(&self) -> &Blake2bArray { - &self.0 - } - - fn from(data: Blake2bArray) -> Self { - Self(data) - } +pub type Digest = Data, BlakeDigest>; + +/// A marker type to declare that this data represents a Blake2b digest. +pub struct BlakeDigest; +impl Public for BlakeDigest {} +impl Bounded for BlakeDigest { + const MIN: Option = Some(1); + const MAX: Option = Some(BLAKE2B_OUTSIZE); } #[derive(Debug, Clone)] diff --git a/src/hazardous/kdf/argon2i.rs b/src/hazardous/kdf/argon2i.rs index 1e73a771..e29657d3 100644 --- a/src/hazardous/kdf/argon2i.rs +++ b/src/hazardous/kdf/argon2i.rs @@ -93,7 +93,6 @@ //! [`zeroize` crate]: https://crates.io/crates/zeroize use crate::errors::UnknownCryptoError; -use crate::hazardous::base::Public; use crate::hazardous::hash::blake2::blake2b::Blake2b; use crate::hazardous::hash::blake2::blake2b_core::BLAKE2B_OUTSIZE; use crate::util; diff --git a/src/high_level/kex.rs b/src/high_level/kex.rs index 30bfeb84..c7c0e8d9 100644 --- a/src/high_level/kex.rs +++ b/src/high_level/kex.rs @@ -89,7 +89,6 @@ pub use crate::hazardous::ecc::x25519::PrivateKey; pub use crate::hazardous::ecc::x25519::PublicKey; use crate::errors::UnknownCryptoError; -use crate::hazardous::base::Public; use crate::hazardous::ecc::x25519; use crate::hazardous::hash::blake2::blake2b::{Blake2b, Digest}; use core::convert::TryFrom; From 52582f9b717de0d082af34b7137b5cee7e88fb3b Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sun, 13 Feb 2022 14:13:14 -0500 Subject: [PATCH 05/25] squash me --- src/hazardous/base.rs | 138 +++++++++++++++++++++++---- src/hazardous/hash/blake2/blake2b.rs | 4 +- src/lib.rs | 3 + 3 files changed, 125 insertions(+), 20 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index 341e27b2..0c12b624 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -21,32 +21,126 @@ pub trait Bounded { const MAX: Option = None; } +/// A trait to express the fact that a type can be (validly) generated +/// from secure random bytes, and the length of that generated type. +/// +/// Note that `Data` implements `Default` if and only if +/// `C` implements +pub trait Generate: Bounded { + /// The size in bytes of the type when generated randomly. Note that + /// it is a logical error for `SIZE` to be less than + /// `::MIN` or `::MAX`. + const SIZE: usize; +} + /// A generic holder for types that are basically just a bag of bytes -/// with extra semantic meaning and restriction on top. We parameterize -/// over the byte storage with parameter `B`. We parameterize over the -/// API-level semantics of the type with phantom type `K`. -#[derive(Clone)] -pub struct Data { +/// with extra semantic meaning and restriction on top. There are two +/// important type parameters to consider: +/// +/// ## Parameter: `B` (bytes) +/// `B` parameterizes over the **byte storage**. In practice, this is +/// either a [`ArrayData`][a] or [`VecData`][b]. This allows us +/// to implement methods on any type that can be converted from +/// or interpreted as a `&[u8]`. This makes it possible to add +/// compatibility with, for example, the [`Bytes`][c] type for +/// zero-copy creation of cryptographic types arriving from the network. +/// +/// TODO: Add example showing how we can use different byte storages. +/// +/// ## Parameter: `C` (context) +/// `C` parameterizes over the **context** of the data. Primarily, +/// this allows us to leverage the type system to protect against +/// misuse of keys (e.g. using one key for two different primitives). +/// In practice, `C` will be a unit struct named after an intended +/// use of the data, such as `chacha::KeyContext`. This will prevent +/// its use in a function that requires instead `aes::KeyContext`. +/// +/// TODO: Add example showing how we cannnot misuse two Data types +/// with different `C` (context) types. +/// +/// ```rust +/// use orion::hazardous::base::{Bounded, Data, Generate}; +/// +/// // Let's say you hypothetically had keys of two different types: +/// // AES and DES secret keys. (Please don't use DES for anything real.) +/// struct DesContext; +/// struct AesContext; +/// +/// impl Bounded for DesContext { +/// const MIN: usize = 32; +/// const MAX: usize = 32; +/// } +/// +/// impl Bounded for AesContext { +/// const MIN: usize = 32; +/// const MAX: usize = 32; +/// } +/// +/// impl Generate for DesContext { +/// const SIZE: usize = 32; +/// } +/// +/// impl Generate for AesContext { +/// const SIZE: usize = 32; +/// } +/// +/// let des_key0: Data, DesContext> = Data::default(); +/// let des_key1: Data, DesContext> = Data::default(); +/// +/// let aes_key0: Data, AesContext> = Data::default(); +/// let aes_key1: Data, AesContext> = Data::default(); +/// +/// // We can compare two DES keys. +/// assert_eq!(&des_key0, &des_key0); +/// assert_ne!(&des_key0, &des_key1); +/// +/// // We can compare two DES keys. +/// assert_eq!(&aes_key0, &aes_key0); +/// assert_ne!(&aes_key0, &aes_key1); +/// +/// // The below code will not compile. This is a good thing. Reusing +/// // keys in different contexts is not only incorrect; it can be +/// // disastrous cryptographically, and can even end up revealing +/// // the secret keys themselves. +/// // +/// // Will error: +/// // assert_eq!(&aes_key0, &des_key0); +/// ``` +/// +/// [a]: crate::hazardous::base::ArrayData +/// [b]: crate::hazardous::base::VecData +/// [c]: https://docs.rs/bytes/latest/bytes/struct.Bytes.html +/// +pub struct Data { bytes: B, - phantom: PhantomData, + context: PhantomData, } -impl<'a, B, K> Data +impl<'a, B, C> Data where B: TryFrom<&'a [u8], Error = UnknownCryptoError>, - K: Bounded, + C: Bounded, { - /// TODO + /// Create `Data` from a byte slice. Only available when the context + /// type parameter is [`Bounded`](crate::hazardous::base::Bounded). + /// + /// ## Errors + /// This function will return an error if: + /// - The length of the given `slice` is not contained by the range + /// specified by `::MIN` and `::MAX`). + /// - The underlying storage type did not have capacity to hold the + /// given slice. In practice, this condition is usually a subset + /// of the above and does not need to be considered separately. pub fn from_slice(slice: &'a [u8]) -> Result { - let min = K::MIN.unwrap_or(0); - let max = K::MAX.unwrap_or(usize::MAX); - if slice.len() < min || slice.len() > max { + let min = C::MIN.unwrap_or(0); + let max = C::MAX.unwrap_or(usize::MAX); + if !(min..=max).contains(&slice.len()) { return Err(UnknownCryptoError); } Ok(Self { bytes: B::try_from(slice)?, - phantom: PhantomData, + context: PhantomData, }) } } @@ -109,14 +203,14 @@ where } /// A convenient type for holding data with a static upper bound on -/// its size. The bytes are held with a static array. +/// its size. The bytes are held with a static array (`[u8; MAX]`). #[derive(Clone, Debug)] -pub struct StaticData { +pub struct ArrayData { bytes: [u8; MAX], len: usize, } -impl TryFrom<&[u8]> for StaticData { +impl TryFrom<&[u8]> for ArrayData { type Error = UnknownCryptoError; fn try_from(slice: &[u8]) -> Result { @@ -141,7 +235,7 @@ impl TryFrom<&[u8]> for StaticData { } } -impl AsRef<[u8]> for StaticData { +impl AsRef<[u8]> for ArrayData { fn as_ref(&self) -> &[u8] { // PANIC: This unwrap is ok because the type's len is checked at // construction time to be less than MAX. @@ -149,8 +243,16 @@ impl AsRef<[u8]> for StaticData { } } -impl PartialEq for StaticData { +impl PartialEq for ArrayData { fn eq(&self, other: &Self) -> bool { self.bytes.get(..self.len).eq(&other.bytes.get(..other.len)) } } + +/// A convenient type for holding data with a dynamic upper bound on +/// its size. The bytes are held with a `Vec`. +#[derive(Clone, Debug)] +pub struct VecData { + bytes: Vec, + len: usize, +} diff --git a/src/hazardous/hash/blake2/blake2b.rs b/src/hazardous/hash/blake2/blake2b.rs index b36d075c..fb0bf37e 100644 --- a/src/hazardous/hash/blake2/blake2b.rs +++ b/src/hazardous/hash/blake2/blake2b.rs @@ -62,7 +62,7 @@ //! [`mac::blake2b`]: crate::hazardous::mac::blake2b use crate::errors::UnknownCryptoError; -use crate::hazardous::base::{Bounded, Data, Public, StaticData}; +use crate::hazardous::base::{ArrayData, Bounded, Data, Public}; use crate::hazardous::hash::blake2::blake2b_core; use crate::hazardous::hash::blake2::blake2b_core::BLAKE2B_OUTSIZE; @@ -75,7 +75,7 @@ use std::io; /// An error will be returned if: /// - `slice` is empty. /// - `slice` is greater than 64 bytes. -pub type Digest = Data, BlakeDigest>; +pub type Digest = Data, BlakeDigest>; /// A marker type to declare that this data represents a Blake2b digest. pub struct BlakeDigest; diff --git a/src/lib.rs b/src/lib.rs index c6f42fea..5328a809 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -113,6 +113,9 @@ pub use high_level::kdf; #[cfg(feature = "safe_api")] pub use high_level::kex; +#[cfg(feature = "safe_api")] +pub use hazardous::base::Data; + #[doc(hidden)] /// Testing framework. pub mod test_framework; From 1a4b50984109eb034fae77993d4b665ea640ae7d Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sat, 2 Apr 2022 23:03:56 -0400 Subject: [PATCH 06/25] split Data into PublicData and SecretData Because marker traits aren't mutually exclusive (as far as the type system is concerned), they didn't end up being a good way to represent whether Data was public or private info. This commit moves to using two separate data container types: one for public info, and one for private. The upside is that this is significantly simpler than using marker traits, and that the PublicData type can be documented separately from the SecretData type. The downside is that we duplicated a bunch of code since PublicData and SecretData are basically identical with the exception of Debug, PartialEq, and the AsRef<[u8]>/unprotected_as_bytes split. --- src/hazardous/base.rs | 353 +++++++++++++++++++++------ src/hazardous/hash/blake2/blake2b.rs | 10 +- src/lib.rs | 2 +- 3 files changed, 282 insertions(+), 83 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index 0c12b624..fbecdd5b 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -1,47 +1,21 @@ use crate::errors::UnknownCryptoError; use std::{convert::TryFrom, fmt, marker::PhantomData}; -/// Marker trait for when a type contains some sensitive information. -pub trait Secret {} - -/// Marker trait for when a type contains only non-sensitive information. -/// Be careful if implementing this trait on your own. It cannot -/// cause memory unsafety, and so is not marked `unsafe`. Implementing -/// it can, however, lead to data types containing sensitive data ending -/// up with APIs meant only for types containing only non-sensitive data. -pub trait Public {} - -/// A small trait containing static information about the minimum and -/// maximum size (in bytes) of a type containing data. -pub trait Bounded { - /// The largest number of bytes this type should be allowed to hold. - const MIN: Option = None; - - /// The smallest number of bytes this type should be allowed to hold. - const MAX: Option = None; -} - -/// A trait to express the fact that a type can be (validly) generated -/// from secure random bytes, and the length of that generated type. -/// -/// Note that `Data` implements `Default` if and only if -/// `C` implements -pub trait Generate: Bounded { - /// The size in bytes of the type when generated randomly. Note that - /// it is a logical error for `SIZE` to be less than - /// `::MIN` or `::MAX`. - const SIZE: usize; +/// A simple container for bytes that are considered non-sensitive, +/// such as message authentication codes (MACs). +pub struct PublicData { + bytes: B, + context: PhantomData, } -/// A generic holder for types that are basically just a bag of bytes -/// with extra semantic meaning and restriction on top. There are two -/// important type parameters to consider: +/// A simple container for bytes that contain sensitive information, +/// such as secret keys. /// /// ## Parameter: `B` (bytes) /// `B` parameterizes over the **byte storage**. In practice, this is -/// either a [`ArrayData`][a] or [`VecData`][b]. This allows us +/// either an [`ArrayData`][a] or [`VecData`][b]. This allows us /// to implement methods on any type that can be converted from -/// or interpreted as a `&[u8]`. This makes it possible to add +/// or interpreted as a `&[u8]`. This also makes it possible to add /// compatibility with, for example, the [`Bytes`][c] type for /// zero-copy creation of cryptographic types arriving from the network. /// @@ -59,42 +33,53 @@ pub trait Generate: Bounded { /// with different `C` (context) types. /// /// ```rust -/// use orion::hazardous::base::{Bounded, Data, Generate}; +/// use orion::hazardous::base::{ +/// Bounded, Generate, NamedContext, +/// SecretData, VecData +/// }; /// /// // Let's say you hypothetically had keys of two different types: -/// // AES and DES secret keys. (Please don't use DES for anything real.) -/// struct DesContext; +/// // AES and Ed25519 secret keys. /// struct AesContext; -/// -/// impl Bounded for DesContext { -/// const MIN: usize = 32; -/// const MAX: usize = 32; -/// } +/// struct EdContext; /// /// impl Bounded for AesContext { -/// const MIN: usize = 32; -/// const MAX: usize = 32; +/// const MIN: Option = Some(32); +/// const MAX: Option = Some(32); /// } /// -/// impl Generate for DesContext { -/// const SIZE: usize = 32; +/// impl Bounded for EdContext { +/// const MIN: Option = Some(32); +/// const MAX: Option = Some(32); /// } /// /// impl Generate for AesContext { -/// const SIZE: usize = 32; +/// const GEN_SIZE: usize = 32; +/// } +/// +/// impl Generate for EdContext { +/// const GEN_SIZE: usize = 32; +/// } +/// +/// impl NamedContext for AesContext { +/// const NAME: &'static str = "AesContext"; +/// } +/// +/// impl NamedContext for EdContext { +/// const NAME: &'static str = "EdContext"; /// } /// -/// let des_key0: Data, DesContext> = Data::default(); -/// let des_key1: Data, DesContext> = Data::default(); +/// let ed_key0: SecretData = SecretData::default(); +/// let ed_key1: SecretData = SecretData::default(); /// -/// let aes_key0: Data, AesContext> = Data::default(); -/// let aes_key1: Data, AesContext> = Data::default(); +/// let aes_key0: SecretData = SecretData::default(); +/// let aes_key1: SecretData = SecretData::default(); /// -/// // We can compare two DES keys. -/// assert_eq!(&des_key0, &des_key0); -/// assert_ne!(&des_key0, &des_key1); +/// // We can compare two Ed25519 keys. +/// assert_eq!(&ed_key0, &ed_key0); +/// assert_ne!(&ed_key0, &ed_key1); /// -/// // We can compare two DES keys. +/// // We can compare two AES keys. /// assert_eq!(&aes_key0, &aes_key0); /// assert_ne!(&aes_key0, &aes_key1); /// @@ -111,17 +96,83 @@ pub trait Generate: Bounded { /// [b]: crate::hazardous::base::VecData /// [c]: https://docs.rs/bytes/latest/bytes/struct.Bytes.html /// -pub struct Data { +pub struct SecretData { bytes: B, context: PhantomData, } -impl<'a, B, C> Data +/// A small trait containing static information about the minimum and +/// maximum size (in bytes) of a type containing data. +pub trait Bounded { + /// The largest number of bytes this type should be allowed to hold. + const MIN: Option = None; + + /// The smallest number of bytes this type should be allowed to hold. + const MAX: Option = None; +} + +/// A trait to express the fact that a type can be (validly) generated +/// from secure random bytes, and the length of that generated type. +/// +/// Note that `PublicData` and `PrivateData` implement +/// `Default` if and only if `C` implements `Generate`. +pub trait Generate: Bounded { + /// The size in bytes of the type when generated randomly. Note that + /// it is a logical error for `SIZE` to be less than + /// `::MIN` or `::MAX`. + const GEN_SIZE: usize; +} + +/// A trait to give contexts a name. Used in Debug impls. +pub trait NamedContext { + /// The type name that will appear in Debug impls. + const NAME: &'static str; +} + +/// A stricter version of `TryFrom<&[u8]>`. By implementing this trait for +/// a type `T`, we prove to the compiler that an *owned* `T` can be +/// generated from a byte slice, versus `TryFrom<&[u8]>` which may be +/// implemented even if the type holds a reference to the slice. +pub trait TryFromBytes: Sized { + /// Convert from a byte slice to an owned `Data`. + fn try_from_bytes(bytes: &[u8]) -> Result; +} + +impl PublicData +where + B: TryFromBytes, + C: Bounded, +{ + /// Create a `PublicData` from a byte slice. Only available when the context + /// type parameter is [`Bounded`](crate::hazardous::base::Bounded). + /// + /// ## Errors + /// This function will return an error if: + /// - The length of the given `slice` is not contained by the range + /// specified by `::MIN` and `::MAX`). + /// - The underlying storage type did not have capacity to hold the + /// given slice. In practice, this condition is usually a subset + /// of the above and does not need to be considered separately. + pub fn from_slice(slice: &[u8]) -> Result { + let min = C::MIN.unwrap_or(0); + let max = C::MAX.unwrap_or(usize::MAX); + if !(min..=max).contains(&slice.len()) { + return Err(UnknownCryptoError); + } + + Ok(Self { + bytes: B::try_from_bytes(slice)?, + context: PhantomData, + }) + } +} + +impl SecretData where - B: TryFrom<&'a [u8], Error = UnknownCryptoError>, + B: TryFromBytes, C: Bounded, { - /// Create `Data` from a byte slice. Only available when the context + /// Create a `PrivateData` from a byte slice. Only available when the context /// type parameter is [`Bounded`](crate::hazardous::base::Bounded). /// /// ## Errors @@ -131,7 +182,7 @@ where /// - The underlying storage type did not have capacity to hold the /// given slice. In practice, this condition is usually a subset /// of the above and does not need to be considered separately. - pub fn from_slice(slice: &'a [u8]) -> Result { + pub fn from_slice(slice: &[u8]) -> Result { let min = C::MIN.unwrap_or(0); let max = C::MAX.unwrap_or(usize::MAX); if !(min..=max).contains(&slice.len()) { @@ -139,13 +190,28 @@ where } Ok(Self { - bytes: B::try_from(slice)?, + bytes: B::try_from_bytes(slice)?, context: PhantomData, }) } } -impl<'a, B, K> Data +impl<'a, B, C> PublicData +where + B: AsRef<[u8]>, +{ + /// Get the length of the contained byte slice. + pub fn len(&self) -> usize { + self.bytes.as_ref().len() + } + + /// Check if the contained byte slice is empty. + pub fn is_empty(&self) -> bool { + self.bytes.as_ref().is_empty() + } +} + +impl<'a, B, C> SecretData where B: AsRef<[u8]>, { @@ -160,10 +226,9 @@ where } } -impl<'a, B, K> AsRef<[u8]> for Data +impl<'a, B, K> AsRef<[u8]> for PublicData where B: AsRef<[u8]>, - K: Public, { /// Get a reference to the underlying byte slice. fn as_ref(&self) -> &[u8] { @@ -171,10 +236,9 @@ where } } -impl<'a, B, K> Data +impl<'a, B, C> SecretData where B: AsRef<[u8]>, - K: Secret, { /// TODO: Grab docs for `unprotected_as_bytes` and insert here. pub fn unprotected_as_bytes(&self) -> &[u8] { @@ -182,8 +246,80 @@ where } } +impl<'a, B, C> Default for PublicData +where + B: TryFromBytes, + C: Bounded + Generate, +{ + /// Use a CSPRNG to fill this type with secure random bytes. + /// + /// # Panic + /// This will panic if the underyling call to + /// [`secure_rand_bytes`](crate::util::secure_rand_bytes) fails; + /// see its documentation for more info. + fn default() -> Self { + let mut data = vec![0u8; C::GEN_SIZE]; + crate::util::secure_rand_bytes(&mut data).unwrap(); + Self { + bytes: B::try_from_bytes(data.as_slice()).unwrap(), + context: PhantomData, + } + } +} + +impl<'a, B, C> Default for SecretData +where + B: TryFromBytes, + C: Bounded + Generate, +{ + /// Use a CSPRNG to fill this type with secure random bytes. + /// + /// # Panic + /// This will panic if the underyling call to + /// [`secure_rand_bytes`](crate::util::secure_rand_bytes) fails; + /// see its documentation for more info. + fn default() -> Self { + let mut data = vec![0u8; C::GEN_SIZE]; + crate::util::secure_rand_bytes(&mut data).unwrap(); + Self { + bytes: B::try_from_bytes(data.as_slice()).unwrap(), + context: PhantomData, + } + } +} + +/// Delegates to [`B::from_bytes`](crate::hazardous::base::TryFromBytes) under the hood. +impl TryFrom<&[u8]> for PublicData +where + B: TryFromBytes, +{ + type Error = UnknownCryptoError; + + fn try_from(value: &[u8]) -> Result { + Ok(Self { + bytes: B::try_from_bytes(value).unwrap(), + context: PhantomData, + }) + } +} + +/// Delegates to [`B::from_bytes`](crate::hazardous::base::TryFromBytes) under the hood. +impl TryFrom<&[u8]> for SecretData +where + B: TryFromBytes, +{ + type Error = UnknownCryptoError; + + fn try_from(value: &[u8]) -> Result { + Ok(Self { + bytes: B::try_from_bytes(value).unwrap(), + context: PhantomData, + }) + } +} + // We implement this manually to skip over the PhantomData. -impl PartialEq for Data +impl PartialEq for PublicData where B: PartialEq, { @@ -193,12 +329,50 @@ where } // We implement this manually to skip over the PhantomData. -impl fmt::Debug for Data +// TODO: Should this be less general? Maybe only implement +// PartialEq<&Self> instead of any U: AsRef. +impl PartialEq for SecretData +where + B: AsRef<[u8]>, +{ + fn eq(&self, other: &Self) -> bool { + use subtle::ConstantTimeEq; + self.unprotected_as_bytes() + .ct_eq(other.unprotected_as_bytes()) + .into() + } +} + +// We implement this manually to skip over the PhantomData. +impl PartialEq<[u8]> for SecretData +where + B: AsRef<[u8]>, +{ + fn eq(&self, other: &[u8]) -> bool { + use subtle::ConstantTimeEq; + self.unprotected_as_bytes().ct_eq(other).into() + } +} + +// We implement this manually to skip over the PhantomData. +impl fmt::Debug for PublicData +where + B: fmt::Debug, + C: NamedContext, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{{ {}: {:?} }}", C::NAME, self.bytes) + } +} + +// We implement this manually to skip over the PhantomData. +impl fmt::Debug for SecretData where B: fmt::Debug, + C: NamedContext, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.bytes.fmt(f) + write!(f, "{{ {}: {:?} }}", C::NAME, self.bytes) } } @@ -210,10 +384,8 @@ pub struct ArrayData { len: usize, } -impl TryFrom<&[u8]> for ArrayData { - type Error = UnknownCryptoError; - - fn try_from(slice: &[u8]) -> Result { +impl TryFromBytes for ArrayData { + fn try_from_bytes(slice: &[u8]) -> Result { if slice.len() > MAX { return Err(UnknownCryptoError); } @@ -243,16 +415,39 @@ impl AsRef<[u8]> for ArrayData { } } +// NOTE: Using non-constant-time comparison here is okay becuase we don't use +// it for timing-sensitive comparisons. `VecData` is always wrapped in a `Data` +// struct which, when its "context" type parameter is marked as `Private`, will +// implement comparisons using constant-time operations. impl PartialEq for ArrayData { fn eq(&self, other: &Self) -> bool { self.bytes.get(..self.len).eq(&other.bytes.get(..other.len)) } } -/// A convenient type for holding data with a dynamic upper bound on -/// its size. The bytes are held with a `Vec`. -#[derive(Clone, Debug)] -pub struct VecData { +/// A convenient type for holding data in dynamically allocated buffer. +// TODO: Should we just use a `Vec` here? We could implement all of the +// same traits for a regular old Vec. +// +// NOTE: Deriving PartialEq here is okay becuase we don't use it for +// timing-sensitive comparisons. `VecData` is always wrapped in a `Data` +// struct which, when its "context" type parameter is marked as `Private`, +// will implement comparisons using constant-time operations. +#[derive(Clone, Debug, PartialEq)] +pub struct VecData { bytes: Vec, - len: usize, +} + +impl TryFromBytes for VecData { + fn try_from_bytes(slice: &[u8]) -> Result { + Ok(Self { + bytes: slice.into(), + }) + } +} + +impl AsRef<[u8]> for VecData { + fn as_ref(&self) -> &[u8] { + self.bytes.as_slice() + } } diff --git a/src/hazardous/hash/blake2/blake2b.rs b/src/hazardous/hash/blake2/blake2b.rs index fb0bf37e..84623925 100644 --- a/src/hazardous/hash/blake2/blake2b.rs +++ b/src/hazardous/hash/blake2/blake2b.rs @@ -62,7 +62,7 @@ //! [`mac::blake2b`]: crate::hazardous::mac::blake2b use crate::errors::UnknownCryptoError; -use crate::hazardous::base::{ArrayData, Bounded, Data, Public}; +use crate::hazardous::base::{ArrayData, Bounded, NamedContext, PublicData}; use crate::hazardous::hash::blake2::blake2b_core; use crate::hazardous::hash::blake2::blake2b_core::BLAKE2B_OUTSIZE; @@ -75,16 +75,20 @@ use std::io; /// An error will be returned if: /// - `slice` is empty. /// - `slice` is greater than 64 bytes. -pub type Digest = Data, BlakeDigest>; +pub type Digest = PublicData, BlakeDigest>; /// A marker type to declare that this data represents a Blake2b digest. pub struct BlakeDigest; -impl Public for BlakeDigest {} + impl Bounded for BlakeDigest { const MIN: Option = Some(1); const MAX: Option = Some(BLAKE2B_OUTSIZE); } +impl NamedContext for BlakeDigest { + const NAME: &'static str = "Blake2bDigest"; +} + #[derive(Debug, Clone)] /// BLAKE2b streaming state. pub struct Blake2b { diff --git a/src/lib.rs b/src/lib.rs index 5328a809..95137ae0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -114,7 +114,7 @@ pub use high_level::kdf; pub use high_level::kex; #[cfg(feature = "safe_api")] -pub use hazardous::base::Data; +pub use hazardous::base::{PublicData, SecretData}; #[doc(hidden)] /// Testing framework. From 651e7a69b63ffce520835ce631dd961379899278 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sat, 2 Apr 2022 23:31:54 -0400 Subject: [PATCH 07/25] remove optional bounds and fix some cfgs --- src/hazardous/base.rs | 37 ++++++++++++++++------------ src/hazardous/hash/blake2/blake2b.rs | 4 +-- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index fbecdd5b..1d9b2d93 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -1,5 +1,8 @@ use crate::errors::UnknownCryptoError; -use std::{convert::TryFrom, fmt, marker::PhantomData}; +use core::{convert::TryFrom, marker::PhantomData}; + +#[cfg(feature = "safe_api")] +use std::fmt; /// A simple container for bytes that are considered non-sensitive, /// such as message authentication codes (MACs). @@ -44,13 +47,13 @@ pub struct PublicData { /// struct EdContext; /// /// impl Bounded for AesContext { -/// const MIN: Option = Some(32); -/// const MAX: Option = Some(32); +/// const MIN: usize = 32; +/// const MAX: usize = 32; /// } /// /// impl Bounded for EdContext { -/// const MIN: Option = Some(32); -/// const MAX: Option = Some(32); +/// const MIN: usize = 32; +/// const MAX: usize = 32; /// } /// /// impl Generate for AesContext { @@ -105,10 +108,10 @@ pub struct SecretData { /// maximum size (in bytes) of a type containing data. pub trait Bounded { /// The largest number of bytes this type should be allowed to hold. - const MIN: Option = None; + const MIN: usize; /// The smallest number of bytes this type should be allowed to hold. - const MAX: Option = None; + const MAX: usize; } /// A trait to express the fact that a type can be (validly) generated @@ -154,9 +157,7 @@ where /// given slice. In practice, this condition is usually a subset /// of the above and does not need to be considered separately. pub fn from_slice(slice: &[u8]) -> Result { - let min = C::MIN.unwrap_or(0); - let max = C::MAX.unwrap_or(usize::MAX); - if !(min..=max).contains(&slice.len()) { + if !(C::MIN..=C::MAX).contains(&slice.len()) { return Err(UnknownCryptoError); } @@ -183,9 +184,7 @@ where /// given slice. In practice, this condition is usually a subset /// of the above and does not need to be considered separately. pub fn from_slice(slice: &[u8]) -> Result { - let min = C::MIN.unwrap_or(0); - let max = C::MAX.unwrap_or(usize::MAX); - if !(min..=max).contains(&slice.len()) { + if !(C::MIN..=C::MAX).contains(&slice.len()) { return Err(UnknownCryptoError); } @@ -246,6 +245,7 @@ where } } +#[cfg(feature = "safe_api")] impl<'a, B, C> Default for PublicData where B: TryFromBytes, @@ -267,6 +267,7 @@ where } } +#[cfg(feature = "safe_api")] impl<'a, B, C> Default for SecretData where B: TryFromBytes, @@ -355,6 +356,7 @@ where } // We implement this manually to skip over the PhantomData. +#[cfg(feature = "safe_api")] impl fmt::Debug for PublicData where B: fmt::Debug, @@ -366,6 +368,7 @@ where } // We implement this manually to skip over the PhantomData. +#[cfg(feature = "safe_api")] impl fmt::Debug for SecretData where B: fmt::Debug, @@ -430,14 +433,15 @@ impl PartialEq for ArrayData { // same traits for a regular old Vec. // // NOTE: Deriving PartialEq here is okay becuase we don't use it for -// timing-sensitive comparisons. `VecData` is always wrapped in a `Data` -// struct which, when its "context" type parameter is marked as `Private`, -// will implement comparisons using constant-time operations. +// timing-sensitive comparisons. For sensitive types, `VecData` is always wrapped +// in a `PrivateData` which implements comparisons using constant-time operations. +#[cfg(feature = "safe_api")] #[derive(Clone, Debug, PartialEq)] pub struct VecData { bytes: Vec, } +#[cfg(feature = "safe_api")] impl TryFromBytes for VecData { fn try_from_bytes(slice: &[u8]) -> Result { Ok(Self { @@ -446,6 +450,7 @@ impl TryFromBytes for VecData { } } +#[cfg(feature = "safe_api")] impl AsRef<[u8]> for VecData { fn as_ref(&self) -> &[u8] { self.bytes.as_slice() diff --git a/src/hazardous/hash/blake2/blake2b.rs b/src/hazardous/hash/blake2/blake2b.rs index 84623925..1139bbfe 100644 --- a/src/hazardous/hash/blake2/blake2b.rs +++ b/src/hazardous/hash/blake2/blake2b.rs @@ -81,8 +81,8 @@ pub type Digest = PublicData, BlakeDigest>; pub struct BlakeDigest; impl Bounded for BlakeDigest { - const MIN: Option = Some(1); - const MAX: Option = Some(BLAKE2B_OUTSIZE); + const MIN: usize = 1; + const MAX: usize = BLAKE2B_OUTSIZE; } impl NamedContext for BlakeDigest { From 26143d76cdda235f330ef34083eb00c34fef6bc7 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sun, 3 Apr 2022 01:29:39 -0400 Subject: [PATCH 08/25] move/fix docs, remove unnecessary lifetimes --- src/hazardous/base.rs | 193 ++++++++++++++++++++++-------------------- 1 file changed, 99 insertions(+), 94 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index 1d9b2d93..fe2639bf 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -1,3 +1,96 @@ +//! ## Parameter: `B` (bytes) +//! `B` parameterizes over the **byte storage**. In practice, this is +//! either an [`ArrayData`][a] or [`VecData`][b]. This allows us +//! to implement methods on any type that can be converted from +//! or interpreted as a `&[u8]`. This also makes it possible to add +//! compatibility with, for example, the [`Bytes`][c] type for +//! zero-copy creation of cryptographic types arriving from the network. +//! +//! TODO: Add example showing how we can use different byte storages. +//! +//! ## Parameter: `C` (context) +//! `C` parameterizes over the **context** of the data. Primarily, +//! this allows us to leverage the type system to protect against +//! misuse of keys (e.g. using one key for two different primitives). +//! In practice, `C` will be a unit struct named after an intended +//! use of the data, such as `chacha::Key`. This will prevent +//! its use in a function that requires instead `aes::Key`. +//! +//! The following example demonstrates how we can leverage the type +//! system and the concept of "contexts" to quickly create types +//! that implement the functionality we want from byte storage objects, +//! but are still logically separate from each other and, in that way, +//! "misuse-resistant". +//! ```rust +//! use orion::hazardous::base::{ +//! Bounded, Generate, NamedContext, +//! SecretData, VecData +//! }; +//! +//! // Let's say you hypothetically had keys of two different types: +//! // AES and Ed25519 secret keys. +//! struct AesContext; +//! struct EdContext; +//! +//! const KEY_SIZE: usize = 32; +//! +//! impl Bounded for AesContext { +//! const MIN: usize = KEY_SIZE; +//! const MAX: usize = KEY_SIZE; +//! } +//! +//! impl Bounded for EdContext { +//! const MIN: usize = KEY_SIZE; +//! const MAX: usize = KEY_SIZE; +//! } +//! +//! impl Generate for AesContext { +//! const GEN_SIZE: usize = KEY_SIZE; +//! } +//! +//! impl Generate for EdContext { +//! const GEN_SIZE: usize = KEY_SIZE; +//! } +//! +//! impl NamedContext for AesContext { +//! const NAME: &'static str = "AesContext"; +//! } +//! +//! impl NamedContext for EdContext { +//! const NAME: &'static str = "EdContext"; +//! } +//! +//! type AesSecretKey = SecretData; +//! type EdSecretKey = SecretData; +//! +//! let aes_key0 = AesSecretKey::default(); +//! let aes_key1 = AesSecretKey::default(); +//! +//! let ed_key0 = EdSecretKey::default(); +//! let ed_key1 = EdSecretKey::default(); +//! +//! // We can compare two Ed25519 keys. +//! assert_eq!(&ed_key0, &ed_key0); +//! assert_ne!(&ed_key0, &ed_key1); +//! +//! // We can compare two AES keys. +//! assert_eq!(&aes_key0, &aes_key0); +//! assert_ne!(&aes_key0, &aes_key1); +//! +//! // The below code will NOT compile. This is a good thing. Reusing +//! // keys in different contexts is not only incorrect; it can be +//! // disastrous cryptographically, and can even end up revealing +//! // the secret keys themselves. +//! // +//! // Will error: +//! // assert_eq!(&aes_key0, &des_key0); +//! ``` +//! +//! [a]: crate::hazardous::base::ArrayData +//! [b]: crate::hazardous::base::VecData +//! [c]: https://docs.rs/bytes/latest/bytes/struct.Bytes.html +//! + use crate::errors::UnknownCryptoError; use core::{convert::TryFrom, marker::PhantomData}; @@ -13,92 +106,6 @@ pub struct PublicData { /// A simple container for bytes that contain sensitive information, /// such as secret keys. -/// -/// ## Parameter: `B` (bytes) -/// `B` parameterizes over the **byte storage**. In practice, this is -/// either an [`ArrayData`][a] or [`VecData`][b]. This allows us -/// to implement methods on any type that can be converted from -/// or interpreted as a `&[u8]`. This also makes it possible to add -/// compatibility with, for example, the [`Bytes`][c] type for -/// zero-copy creation of cryptographic types arriving from the network. -/// -/// TODO: Add example showing how we can use different byte storages. -/// -/// ## Parameter: `C` (context) -/// `C` parameterizes over the **context** of the data. Primarily, -/// this allows us to leverage the type system to protect against -/// misuse of keys (e.g. using one key for two different primitives). -/// In practice, `C` will be a unit struct named after an intended -/// use of the data, such as `chacha::KeyContext`. This will prevent -/// its use in a function that requires instead `aes::KeyContext`. -/// -/// TODO: Add example showing how we cannnot misuse two Data types -/// with different `C` (context) types. -/// -/// ```rust -/// use orion::hazardous::base::{ -/// Bounded, Generate, NamedContext, -/// SecretData, VecData -/// }; -/// -/// // Let's say you hypothetically had keys of two different types: -/// // AES and Ed25519 secret keys. -/// struct AesContext; -/// struct EdContext; -/// -/// impl Bounded for AesContext { -/// const MIN: usize = 32; -/// const MAX: usize = 32; -/// } -/// -/// impl Bounded for EdContext { -/// const MIN: usize = 32; -/// const MAX: usize = 32; -/// } -/// -/// impl Generate for AesContext { -/// const GEN_SIZE: usize = 32; -/// } -/// -/// impl Generate for EdContext { -/// const GEN_SIZE: usize = 32; -/// } -/// -/// impl NamedContext for AesContext { -/// const NAME: &'static str = "AesContext"; -/// } -/// -/// impl NamedContext for EdContext { -/// const NAME: &'static str = "EdContext"; -/// } -/// -/// let ed_key0: SecretData = SecretData::default(); -/// let ed_key1: SecretData = SecretData::default(); -/// -/// let aes_key0: SecretData = SecretData::default(); -/// let aes_key1: SecretData = SecretData::default(); -/// -/// // We can compare two Ed25519 keys. -/// assert_eq!(&ed_key0, &ed_key0); -/// assert_ne!(&ed_key0, &ed_key1); -/// -/// // We can compare two AES keys. -/// assert_eq!(&aes_key0, &aes_key0); -/// assert_ne!(&aes_key0, &aes_key1); -/// -/// // The below code will not compile. This is a good thing. Reusing -/// // keys in different contexts is not only incorrect; it can be -/// // disastrous cryptographically, and can even end up revealing -/// // the secret keys themselves. -/// // -/// // Will error: -/// // assert_eq!(&aes_key0, &des_key0); -/// ``` -/// -/// [a]: crate::hazardous::base::ArrayData -/// [b]: crate::hazardous::base::VecData -/// [c]: https://docs.rs/bytes/latest/bytes/struct.Bytes.html -/// pub struct SecretData { bytes: B, context: PhantomData, @@ -195,7 +202,7 @@ where } } -impl<'a, B, C> PublicData +impl PublicData where B: AsRef<[u8]>, { @@ -210,7 +217,7 @@ where } } -impl<'a, B, C> SecretData +impl SecretData where B: AsRef<[u8]>, { @@ -225,7 +232,7 @@ where } } -impl<'a, B, K> AsRef<[u8]> for PublicData +impl AsRef<[u8]> for PublicData where B: AsRef<[u8]>, { @@ -235,7 +242,7 @@ where } } -impl<'a, B, C> SecretData +impl SecretData where B: AsRef<[u8]>, { @@ -246,7 +253,7 @@ where } #[cfg(feature = "safe_api")] -impl<'a, B, C> Default for PublicData +impl Default for PublicData where B: TryFromBytes, C: Bounded + Generate, @@ -268,7 +275,7 @@ where } #[cfg(feature = "safe_api")] -impl<'a, B, C> Default for SecretData +impl Default for SecretData where B: TryFromBytes, C: Bounded + Generate, @@ -344,7 +351,6 @@ where } } -// We implement this manually to skip over the PhantomData. impl PartialEq<[u8]> for SecretData where B: AsRef<[u8]>, @@ -355,7 +361,6 @@ where } } -// We implement this manually to skip over the PhantomData. #[cfg(feature = "safe_api")] impl fmt::Debug for PublicData where From 0ce38c58001eeec38e56774a336def8cb24e59b8 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sun, 3 Apr 2022 01:30:13 -0400 Subject: [PATCH 09/25] broaden PartialEq impls for {Secret,Public}Data --- src/hazardous/base.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index fe2639bf..05e1f1e3 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -326,24 +326,27 @@ where } } -// We implement this manually to skip over the PhantomData. -impl PartialEq for PublicData +// We define `PartialEq` such that we can compare only with +// other `PublicData` that have the same "context". +impl PartialEq> for PublicData where - B: PartialEq, + B0: AsRef<[u8]>, + B1: AsRef<[u8]>, { - fn eq(&self, other: &Self) -> bool { - self.bytes.eq(&other.bytes) + fn eq(&self, other: &PublicData) -> bool { + self.bytes.as_ref().eq(other.bytes.as_ref()) } } // We implement this manually to skip over the PhantomData. // TODO: Should this be less general? Maybe only implement // PartialEq<&Self> instead of any U: AsRef. -impl PartialEq for SecretData +impl PartialEq> for SecretData where - B: AsRef<[u8]>, + B0: AsRef<[u8]>, + B1: AsRef<[u8]>, { - fn eq(&self, other: &Self) -> bool { + fn eq(&self, other: &SecretData) -> bool { use subtle::ConstantTimeEq; self.unprotected_as_bytes() .ct_eq(other.unprotected_as_bytes()) From f622c92f2152fe1f792ed06bda5559affa270e33 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sun, 3 Apr 2022 01:56:33 -0400 Subject: [PATCH 10/25] base: better module docs --- src/hazardous/base.rs | 72 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 58 insertions(+), 14 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index 05e1f1e3..79839345 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -1,3 +1,16 @@ +//! # Base +//! +//! This module provides convenient, type-parameterized byte-storage types +//! that can be used to quickly create newtypes for keys, digests, etc. +//! The goal is to support all the various cryptography types that are +//! basically just bags of bytes, but **absolutely cannot** afford to +//! be confused with one another. +//! +//! To that end, we define two structs — `PublicData` and `SecretData` — +//! to as generic containers for public and secret information, respectively. +//! These containers are each parameterized by the same two type parameters: +//! `B` (for "bytes") and `C` (for "context"). +//! //! ## Parameter: `B` (bytes) //! `B` parameterizes over the **byte storage**. In practice, this is //! either an [`ArrayData`][a] or [`VecData`][b]. This allows us @@ -6,7 +19,32 @@ //! compatibility with, for example, the [`Bytes`][c] type for //! zero-copy creation of cryptographic types arriving from the network. //! -//! TODO: Add example showing how we can use different byte storages. +//! The following example demonstrates how we can create a type with various +//! types for storage. +//! ```rust +//! use orion::hazardous::base::{ +//! Bounded, Generate, NamedContext, +//! PrivateData, VecData +//! }; +//! +//! struct Password; +//! +//! impl Bounded for Password { +//! const MIN: usize = 8; +//! const MAX: usize = usize::MAX; +//! } +//! +//! impl Generate for Password { +//! const GEN_SIZE: usize = 32; +//! } +//! +//! impl NamedContext for Password { +//! const NAME: &'static str = "Password"; +//! } +//! +//! type PasswordVec = PrivateData; +//! type PasswordArray = PrivateData, Password>; +//! ``` //! //! ## Parameter: `C` (context) //! `C` parameterizes over the **context** of the data. Primarily, @@ -28,38 +66,44 @@ //! }; //! //! // Let's say you hypothetically had keys of two different types: -//! // AES and Ed25519 secret keys. -//! struct AesContext; -//! struct EdContext; //! //! const KEY_SIZE: usize = 32; //! +//! +//! // AES 256-bit keys +//! struct AesContext; +//! //! impl Bounded for AesContext { //! const MIN: usize = KEY_SIZE; //! const MAX: usize = KEY_SIZE; //! } //! +//! impl Generate for AesContext { +//! const GEN_SIZE: usize = KEY_SIZE; +//! } +//! +//! impl NamedContext for AesContext { +//! const NAME: &'static str = "Aes256"; +//! } +//! +//! +//! // Ed25519 256-bit keys +//! struct EdContext; +//! //! impl Bounded for EdContext { //! const MIN: usize = KEY_SIZE; //! const MAX: usize = KEY_SIZE; //! } //! -//! impl Generate for AesContext { -//! const GEN_SIZE: usize = KEY_SIZE; -//! } -//! //! impl Generate for EdContext { //! const GEN_SIZE: usize = KEY_SIZE; //! } //! -//! impl NamedContext for AesContext { -//! const NAME: &'static str = "AesContext"; -//! } -//! //! impl NamedContext for EdContext { -//! const NAME: &'static str = "EdContext"; +//! const NAME: &'static str = "Ed256"; //! } //! +//! //! type AesSecretKey = SecretData; //! type EdSecretKey = SecretData; //! @@ -232,7 +276,7 @@ where } } -impl AsRef<[u8]> for PublicData +impl AsRef<[u8]> for PublicData where B: AsRef<[u8]>, { From 64ab6371bee6fa8d7e20991abb67a068d91e66af Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sun, 3 Apr 2022 02:10:00 -0400 Subject: [PATCH 11/25] rename to ArrayVecData and add actual ArrayData --- src/hazardous/base.rs | 54 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index 79839345..1f8895b4 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -7,7 +7,7 @@ //! be confused with one another. //! //! To that end, we define two structs — `PublicData` and `SecretData` — -//! to as generic containers for public and secret information, respectively. +//! to act as generic containers for public and secret information, respectively. //! These containers are each parameterized by the same two type parameters: //! `B` (for "bytes") and `C` (for "context"). //! @@ -431,15 +431,42 @@ where } } +/// A convenient type for holding data with a statically known size. +/// The bytes are held with a static array (`[u8; LEN]`). +// +// NOTE: Deriving PartialEq here is okay becuase we don't use it for +// timing-sensitive comparisons. `ArrayData` is always wrapped in a +// [`SecretData`](crate::hazardous::base::SecretData) if it's used for +// sensitive information, which implements constant-time comparisons. +// +// Same thing for Debug: the SecretData wrapper will handle it.. +#[derive(Clone, Debug, PartialEq)] +pub struct ArrayData { + bytes: [u8; LEN], +} + +impl TryFromBytes for ArrayData { + fn try_from_bytes(slice: &[u8]) -> Result { + let bytes = <[u8; LEN]>::try_from(slice).map_err(|_| UnknownCryptoError)?; + Ok(Self { bytes }) + } +} + +impl AsRef<[u8]> for ArrayData { + fn as_ref(&self) -> &[u8] { + self.bytes.as_slice() + } +} + /// A convenient type for holding data with a static upper bound on /// its size. The bytes are held with a static array (`[u8; MAX]`). #[derive(Clone, Debug)] -pub struct ArrayData { +pub struct ArrayVecData { bytes: [u8; MAX], len: usize, } -impl TryFromBytes for ArrayData { +impl TryFromBytes for ArrayVecData { fn try_from_bytes(slice: &[u8]) -> Result { if slice.len() > MAX { return Err(UnknownCryptoError); @@ -462,7 +489,7 @@ impl TryFromBytes for ArrayData { } } -impl AsRef<[u8]> for ArrayData { +impl AsRef<[u8]> for ArrayVecData { fn as_ref(&self) -> &[u8] { // PANIC: This unwrap is ok because the type's len is checked at // construction time to be less than MAX. @@ -470,11 +497,13 @@ impl AsRef<[u8]> for ArrayData { } } -// NOTE: Using non-constant-time comparison here is okay becuase we don't use -// it for timing-sensitive comparisons. `VecData` is always wrapped in a `Data` -// struct which, when its "context" type parameter is marked as `Private`, will -// implement comparisons using constant-time operations. -impl PartialEq for ArrayData { +// NOTE: Using non-constant-time comparison here is okay becuase we don't +// use it for timing-sensitive comparisons. `ArrayVecData` is always wrapped +// in a [`SecretData`](crate::hazardous::base::SecretData) if it's used for +// sensitive information, which implements constant-time comparisons. +// +// Same thing for Debug: the SecretData wrapper will handle it.. +impl PartialEq for ArrayVecData { fn eq(&self, other: &Self) -> bool { self.bytes.get(..self.len).eq(&other.bytes.get(..other.len)) } @@ -485,8 +514,11 @@ impl PartialEq for ArrayData { // same traits for a regular old Vec. // // NOTE: Deriving PartialEq here is okay becuase we don't use it for -// timing-sensitive comparisons. For sensitive types, `VecData` is always wrapped -// in a `PrivateData` which implements comparisons using constant-time operations. +// timing-sensitive comparisons. `VecData` is always wrapped in a +// [`SecretData`](crate::hazardous::base::SecretData) if it's used for +// sensitive information, which implements constant-time comparisons. +// +// Same thing for Debug: the SecretData wrapper will handle it.. #[cfg(feature = "safe_api")] #[derive(Clone, Debug, PartialEq)] pub struct VecData { From 6930a0f194180ebe616b76ce690f5d1e4070dd54 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sat, 30 Apr 2022 16:40:23 -0400 Subject: [PATCH 12/25] rename PublicData and SecretData to Public and Secret --- src/hazardous/base.rs | 62 +++++++++++++++++++++---------------------- src/lib.rs | 2 +- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index 1f8895b4..ca825475 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -6,7 +6,7 @@ //! basically just bags of bytes, but **absolutely cannot** afford to //! be confused with one another. //! -//! To that end, we define two structs — `PublicData` and `SecretData` — +//! To that end, we define two structs — `Public` and `Secret` — //! to act as generic containers for public and secret information, respectively. //! These containers are each parameterized by the same two type parameters: //! `B` (for "bytes") and `C` (for "context"). @@ -24,7 +24,7 @@ //! ```rust //! use orion::hazardous::base::{ //! Bounded, Generate, NamedContext, -//! PrivateData, VecData +//! Secret, ArrayData, VecData //! }; //! //! struct Password; @@ -42,8 +42,8 @@ //! const NAME: &'static str = "Password"; //! } //! -//! type PasswordVec = PrivateData; -//! type PasswordArray = PrivateData, Password>; +//! type PasswordVec = Secret; +//! type PasswordArray = Secret, Password>; //! ``` //! //! ## Parameter: `C` (context) @@ -62,7 +62,7 @@ //! ```rust //! use orion::hazardous::base::{ //! Bounded, Generate, NamedContext, -//! SecretData, VecData +//! Secret, VecData //! }; //! //! // Let's say you hypothetically had keys of two different types: @@ -104,8 +104,8 @@ //! } //! //! -//! type AesSecretKey = SecretData; -//! type EdSecretKey = SecretData; +//! type AesSecretKey = Secret; +//! type EdSecretKey = Secret; //! //! let aes_key0 = AesSecretKey::default(); //! let aes_key1 = AesSecretKey::default(); @@ -143,14 +143,14 @@ use std::fmt; /// A simple container for bytes that are considered non-sensitive, /// such as message authentication codes (MACs). -pub struct PublicData { +pub struct Public { bytes: B, context: PhantomData, } /// A simple container for bytes that contain sensitive information, /// such as secret keys. -pub struct SecretData { +pub struct Secret { bytes: B, context: PhantomData, } @@ -168,7 +168,7 @@ pub trait Bounded { /// A trait to express the fact that a type can be (validly) generated /// from secure random bytes, and the length of that generated type. /// -/// Note that `PublicData` and `PrivateData` implement +/// Note that `Public` and `Secret` implement /// `Default` if and only if `C` implements `Generate`. pub trait Generate: Bounded { /// The size in bytes of the type when generated randomly. Note that @@ -192,12 +192,12 @@ pub trait TryFromBytes: Sized { fn try_from_bytes(bytes: &[u8]) -> Result; } -impl PublicData +impl Public where B: TryFromBytes, C: Bounded, { - /// Create a `PublicData` from a byte slice. Only available when the context + /// Create a `Public` from a byte slice. Only available when the context /// type parameter is [`Bounded`](crate::hazardous::base::Bounded). /// /// ## Errors @@ -219,12 +219,12 @@ where } } -impl SecretData +impl Secret where B: TryFromBytes, C: Bounded, { - /// Create a `PrivateData` from a byte slice. Only available when the context + /// Create a `Secret` from a byte slice. Only available when the context /// type parameter is [`Bounded`](crate::hazardous::base::Bounded). /// /// ## Errors @@ -246,7 +246,7 @@ where } } -impl PublicData +impl Public where B: AsRef<[u8]>, { @@ -261,7 +261,7 @@ where } } -impl SecretData +impl Secret where B: AsRef<[u8]>, { @@ -276,7 +276,7 @@ where } } -impl AsRef<[u8]> for PublicData +impl AsRef<[u8]> for Public where B: AsRef<[u8]>, { @@ -286,7 +286,7 @@ where } } -impl SecretData +impl Secret where B: AsRef<[u8]>, { @@ -297,7 +297,7 @@ where } #[cfg(feature = "safe_api")] -impl Default for PublicData +impl Default for Public where B: TryFromBytes, C: Bounded + Generate, @@ -319,7 +319,7 @@ where } #[cfg(feature = "safe_api")] -impl Default for SecretData +impl Default for Secret where B: TryFromBytes, C: Bounded + Generate, @@ -341,7 +341,7 @@ where } /// Delegates to [`B::from_bytes`](crate::hazardous::base::TryFromBytes) under the hood. -impl TryFrom<&[u8]> for PublicData +impl TryFrom<&[u8]> for Public where B: TryFromBytes, { @@ -356,7 +356,7 @@ where } /// Delegates to [`B::from_bytes`](crate::hazardous::base::TryFromBytes) under the hood. -impl TryFrom<&[u8]> for SecretData +impl TryFrom<&[u8]> for Secret where B: TryFromBytes, { @@ -398,7 +398,7 @@ where } } -impl PartialEq<[u8]> for SecretData +impl PartialEq<[u8]> for Secret where B: AsRef<[u8]>, { @@ -409,7 +409,7 @@ where } #[cfg(feature = "safe_api")] -impl fmt::Debug for PublicData +impl fmt::Debug for Public where B: fmt::Debug, C: NamedContext, @@ -421,7 +421,7 @@ where // We implement this manually to skip over the PhantomData. #[cfg(feature = "safe_api")] -impl fmt::Debug for SecretData +impl fmt::Debug for Secret where B: fmt::Debug, C: NamedContext, @@ -436,10 +436,10 @@ where // // NOTE: Deriving PartialEq here is okay becuase we don't use it for // timing-sensitive comparisons. `ArrayData` is always wrapped in a -// [`SecretData`](crate::hazardous::base::SecretData) if it's used for +// [`Secret`](crate::hazardous::base::Secret) if it's used for // sensitive information, which implements constant-time comparisons. // -// Same thing for Debug: the SecretData wrapper will handle it.. +// Same thing for Debug: the Secret wrapper will handle it.. #[derive(Clone, Debug, PartialEq)] pub struct ArrayData { bytes: [u8; LEN], @@ -499,10 +499,10 @@ impl AsRef<[u8]> for ArrayVecData { // NOTE: Using non-constant-time comparison here is okay becuase we don't // use it for timing-sensitive comparisons. `ArrayVecData` is always wrapped -// in a [`SecretData`](crate::hazardous::base::SecretData) if it's used for +// in a [`Secret`](crate::hazardous::base::Secret) if it's used for // sensitive information, which implements constant-time comparisons. // -// Same thing for Debug: the SecretData wrapper will handle it.. +// Same thing for Debug: the Secret wrapper will handle it.. impl PartialEq for ArrayVecData { fn eq(&self, other: &Self) -> bool { self.bytes.get(..self.len).eq(&other.bytes.get(..other.len)) @@ -515,10 +515,10 @@ impl PartialEq for ArrayVecData { // // NOTE: Deriving PartialEq here is okay becuase we don't use it for // timing-sensitive comparisons. `VecData` is always wrapped in a -// [`SecretData`](crate::hazardous::base::SecretData) if it's used for +// [`Secret`](crate::hazardous::base::Secret) if it's used for // sensitive information, which implements constant-time comparisons. // -// Same thing for Debug: the SecretData wrapper will handle it.. +// Same thing for Debug: the Secret wrapper will handle it.. #[cfg(feature = "safe_api")] #[derive(Clone, Debug, PartialEq)] pub struct VecData { diff --git a/src/lib.rs b/src/lib.rs index 95137ae0..19c1a109 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -114,7 +114,7 @@ pub use high_level::kdf; pub use high_level::kex; #[cfg(feature = "safe_api")] -pub use hazardous::base::{PublicData, SecretData}; +pub use hazardous::base::{Public, Secret}; #[doc(hidden)] /// Testing framework. From f45f7ec8558ea16f6f5f0f185ac80c42c7bd3b30 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sat, 30 Apr 2022 16:42:22 -0400 Subject: [PATCH 13/25] change blake2b Digest to use ArrayVecData --- src/hazardous/hash/blake2/blake2b.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hazardous/hash/blake2/blake2b.rs b/src/hazardous/hash/blake2/blake2b.rs index 1139bbfe..23546302 100644 --- a/src/hazardous/hash/blake2/blake2b.rs +++ b/src/hazardous/hash/blake2/blake2b.rs @@ -62,7 +62,7 @@ //! [`mac::blake2b`]: crate::hazardous::mac::blake2b use crate::errors::UnknownCryptoError; -use crate::hazardous::base::{ArrayData, Bounded, NamedContext, PublicData}; +use crate::hazardous::base::{ArrayVecData, Bounded, NamedContext, Public}; use crate::hazardous::hash::blake2::blake2b_core; use crate::hazardous::hash::blake2::blake2b_core::BLAKE2B_OUTSIZE; @@ -75,7 +75,7 @@ use std::io; /// An error will be returned if: /// - `slice` is empty. /// - `slice` is greater than 64 bytes. -pub type Digest = PublicData, BlakeDigest>; +pub type Digest = Public, BlakeDigest>; /// A marker type to declare that this data represents a Blake2b digest. pub struct BlakeDigest; From cae31f76fb18ab4789714d220eecf1cf36c90308 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sat, 30 Apr 2022 16:45:58 -0400 Subject: [PATCH 14/25] restrict PartialEq to exact same type This is a safer default because it prevents users from creating their own data types and comparing them against Orion's. If we want to allow that, it should be a separate discussion. --- src/hazardous/base.rs | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index ca825475..b730a838 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -371,26 +371,28 @@ where } // We define `PartialEq` such that we can compare only with -// other `PublicData` that have the same "context". -impl PartialEq> for PublicData -where - B0: AsRef<[u8]>, - B1: AsRef<[u8]>, +// other `Public` that have the same "context". +impl, C> PartialEq for Public { - fn eq(&self, other: &PublicData) -> bool { + fn eq(&self, other: &Self) -> bool { self.bytes.as_ref().eq(other.bytes.as_ref()) } } -// We implement this manually to skip over the PhantomData. -// TODO: Should this be less general? Maybe only implement -// PartialEq<&Self> instead of any U: AsRef. -impl PartialEq> for SecretData +impl PartialEq<[u8]> for Public where - B0: AsRef<[u8]>, - B1: AsRef<[u8]>, + B: AsRef<[u8]>, +{ + fn eq(&self, other: &[u8]) -> bool { + self.bytes.as_ref().eq(other) + } +} + +// We define `PartialEq` such that we can compare only with +// other `Public` that have the same "context". +impl, C> PartialEq for Secret { - fn eq(&self, other: &SecretData) -> bool { + fn eq(&self, other: &Secret) -> bool { use subtle::ConstantTimeEq; self.unprotected_as_bytes() .ct_eq(other.unprotected_as_bytes()) From e79316ce4fe13589ab2d6b29b628ec7cece8fb28 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sat, 30 Apr 2022 16:46:46 -0400 Subject: [PATCH 15/25] fix base docs line in hazardous module docs --- src/hazardous/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hazardous/mod.rs b/src/hazardous/mod.rs index 2cb27183..4b5728e1 100644 --- a/src/hazardous/mod.rs +++ b/src/hazardous/mod.rs @@ -45,5 +45,5 @@ pub mod stream; /// Elliptic-Curve Cryptography. pub mod ecc; -/// TODO: This probably belongs somewhere else. +/// Primary byte-container types used in the public API. pub mod base; From a086bcf97cc2ac4859f16d67ed3a9949c0de5229 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sat, 30 Apr 2022 22:09:12 -0400 Subject: [PATCH 16/25] move to core::fmt::Debug for Secret and Public --- src/hazardous/base.rs | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index b730a838..623bf927 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -22,6 +22,7 @@ //! The following example demonstrates how we can create a type with various //! types for storage. //! ```rust +//! # #[cfg(feature = "safe_api")] { //! use orion::hazardous::base::{ //! Bounded, Generate, NamedContext, //! Secret, ArrayData, VecData @@ -44,6 +45,7 @@ //! //! type PasswordVec = Secret; //! type PasswordArray = Secret, Password>; +//! # } //! ``` //! //! ## Parameter: `C` (context) @@ -59,7 +61,9 @@ //! that implement the functionality we want from byte storage objects, //! but are still logically separate from each other and, in that way, //! "misuse-resistant". +//! //! ```rust +//! # #[cfg(feature = "safe_api")] { //! use orion::hazardous::base::{ //! Bounded, Generate, NamedContext, //! Secret, VecData @@ -128,6 +132,7 @@ //! // //! // Will error: //! // assert_eq!(&aes_key0, &des_key0); +//! # } //! ``` //! //! [a]: crate::hazardous::base::ArrayData @@ -136,10 +141,7 @@ //! use crate::errors::UnknownCryptoError; -use core::{convert::TryFrom, marker::PhantomData}; - -#[cfg(feature = "safe_api")] -use std::fmt; +use core::{convert::TryFrom, fmt, marker::PhantomData}; /// A simple container for bytes that are considered non-sensitive, /// such as message authentication codes (MACs). @@ -372,8 +374,7 @@ where // We define `PartialEq` such that we can compare only with // other `Public` that have the same "context". -impl, C> PartialEq for Public -{ +impl, C> PartialEq for Public { fn eq(&self, other: &Self) -> bool { self.bytes.as_ref().eq(other.bytes.as_ref()) } @@ -390,8 +391,7 @@ where // We define `PartialEq` such that we can compare only with // other `Public` that have the same "context". -impl, C> PartialEq for Secret -{ +impl, C> PartialEq for Secret { fn eq(&self, other: &Secret) -> bool { use subtle::ConstantTimeEq; self.unprotected_as_bytes() @@ -410,26 +410,24 @@ where } } -#[cfg(feature = "safe_api")] impl fmt::Debug for Public where - B: fmt::Debug, + B: AsRef<[u8]>, C: NamedContext, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{{ {}: {:?} }}", C::NAME, self.bytes) + write!(f, "{}: {:?}", C::NAME, self.bytes.as_ref()) } } // We implement this manually to skip over the PhantomData. -#[cfg(feature = "safe_api")] impl fmt::Debug for Secret where - B: fmt::Debug, + B: AsRef<[u8]>, C: NamedContext, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{{ {}: {:?} }}", C::NAME, self.bytes) + write!(f, "{}: REDACTED", C::NAME) } } From cb8563035191b9d0b5294b03c57f9a050386f5a8 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sat, 30 Apr 2022 23:38:16 -0400 Subject: [PATCH 17/25] add omitted_debug tests to base types This is a step toward converting more types to the generics-based implementations (as opposed to macro-based). There are lots of tests auto-generated by the macros, and test_omitted_debug is one of them. --- src/hazardous/base.rs | 62 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index 623bf927..f0a5f1b9 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -540,3 +540,65 @@ impl AsRef<[u8]> for VecData { self.bytes.as_slice() } } + +#[cfg(test)] +mod tests { + use super::*; + + const MIN_POW2_SIZE: usize = 4; // 2^4 = 16 + const MAX_POW2_SIZE: usize = 10; // 2^10 = 1024 + const MIN_SIZE: usize = 2usize.pow(MIN_POW2_SIZE as u32); + const MAX_SIZE: usize = 2usize.pow(MAX_POW2_SIZE as u32); + + // Archetypal secret information. + type TestKey = Secret, TestContext>; + + // Archetypal public information. Note that we would *NOT* + // normally reuse the same context for two different types. + // This is *ONLY FOR TESTING*. + type TestTag = Secret, TestContext>; + + struct TestContext; + + impl NamedContext for TestContext { + const NAME: &'static str = "Test"; + } + + impl Bounded for TestContext { + const MIN: usize = MIN_SIZE; + const MAX: usize = MAX_SIZE; + } + + fn run_all_sizes(mut f: F) + where + F: FnMut(usize, usize), + { + for min in (MIN_POW2_SIZE..MAX_POW2_SIZE).map(|n| 2usize.pow(n as u32)) { + for max in (MIN_POW2_SIZE..MAX_POW2_SIZE).map(|n| 2usize.pow(n as u32)) { + f(min, max) + } + } + } + + #[test] + #[cfg(feature = "safe_api")] + fn test_omitted_debug() { + run_all_sizes(|_lower_bound, upper_bound| { + let data = vec![0u8; upper_bound]; + let secret = format!("{:?}", data.as_slice()); + let test_debug_contents = format!("{:?}", TestKey::from_slice(&data).unwrap()); + assert_eq!(test_debug_contents.contains(&secret), false); + }); + } + + #[test] + #[cfg(feature = "safe_api")] + fn test_non_omitted_debug() { + run_all_sizes(|_lower_bound, upper_bound| { + let data = vec![0u8; upper_bound]; + let public = format!("{:?}", data.as_slice()); + let test_debug_contents = format!("{:?}", TestTag::from_slice(&data).unwrap()); + assert_eq!(test_debug_contents.contains(&public), false); + }); + } +} From ee63c2a70a5a21b92aa3c7d5d5da5ad460e7bd51 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sun, 1 May 2022 17:36:26 -0400 Subject: [PATCH 18/25] clean up docs --- src/hazardous/base.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index f0a5f1b9..0e236d01 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -143,16 +143,14 @@ use crate::errors::UnknownCryptoError; use core::{convert::TryFrom, fmt, marker::PhantomData}; -/// A simple container for bytes that are considered non-sensitive, -/// such as message authentication codes (MACs). +/// A simple container for bytes that are considered non-sensitive. pub struct Public { bytes: B, context: PhantomData, } -/// A simple container for bytes that contain sensitive information, -/// such as secret keys. -pub struct Secret { +/// A simple container for bytes that contain sensitive information. +pub struct Secret, C> { bytes: B, context: PhantomData, } @@ -304,7 +302,7 @@ where B: TryFromBytes, C: Bounded + Generate, { - /// Use a CSPRNG to fill this type with secure random bytes. + /// Use a CSPRNG to fill a new instance of this type with secure random bytes. /// /// # Panic /// This will panic if the underyling call to @@ -326,7 +324,8 @@ where B: TryFromBytes, C: Bounded + Generate, { - /// Use a CSPRNG to fill this type with secure random bytes. + /// Use a CSPRNG to fill a new instance of this type with a given number + /// of secure random bytes. /// /// # Panic /// This will panic if the underyling call to From 68e03b90fd045b0e97973a5df4fa20a883161cbb Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sun, 1 May 2022 17:37:28 -0400 Subject: [PATCH 19/25] add generate_with_size method to Public and Secret --- src/hazardous/base.rs | 72 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 67 insertions(+), 5 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index 0e236d01..d695b2cd 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -170,10 +170,21 @@ pub trait Bounded { /// /// Note that `Public` and `Secret` implement /// `Default` if and only if `C` implements `Generate`. +/// +/// When a context type `C` implements `Generate`, the following methods +/// are implemented on `Public` and `Secret`. See those methods' +/// documentation for usage information. +/// +/// - [`Public::generate`](Public::generate) +/// - [`Secret::generate`](Secret::generate) +/// - [`Public::generate_with_size`](Public::generate_with_size) +/// - [`Secret::generate_with_size`](Secret::generate_with_size) +// +// TODO: Does `Generate` need to have a `const` GEN_SIZE? pub trait Generate: Bounded { /// The size in bytes of the type when generated randomly. Note that - /// it is a logical error for `SIZE` to be less than - /// `::MIN` or `::MAX`. + /// it is a logical error for `GEN_SIZE` to be less than + /// `::MIN` or greater than `::MAX`. const GEN_SIZE: usize; } @@ -308,7 +319,7 @@ where /// This will panic if the underyling call to /// [`secure_rand_bytes`](crate::util::secure_rand_bytes) fails; /// see its documentation for more info. - fn default() -> Self { + pub fn generate() -> Self { let mut data = vec![0u8; C::GEN_SIZE]; crate::util::secure_rand_bytes(&mut data).unwrap(); Self { @@ -316,10 +327,35 @@ where context: PhantomData, } } + + /// Use a CSPRNG to fill a new instance of this type with secure random bytes. + /// + /// # Errors + /// - If the passed `size` is less than `::MIN`. + /// - If the passed `size` is greater than `::MAX`. + /// - If the configured data storage parameter cannot hold `size` bytes. + /// + /// # Panic + /// This will panic if the underyling call to + /// [`secure_rand_bytes`](crate::util::secure_rand_bytes) fails; + /// see its documentation for more info. + pub fn generate_with_size(size: usize) -> Result { + if size < ::MIN || size > ::MAX { + return Err(UnknownCryptoError); + } + + let mut data = vec![0u8; size]; + crate::util::secure_rand_bytes(&mut data).unwrap(); + + Ok(Self { + bytes: B::try_from_bytes(data.as_slice())?, + context: PhantomData, + }) + } } #[cfg(feature = "safe_api")] -impl Default for Secret +impl Secret where B: TryFromBytes, C: Bounded + Generate, @@ -331,7 +367,7 @@ where /// This will panic if the underyling call to /// [`secure_rand_bytes`](crate::util::secure_rand_bytes) fails; /// see its documentation for more info. - fn default() -> Self { + pub fn generate() -> Self { let mut data = vec![0u8; C::GEN_SIZE]; crate::util::secure_rand_bytes(&mut data).unwrap(); Self { @@ -339,6 +375,32 @@ where context: PhantomData, } } + + /// Use a CSPRNG to fill a new instance of this type with a given number + /// of secure random bytes. + /// + /// # Errors + /// - If the passed `size` is less than `::MIN`. + /// - If the passed `size` is greater than `::MAX`. + /// - If the configured data storage parameter cannot hold `size` bytes. + /// + /// # Panic + /// This will panic if the underyling call to + /// [`secure_rand_bytes`](crate::util::secure_rand_bytes) fails; + /// see its documentation for more info. + pub fn generate_with_size(size: usize) -> Result { + if size < ::MIN || size > ::MAX { + return Err(UnknownCryptoError); + } + + let mut data = vec![0u8; size]; + crate::util::secure_rand_bytes(&mut data).unwrap(); + + Ok(Self { + bytes: B::try_from_bytes(data.as_slice())?, + context: PhantomData, + }) + } } /// Delegates to [`B::from_bytes`](crate::hazardous::base::TryFromBytes) under the hood. From 967e0b9b53607201848f80d18a4c970bf4c8186e Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sun, 1 May 2022 18:10:27 -0400 Subject: [PATCH 20/25] simplify to Context and Data super traits --- src/hazardous/base.rs | 412 +++++++++++++-------------- src/hazardous/hash/blake2/blake2b.rs | 11 +- 2 files changed, 203 insertions(+), 220 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index d695b2cd..5ccfdd3d 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -11,44 +11,8 @@ //! These containers are each parameterized by the same two type parameters: //! `B` (for "bytes") and `C` (for "context"). //! -//! ## Parameter: `B` (bytes) -//! `B` parameterizes over the **byte storage**. In practice, this is -//! either an [`ArrayData`][a] or [`VecData`][b]. This allows us -//! to implement methods on any type that can be converted from -//! or interpreted as a `&[u8]`. This also makes it possible to add -//! compatibility with, for example, the [`Bytes`][c] type for -//! zero-copy creation of cryptographic types arriving from the network. -//! -//! The following example demonstrates how we can create a type with various -//! types for storage. -//! ```rust -//! # #[cfg(feature = "safe_api")] { -//! use orion::hazardous::base::{ -//! Bounded, Generate, NamedContext, -//! Secret, ArrayData, VecData -//! }; //! -//! struct Password; -//! -//! impl Bounded for Password { -//! const MIN: usize = 8; -//! const MAX: usize = usize::MAX; -//! } -//! -//! impl Generate for Password { -//! const GEN_SIZE: usize = 32; -//! } -//! -//! impl NamedContext for Password { -//! const NAME: &'static str = "Password"; -//! } -//! -//! type PasswordVec = Secret; -//! type PasswordArray = Secret, Password>; -//! # } -//! ``` -//! -//! ## Parameter: `C` (context) +//! ## Parameter: `C` (Context) //! `C` parameterizes over the **context** of the data. Primarily, //! this allows us to leverage the type system to protect against //! misuse of keys (e.g. using one key for two different primitives). @@ -64,10 +28,7 @@ //! //! ```rust //! # #[cfg(feature = "safe_api")] { -//! use orion::hazardous::base::{ -//! Bounded, Generate, NamedContext, -//! Secret, VecData -//! }; +//! use orion::hazardous::base::{Context, Generate, Secret, VecData}; //! //! // Let's say you hypothetically had keys of two different types: //! @@ -77,7 +38,8 @@ //! // AES 256-bit keys //! struct AesContext; //! -//! impl Bounded for AesContext { +//! impl Context for AesContext { +//! const NAME: &'static str = "Aes256Key"; //! const MIN: usize = KEY_SIZE; //! const MAX: usize = KEY_SIZE; //! } @@ -86,15 +48,12 @@ //! const GEN_SIZE: usize = KEY_SIZE; //! } //! -//! impl NamedContext for AesContext { -//! const NAME: &'static str = "Aes256"; -//! } -//! //! //! // Ed25519 256-bit keys //! struct EdContext; //! -//! impl Bounded for EdContext { +//! impl Context for EdContext { +//! const NAME: &'static str = "Ed256Key"; //! const MIN: usize = KEY_SIZE; //! const MAX: usize = KEY_SIZE; //! } @@ -103,19 +62,15 @@ //! const GEN_SIZE: usize = KEY_SIZE; //! } //! -//! impl NamedContext for EdContext { -//! const NAME: &'static str = "Ed256"; -//! } -//! //! -//! type AesSecretKey = Secret; -//! type EdSecretKey = Secret; +//! type AesSecretKey = Secret; +//! type EdSecretKey = Secret; //! -//! let aes_key0 = AesSecretKey::default(); -//! let aes_key1 = AesSecretKey::default(); +//! let aes_key0 = AesSecretKey::generate(); +//! let aes_key1 = AesSecretKey::generate(); //! -//! let ed_key0 = EdSecretKey::default(); -//! let ed_key1 = EdSecretKey::default(); +//! let ed_key0 = EdSecretKey::generate(); +//! let ed_key1 = EdSecretKey::generate(); //! //! // We can compare two Ed25519 keys. //! assert_eq!(&ed_key0, &ed_key0); @@ -135,29 +90,74 @@ //! # } //! ``` //! +//! ## Parameter: `D` (Data) +//! `D` parameterizes over the **byte storage**. In practice, this is: +//! +//! - [`ArrayData`][a] for types that hold an amount of data known at +//! compile-time +//! - [`ArrayVecData`][d] for types that hold a compile-time +//! *maximum* amount of data +//! - [`VecData`][b] for types that hold a +//! dynamic amount of data, size known only at runtime. +//! +//! This allows us to implement methods on any type that can be converted +//! from or interpreted as a `&[u8]`. This also makes it possible to add +//! compatibility with, for example, the [`Bytes`][c] type for +//! zero-copy creation of cryptographic types arriving from the network. +//! +//! The following example demonstrates how we can create a type with various +//! types for storage. +//! ```rust +//! # #[cfg(feature = "safe_api")] { +//! use orion::hazardous::base::{ +//! Secret, Context, Generate, ArrayData, VecData, +//! }; +//! +//! struct Password; +//! +//! impl Context for Password { +//! const NAME: &'static str = "Password"; +//! const MIN: usize = 8; +//! const MAX: usize = usize::MAX; +//! } +//! +//! impl Generate for Password { +//! const GEN_SIZE: usize = 32; +//! } +//! +//! type PasswordVec = Secret; +//! type PasswordArray = Secret>; +//! # } +//! ``` +//! +//! //! [a]: crate::hazardous::base::ArrayData //! [b]: crate::hazardous::base::VecData //! [c]: https://docs.rs/bytes/latest/bytes/struct.Bytes.html +//! [d]: crate::hazardous::base::VecData //! use crate::errors::UnknownCryptoError; use core::{convert::TryFrom, fmt, marker::PhantomData}; /// A simple container for bytes that are considered non-sensitive. -pub struct Public { - bytes: B, +pub struct Public { context: PhantomData, + data: D, } /// A simple container for bytes that contain sensitive information. -pub struct Secret, C> { - bytes: B, +pub struct Secret { context: PhantomData, + data: D, } -/// A small trait containing static information about the minimum and -/// maximum size (in bytes) of a type containing data. -pub trait Bounded { +/// A small trait containing static information about the name, and +/// minimum and maximum size (in bytes) of a type containing data. +pub trait Context { + /// The type name that will appear in Debug impls. + const NAME: &'static str; + /// The largest number of bytes this type should be allowed to hold. const MIN: usize; @@ -168,7 +168,7 @@ pub trait Bounded { /// A trait to express the fact that a type can be (validly) generated /// from secure random bytes, and the length of that generated type. /// -/// Note that `Public` and `Secret` implement +/// Note that `Public` and `Secret` implement /// `Default` if and only if `C` implements `Generate`. /// /// When a context type `C` implements `Generate`, the following methods @@ -181,18 +181,16 @@ pub trait Bounded { /// - [`Secret::generate_with_size`](Secret::generate_with_size) // // TODO: Does `Generate` need to have a `const` GEN_SIZE? -pub trait Generate: Bounded { +pub trait Generate: Context { /// The size in bytes of the type when generated randomly. Note that /// it is a logical error for `GEN_SIZE` to be less than - /// `::MIN` or greater than `::MAX`. + /// `::MIN` or greater than `::MAX`. const GEN_SIZE: usize; } -/// A trait to give contexts a name. Used in Debug impls. -pub trait NamedContext { - /// The type name that will appear in Debug impls. - const NAME: &'static str; -} +/// A trait indicating that some basic operations on byte-slice-like types +/// are available. +pub trait Data: AsRef<[u8]> + AsMut<[u8]> + TryFromBytes {} /// A stricter version of `TryFrom<&[u8]>`. By implementing this trait for /// a type `T`, we prove to the compiler that an *owned* `T` can be @@ -203,18 +201,27 @@ pub trait TryFromBytes: Sized { fn try_from_bytes(bytes: &[u8]) -> Result; } -impl Public +impl<'a, C, D> Drop for Secret +where + D: Data, +{ + fn drop(&mut self) { + use zeroize::Zeroize; + self.data.as_mut().iter_mut().zeroize(); + } +} + +impl Public where - B: TryFromBytes, - C: Bounded, + C: Context, + D: Data, { - /// Create a `Public` from a byte slice. Only available when the context - /// type parameter is [`Bounded`](crate::hazardous::base::Bounded). + /// Create a `Public` from a byte slice. /// /// ## Errors /// This function will return an error if: /// - The length of the given `slice` is not contained by the range - /// specified by `::MIN` and `::MAX`). + /// specified by `::MIN` and `::MAX`). /// - The underlying storage type did not have capacity to hold the /// given slice. In practice, this condition is usually a subset /// of the above and does not need to be considered separately. @@ -224,24 +231,39 @@ where } Ok(Self { - bytes: B::try_from_bytes(slice)?, + data: D::try_from_bytes(slice)?, context: PhantomData, }) } + + /// Get the length of the contained byte slice. + pub fn len(&self) -> usize { + self.data.as_ref().len() + } + + /// Check if the contained byte slice is empty. + pub fn is_empty(&self) -> bool { + self.data.as_ref().is_empty() + } + + /// Get a reference to the inner byte slice. + pub fn data(&self) -> &[u8] { + self.as_ref() + } } -impl Secret +impl Secret where - B: TryFromBytes, - C: Bounded, + C: Context, + D: Data, { /// Create a `Secret` from a byte slice. Only available when the context - /// type parameter is [`Bounded`](crate::hazardous::base::Bounded). + /// type parameter is [`Context`](crate::hazardous::base::Context). /// /// ## Errors /// This function will return an error if: /// - The length of the given `slice` is not contained by the range - /// specified by `::MIN` and `::MAX`). + /// specified by `::MIN` and `::MAX`). /// - The underlying storage type did not have capacity to hold the /// given slice. In practice, this condition is usually a subset /// of the above and does not need to be considered separately. @@ -251,67 +273,43 @@ where } Ok(Self { - bytes: B::try_from_bytes(slice)?, context: PhantomData, + data: D::try_from_bytes(slice)?, }) } -} -impl Public -where - B: AsRef<[u8]>, -{ /// Get the length of the contained byte slice. pub fn len(&self) -> usize { - self.bytes.as_ref().len() + self.data.as_ref().len() } /// Check if the contained byte slice is empty. pub fn is_empty(&self) -> bool { - self.bytes.as_ref().is_empty() - } -} - -impl Secret -where - B: AsRef<[u8]>, -{ - /// Get the length of the contained byte slice. - pub fn len(&self) -> usize { - self.bytes.as_ref().len() + self.data.as_ref().is_empty() } - /// Check if the contained byte slice is empty. - pub fn is_empty(&self) -> bool { - self.bytes.as_ref().is_empty() + /// TODO: Grab docs for `unprotected_as_bytes` and insert here. + pub fn unprotected_as_bytes(&self) -> &[u8] { + self.data.as_ref() } } -impl AsRef<[u8]> for Public +impl AsRef<[u8]> for Public where - B: AsRef<[u8]>, + C: Context, + D: Data, { /// Get a reference to the underlying byte slice. fn as_ref(&self) -> &[u8] { - self.bytes.as_ref() - } -} - -impl Secret -where - B: AsRef<[u8]>, -{ - /// TODO: Grab docs for `unprotected_as_bytes` and insert here. - pub fn unprotected_as_bytes(&self) -> &[u8] { - self.bytes.as_ref() + self.data.as_ref() } } #[cfg(feature = "safe_api")] -impl Default for Public +impl Public where - B: TryFromBytes, - C: Bounded + Generate, + C: Context + Generate, + D: Data, { /// Use a CSPRNG to fill a new instance of this type with secure random bytes. /// @@ -323,7 +321,7 @@ where let mut data = vec![0u8; C::GEN_SIZE]; crate::util::secure_rand_bytes(&mut data).unwrap(); Self { - bytes: B::try_from_bytes(data.as_slice()).unwrap(), + data: D::try_from_bytes(data.as_slice()).unwrap(), context: PhantomData, } } @@ -331,8 +329,8 @@ where /// Use a CSPRNG to fill a new instance of this type with secure random bytes. /// /// # Errors - /// - If the passed `size` is less than `::MIN`. - /// - If the passed `size` is greater than `::MAX`. + /// - If the passed `size` is less than `::MIN`. + /// - If the passed `size` is greater than `::MAX`. /// - If the configured data storage parameter cannot hold `size` bytes. /// /// # Panic @@ -340,7 +338,7 @@ where /// [`secure_rand_bytes`](crate::util::secure_rand_bytes) fails; /// see its documentation for more info. pub fn generate_with_size(size: usize) -> Result { - if size < ::MIN || size > ::MAX { + if size < ::MIN || size > ::MAX { return Err(UnknownCryptoError); } @@ -348,17 +346,17 @@ where crate::util::secure_rand_bytes(&mut data).unwrap(); Ok(Self { - bytes: B::try_from_bytes(data.as_slice())?, + data: D::try_from_bytes(data.as_slice())?, context: PhantomData, }) } } #[cfg(feature = "safe_api")] -impl Secret +impl Secret where - B: TryFromBytes, - C: Bounded + Generate, + C: Context + Generate, + D: Data, { /// Use a CSPRNG to fill a new instance of this type with a given number /// of secure random bytes. @@ -371,7 +369,7 @@ where let mut data = vec![0u8; C::GEN_SIZE]; crate::util::secure_rand_bytes(&mut data).unwrap(); Self { - bytes: B::try_from_bytes(data.as_slice()).unwrap(), + data: D::try_from_bytes(data.as_slice()).unwrap(), context: PhantomData, } } @@ -380,8 +378,8 @@ where /// of secure random bytes. /// /// # Errors - /// - If the passed `size` is less than `::MIN`. - /// - If the passed `size` is greater than `::MAX`. + /// - If the passed `size` is less than `::MIN`. + /// - If the passed `size` is greater than `::MAX`. /// - If the configured data storage parameter cannot hold `size` bytes. /// /// # Panic @@ -389,7 +387,7 @@ where /// [`secure_rand_bytes`](crate::util::secure_rand_bytes) fails; /// see its documentation for more info. pub fn generate_with_size(size: usize) -> Result { - if size < ::MIN || size > ::MAX { + if size < ::MIN || size > ::MAX { return Err(UnknownCryptoError); } @@ -397,37 +395,39 @@ where crate::util::secure_rand_bytes(&mut data).unwrap(); Ok(Self { - bytes: B::try_from_bytes(data.as_slice())?, + data: D::try_from_bytes(data.as_slice())?, context: PhantomData, }) } } /// Delegates to [`B::from_bytes`](crate::hazardous::base::TryFromBytes) under the hood. -impl TryFrom<&[u8]> for Public +impl TryFrom<&[u8]> for Public where - B: TryFromBytes, + C: Context, + D: TryFromBytes, { type Error = UnknownCryptoError; fn try_from(value: &[u8]) -> Result { Ok(Self { - bytes: B::try_from_bytes(value).unwrap(), + data: D::try_from_bytes(value).unwrap(), context: PhantomData, }) } } /// Delegates to [`B::from_bytes`](crate::hazardous::base::TryFromBytes) under the hood. -impl TryFrom<&[u8]> for Secret +impl TryFrom<&[u8]> for Secret where - B: TryFromBytes, + C: Context, + D: Data, { type Error = UnknownCryptoError; fn try_from(value: &[u8]) -> Result { Ok(Self { - bytes: B::try_from_bytes(value).unwrap(), + data: D::try_from_bytes(value).unwrap(), context: PhantomData, }) } @@ -435,25 +435,34 @@ where // We define `PartialEq` such that we can compare only with // other `Public` that have the same "context". -impl, C> PartialEq for Public { +impl PartialEq for Public +where + C: Context, + D: Data, +{ fn eq(&self, other: &Self) -> bool { - self.bytes.as_ref().eq(other.bytes.as_ref()) + self.data.as_ref().eq(other.data.as_ref()) } } -impl PartialEq<[u8]> for Public +impl PartialEq<[u8]> for Public where - B: AsRef<[u8]>, + C: Context, + D: Data, { fn eq(&self, other: &[u8]) -> bool { - self.bytes.as_ref().eq(other) + self.data.as_ref().eq(other) } } // We define `PartialEq` such that we can compare only with // other `Public` that have the same "context". -impl, C> PartialEq for Secret { - fn eq(&self, other: &Secret) -> bool { +impl PartialEq for Secret +where + C: Context, + D: Data, +{ + fn eq(&self, other: &Secret) -> bool { use subtle::ConstantTimeEq; self.unprotected_as_bytes() .ct_eq(other.unprotected_as_bytes()) @@ -461,9 +470,10 @@ impl, C> PartialEq for Secret { } } -impl PartialEq<[u8]> for Secret +impl PartialEq<[u8]> for Secret where - B: AsRef<[u8]>, + C: Context, + D: Data, { fn eq(&self, other: &[u8]) -> bool { use subtle::ConstantTimeEq; @@ -471,21 +481,21 @@ where } } -impl fmt::Debug for Public +impl fmt::Debug for Public where - B: AsRef<[u8]>, - C: NamedContext, + C: Context, + D: Data, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}: {:?}", C::NAME, self.bytes.as_ref()) + write!(f, "{}: {:?}", C::NAME, self.data.as_ref()) } } // We implement this manually to skip over the PhantomData. -impl fmt::Debug for Secret +impl fmt::Debug for Secret where - B: AsRef<[u8]>, - C: NamedContext, + C: Context, + D: Data, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}: REDACTED", C::NAME) @@ -506,6 +516,8 @@ pub struct ArrayData { bytes: [u8; LEN], } +impl Data for ArrayData {} + impl TryFromBytes for ArrayData { fn try_from_bytes(slice: &[u8]) -> Result { let bytes = <[u8; LEN]>::try_from(slice).map_err(|_| UnknownCryptoError)?; @@ -519,6 +531,12 @@ impl AsRef<[u8]> for ArrayData { } } +impl AsMut<[u8]> for ArrayData { + fn as_mut(&mut self) -> &mut [u8] { + &mut self.bytes + } +} + /// A convenient type for holding data with a static upper bound on /// its size. The bytes are held with a static array (`[u8; MAX]`). #[derive(Clone, Debug)] @@ -527,6 +545,8 @@ pub struct ArrayVecData { len: usize, } +impl Data for ArrayVecData {} + impl TryFromBytes for ArrayVecData { fn try_from_bytes(slice: &[u8]) -> Result { if slice.len() > MAX { @@ -558,12 +578,20 @@ impl AsRef<[u8]> for ArrayVecData { } } +impl AsMut<[u8]> for ArrayVecData { + fn as_mut(&mut self) -> &mut [u8] { + // PANIC: This unwrap is ok because the type's len is checked at + // construction time to be less than MAX. + self.bytes.get_mut(..self.len).unwrap() + } +} + // NOTE: Using non-constant-time comparison here is okay becuase we don't // use it for timing-sensitive comparisons. `ArrayVecData` is always wrapped // in a [`Secret`](crate::hazardous::base::Secret) if it's used for // sensitive information, which implements constant-time comparisons. // -// Same thing for Debug: the Secret wrapper will handle it.. +// Same thing for Debug: the Secret wrapper will handle it. impl PartialEq for ArrayVecData { fn eq(&self, other: &Self) -> bool { self.bytes.get(..self.len).eq(&other.bytes.get(..other.len)) @@ -586,6 +614,9 @@ pub struct VecData { bytes: Vec, } +#[cfg(feature = "safe_api")] +impl Data for VecData {} + #[cfg(feature = "safe_api")] impl TryFromBytes for VecData { fn try_from_bytes(slice: &[u8]) -> Result { @@ -602,64 +633,19 @@ impl AsRef<[u8]> for VecData { } } -#[cfg(test)] -mod tests { - use super::*; - - const MIN_POW2_SIZE: usize = 4; // 2^4 = 16 - const MAX_POW2_SIZE: usize = 10; // 2^10 = 1024 - const MIN_SIZE: usize = 2usize.pow(MIN_POW2_SIZE as u32); - const MAX_SIZE: usize = 2usize.pow(MAX_POW2_SIZE as u32); - - // Archetypal secret information. - type TestKey = Secret, TestContext>; - - // Archetypal public information. Note that we would *NOT* - // normally reuse the same context for two different types. - // This is *ONLY FOR TESTING*. - type TestTag = Secret, TestContext>; - - struct TestContext; - - impl NamedContext for TestContext { - const NAME: &'static str = "Test"; - } - - impl Bounded for TestContext { - const MIN: usize = MIN_SIZE; - const MAX: usize = MAX_SIZE; +#[cfg(feature = "safe_api")] +impl AsMut<[u8]> for VecData { + fn as_mut(&mut self) -> &mut [u8] { + &mut self.bytes } +} - fn run_all_sizes(mut f: F) - where - F: FnMut(usize, usize), - { - for min in (MIN_POW2_SIZE..MAX_POW2_SIZE).map(|n| 2usize.pow(n as u32)) { - for max in (MIN_POW2_SIZE..MAX_POW2_SIZE).map(|n| 2usize.pow(n as u32)) { - f(min, max) - } - } - } +#[cfg(feature = "safe_api")] +impl<'a> TryFrom<&'a [u8]> for VecData { + type Error = UnknownCryptoError; - #[test] - #[cfg(feature = "safe_api")] - fn test_omitted_debug() { - run_all_sizes(|_lower_bound, upper_bound| { - let data = vec![0u8; upper_bound]; - let secret = format!("{:?}", data.as_slice()); - let test_debug_contents = format!("{:?}", TestKey::from_slice(&data).unwrap()); - assert_eq!(test_debug_contents.contains(&secret), false); - }); - } - - #[test] - #[cfg(feature = "safe_api")] - fn test_non_omitted_debug() { - run_all_sizes(|_lower_bound, upper_bound| { - let data = vec![0u8; upper_bound]; - let public = format!("{:?}", data.as_slice()); - let test_debug_contents = format!("{:?}", TestTag::from_slice(&data).unwrap()); - assert_eq!(test_debug_contents.contains(&public), false); - }); + fn try_from(value: &'a [u8]) -> Result { + let bytes = Vec::from(value); + Ok(VecData { bytes }) } } diff --git a/src/hazardous/hash/blake2/blake2b.rs b/src/hazardous/hash/blake2/blake2b.rs index 23546302..4ffb9aa6 100644 --- a/src/hazardous/hash/blake2/blake2b.rs +++ b/src/hazardous/hash/blake2/blake2b.rs @@ -62,7 +62,7 @@ //! [`mac::blake2b`]: crate::hazardous::mac::blake2b use crate::errors::UnknownCryptoError; -use crate::hazardous::base::{ArrayVecData, Bounded, NamedContext, Public}; +use crate::hazardous::base::{ArrayVecData, Context, Public}; use crate::hazardous::hash::blake2::blake2b_core; use crate::hazardous::hash::blake2::blake2b_core::BLAKE2B_OUTSIZE; @@ -75,20 +75,17 @@ use std::io; /// An error will be returned if: /// - `slice` is empty. /// - `slice` is greater than 64 bytes. -pub type Digest = Public, BlakeDigest>; +pub type Digest = Public>; /// A marker type to declare that this data represents a Blake2b digest. pub struct BlakeDigest; -impl Bounded for BlakeDigest { +impl Context for BlakeDigest { + const NAME: &'static str = "Blake2bDigest"; const MIN: usize = 1; const MAX: usize = BLAKE2B_OUTSIZE; } -impl NamedContext for BlakeDigest { - const NAME: &'static str = "Blake2bDigest"; -} - #[derive(Debug, Clone)] /// BLAKE2b streaming state. pub struct Blake2b { From 1b2a99d209a63e5bf883b110fd500c68df165545 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Wed, 11 May 2022 00:38:07 -0400 Subject: [PATCH 21/25] begin base test_framework --- src/test_framework/base_interface.rs | 162 +++++++++++++++++++++++++++ src/test_framework/mod.rs | 2 + 2 files changed, 164 insertions(+) create mode 100644 src/test_framework/base_interface.rs diff --git a/src/test_framework/base_interface.rs b/src/test_framework/base_interface.rs new file mode 100644 index 00000000..f67f8765 --- /dev/null +++ b/src/test_framework/base_interface.rs @@ -0,0 +1,162 @@ +use crate::{ + hazardous::base::{Context, Data, Generate, VecData}, + Public, Secret, +}; + +pub(crate) fn test_omitted_debug(secret: Secret) +where + C: Context, + D: Data, +{ + let secret_data = format!("{:?}", secret.unprotected_as_bytes()); + let debug_contents = format!("{:?}", secret_data); + assert!(!debug_contents.contains(&secret_data)); +} + +pub(crate) fn test_normal_debug(public: Public) +where + C: Context, + D: Data, +{ + let public_data = format!("{:?}", public.as_ref()); + let debug_contents = format!("{:?}", public_data); + assert!(debug_contents.contains(&public_data)); +} + +#[cfg(feature = "safe_api")] +pub(crate) fn test_as_bytes_secret() +where + C: Context, + D: Data, +{ + if C::MIN == C::MAX { + // Test fixed-length definitions + let data = vec![0; C::MIN]; + let secret = Secret::::from_slice(&data).unwrap(); + + assert_eq!(secret.unprotected_as_bytes().len(), secret.len()); + assert!(!secret.is_empty()); + assert_eq!(secret.len(), C::MIN); + assert_eq!(secret.len(), C::MAX); + } else { + // Test non-fixed-length definitions + let data = vec![0; C::MAX]; + let secret_lower = Secret::::from_slice(&data[..C::MIN]).unwrap(); + let secret_upper = Secret::::from_slice(&data[..C::MAX]).unwrap(); + + assert_eq!(secret_lower.len(), C::MIN); + assert_eq!(secret_upper.len(), C::MAX); + + assert_eq!( + secret_lower.unprotected_as_bytes().len(), + secret_lower.len() + ); + assert_eq!( + secret_upper.unprotected_as_bytes().len(), + secret_upper.len() + ); + + assert_eq!(secret_lower.is_empty(), false); + assert_eq!(secret_upper.is_empty(), false); + } +} + +#[cfg(feature = "safe_api")] +pub(crate) fn test_as_bytes_public() +where + C: Context, + D: Data, +{ + if C::MIN == C::MAX { + // Test fixed-length definitions + let data = vec![0; C::MIN]; + let public = Public::::from_slice(&data).unwrap(); + + assert_eq!(public.as_ref().len(), public.len()); + assert!(!public.is_empty()); + assert_eq!(public.len(), C::MIN); + assert_eq!(public.len(), C::MAX); + } else { + // Test non-fixed-length definitions + let data = vec![0; C::MAX]; + let public_lower = Public::::from_slice(&data[..C::MIN]).unwrap(); + let public_upper = Public::::from_slice(&data[..C::MAX]).unwrap(); + + assert_eq!(public_lower.len(), C::MIN); + assert_eq!(public_upper.len(), C::MAX); + + assert_eq!(public_lower.as_ref().len(), public_lower.len()); + assert_eq!(public_upper.as_ref().len(), public_upper.len()); + + assert_eq!(public_lower.is_empty(), false); + assert_eq!(public_upper.is_empty(), false); + } +} + +#[cfg(feature = "safe_api")] +pub(crate) fn test_generate_public() +where + C: Context + Generate, + D: Data, +{ + let generated = Public::::generate(); + assert!(!generated.as_ref().iter().copied().all(|b| b == 0)); + assert_eq!(generated.len(), C::GEN_SIZE); + assert_eq!(generated.as_ref().len(), C::GEN_SIZE); +} + +#[cfg(feature = "safe_api")] +pub(crate) fn test_generate_secret() +where + C: Context + Generate, + D: Data, +{ + let generated = Secret::::generate(); + + assert!(!generated + .unprotected_as_bytes() + .iter() + .copied() + .all(|b| b == 0)); + + assert_eq!(generated.len(), C::GEN_SIZE); + assert_eq!(generated.unprotected_as_bytes().len(), C::GEN_SIZE); +} + +#[cfg(feature = "safe_api")] +pub(crate) fn test_generate_with_size_public() +where + C: Context + Generate, + D: Data, +{ + // least, middle, greatest possible value + let sizes = Vec::from([C::MIN, (C::MIN + (C::MAX - C::MIN) / 2), C::MAX]); + + for size in sizes { + let generated = Public::::generate_with_size(size).unwrap(); + assert!(!generated.as_ref().iter().copied().all(|b| b == 0)); + assert_eq!(generated.len(), size); + assert_eq!(generated.as_ref().len(), size); + } +} + +#[cfg(feature = "safe_api")] +pub(crate) fn test_generate_with_size_secret() +where + C: Context + Generate, + D: Data, +{ + // least, middle, greatest possible value + let sizes = Vec::from([C::MIN, (C::MIN + (C::MAX - C::MIN) / 2), C::MAX]); + + for size in sizes { + let generated = Secret::::generate_with_size(size).unwrap(); + assert!(!generated + .unprotected_as_bytes() + .iter() + .copied() + .all(|b| b == 0)); + assert_eq!(generated.len(), size); + assert_eq!(generated.unprotected_as_bytes().len(), size); + } +} diff --git a/src/test_framework/mod.rs b/src/test_framework/mod.rs index 8612baec..c5fafba6 100644 --- a/src/test_framework/mod.rs +++ b/src/test_framework/mod.rs @@ -28,3 +28,5 @@ pub mod aead_interface; /// Tests for stream ciphers such as `chacha20`. pub mod streamcipher_interface; + +pub mod base_interface; From 025b823f895165e055c6415f7bd1cc92b0a0a8df Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Wed, 11 May 2022 12:23:00 -0400 Subject: [PATCH 22/25] remove safe_api bound on as_bytes base tests --- src/test_framework/base_interface.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/test_framework/base_interface.rs b/src/test_framework/base_interface.rs index f67f8765..aa84e9b8 100644 --- a/src/test_framework/base_interface.rs +++ b/src/test_framework/base_interface.rs @@ -23,24 +23,20 @@ where assert!(debug_contents.contains(&public_data)); } -#[cfg(feature = "safe_api")] -pub(crate) fn test_as_bytes_secret() +pub(crate) fn test_as_bytes_secret(secret: Secret) where C: Context, D: Data, { if C::MIN == C::MAX { // Test fixed-length definitions - let data = vec![0; C::MIN]; - let secret = Secret::::from_slice(&data).unwrap(); - assert_eq!(secret.unprotected_as_bytes().len(), secret.len()); assert!(!secret.is_empty()); assert_eq!(secret.len(), C::MIN); assert_eq!(secret.len(), C::MAX); } else { // Test non-fixed-length definitions - let data = vec![0; C::MAX]; + let data = secret.unprotected_as_bytes(); let secret_lower = Secret::::from_slice(&data[..C::MIN]).unwrap(); let secret_upper = Secret::::from_slice(&data[..C::MAX]).unwrap(); @@ -61,24 +57,20 @@ where } } -#[cfg(feature = "safe_api")] -pub(crate) fn test_as_bytes_public() +pub(crate) fn test_as_bytes_public(public: Public) where C: Context, D: Data, { if C::MIN == C::MAX { // Test fixed-length definitions - let data = vec![0; C::MIN]; - let public = Public::::from_slice(&data).unwrap(); - assert_eq!(public.as_ref().len(), public.len()); assert!(!public.is_empty()); assert_eq!(public.len(), C::MIN); assert_eq!(public.len(), C::MAX); } else { // Test non-fixed-length definitions - let data = vec![0; C::MAX]; + let data = public.as_ref(); let public_lower = Public::::from_slice(&data[..C::MIN]).unwrap(); let public_upper = Public::::from_slice(&data[..C::MAX]).unwrap(); From 17a7b5423d8498e253b3ba736d178683e6fbf2f4 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Wed, 11 May 2022 12:48:11 -0400 Subject: [PATCH 23/25] add base tests for blake2b Digest --- src/hazardous/hash/blake2/blake2b.rs | 23 +++++++++++++++++++++++ src/test_framework/base_interface.rs | 7 ++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/hazardous/hash/blake2/blake2b.rs b/src/hazardous/hash/blake2/blake2b.rs index 4ffb9aa6..ef6fedee 100644 --- a/src/hazardous/hash/blake2/blake2b.rs +++ b/src/hazardous/hash/blake2/blake2b.rs @@ -417,4 +417,27 @@ mod public { hash_a == hash_b } } + + mod test_base { + use crate::{ + hazardous::hash::blake2::blake2b::{Blake2b, Digest}, + test_framework::base_interface, + }; + + fn example_digest() -> Digest { + let mut hasher = Blake2b::new(64).unwrap(); + hasher.update(b"test data").unwrap(); + hasher.finalize().unwrap() + } + + #[test] + fn test_omitted_debug() { + base_interface::test_normal_debug(example_digest()); + } + + #[test] + fn test_as_bytes_public() { + base_interface::test_as_bytes_public(example_digest()); + } + } } diff --git a/src/test_framework/base_interface.rs b/src/test_framework/base_interface.rs index aa84e9b8..2ad969df 100644 --- a/src/test_framework/base_interface.rs +++ b/src/test_framework/base_interface.rs @@ -2,6 +2,7 @@ use crate::{ hazardous::base::{Context, Data, Generate, VecData}, Public, Secret, }; +use core::marker::PhantomData; pub(crate) fn test_omitted_debug(secret: Secret) where @@ -86,7 +87,7 @@ where } #[cfg(feature = "safe_api")] -pub(crate) fn test_generate_public() +pub(crate) fn test_generate_public(_phantom: PhantomData>) where C: Context + Generate, D: Data, @@ -98,7 +99,7 @@ where } #[cfg(feature = "safe_api")] -pub(crate) fn test_generate_secret() +pub(crate) fn test_generate_secret(_phantom: PhantomData>) where C: Context + Generate, D: Data, @@ -116,7 +117,7 @@ where } #[cfg(feature = "safe_api")] -pub(crate) fn test_generate_with_size_public() +pub(crate) fn test_generate_with_size_public(_phantom: PhantomData>) where C: Context + Generate, D: Data, From 0cc1667df9c4d75cf960eb609bf2109c47387c95 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sun, 17 Jul 2022 21:26:11 -0400 Subject: [PATCH 24/25] move per-type base test impl to macro --- src/hazardous/base.rs | 261 ++++++++++++++------------- src/hazardous/hash/blake2/blake2b.rs | 19 +- src/lib.rs | 1 + src/test_framework/base_interface.rs | 77 +++++--- src/test_framework/mod.rs | 2 + 5 files changed, 200 insertions(+), 160 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index 5ccfdd3d..f449f99e 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -140,6 +140,8 @@ use crate::errors::UnknownCryptoError; use core::{convert::TryFrom, fmt, marker::PhantomData}; +pub use self::{array_data::ArrayData, array_vec_data::ArrayVecData, vec_data::VecData}; + /// A simple container for bytes that are considered non-sensitive. pub struct Public { context: PhantomData, @@ -179,8 +181,6 @@ pub trait Context { /// - [`Secret::generate`](Secret::generate) /// - [`Public::generate_with_size`](Public::generate_with_size) /// - [`Secret::generate_with_size`](Secret::generate_with_size) -// -// TODO: Does `Generate` need to have a `const` GEN_SIZE? pub trait Generate: Context { /// The size in bytes of the type when generated randomly. Note that /// it is a logical error for `GEN_SIZE` to be less than @@ -288,7 +288,9 @@ where self.data.as_ref().is_empty() } - /// TODO: Grab docs for `unprotected_as_bytes` and insert here. + /// Return the object as byte slice. __**Warning**__: Should not be + /// used unless strictly needed. This __**breaks protections**__ that + /// the type implements. pub fn unprotected_as_bytes(&self) -> &[u8] { self.data.as_ref() } @@ -401,7 +403,7 @@ where } } -/// Delegates to [`B::from_bytes`](crate::hazardous::base::TryFromBytes) under the hood. +/// Delegates to [`B::try_from_bytes`](crate::hazardous::base::TryFromBytes) under the hood. impl TryFrom<&[u8]> for Public where C: Context, @@ -417,7 +419,7 @@ where } } -/// Delegates to [`B::from_bytes`](crate::hazardous::base::TryFromBytes) under the hood. +/// Delegates to [`B::try_from_bytes`](crate::hazardous::base::TryFromBytes) under the hood. impl TryFrom<&[u8]> for Secret where C: Context, @@ -502,150 +504,167 @@ where } } -/// A convenient type for holding data with a statically known size. -/// The bytes are held with a static array (`[u8; LEN]`). -// -// NOTE: Deriving PartialEq here is okay becuase we don't use it for -// timing-sensitive comparisons. `ArrayData` is always wrapped in a -// [`Secret`](crate::hazardous::base::Secret) if it's used for -// sensitive information, which implements constant-time comparisons. -// -// Same thing for Debug: the Secret wrapper will handle it.. -#[derive(Clone, Debug, PartialEq)] -pub struct ArrayData { - bytes: [u8; LEN], -} +mod array_data { + use super::{Data, TryFromBytes}; + use crate::errors::UnknownCryptoError; + use std::convert::TryFrom; + + /// A convenient type for holding data with a statically known size. + /// The bytes are held with a static array (`[u8; LEN]`). + // + // NOTE: Deriving PartialEq here is okay becuase we don't use it for + // timing-sensitive comparisons. `ArrayData` is always wrapped in a + // [`Secret`](crate::hazardous::base::Secret) if it's used for + // sensitive information, which implements constant-time comparisons. + // + // Same thing for Debug: the Secret wrapper will handle it.. + #[derive(Clone, Debug, PartialEq)] + pub struct ArrayData { + pub(crate) bytes: [u8; LEN], + } -impl Data for ArrayData {} + impl Data for ArrayData {} -impl TryFromBytes for ArrayData { - fn try_from_bytes(slice: &[u8]) -> Result { - let bytes = <[u8; LEN]>::try_from(slice).map_err(|_| UnknownCryptoError)?; - Ok(Self { bytes }) + impl TryFromBytes for ArrayData { + fn try_from_bytes(slice: &[u8]) -> Result { + let bytes = <[u8; LEN]>::try_from(slice).map_err(|_| UnknownCryptoError)?; + Ok(Self { bytes }) + } } -} -impl AsRef<[u8]> for ArrayData { - fn as_ref(&self) -> &[u8] { - self.bytes.as_slice() + impl AsRef<[u8]> for ArrayData { + fn as_ref(&self) -> &[u8] { + self.bytes.as_slice() + } } -} -impl AsMut<[u8]> for ArrayData { - fn as_mut(&mut self) -> &mut [u8] { - &mut self.bytes + impl AsMut<[u8]> for ArrayData { + fn as_mut(&mut self) -> &mut [u8] { + &mut self.bytes + } } } -/// A convenient type for holding data with a static upper bound on -/// its size. The bytes are held with a static array (`[u8; MAX]`). -#[derive(Clone, Debug)] -pub struct ArrayVecData { - bytes: [u8; MAX], - len: usize, -} +mod array_vec_data { + use super::{Data, TryFromBytes}; + use crate::errors::UnknownCryptoError; -impl Data for ArrayVecData {} + /// A convenient type for holding data with a static upper bound on + /// its size. The bytes are held with a static array (`[u8; MAX]`). + #[derive(Clone, Debug)] + pub struct ArrayVecData { + pub(crate) bytes: [u8; MAX], + pub(crate) len: usize, + } -impl TryFromBytes for ArrayVecData { - fn try_from_bytes(slice: &[u8]) -> Result { - if slice.len() > MAX { - return Err(UnknownCryptoError); - } + impl Data for ArrayVecData {} - let mut bytes = [0u8; MAX]; + impl TryFromBytes for ArrayVecData { + fn try_from_bytes(slice: &[u8]) -> Result { + if slice.len() > MAX { + return Err(UnknownCryptoError); + } - // PANIC: This is ok because we just checked that the length - // was less than MAX above. Violating that condition is the - // only thing that would cause this to panic. - bytes - .get_mut(0..slice.len()) - .unwrap() - .copy_from_slice(slice); + let mut bytes = [0u8; MAX]; - Ok(Self { - bytes, - len: slice.len(), - }) - } -} + // PANIC: This is ok because we just checked that the length + // was less than MAX above. Violating that condition is the + // only thing that would cause this to panic. + bytes + .get_mut(0..slice.len()) + .unwrap() + .copy_from_slice(slice); -impl AsRef<[u8]> for ArrayVecData { - fn as_ref(&self) -> &[u8] { - // PANIC: This unwrap is ok because the type's len is checked at - // construction time to be less than MAX. - self.bytes.get(..self.len).unwrap() + Ok(Self { + bytes, + len: slice.len(), + }) + } } -} -impl AsMut<[u8]> for ArrayVecData { - fn as_mut(&mut self) -> &mut [u8] { - // PANIC: This unwrap is ok because the type's len is checked at - // construction time to be less than MAX. - self.bytes.get_mut(..self.len).unwrap() + impl AsRef<[u8]> for ArrayVecData { + fn as_ref(&self) -> &[u8] { + // PANIC: This unwrap is ok because the type's len is checked at + // construction time to be less than MAX. + self.bytes.get(..self.len).unwrap() + } } -} -// NOTE: Using non-constant-time comparison here is okay becuase we don't -// use it for timing-sensitive comparisons. `ArrayVecData` is always wrapped -// in a [`Secret`](crate::hazardous::base::Secret) if it's used for -// sensitive information, which implements constant-time comparisons. -// -// Same thing for Debug: the Secret wrapper will handle it. -impl PartialEq for ArrayVecData { - fn eq(&self, other: &Self) -> bool { - self.bytes.get(..self.len).eq(&other.bytes.get(..other.len)) + impl AsMut<[u8]> for ArrayVecData { + fn as_mut(&mut self) -> &mut [u8] { + // PANIC: This unwrap is ok because the type's len is checked at + // construction time to be less than MAX. + self.bytes.get_mut(..self.len).unwrap() + } } } -/// A convenient type for holding data in dynamically allocated buffer. -// TODO: Should we just use a `Vec` here? We could implement all of the -// same traits for a regular old Vec. -// -// NOTE: Deriving PartialEq here is okay becuase we don't use it for -// timing-sensitive comparisons. `VecData` is always wrapped in a -// [`Secret`](crate::hazardous::base::Secret) if it's used for -// sensitive information, which implements constant-time comparisons. -// -// Same thing for Debug: the Secret wrapper will handle it.. -#[cfg(feature = "safe_api")] -#[derive(Clone, Debug, PartialEq)] -pub struct VecData { - bytes: Vec, -} - -#[cfg(feature = "safe_api")] -impl Data for VecData {} - -#[cfg(feature = "safe_api")] -impl TryFromBytes for VecData { - fn try_from_bytes(slice: &[u8]) -> Result { - Ok(Self { - bytes: slice.into(), - }) +// // NOTE: Using non-constant-time comparison here is okay becuase we don't +// // use it for timing-sensitive comparisons. `ArrayVecData` is always wrapped +// // in a [`Secret`](crate::hazardous::base::Secret) if it's used for +// // sensitive information, which implements constant-time comparisons. +// // +// // Same thing for Debug: the Secret wrapper will handle it. +// impl PartialEq for ArrayVecData { +// fn eq(&self, other: &Self) -> bool { +// self.bytes.get(..self.len).eq(&other.bytes.get(..other.len)) +// } +// } + +mod vec_data { + use super::{Data, TryFromBytes}; + use crate::errors::UnknownCryptoError; + use std::convert::TryFrom; + + /// A convenient type for holding data in dynamically allocated buffer. + // TODO: Should we just use a `Vec` here? We could implement all of the + // same traits for a regular old Vec. + // + // NOTE: Deriving PartialEq here is okay becuase we don't use it for + // timing-sensitive comparisons. `VecData` is always wrapped in a + // [`Secret`](crate::hazardous::base::Secret) if it's used for + // sensitive information, which implements constant-time comparisons. + // + // Same thing for Debug: the Secret wrapper will handle it.. + #[cfg(feature = "safe_api")] + #[derive(Clone, Debug, PartialEq)] + pub struct VecData { + pub(crate) bytes: Vec, + } + + #[cfg(feature = "safe_api")] + impl Data for VecData {} + + #[cfg(feature = "safe_api")] + impl TryFromBytes for VecData { + fn try_from_bytes(slice: &[u8]) -> Result { + Ok(Self { + bytes: slice.into(), + }) + } } -} -#[cfg(feature = "safe_api")] -impl AsRef<[u8]> for VecData { - fn as_ref(&self) -> &[u8] { - self.bytes.as_slice() + #[cfg(feature = "safe_api")] + impl AsRef<[u8]> for VecData { + fn as_ref(&self) -> &[u8] { + self.bytes.as_slice() + } } -} -#[cfg(feature = "safe_api")] -impl AsMut<[u8]> for VecData { - fn as_mut(&mut self) -> &mut [u8] { - &mut self.bytes + #[cfg(feature = "safe_api")] + impl AsMut<[u8]> for VecData { + fn as_mut(&mut self) -> &mut [u8] { + &mut self.bytes + } } -} -#[cfg(feature = "safe_api")] -impl<'a> TryFrom<&'a [u8]> for VecData { - type Error = UnknownCryptoError; + #[cfg(feature = "safe_api")] + impl<'a> TryFrom<&'a [u8]> for VecData { + type Error = UnknownCryptoError; - fn try_from(value: &'a [u8]) -> Result { - let bytes = Vec::from(value); - Ok(VecData { bytes }) + fn try_from(value: &'a [u8]) -> Result { + let bytes = Vec::from(value); + Ok(VecData { bytes }) + } } } diff --git a/src/hazardous/hash/blake2/blake2b.rs b/src/hazardous/hash/blake2/blake2b.rs index ef6fedee..b3eb627f 100644 --- a/src/hazardous/hash/blake2/blake2b.rs +++ b/src/hazardous/hash/blake2/blake2b.rs @@ -419,25 +419,12 @@ mod public { } mod test_base { - use crate::{ - hazardous::hash::blake2::blake2b::{Blake2b, Digest}, - test_framework::base_interface, - }; - - fn example_digest() -> Digest { + use crate::hazardous::hash::blake2::blake2b::{Blake2b, Digest}; + fn gen_test_data() -> Digest { let mut hasher = Blake2b::new(64).unwrap(); hasher.update(b"test data").unwrap(); hasher.finalize().unwrap() } - - #[test] - fn test_omitted_debug() { - base_interface::test_normal_debug(example_digest()); - } - - #[test] - fn test_as_bytes_public() { - base_interface::test_as_bytes_public(example_digest()); - } + crate::test_base!(Digest, gen_test_data, public); } } diff --git a/src/lib.rs b/src/lib.rs index 19c1a109..20290a8f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -117,5 +117,6 @@ pub use high_level::kex; pub use hazardous::base::{Public, Secret}; #[doc(hidden)] +#[macro_use] /// Testing framework. pub mod test_framework; diff --git a/src/test_framework/base_interface.rs b/src/test_framework/base_interface.rs index 2ad969df..5b5dcb9b 100644 --- a/src/test_framework/base_interface.rs +++ b/src/test_framework/base_interface.rs @@ -1,9 +1,40 @@ use crate::{ - hazardous::base::{Context, Data, Generate, VecData}, + hazardous::base::{Context, Data, Generate}, Public, Secret, }; use core::marker::PhantomData; +// TODO: Do we need to export this? Is it bad if we do? +#[macro_export] +macro_rules! test_base { + ($newtype_alias:ident, $gen_test_data:ident, public) => { + #[test] + fn test_normal_debug() { + crate::test_framework::base_interface::test_normal_debug($gen_test_data()); + } + + #[test] + fn test_as_bytes_public() { + crate::test_framework::base_interface::test_as_bytes_public($gen_test_data()); + } + }; + + ($newtype_alias:ident, generate, public) => { + #[test] + fn test_normal_debug() { + let generated_newtype = $newtype_alias::generate(); + crate::test_framework::base_interface::test_normal_debug($gen_test_data()); + } + + #[test] + fn test_as_bytes_public() { + crate::test_framework::base_interface::test_as_bytes_public($gen_test_data()); + } + }; + + ($newtype_alias:ident, private, $newtype_mod:ident, $gen_test_data: ident) => {}; +} + pub(crate) fn test_omitted_debug(secret: Secret) where C: Context, @@ -86,18 +117,6 @@ where } } -#[cfg(feature = "safe_api")] -pub(crate) fn test_generate_public(_phantom: PhantomData>) -where - C: Context + Generate, - D: Data, -{ - let generated = Public::::generate(); - assert!(!generated.as_ref().iter().copied().all(|b| b == 0)); - assert_eq!(generated.len(), C::GEN_SIZE); - assert_eq!(generated.as_ref().len(), C::GEN_SIZE); -} - #[cfg(feature = "safe_api")] pub(crate) fn test_generate_secret(_phantom: PhantomData>) where @@ -117,20 +136,15 @@ where } #[cfg(feature = "safe_api")] -pub(crate) fn test_generate_with_size_public(_phantom: PhantomData>) +pub(crate) fn test_generate_public(_phantom: PhantomData>) where C: Context + Generate, D: Data, { - // least, middle, greatest possible value - let sizes = Vec::from([C::MIN, (C::MIN + (C::MAX - C::MIN) / 2), C::MAX]); - - for size in sizes { - let generated = Public::::generate_with_size(size).unwrap(); - assert!(!generated.as_ref().iter().copied().all(|b| b == 0)); - assert_eq!(generated.len(), size); - assert_eq!(generated.as_ref().len(), size); - } + let generated = Public::::generate(); + assert!(!generated.as_ref().iter().copied().all(|b| b == 0)); + assert_eq!(generated.len(), C::GEN_SIZE); + assert_eq!(generated.as_ref().len(), C::GEN_SIZE); } #[cfg(feature = "safe_api")] @@ -153,3 +167,20 @@ where assert_eq!(generated.unprotected_as_bytes().len(), size); } } + +#[cfg(feature = "safe_api")] +pub(crate) fn test_generate_with_size_public(_phantom: PhantomData>) +where + C: Context + Generate, + D: Data, +{ + // least, middle, greatest possible value + let sizes = Vec::from([C::MIN, (C::MIN + (C::MAX - C::MIN) / 2), C::MAX]); + + for size in sizes { + let generated = Public::::generate_with_size(size).unwrap(); + assert!(!generated.as_ref().iter().copied().all(|b| b == 0)); + assert_eq!(generated.len(), size); + assert_eq!(generated.as_ref().len(), size); + } +} diff --git a/src/test_framework/mod.rs b/src/test_framework/mod.rs index c5fafba6..755d7122 100644 --- a/src/test_framework/mod.rs +++ b/src/test_framework/mod.rs @@ -29,4 +29,6 @@ pub mod aead_interface; /// Tests for stream ciphers such as `chacha20`. pub mod streamcipher_interface; +#[cfg(test)] +#[macro_use] pub mod base_interface; From d5846af7e4480633ea3b45f23d9bd36d783cd2f5 Mon Sep 17 00:00:00 2001 From: Vince Mutolo Date: Sun, 17 Jul 2022 22:25:15 -0400 Subject: [PATCH 25/25] move aead::SecretKey to const generics --- src/hazardous/base.rs | 6 +- src/high_level/aead.rs | 43 ++++++++++++- src/test_framework/base_interface.rs | 92 +++++++++++++++++++--------- 3 files changed, 106 insertions(+), 35 deletions(-) diff --git a/src/hazardous/base.rs b/src/hazardous/base.rs index f449f99e..78e42e34 100644 --- a/src/hazardous/base.rs +++ b/src/hazardous/base.rs @@ -160,10 +160,11 @@ pub trait Context { /// The type name that will appear in Debug impls. const NAME: &'static str; - /// The largest number of bytes this type should be allowed to hold. + /// The smallest number of bytes this type should be allowed to hold. const MIN: usize; - /// The smallest number of bytes this type should be allowed to hold. + // TODO: Should this be an exclusive bound? + /// The largest number of bytes this type should be allowed to hold. const MAX: usize; } @@ -493,7 +494,6 @@ where } } -// We implement this manually to skip over the PhantomData. impl fmt::Debug for Secret where C: Context, diff --git a/src/high_level/aead.rs b/src/high_level/aead.rs index 4ab5003c..aee5aa4a 100644 --- a/src/high_level/aead.rs +++ b/src/high_level/aead.rs @@ -80,11 +80,11 @@ #![cfg_attr(docsrs, doc(cfg(feature = "safe_api")))] -pub use super::hltypes::SecretKey; use crate::{ errors::UnknownCryptoError, hazardous::{ aead, + base::{Context, Generate, Secret, VecData}, mac::poly1305::POLY1305_OUTSIZE, stream::{ chacha20, @@ -93,6 +93,29 @@ use crate::{ }, }; +/// A type to represent the `SecretKey` used in AEAD. +pub type SecretKey = Secret; + +/// A marker type to declare that this data represents an AEAD secret key. +// TODO: Should this be named something more specific? Like `ChaChaKey`? +pub struct AeadKey; + +impl Context for AeadKey { + const NAME: &'static str = "AeadKey"; + const MIN: usize = 32; // TODO: Is this the right min size? + const MAX: usize = 1024 * 1024; // TODO: Is there a less arbitrary upper bound than a MB? +} + +impl Generate for AeadKey { + const GEN_SIZE: usize = 32; +} + +impl Default for SecretKey { + fn default() -> Self { + Self::generate() + } +} + #[must_use = "SECURITY WARNING: Ignoring a Result can have real security implications."] /// Authenticated encryption using XChaCha20Poly1305. pub fn seal(secret_key: &SecretKey, plaintext: &[u8]) -> Result, UnknownCryptoError> { @@ -399,7 +422,7 @@ mod public { #[test] fn test_secret_length_err() { - let key = SecretKey::generate(31).unwrap(); + let key = SecretKey::generate_with_size(31).unwrap(); let plaintext = "Secret message".as_bytes(); assert!(seal(&key, plaintext).is_err()); @@ -513,7 +536,7 @@ mod public { #[test] fn test_secret_length_err() { - let key = SecretKey::generate(31).unwrap(); + let key = SecretKey::generate_with_size(31).unwrap(); assert!(StreamSealer::new(&key).is_err()); assert!(StreamOpener::new(&key, &Nonce::generate()).is_err()); } @@ -628,4 +651,18 @@ mod public { open(&sk2, &ct).is_err() } } + + mod test_base { + use crate::{ + hazardous::base::VecData, + high_level::aead::{AeadKey, SecretKey}, + }; + + fn gen_test_data() -> SecretKey { + SecretKey::generate_with_size(64).unwrap() + } + + crate::test_base!(SecretKey, gen_test_data, secret); + crate::test_generate!(AeadKey, VecData, secret); + } } diff --git a/src/test_framework/base_interface.rs b/src/test_framework/base_interface.rs index 5b5dcb9b..5079b76f 100644 --- a/src/test_framework/base_interface.rs +++ b/src/test_framework/base_interface.rs @@ -2,7 +2,6 @@ use crate::{ hazardous::base::{Context, Data, Generate}, Public, Secret, }; -use core::marker::PhantomData; // TODO: Do we need to export this? Is it bad if we do? #[macro_export] @@ -19,20 +18,61 @@ macro_rules! test_base { } }; - ($newtype_alias:ident, generate, public) => { + ($newtype_alias:ident, $gen_test_data: ident, secret) => { #[test] - fn test_normal_debug() { - let generated_newtype = $newtype_alias::generate(); - crate::test_framework::base_interface::test_normal_debug($gen_test_data()); + fn test_omitted_debug() { + crate::test_framework::base_interface::test_omitted_debug($gen_test_data()); } #[test] - fn test_as_bytes_public() { - crate::test_framework::base_interface::test_as_bytes_public($gen_test_data()); + fn test_as_bytes_secret() { + crate::test_framework::base_interface::test_as_bytes_secret($gen_test_data()); + } + }; +} + +// TODO: Do we need to export this? Is it bad if we do? +#[macro_export] +macro_rules! test_generate { + ($context_type:ident, $data_type:ident, public) => { + #[cfg(feature = "safe_api")] + #[test] + fn test_generate_public() { + crate::test_framework::base_interface::test_generate_public::< + $context_type, + $data_type, + >(); + } + + #[cfg(feature = "safe_api")] + #[test] + fn test_generate_with_size_public() { + crate::test_framework::base_interface::test_generate_with_size_public::< + $context_type, + $data_type, + >(); } }; - ($newtype_alias:ident, private, $newtype_mod:ident, $gen_test_data: ident) => {}; + ($context_type:ident, $data_type:ident, secret) => { + #[cfg(feature = "safe_api")] + #[test] + fn test_generate_secret() { + crate::test_framework::base_interface::test_generate_secret::< + $context_type, + $data_type, + >(); + } + + #[cfg(feature = "safe_api")] + #[test] + fn test_generate_with_size_secret() { + crate::test_framework::base_interface::test_generate_with_size_secret::< + $context_type, + $data_type, + >(); + } + }; } pub(crate) fn test_omitted_debug(secret: Secret) @@ -41,7 +81,7 @@ where D: Data, { let secret_data = format!("{:?}", secret.unprotected_as_bytes()); - let debug_contents = format!("{:?}", secret_data); + let debug_contents = format!("{:?}", secret); assert!(!debug_contents.contains(&secret_data)); } @@ -60,32 +100,26 @@ where C: Context, D: Data, { + // Test fixed-length definitions + assert_eq!(secret.unprotected_as_bytes().len(), secret.len()); + assert!(!secret.is_empty()); + if C::MIN == C::MAX { - // Test fixed-length definitions - assert_eq!(secret.unprotected_as_bytes().len(), secret.len()); - assert!(!secret.is_empty()); assert_eq!(secret.len(), C::MIN); assert_eq!(secret.len(), C::MAX); - } else { + } else if C::MIN != C::MAX { // Test non-fixed-length definitions let data = secret.unprotected_as_bytes(); - let secret_lower = Secret::::from_slice(&data[..C::MIN]).unwrap(); - let secret_upper = Secret::::from_slice(&data[..C::MAX]).unwrap(); - assert_eq!(secret_lower.len(), C::MIN); - assert_eq!(secret_upper.len(), C::MAX); + let secret_subset = Secret::::from_slice(&data[..C::MIN]).unwrap(); + + assert_eq!(secret_subset.len(), C::MIN); + assert_eq!(secret_subset.is_empty(), false); assert_eq!( - secret_lower.unprotected_as_bytes().len(), - secret_lower.len() - ); - assert_eq!( - secret_upper.unprotected_as_bytes().len(), - secret_upper.len() + secret_subset.unprotected_as_bytes().len(), + secret_subset.len() ); - - assert_eq!(secret_lower.is_empty(), false); - assert_eq!(secret_upper.is_empty(), false); } } @@ -118,7 +152,7 @@ where } #[cfg(feature = "safe_api")] -pub(crate) fn test_generate_secret(_phantom: PhantomData>) +pub(crate) fn test_generate_secret() where C: Context + Generate, D: Data, @@ -136,7 +170,7 @@ where } #[cfg(feature = "safe_api")] -pub(crate) fn test_generate_public(_phantom: PhantomData>) +pub(crate) fn test_generate_public() where C: Context + Generate, D: Data, @@ -169,7 +203,7 @@ where } #[cfg(feature = "safe_api")] -pub(crate) fn test_generate_with_size_public(_phantom: PhantomData>) +pub(crate) fn test_generate_with_size_public() where C: Context + Generate, D: Data,