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

Update README.md to include an example for PBKDF in browser with js-sha256 #115

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mohsenasm
Copy link

Using the pbkdf2 library in the browser is not very straightforward.
Including a simpler yet secure way for the browsers in the readme is good.

…ha256

Using the `pbkdf2` library in the browser is not very straightforward. 
Including a simpler yet secure way for the browsers in the readme is good.
@ricmoo
Copy link
Owner

ricmoo commented Jan 6, 2023

I think this is out of scope for this readme though, no?

There are so many ways, and sha2-256 is not a great way to create a key from a password. It is subject to rainbow table attacks but also, in your example could yield different keys on computers in different countries, since the password isn’t normalized to a specific form.

Algorithms like pbkdf2 explicitly state which normalized form to use and there are a lot of other nuances that may be missed, which when it comes to deterministic key generation is important.

I’m also a firm believer in key stretching, which is why I usually use scrypt. ;)

@mohsenasm
Copy link
Author

There is a pbkdf2 example a couple of lines before, and for that, I thought we can add another example that can be used in the browsers. So, I don't think it's out of scope and I think it can help new users a lot.

I don't think sha256 can be subjected to rainbow table attacks with the presents of salt.
I added the NFKC normalization to address the issue you mentioned.
Thanks :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants