fix(key-derivation): use stored Argon2 parameters instead of default values #2360
Conversation
|
Should we maybe add regression tests to ensure data encrypted with old code can be always decrypted? |
It will not work as saved params in keystore file are not what was used to actually encrypt. This is the problem we are trying to fix here and as mentioned here #2360 (comment) this encryption was not used by anyone yet as it's still to be released in next GUI release. |
There was a problem hiding this comment.
LGTM, but we should keep in mind that this change will make previously saved .dat files incompatible with the new version. This isn't just due to changes in the JSON structure (such as adding the version field and modifying some argument types), but also because the earlier version actually used m_cost = 19456, while storing it in JSON as 65536.
To make old .dat files compatible with these new changes, we need to manually apply the following modifications:
But since all of these changes were in the dev branch and no one actually saved mnemonics using the old code, this is acceptable.
p.s. Also, it might be useful to remind that using the change_mnemonic_password RPC will also update the Argon2 parameters to the current defaults.
For example, if you start mm2 with a .dat file where "m_cost" is 19456, after using change_mnemonic_password, it will be upgraded to "m_cost": 65536.
* dev: feat(rpc): add is_success field to legacy MySwapStatusResponse (#2371) fix(key-derivation): use stored Argon2 parameters instead of default values (#2360) fix(tests): stabilize `tendermint_coin::test_claim_staking_rewards` (#2373) improvement(RPCs): group staking rpcs under a namespace (#2372) feat(tendermint): claim delegation rewards (#2351) fix(eth-tpu): remove state from funding validation (#2334) improvement(rpc-server): rpc server dynamic port allocation (#2342) fix(tests): fix or ignore unstable tests (#2365) fix(fs): make `filter_files_by_extension` return only files (#2364) fix(derive_key_from_path): check length of current_key_material (#2356) chore(release): bump mm2 version to 2.4.0-beta (#2346) fix(tests): add additional testnet sepolia nodes to test code (#2358) fix(swaps): maintain legacy compatibility for negotiation messages (#2353) refactor(SwapOps): impl defaults for protocol specific swapops fns (#2354) feat(tpu-v2): provide swap protocol versioning (#2324) feat(wallet): add change mnemonic password rpc (#2317)
Previously, the key derivation function for mnemonic password always used
Argon2::default(), which depends on thelibrary’s default parameters. Since those defaults may change over time (e.g., following changes
in OWASP recommendations), this PR ensures that the Argon2 instance is built using the
parameters stored in the keystore file. This prevents breaks in mnemonic decryption when updating the Argon2 crate.
Additionally, a version field was added to the
EncryptedDatastruct to enable easier handling of backward compatibility.fixes #2341