|
Hans0
Member
Offline
Activity: 91
Merit: 10
|
|
June 06, 2011, 10:20:58 AM |
|
Reading "CBC mode" made me remember this paper (causing many ASP.NET applications to become remotely vulnerable because they used CBC mode): http://www.isg.rhul.ac.uk/~kp/padding.pdfI do not fully understand the problem but the CBC mode seems to be vulnerable. Maybe somebody can look at this.
|
|
|
|
Hans0
Member
Offline
Activity: 91
Merit: 10
|
|
June 06, 2011, 10:32:51 AM |
|
Idea: Launch a new process that displays the password prompt and sends it back via IPC. That way the GUI will not hold to the password as all memory is destroyed when the process exits. Reduces the chance of it going into swap or hibernation file.
|
|
|
|
twmz
|
|
June 06, 2011, 09:39:45 PM |
|
The only minor thing that I noticed you might want to fix is that the walletpasswordchange RPC method was not added to the RPC help results.
Thanks fixed that and fixed some minor logic issues to make it a bit cleaner for -nocrypt. Any thoughts on how a client might detect that a particular bitcoind had encryption enabled (thereby requiring walletpassword) vs a bitcoind that doesn't have encryption enabled.
|
Was I helpful? 1 TwmzX1wBxNF2qtAJRhdKmi2WyLZ5VHRs WoT, GPGBitrated user: ewal.
|
|
|
Matt Corallo (OP)
|
|
June 07, 2011, 12:20:46 AM |
|
Any thoughts on how a client might detect that a particular bitcoind had encryption enabled (thereby requiring walletpassword) vs a bitcoind that doesn't have encryption enabled.
Its currently pretty smart. If -nocrypt is on, walletpassword and walletpasswordchange will not appear in help. You could check that, or just call one of the two and see if you get an error saying that you have -nocrypt on. Idea: Launch a new process that displays the password prompt and sends it back via IPC. That way the GUI will not hold to the password as all memory is destroyed when the process exits. Reduces the chance of it going into swap or hibernation file.
At that point you are putting *way* too much effort into protecting against one attack, when others are easier to exploit anyway, such as a keylogger or memory dumper that dumps the keys out of bitcoin anyway. Reading "CBC mode" made me remember this paper (causing many ASP.NET applications to become remotely vulnerable because they used CBC mode): http://www.isg.rhul.ac.uk/~kp/padding.pdfI do not fully understand the problem but the CBC mode seems to be vulnerable. Maybe somebody can look at this. That attack appears to require a padding oracle which is a function which can check if arbitrary data fits the necessary padding scheme. For that to work, the padding oracle must have the private key, so an attacker could not exploit it without your private key. Thus it doesnt really apply. Also, it appears to only apply to some padding scheme, and although I didnt bother looking into that, I'd hope openssl defaults to a secure padding scheme.
|
|
|
|
twmz
|
|
June 09, 2011, 12:54:12 PM |
|
Today I decided to try changing my password with walletpasswordchange and it didn't seem to work. Well, the RPC call succeeded as if it had worked, but the password didn't seem to have changed. The walletpassword RPC call still only accepted the old password and not the new one.
I should have tested this earlier, sorry.
|
Was I helpful? 1 TwmzX1wBxNF2qtAJRhdKmi2WyLZ5VHRs WoT, GPGBitrated user: ewal.
|
|
|
Matt Corallo (OP)
|
|
June 09, 2011, 01:11:27 PM |
|
Today I decided to try changing my password with walletpasswordchange and it didn't seem to work. Well, the RPC call succeeded as if it had worked, but the password didn't seem to have changed. The walletpassword RPC call still only accepted the old password and not the new one.
Oops, had a 1-character typo in rpc.cpp, should work now.
|
|
|
|
lachesis
|
|
June 11, 2011, 05:48:11 PM Last edit: June 11, 2011, 10:49:47 PM by lachesis |
|
I've found what appears to be a nasty bug, and has made me potentially lose 65 BTC so far: I called "bitcoind getaccountaddress Testing" or something similar. It returned the address "1HT7xU2Ngenf7D4yocz2SAcnNLW7rK8d4E". I then sent 10BTC to this address (showing my friend how the unlock RPC command works). A few minutes later, I called "bitcoind getaccountaddress FromMtGox" to withdraw some BTC from MtGox. It also returned "1HT7xU2Ngenf7D4yocz2SAcnNLW7rK8d4E" although I didn't notice it at the time. In fact, no matter what account I ask for, I get that address. Even worse, I don't seem to have the private key for it. I don't see the recv part of any of the transactions that I just described in my listtransactions output. bitcoind validateaddress 1HT7xU2Ngenf7D4yocz2SAcnNLW7rK8d4E { "isvalid" : true, "address" : "1HT7xU2Ngenf7D4yocz2SAcnNLW7rK8d4E", "ismine" : false }
If you check blockexplorer, you'll see that address has clearly received the coins. The 0.01 is from me as well. However, the 3.79 transaction is not from me. What's going on here!? Furthermore, who owns that address and how the hell did my client get it and decide to use it for all my accounts? --Lachesis
|
|
|
|
Matt Corallo (OP)
|
|
June 11, 2011, 10:37:50 PM |
|
I've found what appears to be a nasty bug, and has made me potentially lose 65 BTC so far: If you check blockexplorer, you'll see that address has clearly received the coins. The 0.01 is from me as well. However, the 3.79 transaction is not from me. What's going on here!? Furthermore, who owns that address and how the hell did my client get it and decide to use it for all my accounts? Looks like the bug was a missing { which caused it to return the address of a blank pubkey (which is impossible to generate). Those coins are gone. Really sorry about this, just poor coding on my part. I posted a separate commit so that the fix is clearly visible. If you want proof I didn't steal the coins, you can change rpc.cpp:381 (of the new version) to vector<unsigned char> vchBlankVector; strAddress = PubKeyToAddress(vchBlankVector);
and you will notice that it always returns that address (an address derived from pubkey of length 0 which cannot exist). Again, Im really sorry about all of this, Ill be sending Lachesis a couple coins, and if anyone else has a valid problem with this as well, Ill send you a couple, just pm me. It was just really poor syntax in my coding, I really need to stop coding after 2 am.
|
|
|
|
Hans0
Member
Offline
Activity: 91
Merit: 10
|
|
June 11, 2011, 10:47:33 PM |
|
I've found what appears to be a nasty bug, and has made me potentially lose 65 BTC so far: If you check blockexplorer, you'll see that address has clearly received the coins. The 0.01 is from me as well. However, the 3.79 transaction is not from me. What's going on here!? Furthermore, who owns that address and how the hell did my client get it and decide to use it for all my accounts? Looks like the bug was a missing { which caused it to return the address of a blank pubkey (which is impossible to generate). Those coins are gone. Really sorry about this, just poor coding on my part. I posted a separate commit so that the fix is clearly visible. If you want proof I didn't steal the coins, you can change rpc.cpp:381 (of the new version) to vector<unsigned char> vchBlankVector; strAddress = PubKeyToAddress(vchBlankVector);
and you will notice that it always returns that address (an address derived from pubkey of length 0 which cannot exist). Again, Im really sorry about all of this, Ill be sending Lachesis a couple coins, and if anyone else has a valid problem with this as well, Ill send you a couple, just pm me. It was just really poor syntax in my coding, I really need to stop coding after 2 am. Don't send him your coins. You are not his insurance against software bugs. We are grateful that you help develop bitcoin and that must be enough for the guy who lost his coins.
|
|
|
|
lachesis
|
|
June 11, 2011, 10:49:08 PM |
|
Don't send him your coins. You are not his insurance against software bugs. We are grateful that you help develop bitcoin and that must be enough for the guy who lost his coins.
Nobody's forcing him to do anything. I didn't ask him for coins - he offered.
|
|
|
|
Hans0
Member
Offline
Activity: 91
Merit: 10
|
|
June 11, 2011, 10:51:31 PM |
|
Don't send him your coins. You are not his insurance against software bugs. We are grateful that you help develop bitcoin and that must be enough for the guy who lost his coins.
Nobody's forcing him to do anything. I didn't ask him for coins - he offered. I know. I am advising him. It is not his duty to undo damages caused by unintentional bugs. Mistakes happen.
|
|
|
|
Matt Corallo (OP)
|
|
June 12, 2011, 02:19:03 PM |
|
Don't send him your coins. You are not his insurance against software bugs. We are grateful that you help develop bitcoin and that must be enough for the guy who lost his coins.
That said, I feel bad that he lost a couple coins due to a poor coding mistake. I'm not sending him the full amount (largely because I dont have much left myself) but still, I fucked up, and, if nothing else, should pay him in appreciation of his willingness to test my code. In any case, this is a good opportunity to reiterate the standard "this is code written once and only mostly tested, it has not been looked over by others, and not even much by me since I wrote it, it is beta and should not be expected to work 100%"
|
|
|
|
jgarzik
Legendary
Offline
Activity: 1596
Merit: 1099
|
|
June 14, 2011, 12:02:07 AM |
|
Hoping to make this a priority for the next release. Review requested! https://github.com/bitcoin/bitcoin/pull/232
|
Jeff Garzik, Bloq CEO, former bitcoin core dev team; opinions are my own. Visit bloq.com / metronome.io Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
|
|
|
Hans0
Member
Offline
Activity: 91
Merit: 10
|
|
June 14, 2011, 10:51:54 AM |
|
Oh, the missing braces were a nasty bug! What about adding an assert to PubKeyToAddress that ensures "!vector.empty()" ?
|
|
|
|
passerby
Member
Offline
Activity: 112
Merit: 10
|
|
June 17, 2011, 11:49:44 PM |
|
Okay, maybe I'm being dumb like a rock here, but do I understand correctly that the threat model in the context of which this is supposed to operate is bitcoin-hunting malware infection?
Y/N?
If Y, then what prevents malware from stealing the crypto key material from memory when user authenticates or just plain intercept authentication (assuming no HSM/smartcards being employed to store valet away from malware tampering)?
If N, then sorry my bad.
|
|
|
|
jgarzik
Legendary
Offline
Activity: 1596
Merit: 1099
|
|
June 18, 2011, 12:20:14 AM |
|
Nobody claims this will solve malware infections.
A simple keylogger can defeat this.
But we need to raise bar so that "cat wallet.dat | mail" thefts will not work.
|
Jeff Garzik, Bloq CEO, former bitcoin core dev team; opinions are my own. Visit bloq.com / metronome.io Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
|
|
|
passerby
Member
Offline
Activity: 112
Merit: 10
|
|
June 18, 2011, 05:43:34 PM |
|
I think it's pretty measly a defense that would be primarily a PR tool (SEE? WE DO CARE ABOUT SIMPLETON J. USER!) and not an actual security measure.
To really secure a wallet against malware, you'd be best to implement some smartcard/HSM support or something, so that wallet is manipulated in a strongly isolated environment.
|
|
|
|
bzzard
|
|
June 18, 2011, 06:51:09 PM |
|
Nobody claims this will solve malware infections.
A simple keylogger can defeat this.
Maybe some screen keyboard with A-Z keys generated at random positions? But then still some malware can do a screenshot File as a key can be also good solution (like in Truecrypt).
|
|
|
|
passerby
Member
Offline
Activity: 112
Merit: 10
|
|
June 18, 2011, 07:21:05 PM |
|
Nobody claims this will solve malware infections.
A simple keylogger can defeat this.
Maybe some screen keyboard with A-Z keys generated at random positions? But then still some malware can do a screenshot File as a key can be also good solution (like in Truecrypt). I don't want to sound like some patronizing ass, especially since I can't code 'fo shit, but I really think people should read some books by Bruce Schneier - our IT security guy had me do it after we got into an argument over somewhat similar type of thing and it indeed changed the way I see those things. What you are currently doing is concocting a very specific countermeasure against a very narrow implementation of a certain attack vector. It is futile. If worst comes to worst, as long as the cryptographic material shares the same memory with malware, you can't really expect to be able to keep the bad guys at bay. The only way to reliably ward off such an attack is to keep the wallet or some part thereof on an isolated module 100% of the time, such as smart card or that USB security dongle thingus. Or implement some mindboggling virtualization set-up which no sane user would actually use. Having the wallet "encrypted" is, however, important for PR (Otherwise people will claim Devs don't care about "average joe six-pack" and his hard earned bitcoins). Thus, the simplest possible encryption/password scheme should be implemented, so that as little time as possible is lost on such technologically pointless endeavor. Also, someone of those people who have thousands upon thousands of dollars worth of coins in their wallet should post a bounty for development of smartcard/HSM support of some sort. Srsly, that's in their own enlightened self-interest ...
|
|
|
|
|