-
Notifications
You must be signed in to change notification settings - Fork 103
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
Enhanced Entropy: sha384 and NUL bytes #15
Comments
I've added further tests to the null check (it runs the test cases against enhanced and normal) The spec says we should treat null like any other character so I don't believe we've any issues with null bytes surfaced through direct cryptraw access (which is a private method) or direct-via-hmac. I'm not discounting adding in b64 wrapping the hmac as this seems to be the default in multiple nix platforms and I love compatibility. I need to get some test vectors so I can validate mine vs say php/ruby etal. Didn't realise @paragonie-scott had linked it. |
Well, the NULL char is actually not related to the fact that the password can contain a NULL char, but instead that the sha384 function can return See this Paragonie article (once again!) but it's about sha256:
|
Run a quick test with SHA384: The sha384 hash value of The sha384 hash value of And the sha384 hash value of These values could be used as test passwords when enhancedEntropy is used and check if it gets truncated... |
I'm aware of the why, I was trying to avoid a rambling answer.
|
Just to show it at a lower level; I don't push the crypt-raw test as the method is private; as is decode. I may add them and change private to internal so the tests can access those areas of the method. Similar test using the value you provided You'll probably find this interesting also https://blog.ircmaxell.com/2015/03/security-issue-combining-bcrypt-with.html < most annoying part about this implementation but of course being that it was at the module level so pretty ubiquitous. |
So this means that the C# implementation of bcrypt isn't affected by the NULL byte issue? However, as you noted, there is still a compatibility problem, as other libs uses base64 and not this one. |
@Indigo744 tbh I'll just bump the major version and add a legacy option. I want to make the HMAC an option between current and 512... it adds bugger all real gain but some folk seem to want it 🙄 Ta for the nudge I'd been meaning to finish off these changes and add in better test coverage for a while |
Since you are adding sha512, maybe also add an option to add sha256 for interoperability with other lib (such as Passlib)? I don't know if it's worth it actually... Anyway, I'm glad we were able to work things out and that everything is OK and secure 😄 |
Oh, I just noticed that Passlib uses a specific algorithm name in the crypt format: A quick search tells me they are the only one doing it (I may be wrong tho). However, it looks interesting, since it clearly explains the underlying algorithm. |
@Indigo744 suprised the didnt go with It would be handy from a standpoint of auto-picking which route needs to be taken (similar to extracting salt/workload). Hmmmmm 🤔 |
You're right! I didn't know this format... which seems to be the the format for Argon2. Well, since this is a bcrypt lib, I don't think there is a point of using PHF... It should remain compatible with crypt. |
Yeah, I won't be breaking the version standard for normal bcrypt. I'd considered upping the workload but with the crappy processor bug patches I'd risk shooting people in the face by accident. |
Alright. Looking forward the new version. Thanks! |
Hello,
I was reading this article (which actually recommend this library) about using bcrypt for password hashing.
It suggests to pass the password through SHA384 before using bcrypt in order to circumvent the bcrypt limitation.
I was glad to find that your library implemented it through the
enhancedEntropy
parameter.However, looking at the code, I couldn't find any mention of base64 encoding after the SHA384 and prior bcrypt.
base64 is suggested in the article because:
And
Thanks for your insight.
EDIT Adding some reference implementation seen elsewhere:
In PasswordLock PHP library, they effectively perform base64 after sha384.
In passlib Python library, they also perform base64 after sha256.
The text was updated successfully, but these errors were encountered: