crypto-common: add SerializableState trait

[rpiasetskyi]

This trait is used for saving internal state of the hash core and restoring it later.

The original issue was discussed here: https://github.com/RustCrypto/hashes/issues/310

[tarcieri]

I think the core notion here is a "serializable state" rather than an "internal state"

[rpiasetskyi]

Sorry, missed the last comment. I will move the trait to crypto-common and use serialize/deserialize methods.

Should deserialize method return Result (if the buffer contains invalid values)?

[rpiasetskyi]

The versions of the crates have to be updated: should they be 0.1.7/0.10.4 or 0.2.0/0.11.0?

Right now crypto-common is 0.1.6 and digest is 0.10.3.

[rpiasetskyi]

Do not forget to also implement it for CoreWrapper and ideally for RtVariableCoreWrapper.

There is a small issue with RtVariableCoreWrapper: it has a member that has usize type.

It is needed for SerializedStateSize definition. It should be something like:

type SerializedStateSize =  Sum<Sum<Add1<T::SerializedStateSize>, UsizeSize>, T::BlockSize>;

Is there any known UsizeSize definition?

Or should it be implemented via cfg?

#[cfg(target_pointer_width = "16")]
type UsizeSize = U2;
#[cfg(target_pointer_width = "32")]
type UsizeSize = U4;
#[cfg(target_pointer_width = "64")]
type UsizeSize = U8;

What should be the best place to put it In this case?

[tarcieri]

The serialized state needs to be consistent regardless of the target pointer width.

You should pick a sensible on-the-wire representation (u32 or u64) and use a try_from conversion when deserializing it.

[newpavlov]

I think we can simply transform output_size to u8 using TryInto and unwrap. I am not aware about variable hashes which support output size bigger than 255 bytes (XOF does not count). In future versions it could be worth to replace usize with u8.

[rpiasetskyi]

Converted to draft to avoid emails about failed tests.

