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

Added PBKDF2 (300000 iterations) hashing support in addition to SHA256 #27

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@ychin

ychin commented Oct 29, 2013

Most passphrases don't tend to be very secure or high entropy so I added PBKDF2 support to make the hashes less prone to attacks since they are more expensive to guess. By default it's still using SHA256 but you can now select "Use Secure" to toggle the secure hash mode which uses PBKDF2 with 300000 iterations. Right now using Stanford's sjcl library since that's faster than CryptoJS. Still need to download the JS file and cache it in the repository like the other JS files instead of direct reference to external server.

Also added a button to hide the passphrase although the private keys are still visible. Maybe when we're in hiding mode should hide all private keys?

ychin added some commits Oct 29, 2013

Added button to hide passphrase so they are not displayed as plaintex…
…t. The private key will still be displayed though.
Added PBKDF2 support (right now using 100000 iterations) for more sec…
…ure hashing. This gives some extra safety for lower entropy passwords, and since it's unlikely for this page to be used too often we want to use as high number of iterations as possible (maybe higher?)

Originally used CryptoJS but their PBKDF2 implementation was too slow so switched to using Stanford's more efficient sjcl library. Still need to download and cache the code.
@ychin

This comment has been minimized.

Show comment
Hide comment
@ychin

ychin Oct 29, 2013

Sorry the code is actually using 100000 iterations now rather than 300000.

ychin commented Oct 29, 2013

Sorry the code is actually using 100000 iterations now rather than 300000.

@ychin

This comment has been minimized.

Show comment
Hide comment
@ychin

ychin Oct 29, 2013

Another thing is my two buttons kind of take a lot of space on a smaller mobile screen. Should probably make them smaller or something.

ychin commented Oct 29, 2013

Another thing is my two buttons kind of take a lot of space on a smaller mobile screen. Should probably make them smaller or something.

@brainwallet

This comment has been minimized.

Show comment
Hide comment
@brainwallet

brainwallet Oct 29, 2013

Owner

You don't really need that. It's actually not possible to calculate MILLIONS of passphrase-based addresses per second in order to check their existence in the network (you can, however, relatively fast calculate random addresses using affine addition, as in vanitygen). People always forget that the entire process is much more complex than SHA256, some even mention rainbow tables, which are totally useless in this case. The matter is, if you're calculating an address from the passphrase, you have to perform ECDSA multiplication which is a few orders of magnitude slower than SHA256, not to mention address hashing.

Regarding private key hiding, it could be done with an "input-append" button (like a "Random" button for the secret exponent). It might be actually useful and probably would be implemented in the near future.

Owner

brainwallet commented Oct 29, 2013

You don't really need that. It's actually not possible to calculate MILLIONS of passphrase-based addresses per second in order to check their existence in the network (you can, however, relatively fast calculate random addresses using affine addition, as in vanitygen). People always forget that the entire process is much more complex than SHA256, some even mention rainbow tables, which are totally useless in this case. The matter is, if you're calculating an address from the passphrase, you have to perform ECDSA multiplication which is a few orders of magnitude slower than SHA256, not to mention address hashing.

Regarding private key hiding, it could be done with an "input-append" button (like a "Random" button for the secret exponent). It might be actually useful and probably would be implemented in the near future.

@ychin

This comment has been minimized.

Show comment
Hide comment
@ychin

ychin Oct 30, 2013

That's fair enough. It's true that ECDSA is a lot harder to calculate and not easily done on GPU. I guessed my rationale was just that if you have the CPU power to burn (since we don't need to look up our brain wallet that often) there's no harm in trying to be as secure as possible since with PBKDF2 we can set the work factor to anything we want which seems more flexible.

ychin commented Oct 30, 2013

That's fair enough. It's true that ECDSA is a lot harder to calculate and not easily done on GPU. I guessed my rationale was just that if you have the CPU power to burn (since we don't need to look up our brain wallet that often) there's no harm in trying to be as secure as possible since with PBKDF2 we can set the work factor to anything we want which seems more flexible.

@lyomi

This comment has been minimized.

Show comment
Hide comment
@lyomi

lyomi Aug 9, 2015

Another example of a "holier-than-thou" developer scoffing off honest help, thinking they are the only competent being in this world and everyone else isn't as good at them.

Except that denying this merge request was what effectively killed brainwallet completely; its site as of 9/8/2015 reads "Closed Permanently", and millions of dollars have been stolen. A fatal misjudgment by the single person who wouldn't accept any help, because after all, they're a developer, and they got nothing to do with common rabble.

Lose the God-complex, developers!

lyomi commented Aug 9, 2015

Another example of a "holier-than-thou" developer scoffing off honest help, thinking they are the only competent being in this world and everyone else isn't as good at them.

Except that denying this merge request was what effectively killed brainwallet completely; its site as of 9/8/2015 reads "Closed Permanently", and millions of dollars have been stolen. A fatal misjudgment by the single person who wouldn't accept any help, because after all, they're a developer, and they got nothing to do with common rabble.

Lose the God-complex, developers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment