[PULL] Wallet Private Key Encryption - page 4.

Matt Corallo

hero member

Activity: 755

Merit: 515

Quote from: Luke-Jr on May 18, 2011, 08:34:32 AM

It will still show up on the process list ('ps aux') at least for some duration.

Hence the "Doesn't mean the point isn't valid but still..."

Quote from: Luke-Jr on May 18, 2011, 08:34:32 AM

I would be concerned with the timeout mechanism creating a race condition. A virus/trojan is going to be able to spend the coins faster than the user. But I guess it could just be added to the already-long list of JSON-RPC problems to be addressed by any contending protocol.

If you are concerned about getting the security that tight, there are way more pressing problems. Memory dumping bitcoin, replacing the bitcoin binary which makes the rpc calls, etc, etc, etc. But yea, you would have to enter password the way they were originally, and I agree with gavin, that is not nearly as good.

Luke-Jr

legendary

Activity: 2576

Merit: 1186

Quote from: Matt Corallo on May 18, 2011, 08:32:05 AM

Quote from: khal on May 18, 2011, 07:05:15 AM

This will avoid your password to be stored in history files

If you dont want something in .bash_history (probably works for others too) you can just prefix the command with a space.
Doesn't mean the point isn't valid but still...

It will still show up on the process list ('ps aux') at least for some duration.

I would be concerned with the timeout mechanism creating a race condition. A virus/trojan is going to be able to spend the coins faster than the user. But I guess it could just be added to the already-long list of JSON-RPC problems to be addressed by any contending protocol.

Matt Corallo

hero member

Activity: 755

Merit: 515

Quote from: khal on May 18, 2011, 07:05:15 AM

This will avoid your password to be stored in history files

If you dont want something in .bash_history (probably works for others too) you can just prefix the command with a space.
Doesn't mean the point isn't valid but still...

Gavin Andresen

legendary

Activity: 1652

Merit: 2301

Chief Scientist

Would it be good enough if I volunteered to write a little perl or python or php or bash script that reads the passphrase from a file descriptor and then posts the right JSON-HTTP request?

If the consensus is to teach bitcoind to read arguments from file descriptors, somebody please figure out if there's a standard for how other unix apps do that. Here's what curl does:

Quote

If you start the data with the letter @, the rest should be a file name to read the data from, or - if you want curl to read the data from stdin.

Pieter Wuille

legendary

Activity: 1072

Merit: 1181

I'd like to see a way to pass the passphrase through a file descriptor instead of on the command-line, so it isn't visible on the process list.

khal

hero member

Activity: 540

Merit: 500

I would suggest :

Quote

walletpassword [password]

If password is not on the command line, bitcoin will prompt for it. This will avoid your password to be stored in history files (or anything else) and still allow putting all in 1 command if you prefer. Same for walletpasswordchange rpc.

Matt Corallo

hero member

Activity: 755

Merit: 515

Quote from: Gavin Andresen on May 17, 2011, 07:15:01 PM

walletpassword
A walletpasswordchange seems like it would be very handy, too.

Added, committed, rebased, pushed.

Matt Corallo

hero member

Activity: 755

Merit: 515

Quote from: Gavin Andresen on May 17, 2011, 07:15:01 PM

walletpassword

... which would store in memory for seconds. If you know your server is secure, you'd give a very long at startup.

Ah, good idea. Hadn't really given any real thought towards RPC stuff, just kind of did it, I suppose just doing it at 1 AM is never the right approach. The fact that that that fits perfectly with the current method of password handling is also quite nice

.

Gavin Andresen

legendary

Activity: 1652

Merit: 2301

Chief Scientist

Random thought RE: passwords and RPC:

I was thinking a better way of handling the password might be a new RPC command:

walletpassword

... which would store in memory for seconds. If you know your server is secure, you'd give a very long at startup.

That same mechanism might be very handy in the GUI (somebody who knows more about password security might have something intelligent to say about the tradeoff between the risk of storing hashed-password in memory versus the convenience of not having to constantly re-enter it).

A walletpasswordchange seems like it would be very handy, too.

Tacking onto the beginning of RPC argument lists seems like the wrong thing to do.

Matt Corallo

hero member

Activity: 755

Merit: 515

I suppose the title speaks for itself, but here is the commit blurb:

Quote

Add wallet privkey encryption.

This commit adds support for ckeys, 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 (by default) 1000 rounds and
a random salt.

By default, the user's wallet remains unencrypted until they call the RPC
command encryptwallet or, from the GUI menu, Options->
Encrypt Wallet.

When the user is attempting to call RPC functions which require the password
to unlock the wallet, an error will be returned unless they call
walletpassword first.

A keypoolrefill command has been added which tops up the users keypool
(requiring the password via walletpassword first).
keypoolsize has been added to the output of getinfo to show the user the
number of keys left before they need to specify their password and call
topupkeypool.

A walletpasswordchange has been added to allow
the user to change their password via RPC.

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.

Pull request is at ~~https://github.com/bitcoin/bitcoin/pull/232~~
https://github.com/bitcoin/bitcoin/pull/352
Now rebased on wallet class + mostly based off of Pieter's suggested model.

EDIT: New commit blurb to match changes.

Topic: [PULL] Wallet Private Key Encryption - page 4. (Read 16708 times)