mlabs-haskell / godot-cardano

Integrated light wallet and off-chain SDK for the Godot engine and Cardano blockchain
https://mlabs-haskell.github.io/godot-cardano/
MIT License
3 stars 0 forks source link

Find balanced scrypt parameters #43

Open danielfarrelly opened 6 months ago

rmgaray commented 4 months ago

To clarify: the idea here is to find a good set of parameters for scrypt encryption/decryption that reduces the amount of time it takes to open a wallet. The "recommended" parameters seem to be too taxing for a mid-range laptop (and maybe acceptable for a higher-end PC).

rmgaray commented 3 months ago

I will expand on this issue.

Our framework currently encrypts the Cardano master private key using the PBES2 encryption scheme, as implemented in the pkcs5 Rust crate.

Currently, our wallet implementation keeps a copy of the encrypted master key in memory. We only decrypt the master key when necessary, such as when a user signature is needed. When we do it, we do it inside a closure - but we don't zero out the memory yet (see issue #36).

The main issue with our choice of encryption scheme is that the recommended set of parameters is too slow for interactive use. While we do the decryption of the private key in a separate thread (to make sure the UI does not freeze) the user still needs to wait an unacceptable amount of time (more than 10 seconds, depending on the user machine).

We know the set of parameters is the culprit because we saw a noticeable improvement after lowering these to be less taxing. These are currently hardcoded here.

We want a more principled solution, one that answers the following questions:

If it's not possible to have a set of encryption parameters that is safe for both uses, what encryption scheme alternative can we use?

bladyjoker commented 3 months ago

Is it a possibility to have the lock/unlock methods on your key container? Perhaps keeping the key decrypted when unlocked, with a timeout feature that then locks the key (and perhaps zeroes out the memory).

My point is that maybe, leaving the key unencrypted for the duration of the user session works around this problem. Then perhaps you get to use strongest encryption parameters as unlocking and locking would be done sparingly, and not for each interactive key use.

rmgaray commented 3 months ago

@bladyjoker That's a very good idea and I think we could definitely improve our implementation by keeping the key unencrypted for a limited amount of time, avoiding user signatures in the process. This could even be desirable as a feature, since blockchain games may have a need for signing data or transactions more frequently, something that the more standard "enter your password for every signing operation" paradigm does not help to do.

But still, even my password manager does not take 10 seconds to unlock. There is only so much wait time a user may accept, even for an initial load. I did some measurements on my laptop:

So it's actually far worse than I expected, it takes 20 seconds to encrypt/decrypt the key on my (admittedly old) Intel Core i5 laptop from 2017. I think there should be some middle ground here, but I'm not sure how to evaluate whether we are compromising too much the security by reducing the load times.

rmgaray commented 3 months ago

I found this book which gives some helpful tips for choosing scrypt parameters. It seems like one must either choose to encrypt something for interactive use or permanent storage.

Perhaps a good solution would be to persist the key to disk using very strong encryption parameters (something similar to the recommended set) but, upon loading to memory, encrypt/decrypt using a faster set of parameters (more suited to interactive use).

What do you think?

danielfarrelly commented 3 months ago

@rmgaray Another noteworthy line from the link you provided:

In the MyEtherWallet crypto wallet, the default Scrypt parameters are N=8192, r=8, p=1. These settings are not strong enough for crypto wallets, but this is how it works. The solution is to use long and complex password to avoid password cracking attacks.

Maybe enforcing some minimum password requirements could be another part of balancing this?

rmgaray commented 3 months ago

Good point, password length impacts encryption/decryption time, so I suppose I should run a couple of scenarios with a more realistic password (no more 1234).

bladyjoker commented 3 months ago

Perhaps a good solution would be to persist the key to disk using very strong encryption parameters (something similar to the recommended set) but, upon loading to memory, encrypt/decrypt using a faster set of parameters (more suited to interactive use).

I think that's a very good idea. I would go with that.