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

Provide a constructor for Hash #25

Closed
tailhook opened this issue Nov 3, 2021 · 8 comments
Closed

Provide a constructor for Hash #25

tailhook opened this issue Nov 3, 2021 · 8 comments

Comments

@tailhook
Copy link

tailhook commented Nov 3, 2021

It would be nice to be able to construct Hash from string or bytes to compare it to the computed hash, instead of comparing the string/bytes representation.

Optional serde support would be even better.

@oconnor663
Copy link
Owner

oconnor663 commented Nov 3, 2021

I forgot that the Hash type in these crates were missing the From<[u8; N]> impl. I should probably just add it.

The version of this type in the blake3 crate has more methods, and probably all of them would make sense here too. I wonder if it's time to consider splitting it out into a standalone crate, which could be shared by many different crates? On the other hand, would we rather keep the property that comparing a BLAKE2 hash to a BLAKE3 hash with == is a type error? Not sure.

Also I'm sure the RustCrypto folks (who maintain the digest crate) have some thoughts about this. @tarcieri is working on merging implementations from this repo into their blake2 crate also. Tony, have you guys ever considered defining a shared Hash type or something like that? I know the Digest trait works with GenericArray, but now that const generics don't require any sort of hacks, it might make sense to define a new type (with constant-time equality, hexification, and so on).

@oconnor663
Copy link
Owner

@tailhook what do you think of #26 as a first step?

@tailhook
Copy link
Author

tailhook commented Nov 3, 2021

Yes. This is a good start! At least I can do all other stuff (parse from string) myself now.

On the other hand, would we rather keep the property that comparing a BLAKE2 hash to a BLAKE3 hash with == is a type error?

I like this idea. It's better than having hash type by length (i.e. Hash32/Hash64), and clearer than using generic ConstTimeCmpArray<u8, 32>.

@tarcieri
Copy link

tarcieri commented Nov 4, 2021

Tony, have you guys ever considered defining a shared Hash type or something like that?

This is what we presently have in the digest crate:

https://docs.rs/digest/0.9.0/digest/type.Output.html

I know the Digest trait works with GenericArray, but now that const generics don't require any sort of hacks...

Unfortunately min_const_generics are not sufficient for our use cases. See ongoing discussion here:

RustCrypto/hashes#303

@tarcieri
Copy link

tarcieri commented Nov 4, 2021

define a new type (with constant-time equality, hexification, and so on).

This sounds a bit more like the current crypto_mac::Output newtype:

https://docs.rs/crypto-mac/0.11.1/crypto_mac/struct.Output.html

I believe @newpavlov has plans to unify digest and crypto_mac, although I don't know the plans regarding the Output types specifically.

@oconnor663
Copy link
Owner

I just landed the PR above. As part of publishing these changes, I'm leaning towards just tagging these crates v1.0.0 and calling everything stable. (Which wouldn't rule out any future enhancements to the Hash type, of course.) @tarcieri would that be alright with you? Any impact to blake2 from doing that?

@tarcieri
Copy link

That sounds fine to me. I guess the only thing to worry about is porting over the changes to blake2.

@oconnor663
Copy link
Owner

oconnor663 commented Nov 19, 2021

Published blake2b_simd, blake2s_simd, and blake2_bin versions 1.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants