-
Notifications
You must be signed in to change notification settings - Fork 93
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 hashing algorithms + HMac support #193
Conversation
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.
Took a quick look and everything seems pretty straightforward. Not actually reviewing yet since this is a draft PR, but I thought I'd mention that this PR will need a bit of file refactoring since it is from before the split-into-crates merger / lune 0.8.4.
Would you like to make those changes in this PR or open a new one? Might be easier to open a new one and copy & paste the files since 0.8.4 was a pretty big change 😬
Yeah this'll need a bit of a refactor. I've been putting it off since I got a new laptop and haven't set up a Rust environment on it yet. I think using this PR is more than feasible since I can just force push this branch, but it might look weird. If you have a preference I'll respect it but otherwise that's what I'll do. |
Force push is no problem! The squash and final changeset looking good is all that matters to me 👍 |
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.
Everything looks great! Thank you for including all the different test cases, too. Maybe we can consider including some conversion utilities for hex, base64 and similar things in serde
in the future but for now returning a hex string works well.
For some reason, the changelogs and release notes don't include anything about this being merged? I only found out that this exists today while going through the serde builtin source. Can this be fixed (I doubt now - it might be too late)? |
Both this PR and the main branch have up to date changelogs with the new functions included and example usage for them. They haven't been released yet. |
Whoops - thought they were released in 0.8.4! |
Implements the basics of hashing algorithm support, along with hmac.
Does so via 2 new functions:
Algorithm Choice
After the discussion in #105, I elected to include sha1, sha2, sha3, blake3, and md5 for this implementation. This is mostly for practical reasons: algorithms for passwords are more complicated, and other algorithms (like xxhash) don't HMAC well.
I do want to call special attention to GxHash, as it uses
aes
and thus requires us to mess with our target features if we want to include it. I'm not going to make the call for that, since I'm not Lune's maintainer.If merged would close #105.