On Sat, Apr 30, 2022 at 3:48 PM Jason A. Donenfeld <[email protected]> wrote: > On Sat, Apr 30, 2022 at 3:12 PM Denys Vlasenko <[email protected]> > wrote: > > > + /* The fsync() here is necessary for safety here, so that power > > > being pulled > > > + * at the wrong moment doesn't result in the seed being used > > > twice by accident. */ > > > if ((unlink(filename) < 0 || fsync(dfd) < 0) && seed_len) { > > > bb_perror_msg("can't%s seed", " remove"); > > > return -1; > > > > I don't understand the scenario mentioned here. > > Let's say we removed fsync() above. > > We read /var/lib/seedrng/seed.credit contents. > > We unlink() it, but this change does not go to disk yet. > > We seed the RNG. > > We recreate /var/lib/seedrng/seed.credit with new contents, > > but this change does not go to disk yet too. > > We exit. > > Only now, after we are done, RNG can be used for e.g. key generation. > > > > And only if the power gets pulled AFTER this key generation, > > and the write cached data is still not on the disk > > (for example, if you use ext4 or xfs, this won't happen since > > they synchronously wait for both O_TRUNC truncations > > and renames), the previous /var/lib/seedrng/seed.credit might > > still exist and might get reused on the next boot, > > and a new key can be generated from the same seed. > > > > Do you often pull power cords from machines you use for > > somewhat important crypto operations, such as generating keys? > > What are the chances that you also do it on a machine without RTC > > clock (which would otherwise supply randomness > > from the current time)? > > > > IOW: To me, this does not seem to be a realistic threat. > > It's a theoretical one: you can concoct a scenario where it might happen, > > but triggering it for real, even on purpose, would be difficult. > > Again, stop charging steadfastly toward creating vulnerabilities > because you don't understand things.
I don't understand things because there is no detailed explanation of said things. This is why I ask for a good comment to be here. "The fsync() here is necessary for safety here" is not a detailed explanation. It merely says that fsync() is needed (well duh... _of course_ whoever added it thinks it's needed, we do not assume people add fsync()s just for fun... IOW: this comment is not useful...) > The scenario is: > > - RNG is seeded and credited using file A. > - File A is unlinked but not fsync()d. > - TLS connection does something and a nonce is generated. > - System loses power and reboots. > - RNG is seeded and credited using same file A. > - TLS connection does something and the same nonce is generated, > resulting in catastrophic cryptographic failure. > > This is a big deal. Crediting seeds is not to be taken lightly. > Crediting file-based seeds is _only_ safe when they're never used > twice. Thank you for the explanation. I re-adding the fsync and adding a comment. Please take a look at current git. _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
