| ▲ | When random.bytes() runs but doesn't work(insider.btcpp.dev) | ||||||||||||||||||||||||||||||||||||||||
| 47 points by Funes- 8 hours ago | 21 comments | |||||||||||||||||||||||||||||||||||||||||
| ▲ | londons_explore 2 hours ago | parent | next [-] | ||||||||||||||||||||||||||||||||||||||||
I am getting suspicious of "random number generator not actually returning random numbers" bugs.... They are the perfect bug for someone trying to "accidentally" make a secure system insecure. I wonder if bribe money was involved or three letter agencies... | |||||||||||||||||||||||||||||||||||||||||
| |||||||||||||||||||||||||||||||||||||||||
| ▲ | nonfamous 2 hours ago | parent | prev | next [-] | ||||||||||||||||||||||||||||||||||||||||
From the Twitter advisory on the issue being referenced here [1]: >>> To every other developer: we believe this is a sober reality of the new AI paradigm. AI-assisted code review can now find latent bugs at a speed that is outpacing even the industry’s most seasoned experts. If your firmware is open-source or has ever been public, assume it's already being read by attackers and defenders alike. Kinda turns the “many eyes” principle of OSS on its head, eh? | |||||||||||||||||||||||||||||||||||||||||
| |||||||||||||||||||||||||||||||||||||||||
| ▲ | anonymousiam 6 hours ago | parent | prev | next [-] | ||||||||||||||||||||||||||||||||||||||||
This post assumed that the author of the "runs" commit was not acting maliciously. Has that been established? It's not mentioned in this article. | |||||||||||||||||||||||||||||||||||||||||
| |||||||||||||||||||||||||||||||||||||||||
| ▲ | nullc 3 hours ago | parent | prev | next [-] | ||||||||||||||||||||||||||||||||||||||||
This writeup isn't very good and misses/misunderstands the programming error that leads to the flaw. I'm commenting because I think it's important to understand the issue. The article would have you think that the change in question was a tiny change to a flag to make it compile, but in reality the commit in question is a 1533 line addition of the entire RNG infrastructure. The fundamental cause is a mixup between a value test and a definedness test. Coldcard attempted to replace the micropython wrapper on the hardware TRNG, apparently in order to provide a more aggressive handling of fault/error conditions. The micropython hwrng code is gated by an #if check, the replacement HWRNG code is gated by an ifndef. So "#define MICROPY_HW_ENABLE_RNG (0)" deactivated the micropython implementation but failed to activate the internal one (which was #ifndef MICROPY_HW_ENABLE_RNG ... which didn't fire because MICROPY_HW_ENABLE_RNG was _defined_). This was easier to miss because the usages weren't only in different files-- they were in different repositories. There is a more abstract point to make that in cryptographic software the absence of a secure randomness source (the STM32 TRNG) should never fall back to an insecure source (a trivial PRNG which might have only had on the order of 20-bits of uncertainty in its input). But the code that had the fallback was micropython which was not authored by the coldcard creators and is presumably not intended for cryptographic applications... In later code (for MK4+ devices) the issue was further masked without being corrected by xor-ing in another insecure PRNG seeded by 32-bits from another TRNG. ... itself acting like an additional insecure fallback. (Why it first hashes 64-bits of TRNG output then throws away half the entropy is a mystery...) RNG failures can be difficult to detect because the real randomness and a PRNG are indistinguishable by any simple tests of the output. I understand the coldcard developers ran extensive tests on the randomness generated by these devices-- they may well have been just testing the PRNG. It's something of a "color of your bits" issue ( https://ansuz.sooke.bc.ca/entry/23 ). The same sort of issue happens at multiple levels of the stack, e.g. IIRC the STM32 TRNG itself does some kind of whitening that could have the same effect of concealing an RNG failure. The existence of insecure fake randomness in the code at all was a red flag that had been noted previously -- though the really bad one was hidden away in the micropython code and not even obviously at play. I think this is a rare case of a bug that would be more easily found from binary analysis than review of the source code (e.g. no access to the STM32 TRNG at all). Without these fallbacks the failure to use the TRNG would have been immediately detected by the developers (e.g. when every attempt generated the same seed), and a review could be validated by fault injection (NOP out the hwrng and verify that the test fails). | |||||||||||||||||||||||||||||||||||||||||
| |||||||||||||||||||||||||||||||||||||||||
| ▲ | PunchyHamster an hour ago | parent | prev | next [-] | ||||||||||||||||||||||||||||||||||||||||
> The commit message is 235 characters, and the commit changes 15 lines of code. That’s a ratio of 235/15 = ~16. the commit message is not 235 character long, the commit message is this > splice-script: Test for msat chan balances > splice-script: Round channel balances down author mistakes commit message for PR which doesn't get persisted into code's history and don't even contain the ID of bug being fixed. So yeah it is better but nothing to actually brag about | |||||||||||||||||||||||||||||||||||||||||
| ▲ | dmitrygr 3 hours ago | parent | prev | next [-] | ||||||||||||||||||||||||||||||||||||||||
Amen. | |||||||||||||||||||||||||||||||||||||||||
| ▲ | systemsweird 3 hours ago | parent | prev [-] | ||||||||||||||||||||||||||||||||||||||||
Very good writeup and an astonishing level of negligence for an extremely security critical piece of code. The losses are going to surpass 100 million, these guys should be thrown in jail. | |||||||||||||||||||||||||||||||||||||||||
| |||||||||||||||||||||||||||||||||||||||||