Topic: [PATCH] wallet private key encryption (Read 8211 times)

Would it be cryptographically safe to use the (hash of) the public key as IV when encrypting the private key part?

Another idea: store the actual randomly-generated AES key for encryption of the private keys in the wallet itself, encrypted itself using a key derived (using the nice key derivation function you have for scratch-cards?) from the passphrase. This way, you can easily change the passphrase without needing to decrypt/reencrypt everything in the wallet. This encrypted key could also contain a hashed version of itself, to verify integrity.

A change passphrase RPC call / wx thing could be added, and setting it to the empty string would remove encryption (and turn all ekey fields back to key fields).

I think the solution is to fix the client...this is an odd behavior.  It's not really necessary to have an address displayed at all times.  When receiving payment, the client could just clear the current address from the display.  If no user is around, then it's impossible for them to share that address anyway...so no point in generating a new address until a user actually wants to send it to someone for use in a payment.  The copy to clipboard button could trigger the generation of a new address when there isn't a current one.

Steve, the way the client works now, all the wallet keys are read into memory at startup. Subsequent changes are made in memory and written to disk. This proposed patch decrypts keys as they are read into memory at startup, and encrypts new keys as they are written out to disk.

Decrypting keys on use wouldn't be too hard. Add a popup to read the passphrase, perhaps cache it for a while as you suggest.

Strangely, the hard part is encrypting new keys, because Bitcoin creates keys at odd times. The oddest time is when receiving a payment to the "current address" displayed at the top of the window. Bitcoin likes to keep things fresh so it generates a new address to display, and new key. But it can't encrypt the new key without the passphrase, and in general no user is around to respond to a popup at random times.

This is a great step forward.  I would say that the solution needs to stay focused on the individual user wanting to easily hold some bitcoins and use it for transactions.  Thus, I would vote against things like allowing for multiple keys to be used within the same wallet for different subsets of keys.  Merchants or service providers needing such advanced features can customize the basic client code.

Also, assuming it doesn't weaken the security, I don't think encrypted the entire wallet is necessary (just the private bitcoin keys in the DB is fine).

I would also suggest that you not use the users passphrase directly, but instead use a hash (i..e sha256) of their passphrase for the encryption.  This is not to add any additional security to the wallet itself, but to prevent a user's passphrase from being exposed should the bitcoin client be compromised and the decryption key is retrieved (and thereby offering some protection for users that use the same passphrase for many things).  You also need to ensure the user's unhashed passphrase doesn't stick around anywhere in bitcoins' memory by accident.

From a UI perspective, I think on startup, and when accessing the wallet...if it is an encrypted wallet, the user should be prompted for their password...there should be an option the user can set on how long the client will retain the hashed password before overwriting and discarding it from memory...after such time, if the user needs to do something involving a private bitcoin key, they will once again be prompted for a password.  If this is set to 5 minutes or so by default, it's a fair amount of time to have the wallet lock itself in case a user leaves the client unattended.  Another option could also allow the user to require a password on ever use of a private key and for the client to never retain the hashed password beyond the time it takes to decrypt and use the private key (also, decrypted keys should never be retained in memory for longer than the time necessary to use them).

Note, it might also be a good idea to lock the entire client and mask the UI when the passphrase timeout expires (this is effectively re-using the wallet encryption feature to lock the entire client, but I wouldn't want to have to use a separate passphrase for this purpose).

That sort of key-recovery attack is specific to RC4 (and maybe other stream ciphers), I believe.

If you manage to gather lots of IVs then you can use ones that are the same/similar to do a statistical attack to discover the secret key. This is how WEP is cracked because it's IV is only 24 bit, and that's not long enough to ensure that the same key isn't used twice. There's also another flaw whereby you can inject special packets to make the AP spew IVs but that's another story :p

An IV is always transmitted before the ciphertext (or stored, in this case) and is necessary for decryption.  The IV's purpose is to start the encryption from a random state, so that encrypting the same plaintext twice does not result in the same ciphertext.  This prevents comparing two records that have the same initial segment.

I'm not an AES expert, so permit a dumb question:  does AES decryption require knowledge of the IV used to encrypt a given ciphertext?

With the only information persisting between sessions being the user's passphrase, that would seem to imply storing the IV for each encryption?

I think that's the right way to think about it.  And I think Jeff actually implementing a straw-man proposal is exactly the right thing to do.

So:  I say we don't try to defend against (3), at least not right now.  If you have root then you can install a keylogger, read memory, intercept any system call, etc etc etc.   (I would like to see somebody implement a bitcoin client that required payment verification using a cell phone app or telephone call or PIN-sent-to-email and did all the magic key management to make that work securely, but I think that's beyond the scope of what we can reasonably do right now).

