-
Notifications
You must be signed in to change notification settings - Fork 85
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
yescrypt: initial translation #509
Conversation
Lots of this is ugly verbatim |
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.
This certainly needs a lot of cleanup, so I haven't checked the implementation itself.
Is there a proper specification of the algorithm? I couldn't find it after a cursory search. I am not sure it's worth to start with machine translation of the reference implementation.
Pure Rust implementation of the yescrypt password hashing function | ||
""" | ||
authors = ["RustCrypto Developers"] | ||
license = "BSD-2-Clause" |
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 think BSD-2-Clause is compatible with MIT/Apache-2.0, so we can change the license for our modification.
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.
While they're "compatible" in that they're allowed to be used in the same program, that doesn't mean we can arbitrarily relicense code under one license under another.
We could potentially consider changing to MIT OR Apache-2.0
when the code has been extensively rewritten and is no longer an obvious derived work of the original.
Or we could attempt to get permission from the original authors to relicense it, which is what I've done (with a surprising amount of success) in the past.
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.
Yeah... The copyright notices effectively state the same thing, but with slightly different wording. It would be great to have a "greenfield" implementation based on algorithm specification, but I don't know how hard would it be to do compared to the automatic translation (+ cleanup).
Note that right now you state MIT/Apache-2.0 licensing terms in the crate README.
Probably should've noted this isn't ready for extensive comments / followup PRs yet. Just got it working. There's obviously a lot to be done. I'd like to get 32-bit support working first, then get it merged, then do a cleanup pass PR which makes it closer to the original C source code. |
I'm not sure what else to use but the reference implementation. Even as a PHC judge, I am not sure where there is a detailed description of the algorithm beyond the reference implementation, besides possibly floating around in my email history as the PHC judges list lacks a public archive. This paper discusses some of the security properties: https://eprint.iacr.org/2015/227.pdf This site provides a general overview, including links to slide decks which have some of the best descriptions of the algorithm I've seen: https://www.openwall.com/yescrypt/ Regardless, this is the same way the |
@newpavlov also, can you add |
Does the reference implementation confirm to this PHC submission?
Done. |
@newpavlov aha, there it is! The changelog (which is buried in a tarball, not sure if it's on GitHub somewhere) lists several changes since the PHC submission, not sure if any cause the implementation to deviate from the specification |
33c3e5f
to
2784bba
Compare
Translation of the reference implementation as implemented in C, as of this commit: commit e5873f89015c9c7c4ae56e3a6d17cefd47fef239 Author: Solar Designer <[email protected]> Date: Sun Sep 24 16:00:15 2023 +0200
Sourced from `TESTS-OK`: https://github.com/openwall/yescrypt/blob/e5873f8/TESTS-OK
no point rewriting pbkdf2, hmac, sha2, salsa20 etc...
Translation of the reference implementation as implemented in C, as of this commit:
openwall/yescrypt@e5873f8