[Feature] Use ring crypto crate, add generic parsing of PHC format hash fingerprints

[YetAnotherMinion]

• The rust-crypto crate has been replaced with ring
• Dropped support for using SHA1 as a PRF
• Added generic parsing of PHC format (the successor to modular crypt format)
• rustfmt the entire codebase

Why Use Ring

The ring crate is under active development while development of rust-crypto has stalled. Using a crate that is being actively improved is important especially important for crypto because the validity of any crypto algorithm is eliminated over time by adversaries. In the short term ring provides a constant time pbkdf2 verify function. Additionally ring can be statically linked, which is important for my application.

Why move to PHC format

The library is already using PHC format instead of modular crypt format for producing and deriving portable fingerprints. The wikipedia page referenced for modular crypt format is actually about the updated PHC format, and that format is the one that the code on master is parsing and generating. I factored out the code that parses the PHC format fingerprints from the derivation and validity logic. I could not find any crates on crates.io that currently handle this format generically. rust-argon2 uses this format to create and read fingerprints internally, but it only exposes the completed String from the public API. I think parsing the format is a valuable distinct feature because it allows writing programs that work with hashed fingerprints outside of the realm of just accepting a password string and hashing it. For example, you could write short script to migrate a legacy database that did not use PHC format fingerprints to using PHC formated fingerprints without having to worry about serialization.

I have replaced the PasswordDerivationFunctionBuilder workflow with a single type that represents the PHC formatted string as a Rust struct and a higher order function.

• string can be converted into a PHCEncoded struct
• PHCEncoded struct can be converted into a string
• PHCEncoded can be converted into a Boxed pointer to a function that will hash a password using that same algorithm, salt, and settings as the reference hash fingerprint. This performs the same way as PasswordDerivationFunctionBuilder, except now the state is explicitly passed in.

Further Work

This pull request is suggestion, I do not expect to merge this directly in without edits. Right now these changes are internal only, with the exception of dropping support for SHA1 based pbkdf2, the public API has neither been changed or increased. I am not sure why ring does not support SHA1 for pbkdf2. My understanding is that collisions in SHA1 do not affect its security as a PRF, however Brian Smith who is a much better cryptographer than I am has unit tests that explicitly reject using SHA1 for pbkdf2.

Hopefully you can try out the new version in your internal application and verify that its behavior is identical. I only modified the three tests for the pass module, and that was to remove SHA1.

[YetAnotherMinion]

@breard-r Let me know when you get a chance to look at this PR to use ring for crypto.

[breard-r]

Well, changing the crypto library isn't something that should be taken lightly. I am currently considering this choice, however it takes time.

[YetAnotherMinion]

changing the crypto library isn't something that should be taken lightly.

Heartily agree. On the other hand, rust-crypto looks abandoned by the maintainer. It has been over 6 months since the owner DaGenix has commented or merged a PR. At this point I feel pretty confident that support for rust-crypto will not be coming. On the other hand the author of ring has been shown to personally support random projects that use it (see Keats/jsonwebtoken#23 for an example from last week).

[YetAnotherMinion]

@breard-r I merged in your parser changes to this PR. If you want to discuss anything you can also email me at

yam at thinkalexandria.com

[YetAnotherMinion]

@breard-r I don't want to impose, but I would appreciate a decision on this Pull Request.

Please either:

• accept the concept of moving to ring from rust-crypto (which now has been dead for 9 months). This does not mean merging the PR as I am sure there are plenty of things in the PR that could benefit from your review and vision.
• or reject the idea, please close this PR, and stay on rust-crypto (so my employer can hard fork)

[breard-r]

Well, I guess I have to close this PR then. Let me explain you my choice.

Ring does not support MD5, causing regressions in the code. Ring also deprecates SHA1, which may lead in the future to unacceptable regressions. Yes, rust-crypto is close to dead, however I think it is still reasonable to use it for MD5, SHA1 and HMAC calculations. No, I do not encourage to use MD5 or SHA1, but the fact is they are still needed. SHA1 being the default hash function for HOTP and TOTP, it is not a good idea to get rid of it.

Concerning the password authentication module, as I said before it is experimental. I am currently rewriting it. I used PBKDF2 only as a convenience for testing, the default password hashing function will be Argon2. Ring does not support Argon2, therefore it is completely useless there. Maybe I'll add support for other algorithms only so it is possible to migrate from an older system to this one. This is why removing SHA1 based PBKDF2 is not a good idea.

About the other things in this PR, I don't think I will keep them. I'm moving towards the PHC format, however I am now using nom for all my parsers, including this one. The new PHC parser is already written and I am currently integrating it into the code.

[YetAnotherMinion]

Thanks for the reply. Good luck.