interledger-deprecated / five-bells-shared

Common elements that are shared between Five Bells components
Other
4 stars 5 forks source link

fix: reduce absurdly large password hash size #166

Closed justmoon closed 7 years ago

justmoon commented 7 years ago

Previously, the password hashes generated would be 584 bytes (512 + 64 + 8) which is far larger than necessary for security. As a result, they would unnecessarily bloat the ledger database.

Cause of the bug was a misinterpretation on my part of the crypto.pbkdf2 function. I thought it expected the key length in bits, but it expects it in bytes. (In the example a keylen of 512 is used with SHA-512, so I think the documentation writer made the same error.)

Since a smaller output size also causes a speed-up, I'm increasing the default iteration count to compensate.

In this commit, I'm choosing sane values for salt and hash size that lead to an overall password hash size of 40 bytes (instead of 548).

Also note that since the salt size, hash size and iteration count are parsed from existing password hash database entries, this change is fully backward-compatible.

codecov-io commented 7 years ago

Current coverage is 53.85% (diff: 100%)

Merging #166 into master will decrease coverage by 0.06%

@@             master       #166   diff @@
==========================================
  Files            23         23          
  Lines           753        752     -1   
  Methods         133        133          
  Messages          0          0          
  Branches        124        124          
==========================================
- Hits            406        405     -1   
  Misses          347        347          
  Partials          0          0          

Powered by Codecov. Last update 1f5f46a...9656261