libauth
libauth copied to clipboard
PBKDF2 and BIP39 Support
This is a PBKDF2 pure JS implementation to progress towards supporting BIP39 mnemonics (I'll branch mnemonics off of this - want to try to keep reviews small and make sure I'm on the right track code-style-wise).
It's based on this implementation https://github.com/browserify/pbkdf2/blob/master/lib/sync-browser.js with a change to support derivedKeyLength that is less than the hmacLength (both NodeJS's PBKDF2 implementation and Python's support this - the test vectors I've found also have this).
The interface is a it clunky - I've tried to keep to the "under three params" lint rule and, unfortunately, we need to know the HMAC length in advance (we could determine this at run-time by "trying it", but I'm not sure that feels proper or efficient?j). Currently, the Pbkdf2PHmac
interface breaks the functional/no-mixed-type
linter rule too - but I'm not too sure what's a good approach for this (maybe I remove this interface and just pass those params direct into function? Let me know if you have any suggestions).
The implementation itself breaks many linter rules. At the moment, I've set a lot of these to be ignored as there's some precedent for this in the CashAddress implementation and rewriting around them may be less efficient considering we're dealing with an algo that iterates into the thousands in practice. Let me know what's preferred here and I can try to rewrite.
Overall, if you could take a look and just let me know which parts you'd like refactored and whether the general direction (e.g. the tests) look appropriate, that'd be great. I'm still digesting and learning the styles from elsewhere in Libauth.
Hey Jim, really fantastic work, thank you! 🚀
On first review the code and interface look great, and code/comment style is excellent!
Did you consider turning pbkdf2HmacSha256
and pbkdf2HmacSha512
into exported functions themselves? So the pbkdf2
function might become something more like instantiateHmacFunction
, maybe instantiatePbkdf2Function
?
Since your ultimate goal with this is to add BIP39 support, if you're willing, I'd love to see that in this PR too! That would let us confirm that the interface is exactly what we need + improve test coverage.
@bitjson probably ready for another look now. Note the long list of ignored linter rules and which ones we should try to fix up.
Original algo is taken from https://github.com/browserify/pbkdf2/blob/master/lib/sync-browser.js
Looking great so far! For the BIP39 implementation, maybe a reasonable strategy is including a bip39.ts
under src/lib/key
, and importing the const string bip39WordListEnglish
from bip39.english.ts
? (Similar to what we do with ripemd160.base64.ts
currently.)
Just pushed my initial recommendation, but feel free to experiment with other options 👍
I think this is finally ready for review (sorry about how long this took!)
A few notes:
- It was hard to find suitable vectors - those referenced in BIP39 all used the passphrase "TREZOR" and did not provide the seed value without the passphrase. In the end, I decided to just generate using the following script so that I could have both a) without passphrase and b) with passphrase. If preferred, I can do a conversion of the Trezor vectors to derive seed value without passphrase, but I thought we might want to tinker with the number of vectors to reduce overall test time (I've done four per wordlist in the below).
import bip39 from 'bip39';
const wordlists = ['english', 'chinese_simplified', 'japanese', 'spanish', 'italian', 'french', 'korean', 'czech', 'portuguese', 'chinese_traditional'];
let validData = [];
// Create tests for each wordlist.
for(let wordList of wordlists) {
// Set the wordlist to use.
bip39.setDefaultWordlist(wordList)
// Generate four mnemonics using the given wordlist.
for (let i = 0; i < 4; i++) {
const passphrase = 'libauth';
const mnemonic = bip39.generateMnemonic();
const seed = bip39.mnemonicToSeedSync(mnemonic).toString('hex');
const seedUsingPassphrase = bip39.mnemonicToSeedSync(mnemonic, passphrase).toString('hex');
const entropy = bip39.mnemonicToEntropy(mnemonic);
const test = {
entropy,
mnemonic,
seed,
wordList,
passphrase,
seedUsingPassphrase,
}
validData.push(test)
}
}
console.log(JSON.stringify(validData, null, 2));
-
I've cheated in some places with eslint ignore where I couldn't find an otherwise elegant solution (or name).
-
Japanese is exceptional - it uses a
\u3000
separator as opposed to a space character. Detection of Japanese wordlist is dirty - but it seems most other implementations use a similar approach. -
There is almost 100% test coverage. The one exception is with an entropy check function which I think is redundant - but is in the bip39 npm package I based this on. I've left it there just in case there's some undocumented edge-case that I'm unaware of but please let me know if you think I can safely remove it.
Wow, fantastic work @jimtendo! Thanks for the attention to detail, this looks great. I'm going to dedicate some more time to reviewing it, and I'll pull it in with the next release. 🚀
Codecov Report
Attention: 3 lines
in your changes are missing coverage. Please review.
Comparison is base (
7c7e74d
) 0.00% compared to head (4aefb8a
) 84.26%. Report is 2 commits behind head on master.
Files | Patch % | Lines |
---|---|---|
src/lib/key/bip39.ts | 98.86% | 3 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #109 +/- ##
===========================================
+ Coverage 0 84.26% +84.26%
===========================================
Files 0 144 +144
Lines 0 51448 +51448
Branches 0 2136 +2136
===========================================
+ Hits 0 43355 +43355
- Misses 0 8076 +8076
- Partials 0 17 +17
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Thanks again for all your work here @jimtendo!
Merged manually in https://github.com/bitauth/libauth/pull/116/commits/8e032c2f9878d6f20cf805be6b21274534622d56. I added a few more functions, expanded the docs, and added a few more tests (including fuzzing/property-based testing) in https://github.com/bitauth/libauth/pull/116/commits/4709c17c2e2876c6f7a1910c84c292991bc84fba.
Now released in @bitauth/[email protected]
🚀