Fix secret key decryption #59
Conversation
|
Can you expand on why you think this is broken? (I haven't looked at the code in a while so I might be missing context) |
| if kdf_rounds == 0 { | ||
| PrivateKey::verify_keypair(&complete_key)?; | ||
| } |
There was a problem hiding this comment.
so I think before this change, this check would have never passed for encrypted keys. the reason is the public key derived from the encrypted secret key would never match the decrypted public key. I was hitting this issue when decoding encrypted secret keys.
There was a problem hiding this comment.
Yes this seems like a bug in hindsight of writing it. I even seem to have ignored the second part of my own comment :D It makes no sense to try validating a ciphertext blob.
There was a problem hiding this comment.
Do you mind adding a regression test of PrivateKey::from_bytes that checks both cases of this branch? (doesn't need to test a failed decryption, just that it is only ran when kdf_rounds is set appropriately)
There was a problem hiding this comment.
PRed this as its own fix in #61
There was a problem hiding this comment.
thanks, I totally forgot about this, as life got in the way 😅 my bad!
| @@ -119,7 +119,7 @@ impl PrivateKey { | |||
| } = &derivation_info | |||
| { | |||
| let kdf_rounds = *kdf_rounds; | |||
| Self::inner_kdf_mix(&mut complete_key.0[..32], kdf_rounds, &salt, passphrase)?; | |||
| Self::inner_kdf_mix(&mut complete_key.0[..], kdf_rounds, &salt, passphrase)?; | |||
| @@ -146,7 +146,7 @@ impl PrivateKey { | |||
| let mut encrypted_key = self.complete_key.clone(); // Cheap :) | |||
|
|
|||
| match Self::inner_kdf_mix( | |||
| &mut encrypted_key[..32], | |||
| &mut encrypted_key[..], | |||
There was a problem hiding this comment.
...this change, to be honest I'm still not 100% sold on the reason why it fixes the issues I was having during decryption. what it essentially fixes is XOR-ing the complete key with the derived key from kdf vs. only XOR-ing the secret key bytes, I believe. before I ran into Error:WrongKey I think.
There was a problem hiding this comment.
I'm wondering if I've missed a detail or if there is something amiss elsewhere if that's the case. From my knowledge it should just be the secret key part that gets passed into XOR.
What error are you getting back, whoops can't read today.WrongKey or BadPassword?
Fixes decryption of secret keys