Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

convert newtypes to const generics #274

Draft
wants to merge 26 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
e5e45c0
convert blake2b::Digest to const generics
Feb 3, 2022
fb394f4
simplified and documented const generics impls
Feb 3, 2022
7c71df1
remove duplicate docs on PublicVec
Feb 3, 2022
d950d04
move to marker types and traits
Feb 4, 2022
4835b7c
Merge branch 'master' into const-generics
Feb 4, 2022
52582f9
squash me
Feb 13, 2022
1a4b509
split Data into PublicData and SecretData
Apr 3, 2022
651e7a6
remove optional bounds and fix some cfgs
Apr 3, 2022
26143d7
move/fix docs, remove unnecessary lifetimes
Apr 3, 2022
0ce38c5
broaden PartialEq impls for {Secret,Public}Data
Apr 3, 2022
f622c92
base: better module docs
Apr 3, 2022
64ab637
rename to ArrayVecData and add actual ArrayData
Apr 3, 2022
6930a0f
rename PublicData and SecretData to Public and Secret
Apr 30, 2022
f45f7ec
change blake2b Digest to use ArrayVecData
Apr 30, 2022
cae31f7
restrict PartialEq to exact same type
Apr 30, 2022
e79316c
fix base docs line in hazardous module docs
Apr 30, 2022
a086bcf
move to core::fmt::Debug for Secret and Public
May 1, 2022
cb85630
add omitted_debug tests to base types
May 1, 2022
ee63c2a
clean up docs
May 1, 2022
68e03b9
add generate_with_size method to Public and Secret
May 1, 2022
967e0b9
simplify to Context and Data super traits
May 1, 2022
1b2a99d
begin base test_framework
May 11, 2022
025b823
remove safe_api bound on as_bytes base tests
May 11, 2022
17a7b54
add base tests for blake2b Digest
May 11, 2022
0cc1667
move per-type base test impl to macro
Jul 18, 2022
d5846af
move aead::SecretKey to const generics
Jul 18, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 156 additions & 0 deletions src/hazardous/base.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
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<usize> = None;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see no reason to leave these as Option. Aren't we always going to define a bound over all types?

Copy link
Contributor Author

@vlmutolo vlmutolo Feb 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I can't think of a case when we'd want no bound. I actually don't remember why I put it there in the first place. At the time I was seven layers deep in thinking about the trait system, so this didn't get a ton of thought haha.


/// The smallest number of bytes this type should be allowed to hold.
const MAX: Option<usize> = None;
}

/// 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<B, K> {
bytes: B,
phantom: PhantomData<K>,
Copy link
Contributor Author

@vlmutolo vlmutolo Feb 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was probably overambitious.

It ended up working out fine implementation-wise, but the generated documentation is pretty difficult to parse. By conditionally implementing "secret" methods like unprotected_as_bytes based on type parameter bounds, the "secret" methods are mixed in with the "public" methods (like as_ref) all in the same docs page. That's not great when compared to the status quo.

As a solution, I think it would be best to split up Data<B, K> into Secret<B, K> and Public<B,K>. It's still necessary to have both of the type parameters: the first is for parameterizing over storage type (Array vs Vec) and the second is for labeling the data with its purpose, like Secret<Array<32>, Argon2iKey>, which precents key misuse by making it a distinct type. Then we wrap the whole thing in a type alias.

pub type SecretKey = Secret<Array<32>, Argon2iKey>;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that splitting Data<B, K> into Secret<B, K> and Public<B,K> is probably the best choice here.

}

impl<'a, B, K> Data<B, K>
where
B: TryFrom<&'a [u8], Error = UnknownCryptoError>,
K: Bounded,
{
/// TODO
pub fn from_slice(slice: &'a [u8]) -> Result<Self, B::Error> {
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);
}

Ok(Self {
bytes: B::try_from(slice)?,
phantom: PhantomData,
})
}
}