UPD: Did not help :(

[rpiasetskyi]

Does it make sense to implement SerializableState for some commonly used types?

impl SerializableState for u8 {
    type SerializedStateSize = U1;

    fn serialize(&self) -> SerializedState<Self> {
        [*self].into()
    }

    fn deserialize(
        serialized_state: &SerializedState<Self>,
    ) -> Result<Self, DeserializeStateError> {
        Ok(serialized_state[0])
    }
}

impl SerializableState for u64 {
    type SerializedStateSize = U8;

    fn serialize(&self) -> SerializedState<Self> {
        self.to_le_bytes().into()
    }

    fn deserialize(
        serialized_state: &SerializedState<Self>,
    ) -> Result<Self, DeserializeStateError> {
        Ok(u64::from_le_bytes((*serialized_state).into()))
    }
}

impl SerializableState for [u64; 16] {
    type SerializedStateSize = U128;

    fn serialize(&self) -> SerializedState<Self> {
        let mut serialized_state = SerializedState::<Self>::default();
        for (val, chunk) in self.iter().zip(serialized_state.chunks_exact_mut(8)) {
            chunk.copy_from_slice(&val.to_le_bytes());
        }

        serialized_state
    }

    fn deserialize(
        serialized_state: &SerializedState<Self>,
    ) -> Result<Self, DeserializeStateError> {
        let mut array = Self::default();

        for (val, chunk) in array.iter_mut().zip(serialized_state.chunks_exact(8)) {
            *val = u64::from_le_bytes(chunk.try_into().unwrap());
        }

        Ok(array)
    }
}

It can be implemented in a similar way to this https://github.com/fizyk20/generic-array/blob/master/src/impls.rs#L183.

This can move the for loop (array serialization/deserialization) in one place and make the code a little bit simpler.

type State = [u64; 16];

impl SerializableState for GroestlLongVarCore {
    type SerializedStateSize = Sum<
        <u64 as SerializableState>::SerializedStateSize,
        <State as SerializableState>::SerializedStateSize,
    >;

    fn serialize(&self) -> SerializedState<Self> {
        self.state.serialize().concat(self.blocks_len.serialize())
    }

    fn deserialize(
        serialized_state: &SerializedState<Self>,
    ) -> Result<Self, DeserializeStateError> {
        let (serialized_state, serialized_blocks_len) =
            Split::<_, <State as SerializableState>::SerializedStateSize>::split(serialized_state);

        Ok(Self {
            state: State::deserialize(serialized_state)?,
            blocks_len: u64::deserialize(serialized_blocks_len)?,
        })
    }
}

And it might be possible to implement #[derive(SerializableState)] for types that do not need to check anything.

[rpiasetskyi]

With the introduction of the AssociatedOid trait and new generic parameter for CtVariableCoreWrapper, is it needed to add it to serialized state?

[tarcieri]

No, those are type-level properties which are identical regardless of the state.

[rpiasetskyi]

Does it make sense to implement SerializableState for some commonly used types?

impl SerializableState for u8 {
    type SerializedStateSize = U1;

    fn serialize(&self) -> SerializedState<Self> {
        [*self].into()
    }

    fn deserialize(
        serialized_state: &SerializedState<Self>,
    ) -> Result<Self, DeserializeStateError> {
        Ok(serialized_state[0])
    }
}

impl SerializableState for u64 {
    type SerializedStateSize = U8;

    fn serialize(&self) -> SerializedState<Self> {
        self.to_le_bytes().into()
    }

    fn deserialize(
        serialized_state: &SerializedState<Self>,
    ) -> Result<Self, DeserializeStateError> {
        Ok(u64::from_le_bytes((*serialized_state).into()))
    }
}

impl SerializableState for [u64; 16] {
    type SerializedStateSize = U128;

    fn serialize(&self) -> SerializedState<Self> {
        let mut serialized_state = SerializedState::<Self>::default();
        for (val, chunk) in self.iter().zip(serialized_state.chunks_exact_mut(8)) {
            chunk.copy_from_slice(&val.to_le_bytes());
        }

        serialized_state
    }

    fn deserialize(
        serialized_state: &SerializedState<Self>,
    ) -> Result<Self, DeserializeStateError> {
        let mut array = Self::default();

        for (val, chunk) in array.iter_mut().zip(serialized_state.chunks_exact(8)) {
            *val = u64::from_le_bytes(chunk.try_into().unwrap());
        }

        Ok(array)
    }
}

It can be implemented in a similar way to this https://github.com/fizyk20/generic-array/blob/master/src/impls.rs#L183.

This can move the for loop (array serialization/deserialization) in one place and make the code a little bit simpler.

type State = [u64; 16];

impl SerializableState for GroestlLongVarCore {
    type SerializedStateSize = Sum<
        <u64 as SerializableState>::SerializedStateSize,
        <State as SerializableState>::SerializedStateSize,
    >;

    fn serialize(&self) -> SerializedState<Self> {
        self.state.serialize().concat(self.blocks_len.serialize())
    }

    fn deserialize(
        serialized_state: &SerializedState<Self>,
    ) -> Result<Self, DeserializeStateError> {
        let (serialized_state, serialized_blocks_len) =
            Split::<_, <State as SerializableState>::SerializedStateSize>::split(serialized_state);

        Ok(Self {
            state: State::deserialize(serialized_state)?,
            blocks_len: u64::deserialize(serialized_blocks_len)?,
        })
    }
}

And it might be possible to implement #[derive(SerializableState)] for types that do not need to check anything.

What do you think about https://github.com/RustCrypto/traits/pull/1078/commits/3447449814c3ee2bae77834bd23d1984895603fa? If this is okay, I will also add #[derive(SerializableState)].

[rpiasetskyi]

Could you please have a look at https://github.com/RustCrypto/traits/pull/1078/commits/afc029e5f43167d3787e5b5300e9a3961d77a256 ? This is the implementation of #[derive(SerializableState)] macro. It makes it very easy to implement SerializableState trait for cores (e.g. for fsb https://github.com/RustCrypto/hashes/pull/385/commits/589b43bb6009fadb34f13b867d9d06cdd288a3f5).

The current implementation doesn't support generics for now.

Also, crypto_common should be imported in the file that makes derivation.

[waynr]

:wave: any chance of getting this merged any time in the near future?

[tarcieri]

@waynr it needs a rebase, at the very least

[waynr]

@tarcieri I'm attempting to rebase but running into compile errors that are pretty much totally incomprehensible to me. This repo is like a firehose of generic types and my puny little brain only has capacity for a trickle.

[tarcieri]

I think @rpiasetskyi said he would take a look

[waynr]

I figured out that the problem had to do with updating the crypto-common dependency in the digest crate and opened a PR to show the same problem on master branch: https://github.com/RustCrypto/traits/pull/1359

[waynr]

Just a quick note - it looks like the issue of the local dependency on crypto-common being broken in master branch for the digest crate is fixed in #1358 so this PR might need to wait for that before being rebased.

[rpiasetskyi]

I am doing a rebase (and migration to hybrid_array) using https://github.com/RustCrypto/traits/pull/1358.

[rpiasetskyi]

@tarcieri Sorry, I was expecting it to update the PR but not to merge it :(

[rpiasetskyi]

Reopen.

[tarcieri]

I think you pushed 0 commits which auto-closed the PR? Regardless I can't reopen it... I think you'd need to push some new commits