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

[Question] Design goals #2

Closed
YetAnotherMinion opened this issue Apr 13, 2017 · 2 comments
Closed

[Question] Design goals #2

YetAnotherMinion opened this issue Apr 13, 2017 · 2 comments

Comments

@YetAnotherMinion
Copy link

I been reading the source code and I am hoping you will correct my current understanding of what this crate does.

My understanding is that this crate only has 3 areas of functionality

  • Generate and verify TOTP codes from a key passed in
  • Generate and verify HOTP codes from a a key passed in
  • Generate PHC format from password and salt and verify password against PHC string?

Additionally each of these areas have excellent user facing documentation, C library bindings, and thorough tests which are very nice. The API is simple and easy to use.

The password verification functionality is what I am most interested in because I do not understand what is going on inside pass::is_valid(password: &str, reference: &str) -> bool. I think reference is the PHC format password hash information string. First the PHC string is inspected to choose the right hash function (with same salt and workfactor). Once the hash function "closure" is set up, hash the user provided password. Next is where I get lost. A random 32 byte key is generated and used to sign and HMAC value for both the reference PHC string and the PHC string generated from the user password. The password is verified only if the HMAC values match.

To me there is real value with parsing and serializing the PHC format. I like that API boundary decision because it lets me use a single DB column value. What I do not understand is why not use the crypto library's pbkdf2::verify implementation instead of this double HMAC result checking scheme?

@breard-r
Copy link
Owner

Your analysis is correct. Before going any further, I must warn you against the password authentication: this part is still highly experimental, not considered secured and the interface will change in next versions. You should not use it in production yet. Using the HOTP/TOTP part is OK though.

Just a quick focus on the password validation process, mostly in case of someone else read this post because you already got it right. After hashing the user supplied password, we need to compare it against the reference hash that was stored on the "server" side. In some cases, mostly if you the authentification is done on a local machine, there may be a side channel attack by closely watching the time the comparison function uses to execute. One solution is to have a time-insensitive function, but this is very difficult if not impossible to securely achieve (this is a fight against compiler optimisation). Unfortunately, many people try to do it this way. The second option, which I chose and recommend, is not to compare the two hashes directly. You first hash them with a secret salt (a random single usage one in this case) so the attacker cannot guess what will be compared and therefore cannot extract any information by looking at the comparison. As you mentioned, this is a derivation from the double HMAC verification. It is also used for the HOTP/TOTP verification.

About not using pbkdf2::verify, I don't remember seeing this in rust-crypto. Furthermore, the goal of this password authentication mechanism isn't to use pbkdf2 but different algorithms so it is possible to "upgrade" to a safer one upon user login. This is inspired from the password management in Django. Doing so requires the comparison function to be independent from the algorithm. I chose to start with PBKDF2, however that is not what I would recommend for password storage right now. This choice was guided by simplicity in order to get a functional prototype to work on before adding the serious stuff.

@YetAnotherMinion
Copy link
Author

I have a fork where I swapped out rust-crypto (which has not been updated in 7 months) for ring (which is updated today). ring does have a near constant time verify function for pbkdf2.

I actually only need the HOTP/TOTP, but since the password stuff was in there I had to modify it as well to use ring. I do quite like the interface of the password derivation routines, so I rewrote the internals completely to abstract the parsing of PHC formated hash information strings (which is recommended to use over Modular Crypt Format by the authors of MCF for new implementations), and then use that information to dispatch the appropriate hash function. The external C interface remains the same (except for dropping SHA1 support). The tests are not currently passing because PHC format requires B64 encoding (which is a customized version of base64 where they drop the padding characters), while the existing MCF implementation uses base16 encoding for hash. I am going to get all the tests passing using base16 encoding today, and then see about changing the encoding to comply with PHC spec.

breard-r added a commit that referenced this issue Jun 10, 2017
breard-r added a commit that referenced this issue Nov 26, 2017
The rust-crypto crate does not seems to be maintained anymore.
Because of its monolithic architecture, it is not possible to
expand it any further without forking it. Furthermore, many
important issues are still pending. Because of all of this,
rust-crypto is not a good choice anymore.
In issue #2 ring was suggested as a replacement. A fork using
it already exists. However, ring has two main downsides. First of
all, it deprecates SHA1 which is the default hashing function for
HOTP and TOTP. If SHA1 is to be removed, it would break
compatibility with existing systems. The second downside is, like
rust-crypto, its monolithic architecture which does not enable
third-party addons. Such addons would have enabled the possibility
to add back SHA1 support.
On the opposite, the @RustCrypto project seems to be a good
replacement. Development seems to be active and is has a modular
architecture.
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

2 participants