5 ms·
As far as I know that is NOT correct. The wallets in question were all -- to my knowledge -- using the Android platform-provided Java SecureRandom generator.
by gwillen 13y ago
As far as I know that is NOT correct. The wallets in question were all -- to my knowledge -- using the Android platform-provided Java SecureRandom generator.
- tptacek 13y agoIf the story here is that Java SecureRandom on Android is bad enough to break DSA, the headline on this story is wrong; it should be something more like "Android Doomed". Can you provide a link to something corroborating this?
- VMG 13y agoWell there is this paper: http://www.scribd.com/doc/131955288/Randomly-Failed-The-State-of-Randomness-in-Current-Java-Implementations http://www.scribd.com/doc/131955288/Randomly-Failed-The-Stat... Quote: > Apache Harmony revealed multiple weaknesses caused by implementation bugs. As a part of Android a plethora of cryptographic functions [17] rely on this PRNG. One of the bugs addresses directly the Android platform, where as the second one only targets Apache Harmony.
- tptacek 13y agoThe bug described here isn't sufficient to generate repeated DSA k-values, even if it is still in the code†, which is unlikely. It seems more likely that the application code used SecureRandom insecurely, as described upthread. † It's virtually certain not to be in the code, since Android's CSPRNG is based on OpenSSL now, not Harmony's built-in CSPRNG.
- makomk 13y agoAccording to Mike Hearn's comment over at the Bitcoin forums[1]: > You should not assume that using OpenSSL directly is safe. On Jellybean+ the SecureRandom provider is just a shim over the OpenSSL RAND_* functions and Jellybean+ is also affected. However TLS connections are OK. > I realise there's going to be a lot of questions about what exactly is going on here, but I'm not on the Android team and can't talk on their behalf. It's up to them to document exactly how the RNG is broken. Suffice it to say, if you aren't sure if you're affected, you probably are but you are welcome to send me a private message detailing what you're doing and I'll let you know. [1] https://bitcointalk.org/index.php?topic=271486.msg2911376#msg2911376 https://bitcointalk.org/index.php?topic=271486.msg2911376#ms...
- benmmurphy 13y agothat is kind of scary. i had a look over the code and the problem seems not obvious. the only issue i saw was multiple instances of SecureRandom share the same state which could cause some problems if you do [1]: x = new SecureRandom(); y = new SecureRandom(); y.setSeed(predictable); x.generateBytes(zz); and also i think the seed is thread local so you can have: x = new SecureRandom(); x.setSeed(xxx); then on another thread x.generateBytes() won't do what you think it will do. i don't think this would cause dupes because i assume apps are isolated from each other and these apps aren't calling setSeed... [1] this is assuming android people haven't patched openssl to have different behaviour and also setSeed might be safe in openssl because it uses the value to augment and not replace the state.
- gwillen 13y agoI have done a bit of source diving in response to your question, and the answer is that all I can provide is the bitcoiners' announcement: https://bitcointalk.org/index.php?topic=271831.0 https://bitcointalk.org/index.php?topic=271831.0 They claim the problem lies with 'a component of Android'. One of them told me that the solution was to switch from using SecureRandom to reading /dev/urandom directly. The actual source changes appear not to be public, and he wouldn't tell me details about the issue. I tried to find the usages of SecureRandom in a couple of the apps that are supposed to be affected, but a cursory search didn't turn up much. I suspect it's inside some library that I don't know to look inside. The question, I guess, is whether all the affected apps share the same library or not -- I don't _think_ so, but if they do it would dramatically reduce my confidence that the issue lies with the Android platform, as claimed.
- glurgh 13y agoThat's the lib, if you're bored enough to keep diving https://code.google.com/p/bitcoinj/ https://code.google.com/p/bitcoinj/
- gwillen 13y agoOK, I dove a little more. The actual signing happens in org.spongycastle.crypto.signers.ECDSASigner . I haven't dug into _that_ yet. But one thing I'm wondering is: would any setSeed on any SecureRandom instance cause a potential problem? Or just on the same instance being used for signing? In other words, do I only need to check that spongycastle -- which I presume is a fork of bouncycastle -- handles things reasonably? Or could any other messing about with SecureRandom mess it up?
- tptacek 13y agoNot seeing any relevant code that sets seeds in SpongyCastle, either.
- benmmurphy 13y agoif they are using SecureRandom backed by openssl then i think any instance is a problem. However, the 'seed' is used to augment the state and not override it. I'm not sure if it is possible or how much data would be required to get predictable output from SecureRandom by calling setSeed. http://www.openssl.org/docs/crypto/RAND_add.html http://www.openssl.org/docs/crypto/RAND_add.html
- gwillen 13y agoActually, someone elsewhere in the thread has found, and linked, the bugfix commit, in another Android bitcoin wallet: https://code.google.com/p/bitcoin-wallet/source/detail?name=bitcoinj-0.10&r=04d2044880d88107ee4a939a516fb4be4cedeaf9# https://code.google.com/p/bitcoin-wallet/source/detail?name=... So literally they're just ripping out SecureRandom and replacing it with their own stub that reads /dev/urandom.
- deleted 13y ago[deleted]
- gwillen 13y agoOk, more source diving. Bitcoin Wallet calls into bitcoinj java/com/google/bitcoin/core/Wallet.java sendCoinsOffline(...), which calls completeTx(...), which calls core/Transaction.java signInputs(...), which calls calculateSignature(...), which calls core/ECKey.java sign(...), which performs: ECDSASigner signer = new ECDSASigner(); ECPrivateKeyParameters privKey = new ECPrivateKeyParameters(privateKeyForSigning, ecParams); signer.init(true, privKey); BigInteger[] sigs = signer.generateSignature(input.getBytes()); return new ECDSASignature(sigs[0], sigs[1]); This _appears_ to be a correct invocation of spongycastle-formerly-bouncycastle, initializing the signer with an instance of ECPrivateKeyParametes, and NOT an instance of ParametersWithRandom, so that, on org/spongycastle/crypto/signers/ECDSASigner.java line 41, we call SecureRandom() with no arguments. I don't see spongycastle ever calling setSeed, and I don't see it ever leaking its SecureRandom instance, so unless calling setSeed on ANY SecureRandom instance is a problem, this _looks_ like a correct usage. Also, I now remember how much I hate reading Java.
- patio11 13y agoAs tptacek has mentioned many, many times, SecureRandom is one of those things which is very secure if you understand exactly what it is doing and do not shoot yourself in the foot. One easy way to shoot yourself in the foot with SecureRandom is to use seed values from a source with low entropy. http://developer.android.com/reference/java/security/SecureRandom.html http://developer.android.com/reference/java/security/SecureR... If one were to copy/paste the sort of code samples which show up when one Googles for [Android SecureRandom], one would more than likely call setSeed, possibly in such a fashion as to duplicate seeds and, predictably, make the output of the random number generator deterministic. I will leave it to one's individual judgement as to whether "It is unlikely a serious developer would copy/paste in crypto code into a project that has real money on the line" is descriptive of the prevalent standard of care in engineering in the Bitcoin community.
- tptacek 13y agoThey were explicitly seeding SecureRandom?