Hey Etienne, On Tue, Mar 29, 2022 at 7:21 AM Jason A. Donenfeld <ja...@zx2c4.com> wrote: > > Hi Etienne, > > On Tue, Mar 29, 2022 at 1:06 AM Etienne Champetier > <champetier.etie...@gmail.com> wrote: > > > Oh that's an interesting set of considerations and it's possible I > > > didn't understand some aspect of this. Most OSes should call seedrng > > > once at boot and once at shutdown. > > > > As routers are always on devices, it's rare to have clean shutdown. > > Personally, my routers boot after an upgrade or after a power loss, > > so they almost never shutdown properly. > > That's a good point indeed. > > > > 1) read seed into memory, delete seed from disk, write into rng & > > > credit if good seed, write new seed to disk; repeat at shutdown/some > > > other time > > > 2) read seed into memory, write into rng w/o crediting, re-use the > > > same seed next boot > > > > Before this patch we had 2 and users could opt-in to renew seed on > > each boot, so closer to 1. > > I guess the issue is that the implementation of (1) was somewhat > non-optimal, but not exactly catastrophic either. > > > Looking at random.c, I would love add_device_randomness() behavior. > > Maybe it was already answered on LKML, > > but why can't writes to /dev/urandom from a process with CAP_SYS_ADMIN > > be mixed in right away a la add_device_randomness() without being credited ? > > This would not init the RNG faster, but this would make early > > /dev/urandom reads "safer". > > add_device_randomness() does not mix in immediately. It goes into the > entropy pool, but that doesn't get extracted into a new key until the > next reseeding. It does get mixed in directly for crng_init=0, but not > for crng_init=1 or crng_init=2, which is a big gap. Making > /dev/urandom writes behave like that for crng_init=0 doesn't address > the crng_init=1 and crng_init=2 cases, unfortunately. The bigger > problem, though, is that some users of /dev/urandom credit the entropy > via the RNDADDTOENTCNT ioctl _afterwards_. If we mixed it directly in, > then programs with the pattern of write 4 bytes, credit 32 bits, > writes 4 bytes, credit 32 bits, etc could have those 4 written bytes > brute forced each time in what's called a "premature next". For that > reason the key is only modified when 256 bits have accumulated first. > > > I'm fine with writing on each boot, but as we can't rely on shutdown, > > what we could do with the seeds: > > 1) load seed.no-credit, leave it on disk > > 2) mv seed.credit seed.no-credit && load seed.no-credit (and credit it) > > 3) read from getrandom a new seed.credit > > > > This would allow to always keep a seed on disk, only use seed.credit once, > > and actually write seed.credit. > > I would get rid of the whole hashing part as all our seeds would come > > from getrandom(). > > If possible, it's better to not leave a seed on disk after using it, > even if not credited. If that's the only entropy, it's better to > "forget" it after use, so that you can't compromise past secrets. At > the very least, if you have poor entropy, you can replace the seed > with HASH(seed), so at least it ratchets forward. Another thing to > consider is that if you _do_ credit it, that'll initialize the RNG, so > getrandom() automatically works without blocking. These two > observations have lead to seedrng's current scheme, where the sequence > is: > > - load > - delete > - seed & credit, or seed & don't credit, depending > - save new seed, which may be creditable or not, depending on whether > previous things made the rng init > > It sounds like maybe a modification of your suggestion might be to make this: > > - load > - delete > - seed & credit, or seed & don't credit, depending > - save new seed using getrandom(0), so that it's always creditable > > Would that satisfy your concerns? Or are you also trying to preserve a > mode where the filesystem doesn't need to be written to on each boot? > > > > > /var is a symlink to /tmp > > Oh, then in these cleanups, we should change that /tmp/run to /var/run > just to be more "correct". > > > > > > Is there a different place for it that would be good? > > > > Maybe we can leave it in etc and just make sure to exclude it from backups > > That seems like a good course of action. > > If you have a firm idea of what you want this to look like, would you > like to send a series and I'll take a look?
I never heard back from you, but all the concerns you raised strike me as kind of important. Did you intend to move forward with those? Or should I just send a revert for this whole thing, so that you can address it some other time? Jason _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