Defending against (1) and (2) would help with:

a) you forget to logout so attacker sits down at your computer, starts bitcoin and empties your wallet.
b) attacker gets a hold of a filesystem backup that is not encrypted.
c) sysadmin sets file permissions incorrectly so attacker on multi-user system can read your wallet.dat
d) attacker guesses or finds out your ssh password, logs in remotely and steals your wallet.dat.

It won't help with:
- sysadmin with root privileges is evil
- system compromised by rootkit/trojan/keylogger


RE: encrypt everything:  I say maybe later.  Just encrypt everything isn't trivial: users would have to wait a minute or two or ten for Berkeley DB to rewrite all of blkindex.dat (bottleneck will be disk I/O, not the encryption), and we have to deal with "my disk filled up when I changed my password, things are half-encrypted and half-not, what do I do now?"   And I don't see a lot of value in encrypting all of wallet.dat; forget to shutdown bitcoin and an attacker that wants to know your public addresses can just open up the address book and take a screenshot.

It doesn't seem to hurt to encrypt everything?  I think symmetric encryption can do the entire block chain in about a second.

http://www.cryptopp.com/benchmarks.html

A setup that user might reasonably be expected to understand could be:  one password to unlock the wallet (whole-file), and a second to spend money (ECDSA private keys).

However...    It remains a PITA with the current bitcoin implementation to perform whole-file encryption on the wallet.  db4 has whole-environment encryption, so that implies an encrypt-everything solution would encrypt the wallet, addr and blkindex databases.

One could encrypt the 'value' part of each db4 key/value pair, by modifying bitcoin's database Read() and Write() methods.  But with the keys being plaintext, I'm not sure that's useful.

With the current implementation, you are locked into a lot of design decisions, where if you deviate you must rewrite a lot of code all over the codebase.

For that reason, I had hoped a patch would focus discussion on what is reasonable with the current codebase, rather than a perfect solution that will never see the light of day.

There should be two layers of encryption.

The entire wallet should have (optional) AES encryption.

Nested inside this wallet package there should be a PGP private and public key.  This private key should be also encrypted with AES (but with a different passphase)

Every Bitcoin private key should be encrypted independently to the public key, so making a new Bitcoin address will not require entering the private key passphase.

When first loading, Bitcoin will ask for the AES wallet key, if there is one. (this step is optional)

Once the wallet is decrypted, the client will ask "How much funds do you want to make available for spending, and for how long?"

If the answer is more than 'none,'  the client will ask for the private key passphase, and decrypt the minimum number of bitcoin private keys required to make the funds available and store them only in memory.  The passphase and decrypted global private key will automatically be deleted (from memory) after this operation.

If the user wishes to unlock more funds, or wants to send funds after the unlocked time has expired, the Bitcoin client will ask for the private key passphase again.

A flexible system could do the following, with the user making some usability choices:

* Have the user choose a passphrase, and derive one or two secret keys OR the user chooses two passphrases
* Optionally encrypt the wallet as a whole with one secret key
* Always encrypt the private keys with the second secret key
* If the whole wallet is encrypted, prompt for passphrase on startup
* Optionally forget the passphrase after startup, while keeping the wallet secret key
* If passphrase was forgotten, or there is no whole-wallet encryption, and the user wishes to spend, prompt for the passphrase again
* Again, optionally forget the passphrase after spending

I think new keys can still be created automatically.  Just create them in batch ahead of time during spend operations.

Different passphrases for different key sets are an interesting idea, but usability would start to suffer due to complexity.  Might be better to have separate wallets.


I don't think there's any difference between #2 and #3.  In either case, the attacker can listen for the user's passphrase.

There's good discussion of this feature here:

https://github.com/bitcoin/bitcoin/issues#issue/3

https://gist.github.com/803170