impl<'a, B, K> Data<B, K>
where
B: AsRef<[u8]>,
{
/// Get the length of the contained byte slice.
pub fn len(&self) -> usize {
self.bytes.as_ref().len()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot get length based on the bytes here. If like BLAKE2b, it has valid ranges this will return the upper bound, because that's the array which has been allocated, not the actual length requested.

Copy link
Contributor Author

@vlmutolo vlmutolo Feb 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was assuming here that the B's AsRef implementation would take care of returning the correct subset. I was also assuming that B would be slightly more than an array. Something like (ignoring naming and syntax):

struct ArrayBytes {
    bytes: [u8: MAX],
    len: usize,
}

Though I think it's definitely possible to just use basic types for B like Data<[u8; 24]> and then stick the extra logic in Bound maybe.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure at the moment, which one would work best tbh.

Copy link
Contributor Author

@vlmutolo vlmutolo Apr 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this a bit more, and I see two separate use cases: an array of known size, and an array of known maximum size.

If we have a function that returns a precise number of bytes, we should probably use an ArrayData like:

struct ArrayData<const LEN: usize> {
   bytes: [u8; LEN]
}

and then define a new type like:

struct Sha256Ctx;
type Sha256Tag = PublicData<ArrayData<32>, Sha256Ctx>;

However, if we only know the maximum number of bytes to hold, maybe we define an ArrayVecData:

struct ArrayData<const MAX: usize> {
   bytes: [u8; MAX],
   len: usize,
}

and then define types like:

struct PasswordCtx;

// I don't know why we'd want to restrict the password length but let's pretend.
type Password = SecretData<ArrayVecData<32>, PasswordCtx>;

let pw0 = Password::from_slice(&b"rockyou");  // seven-byte password
let pw1 = Password::from_slice(&b"pa$$word"); // eight-byte password

}

/// 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<B, K>
where
B: AsRef<[u8]>,
K: Public,
{
/// Get a reference to the underlying byte slice.
fn as_ref(&self) -> &[u8] {
self.bytes.as_ref()
}
}

impl<'a, B, K> Data<B, K>
where
B: AsRef<[u8]>,
K: Secret,
{
/// TODO: Grab docs for `unprotected_as_bytes` and insert here.
pub fn unprotected_as_bytes(&self) -> &[u8] {
self.bytes.as_ref()
}
}

// We implement this manually to skip over the PhantomData.
impl<B, K> PartialEq for Data<B, K>
where
B: PartialEq<B>,
{
fn eq(&self, other: &Self) -> bool {
self.bytes.eq(&other.bytes)
}
}

// We implement this manually to skip over the PhantomData.
impl<B, K> fmt::Debug for Data<B, K>
where
B: fmt::Debug,
{
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.bytes.fmt(f)
}
}

/// 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)]
vlmutolo marked this conversation as resolved.
Show resolved Hide resolved
pub struct StaticData<const MAX: usize> {
bytes: [u8; MAX],
len: usize,
}

impl<const MAX: usize> TryFrom<&[u8]> for StaticData<MAX> {
type Error = UnknownCryptoError;

fn try_from(slice: &[u8]) -> Result<Self, Self::Error> {
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 {
bytes,
len: slice.len(),
})
}
}

impl<const MAX: usize> AsRef<[u8]> for StaticData<MAX> {
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()
}
}

impl<const MAX: usize> PartialEq for StaticData<MAX> {
fn eq(&self, other: &Self) -> bool {
self.bytes.get(..self.len).eq(&other.bytes.get(..other.len))
}
}
23 changes: 15 additions & 8 deletions src/hazardous/hash/blake2/blake2b.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,27 @@
//! [`mac::blake2b`]: crate::hazardous::mac::blake2b

use crate::errors::UnknownCryptoError;
use crate::hazardous::base::{Bounded, Data, Public, StaticData};
use crate::hazardous::hash::blake2::blake2b_core;
use crate::hazardous::hash::blake2::blake2b_core::BLAKE2B_OUTSIZE;

#[cfg(feature = "safe_api")]
use std::io;

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)
/// 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.
pub type Digest = Data<StaticData<BLAKE2B_OUTSIZE>, 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<usize> = Some(1);
const MAX: Option<usize> = Some(BLAKE2B_OUTSIZE);
}

#[derive(Debug, Clone)]
Expand Down
3 changes: 3 additions & 0 deletions src/hazardous/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,6 @@ pub mod stream;

/// Elliptic-Curve Cryptography.
pub mod ecc;

/// TODO: This probably belongs somewhere else.
pub mod base;