-
Notifications
You must be signed in to change notification settings - Fork 258
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
Implement SerializableState for hashes #385
Conversation
sha2/src/lib.rs
Outdated
pub type Sha512_224 = CoreWrapper<Sha512_224Core>; | ||
/// SHA-512/224 core serialized state. | ||
pub type Sha512_224CoreSerializedState = | ||
GenericArray<u8, <Sha512_224Core as HashCoreSerializableState>::SerializedStateSize>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to introduce a whole bunch of type aliases for serialized states.
sha2/tests/mod.rs
Outdated
use hex_literal::hex; | ||
use sha2::{Digest, Sha224, Sha256, Sha384, Sha512, Sha512_224, Sha512_256}; | ||
|
||
macro_rules! test_serializable_state { | ||
($name:ident, $hasher:ty) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should add tests which check stability of the serialized state, i.e. that every version of this crate results in the same serialized state for a given hasher state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test macro is moved to traits
, added expected_serialized_state
. RustCrypto/traits@793f9f2.
sha2/tests/mod.rs
Outdated
let mut h = <$hasher>::new(); | ||
|
||
// First part of data. | ||
feed_rand_16mib(&mut h); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
16 MiB is not a great value for testing serialization, we should use an odd number of bytes to check that buffer gets serialized properly. Also it's a bit too big in my opinion, we can simply use small fixed strings with length equal to block size plus several bytes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test macro is moved to traits
, input data is changed to BlockSize + 1
.
RustCrypto/traits@793f9f2.
Could you please suggest the best solution for tests if the expected values are very long?
|
You can split it into several lines: let bytes = hex!(
"0001020304050607"
"08090a0b0c0d0e0f"
);
// equivalent to:
let bytes = hex!("000102030405060708090a0b0c0d0e0f"); It requires |
Is it okay if I bump |
Ah, I forgot about that. In this case simply use new lines: let bytes = hex!("
0001020304050607
08090a0b0c0d0e0f
"); Unfortunately, rustfmt does not like it too much, so you may need to add |
0a9053d
to
a96197c
Compare
13131313131313131313131313131313 | ||
01130000000000000000000000000000 | ||
00000000000000000000000000000000 | ||
00 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm... This redundant byte for eager hashes is certainly an eyesore. It could be worth to add serialize
/deserialize
methods to the block-buffer
crate.
4b162c7
to
fabd1e7
Compare
This is a rebase of #385 on master. RustCrypto/traits#1369 introduced a `SerializableState` trait to serialize the hasher. This implements it for each of the hashers we have. Co-authored-by: Ruslan Piasetskyi <[email protected]>
Merged as #574 |
The internal state of different hashes can be used for serialization.