Issues:

 - Symmetric (aes) vs public key (rsa) encryption

 - Decrypt at startup vs decrypt on use

 - Create new keys automatically (as now) vs create only on user action

 - Encrypt all keys with same passphrase vs different passphrases for different key sets

One way to analyze these is via a threat model. What can the attacker do, that we will try to defend against? And what will we not defend against?

We can distinguish three levels of attacker:

1. Can read user files

2. Can read/write user files but only read system files

3. Can read/write everything, root privileges


Sorry, getting tired, will write more later.".

That's a very concise implementation. I did spot one bug:

The max ciphertext size is actually len + 2*AES_BLOCK_SIZE, so you should set c_len to that, and allocate that much space.

Also a security flaw, you are using a constant IV everywhere, it looks like. You need to use a different IV for each encryption.

One other point, Bitcoin uses a CPrivKey type for sensitive data like private keys. It zeroes memory when it's freed.

SSL recommends to use SEAL which does RSA + AES with IVs (as above). You shouldn't use constant IVs because that can lead to a statistical attack.

Entire wallet. Decrypt into memory once at startup. Randomise location in memory somehow.


The above code I wrote, will try to load the PEM but throw an exception when an invalid passphrase is used. So the code is as simple as:

try {
  // load
} catch (ReadError err) {
}

I would vote no - encrypt private keys only - otherwise passphrase must be typed more often, making it more vulnerable to interception.


If each record were encrypted with a properly cryptographically random initialization vector, the two should be equivalent.  On the other hand, encrypting everything as a block guarantees that there's no possibility of different records being encrypted with different keys for whatever reason.


Assuming the policy isn't "encrypt everything", wouldn't it be simple and straightforward to correlate the private key with either the public key, or the bitcoin address?  Seems either it would check out or it would not.  If the policy were encrypt everything, simply storing perhaps the last 16 bits of the hash of (password+salt), where this salt was only used for the password-verification hash, so that 16 bits of the actual key weren't being given away, you'd have a method that catches nearly all typos, without being very useful to a password cracker.

There's been a lot of feature-requesting and demoing and such.  By posting real code against current bitcoin, it was hoped that we could move on to a more focused discussion (w/ real code) about what wallet encryption should look like.

We need to answer questions such as

Is the presented method cryptographically strong?
Should we encrypt the entire wallet, and not just private keys?
Should we encrypt the plaintext data concatenated together as a single huge memory buffer, enhancing cipher block chaining, or encrypt invididual records as my patch does?
How to handle wallet upgrade + downgrade?  "ekey" creates unspendable coins, when moving to older clients.
How to check for invalid passphrase?  It seems cryptographically unwise to use the naive implementation (encrypt a static, well known string; check for invalid passphrase by attempting to decrypt to well known plaintext).

I was actually also working on this using SSL's SEAL:





Not sure if that code is useful now, but dumping it anyway...

g++ a.cpp -lssl -o enctest
./enctest
g++ b.cpp -lssl -o seal
./seal < INPUT > ENCOUT
g++ c.cpp -lssl -o unseal
./unseal < ENCOUT

This patch adds one form of an opt-requested feature, wallet encryption:

     http://yyz.us/bitcoin/patch.bitcoin-wallet-crypter
     or git://github.com/jgarzik/bitcoin.git#crypter

BIG FAT WARNING: This patch is dangerous, and could create unspendable coins.  Do not use, unless you really know what you are doing.

Implementation details:

With this change, encryption (and associated passphrase) is now required, otherwise the program will exit.  The user may set their passphrase in the WALLET_PASSPHRASE environment variable, or in a GUI dialog if GUI is present.

A new type of wallet record, "ekey", is used to store a public key + AES-encrypted private key.  All new private keys are stored in "ekey" records.  Old keys stored in "key" and "wkey" records will continue to be read and supported indefinitely, but bitcoin no longer writes these records.

Caveats:

• Totally untested rough draft.  It compiles and makes logical sense, that's about it.  I haven't even bothered to test it, though I welcome feedback from brave souls!
• The implementation is bare-bones.  No provisions for wallet recovery, or detecting use of the wrong passphrase (easy way to corrupt!), etc.

Let the comments begin.  Hopefully this will spark discussion about the proper solution for wallet encryption, now that code is out there.