decred / tinydecred

Python tools for Decred
ISC License
27 stars 14 forks source link

crypto: fix encryption key authentication #128

Closed buck54321 closed 4 years ago

buck54321 commented 4 years ago

The checkphrase check really should never have been there. The key params already include a hash that is used to verify the key in SecretKey.rekey. Also reworked the key authentication based on a previous conversation in dcrdex.

Will need to nuke your wallet, since the old password parameters won't work anymore.

buck54321 commented 4 years ago

@JoeGruffins, I had pushed the wrong commit here. Mind giving this another look?

buck54321 commented 4 years ago

@JoeGruffins: Sorry to keep moving this on you, but I made a couple more changes. It's now been brought in line with https://github.com/decred/dcrdex/pull/155, so the conversation there might be informative.

Also brought tests up to standard.

buck54321 commented 4 years ago

Is there a reason that the auth is a hash of the first half of the key and not just the first half of the 64 byte key? Does the first half reveal some information about the second half?

Small correction: The authentication check is not based on the hash, but on verifying the MAC tag created from the second half of the KDF key and the serialized "base parameters".

I originally had it as simply checking the hash, but I changed it to checking the MAC based on the pattern recommended by @jrick in the dex work. As for why that pattern is better, it's not exactly clear to me, but I trust @jrick for this stuff.