Add wallet privkey encryption. #232

This commit adds support for ekeys, or encrypted private keys, to the wallet. All keys are stored in memory in their encrypted form and thus the passphrase is required from the user to spend coins, or to create new addresses.

Keys are encrypted with AES-256-CBC through OpenSSL's EVP library. The key is calculated via EVP_BytesToKey using AES256 with 1000 rounds.

Each RPC command which requires the password in practice has an additional parameter, namely that password. Due to the need to convert the types of other parameters before sending them, users who do not use encryption (via the -nocrypt option) will have to specify a blank string (or any string) as the first parameter, followed by the command they currently use.

Whenever keying material (unencrypted private keys, the user's password, the wallet's AES key) is stored unencrypted in memory, any reasonable attempt is made to mlock/VirtualLock that memory before storing the keying material. This is not true in several (commented) cases where mlock/VirtualLocking the memory is not possible.

Although encryption of private keys in memory can be very useful on desktop systems (as some small amount of protection against stupid viruses), on an RPC server, the password is entered fairly insecurely. Thus, the only main advantage encryption has for RPC servers is for RPC servers that do not spend coins, except in rare cases, eg. a webserver of a merchant which only receives payment except for cases of manual intervention.

Thanks to jgarzik for the original patch and sipa for all his input.

Forum thread: http://forum.bitcoin.org/index.php?topic=8728.0

Instead of using EVP_BytesToKey for key derivation I think scrypt (https://www.tarsnap.com/scrypt.html) should provide a somewhat better security against brute-force attacks.

@sysfrog Im not sure. Though it may claim to be more secure, 1000 rounds on OpenSSL's algorithm is also very secure and I think I'd rather use a more common/well used/well analysed algorithm like OpenSSL's entire code base is.

@dizzyd Ill take a look at updating to your other recommendations when I have a chance.

re: the unused IV -- "may be some kind of crazy issue.." is not a logical argument. Being paranoid about security is good, but there needs to be some basis in reality for it. Otherwise, you wind up with a lot of code that doesn't do anything and may obscure what's really going on.

re: calls to SetKey mlocking -- if that's the case, then you really don't need to copy it to vchKey. All that serves to do is increase the number of copies of keys you have in memory and increases your attack surface.

Security-wise: careful, minimal code > paranoid code. B0.02 from a random coder. :)

Is there a reason you are not using a random salt?

witkamp:

Is there a reason you are not using a random salt?

Yeah, password data should be salted - I mean we DO already have a giant network of people with high-end GPUs and the drivers/software necessary to brute force such things ;)

@TheBlueMatt As far as I understand scrypt the main advantage over older techniques is that it not only adds computational complexity to calculate a given key from a password, but that each calculation also requires a given amount of RAM. So it's much harder to parallelize an attack with, e.g., an ASIC, as you would need lots of RAM to do so.

But I agree with you: There have been much more reviews of the SSL code base, and so the chance of critical errors in OpenSSL is much lower compared to scrypt.

@witkamp mostly because it would be a pain to store. This patch already uses the public key as the IV for the encryption of each private key so any kind of brute force/dictionary attack is already ridiculously difficult, I dont see a major advantage to adding a random salt to be used for a given wallet (and then stored in the wallet) as to brute force a wallet, you already need to know some of the information in the wallet (namely the public keys). But maybe others disagree? @sysfrog OK, so its agreed then that OpenSSL key derivation is probably the best way to go? @dizzyd Still planning on updating with some of your original recommendations, but I'm on vacation atm and dont feel like doing much coding.

@TheBlueMatt gotcha, didn't see the public key = IV bit but it makes sense and stops dictionary/BF attacks at least as well as a random salt so I'm happy with it as a solution.

On Thu, Jun 16, 2011 at 11:24 AM, TheBlueMatt < reply@reply.github.com>wrote:

@dizzyd Still planning on updating with some of your original recommendations, but I'm on vacation atm and dont feel like doing much coding.

No worries -- I read your last reply first and was confused. :)

D.

It would be best to use AES128 and not AES256. The key schedule in AES256 is known to be weak.

http://www.schneier.com/blog/archives/2009/07/another_new_aes.html

Please salt that passphrase properly as suggested in gmaxwell's forum post.

@phantomcircuit That attack is effective against 11-round AES-256, but not full 14-round AES-256. It'd be better to increase the number of rounds (he recommends 28 or more in that article) used instead of using a smaller key size. There are plenty of attacks on AES-128 as well, many of which were published after that article was written.

Closing this in anticipation of the new branch.