-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
crypto/hash/digest reorganization #7503
Comments
Ping @DaGenix - since you moved the stuff into crypto/, I assume you would have an opinion on this. |
Just a thought, maybe the digest types should implement io::Writer, when that trait arrives in its new form. Or there could be a trivial wrapper for it. |
I exported Sha1/Sha2 as extra::{Sha1,Sha2} instead of extra::crypto::{Sha1,Sha2} only because as best as I could tell, that was what was being done in other places in libextra / libstd. Longer term it does seem like as there are more functions implemented, exporting them all together under extra::crypto (or something similar) would be easier to maintain than exporting them all in the main namespace. When I created the Digest trait, I left the md4 code alone simply because its current implementation can't support the Digest trait I created - the md4 code wants to process an entire message at a time, while the Digest trait specifies an interface to process chunks of a message. I didn't want to bundle the md4 code alongside the Sha1/Sha2 code until they supported the same traits. Updating the md4 code to support processing a message incrementally and implement the same traits as Sha1/Sha2 sounds like a great idea to me. There are a wide variety of different cryptographic functions that I would imagine would be nice to have in libextra some day: symmetric ciphers, asymmetric ciphers, message authentication codes, key derivation functions, key agreement protocols, etc. I think the question is how to organize it all. A few options I can think of (and there are probably more):
Personally, I think I prefer #3, though, there are pros and cons for every approach. Another question that doesn't seem to have a widely agreed upon answer is what to call a function like Sha2 - a Hash or a Digest. Personally, I prefer Digest since I think the term hash function is a bit ambiguous: If someone says "hash function," does that mean a hash function suitable for use with a hash table or a hash function suitable for securing communication? Well, those are my initial thoughts. I don't really feel too strongly about anything above, though. I'm fairly new to Rust, so, I'm not sure if any particular options are more appropriate for Rust style than others. |
md4: OK I did not realize the Digest interface meant it had to be able to process messages in chunks. Makes sense. Regarding naming, Digest IMO is ok, but as said above though crypto can be misleading if we shove all sorts of hashing algorithms in there. Older/simpler algorithms with known vulnerabilities should not be treated as cryptographically secure, and labeling crypto sort of infers that. As for the grouping, I think I would favor 1 because having to remember what group the algorithm you want to use belongs to can be a pain and isn't very developer-friendly. I am also very new to rust, but I would be happy to help with this if the core folks set a direction. |
I agree that crypto isn't a perfect name. What other options are there? If the goal is the name the module structure to differentiate broken algorithms from non-broken ones, how do we handle something like Sha1? Sha1 is considered broken for use in digital signatures, but, I think, is still considered secure for its use in HMAC functions: http://www.schneier.com/blog/archives/2005/02/sha1_broken.html. That article is old, but, I couldn't find anything newer. Regardless, even if that no longer applies to Sha1, how can the module structure handle an algorithm that is insecure for one purpose but secure for another? Sha2 is generally considered to still be secure. So, it would got in a module indicating that its secure. But, what happens if Sha2 is broken next week? Does it get moved to a new module to indicate that its no longer secure? That would brake any applications that are making use of the algorithm. |
Well I would use extra::hash (or extra::digest) because that's more generic and does not convey a notion of security. I was not suggesting that we split the algos by level of security, I think it's better people educate themselves a little bit about that, because indeed we can't just move stuff around like that after 1.0. |
Ah, I guess I misunderstood you earlier. That sounds reasonable to me. |
Nominating for backwards compatible. |
libextra is out of scope for milestones. De-nominating |
Closing since we've decided to remove these from libextra. |
Rustup r? `@ghost` changelog: none
Right now extra has md4/sha1/sha2 implemented, yet many more hashing algos exist so I'm thinking consolidating it now might make sense. sha1/sha2 are in extra/crypto/ but exported as
extra::{sha1,sha2}
, they both implement the Digest trait. md4 is simply in extra/ and has its own API.Therefore I would like to suggest the following changes (and volunteer to realize them should they be accepted):
extra/md4.rs
toextra/crypto/md4.rs
and make it implement Digestextra::crypto::{md4,sha1,sha2}
module, although I'd rather call itextra::hash::
because not all hash algos are to be considered cryptographically secure. Perhaps that is the reason md4 is not in crypto right now, but I think it'd be best to clean this up now before more hashes get added and the BC requirements becomes stronger.The text was updated successfully, but these errors were encountered: