standardnotes / snjs

Core JavaScript logic shared by all Standard Notes clients.
32 stars 15 forks source link

Key derivation returns incorrect value #176

Closed jonhadfield closed 3 years ago

jonhadfield commented 3 years ago

Hi,

I have debugged the client side sign-in flow by setting breakpoints in the JS to see the various function calls and their results. I have recreated the code in go, using argon2 from the standard library, and copying the parameters in your code.

My test is a sign-in using: identifier: sn004@lessknown.co.uk password: debugtest and a nonce returned from your server: 2c409996650e46c748856fbd6aa549f89f35be055a8f9bfacdf0c4b29b2152e9

The sha256 hash source is therefore: sn004@lessknown.co.uk:2c409996650e46c748856fbd6aa549f89f35be055a8f9bfacdf0c4b29b2152e9 That hashes to: 7129955dbbbfb376fdcac49890ef17bc1b4f73dad21c480f0b28a36849a7459e You reduce to 128 bit, 32 bytes being: 7129955dbbbfb376fdcac49890ef17bc

password: debugtest hash: 7129955dbbbfb376fdcac49890ef17bc iterations: 5 memory: 65M parallel: 1 thread keyLength: 64 bytes

I get the resulting argon2id hash: 7cf34ec893c9b36f455e594280039b7557c1b51067eb2254dd6eb2666b7fd48fe7b47840c731cdc941e56dbb2d8f30c66428e472eacc1b2809bf7ab9bb3aeb58

But your code produces: 2396d6ac0bc70fe45db1d2bcf3daa522603e9c6fcc88dc933ce1a3a31bbc08eda5eb9fbc767eafd6e54fd9d3646b19520e038ba2ccc9cceddf2340b37b788b47

I have tried with the reference tool: https://github.com/P-H-C/phc-winner-argon2 and it confirms my result:

echo -n "debugtest" | ./argon2 7129955dbbbfb376fdcac49890ef17bc -t 5 -m 16 -p 1 -l 64 -id
Type:       Argon2id  
Iterations: 5  
Memory:     65536 KiB  
Parallelism:    1  
Hash:       7cf34ec893c9b36f455e594280039b7557c1b51067eb2254dd6eb2666b7fd48fe7b47840c731cdc941e56dbb2d8f30c66428e472eacc1b2809bf7ab9bb3aeb58  
Encoded:    $argon2id$v=19$m=65536,t=5,p=1$NzEyOTk1NWRiYmJmYjM3NmZkY2FjNDk4OTBlZjE3YmM$fPNOyJPJs29FXllCgAObdVfBtRBn6yJU3W6yZmt/1I/ntHhAxzHNyUHlbbstjzDGZCjkcurMGygJv3q5uzrrWA
0.163 seconds
Verification ok

I'm almost certain I am doing something wrong! Please could you check and let me know where I'm going wrong (hopefully) or if your code/dependency is incorrect (hopefully not)?

moughxyz commented 3 years ago

This was our reference: https://github.com/jedisct1/libsodium.js/blob/master/test/sodium_utils.js#L182

Can you check what results you get with those inputs?

moughxyz commented 3 years ago

Also, formats matter. In this case I'm not sure how "7129955dbbbfb376fdcac49890ef17bc" is being interpreted. In our case, we treat this as a hex string that we convert to an array buffer before processing. Your script may be treating this as a UTF8 string for example.

https://github.com/standardnotes/sncrypto/blob/004/packages/web/src/crypto.ts#L258

moughxyz commented 3 years ago

I think that's the issue. From their readme:

For example, to hash "password" using "somesalt" as a salt and doing 2 iterations, consuming 64 MiB, using four parallel threads and an output hash of 24 bytes

$ echo -n "password" | ./argon2 somesalt -t 2 -m 16 -p 4 -l 24

So they are treating salt as a plaintext string here, but in our case we don't.

jonhadfield commented 3 years ago

A quick search suggests an array buffer is the equivalent of a byte slice in go. So the issue may be that I'm passing in a byte array of the string representation of the salt hex, rather than the byte representation of the salt hex?

moughxyz commented 3 years ago

Yep, same concepts, different words. "string representation of the salt hex" == "plaintext/UTF8 string". And correct, ultimately you want the byte representation of the salt hex.

jonhadfield commented 3 years ago

That did it! My results now match yours. Thanks, as always, for your help.

moughxyz commented 3 years ago

Glad to hear!