Hi Denys, On Sat, Apr 30, 2022 at 3:12 PM Denys Vlasenko <vda.li...@googlemail.com> 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. 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. > > @@ -190,6 +192,8 @@ int seedrng_main(int argc UNUSED_PARAM, char *argv[]) > > if (mkdir(seed_dir, 0700) < 0 && errno != EEXIST) > > bb_perror_msg_and_die("can't %s seed directory", "create"); > > dfd = open(seed_dir, O_DIRECTORY | O_RDONLY); > > + /* The flock() here is absolutely necessary, as the consistency of > > this > > + * program breaks down with concurrent uses. */ > > if (dfd < 0 || flock(dfd, LOCK_EX) < 0) > > bb_perror_msg_and_die("can't %s seed directory", "lock"); > > The locking is notoriously not reliable across networked filesystems, > and people often find more reliable ways to ensure safety wrt concurrency. > > E.g. renaming the file before use (rename is atomic even on NFS). > > Or, for example, what if we open /var/lib/seedrng/seed.credit, > then try to unlink it. if unlink fails with ENOENT, this means we have > a concurrent user. Thus, we bail out with an error message. > Would this work? No, because a concurrent user might have replaced seed.credit at just the wrong moment: readfile() readfile() unlink() = success createnewseed() unlink() = success > > > printf("Saving %u bits of %screditable seed for next boot\n", > > (unsigned)new_seed_len * 8, new_seed_creditable ? "" : "non-"); > > fd = open(NON_CREDITABLE_SEED_NAME, O_WRONLY | O_CREAT | O_TRUNC, > > 0400); > > + /* The fsync() here is necessary to ensure the data is written to > > disk before > > + * we attempt to make it creditable. */ > > if (fd < 0 || full_write(fd, new_seed, new_seed_len) != > > (ssize_t)new_seed_len || fsync(fd) < 0) { > > bb_perror_msg("can't%s seed", " write"); > > return program_ret | (1 << 4); > > Are you worrying that /var/lib/seedrng/seed.no-credit can be renamed to > /var/lib/seedrng/seed.credit (and this metadata change can hit the disk) > before its _contents_ is flushed to disk? Yes. Some file systems will do this. Jason _______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox