| Message ID | 20220328042556.1986939-1-Jason@zx2c4.com |
|---|---|
| State | Accepted |
| Delegated to: | Petr Štetiar |
| Headers | show |
| Series | urandom-seed: use seedrng for seeding the random number generator | expand |
Hi All, Jason, @Petr Štetiar this merge was a bit too fast to get reviews ... Some comments inline Le lun. 28 mars 2022 à 00:26, Jason A. Donenfeld <Jason@zx2c4.com> a écrit : > > The RNG can't actually be seeded from a shell script, due to the > reliance on ioctls. For this reason, the seedrng project provides a > basic script meant to be copy and pasted into projects like OpenWRT > and tweaked as needed: <https://git.zx2c4.com/seedrng/about/>. > > This commit imports it into the urandom-seed package and wires up the > init scripts to call it. This also is a significant improvement over the > current init script, which does not robustly handle cleaning up of seeds > and syncing to prevent reuse. When urandom-seed was introduced in 2016 it was decided during review that writing on each boot might cause too much wear to the flash. Maybe we can say that 6 years later this is not a problem anymore, but would love to have more devs comment Old thread: https://www.mail-archive.com/lede-dev@lists.infradead.org/msg01225.html Now if I understand correctly, with this patch we are writing a seed to flash twice per boot, in preinit/81_urandom_seed and in init.d/urandom_seed. Also there are good chances we will never have a seed.credit at all on many devices, would be great if seedrng had an option "writeseed" that blocks on getrandom(). > Additionally, the existing script creates > a new seed immediately after writing an old one, which means that the > amount of entropy might actually regress, due to failing to credit the > old seed. We used getrandom to create the seeds, so when configured to write new seed on each boot, either we reboot too fast and we will reuse the seed (not ideal but not catastrophic as we don't credit), or the new seed is created after getrandom() unblock, so not sure to understand how entropy regress. > Closes: https://github.com/openwrt/openwrt/issues/9570 > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> [snip] > diff --git a/package/system/urandom-seed/seedrng.c b/package/system/urandom-seed/seedrng.c > new file mode 100644 > index 0000000000..9a2cb10f55 > --- /dev/null > +++ b/package/system/urandom-seed/seedrng.c [snip] > + > +#define SEED_DIR "/etc/seedrng" If we worry about seed reuse, we should not use /etc as it can be restored from a backup Best Etienne
Hey Etienne, On Mon, Mar 28, 2022 at 10:19 AM Etienne Champetier <champetier.etienne@gmail.com> wrote: > > Hi All, Jason, > > @Petr Štetiar this merge was a bit too fast to get reviews ... > Some comments inline We can apply fixups on top, no big deal. > When urandom-seed was introduced in 2016 it was decided during review > that writing on each boot might cause too much wear to the flash. > Maybe we can say that 6 years later this is not a problem anymore, but > would love to have more devs comment > Old thread: https://www.mail-archive.com/lede-dev@lists.infradead.org/msg01225.html > > Now if I understand correctly, with this patch we are writing a seed > to flash twice per boot, in preinit/81_urandom_seed and in > init.d/urandom_seed. > Also there are good chances we will never have a seed.credit at all on > many devices, > would be great if seedrng had an option "writeseed" that blocks on getrandom(). 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. It's also fine to call seedrng at any other specific time during runtime too. Because it's involved with crediting, it always always removes the seed file after reading but before using, and after it's used, it immediately writes a new seed file. It sounds like what you might want here is, perhaps, the original behavior? Namely, the seed is never credited, but it never changes either? That won't help you initialize the RNG, but since you're not crediting it, you can argue that all new rng inputs are good inputs, even if they've been used before. So these are the two schemes to choose from: 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 If the second scheme is what you prefer, then your original bug report suggesting this was an issue for OpenWRT might not really be so, and we can just go back to what we were doing before. OTOH, if you want to have a good mechanism that actually initializes the RNG, perhaps we can move forward with some tweaks to seedrng. > > > + > > +#define SEED_DIR "/etc/seedrng" > > If we worry about seed reuse, we should not use /etc as it can be > restored from a backup Indeed you're right. Most other distros use /var/lib/seedrng; is /var/lib available on OpenWRT? Is there a different place for it that would be good? Jason
Hey Jason, Le lun. 28 mars 2022 à 13:04, Jason A. Donenfeld <Jason@zx2c4.com> a écrit : > > Hey Etienne, > > On Mon, Mar 28, 2022 at 10:19 AM Etienne Champetier > <champetier.etienne@gmail.com> wrote: > > > > Hi All, Jason, > > > > @Petr Štetiar this merge was a bit too fast to get reviews ... > > Some comments inline > > We can apply fixups on top, no big deal. > > > When urandom-seed was introduced in 2016 it was decided during review > > that writing on each boot might cause too much wear to the flash. > > Maybe we can say that 6 years later this is not a problem anymore, but > > would love to have more devs comment > > Old thread: https://www.mail-archive.com/lede-dev@lists.infradead.org/msg01225.html > > > > Now if I understand correctly, with this patch we are writing a seed > > to flash twice per boot, in preinit/81_urandom_seed and in > > init.d/urandom_seed. > > Also there are good chances we will never have a seed.credit at all on > > many devices, > > would be great if seedrng had an option "writeseed" that blocks on getrandom(). > > 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. > It's also fine to call seedrng at > any other specific time during runtime too. Because it's involved with > crediting, it always always removes the seed file after reading but > before using, and after it's used, it immediately writes a new seed > file. > > It sounds like what you might want here is, perhaps, the original > behavior? Namely, the seed is never credited, but it never changes > either? That won't help you initialize the RNG, but since you're not > crediting it, you can argue that all new rng inputs are good inputs, > even if they've been used before. > > So these are the two schemes to choose from: > > 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. > If the second scheme is what you prefer, then your original bug report > suggesting this was an issue for OpenWRT might not really be so, and > we can just go back to what we were doing before. OTOH, if you want to > have a good mechanism that actually initializes the RNG, perhaps we > can move forward with some tweaks to seedrng. 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". 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(). > > > + > > > +#define SEED_DIR "/etc/seedrng" > > > > If we worry about seed reuse, we should not use /etc as it can be > > restored from a backup > > Indeed you're right. Most other distros use /var/lib/seedrng; is > /var/lib available on OpenWRT? /var is a symlink to /tmp > 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 Etienne > > Jason
Hi Etienne, On Tue, Mar 29, 2022 at 1:06 AM Etienne Champetier <champetier.etienne@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? Jason
Hey Etienne, On Tue, Mar 29, 2022 at 7:21 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > Hi Etienne, > > On Tue, Mar 29, 2022 at 1:06 AM Etienne Champetier > <champetier.etienne@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
Hi Jason, Le lun. 4 avr. 2022 à 14:39, Jason A. Donenfeld <Jason@zx2c4.com> a écrit : > > Hey Etienne, > > On Tue, Mar 29, 2022 at 7:21 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > Hi Etienne, > > > > On Tue, Mar 29, 2022 at 1:06 AM Etienne Champetier > > <champetier.etienne@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? I meant to work on it last week but had 0 time, hopefully this week will be less busy :( Master is not less secure than before, so let's wait a bit to revert Best Etienne > > Jason
On 04/04/2022 21:35, Etienne Champetier wrote: > Hi Jason, > > Le lun. 4 avr. 2022 à 14:39, Jason A. Donenfeld <Jason@zx2c4.com> a écrit : >> >> Hey Etienne, >> >> On Tue, Mar 29, 2022 at 7:21 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote: >>> >>> Hi Etienne, >>> >>> On Tue, Mar 29, 2022 at 1:06 AM Etienne Champetier >>> <champetier.etienne@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? > > I meant to work on it last week but had 0 time, hopefully this week > will be less busy :( > Master is not less secure than before, so let's wait a bit to revert Any progress on this? PS: Since the last busybox bump we even have BUSYBOX_CONFIG_SEEDRNG now. Cheers, Andre > > Best > Etienne > >> >> Jason > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Le mer. 1 févr. 2023 à 10:29, Andre Heider <a.heider@gmail.com> a écrit : > > On 04/04/2022 21:35, Etienne Champetier wrote: > > Hi Jason, > > > > Le lun. 4 avr. 2022 à 14:39, Jason A. Donenfeld <Jason@zx2c4.com> a écrit : > >> > >> Hey Etienne, > >> > >> On Tue, Mar 29, 2022 at 7:21 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > >>> > >>> Hi Etienne, > >>> > >>> On Tue, Mar 29, 2022 at 1:06 AM Etienne Champetier > >>> <champetier.etienne@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? > > > > I meant to work on it last week but had 0 time, hopefully this week > > will be less busy :( > > Master is not less secure than before, so let's wait a bit to revert > > Any progress on this? > > PS: Since the last busybox bump we even have BUSYBOX_CONFIG_SEEDRNG now. None on my side :( My understanding is that most random.c improvements from Jason have been backported to stable kernels making userspace seeding less important, but I might have misunderstood (master branch is on 5.15) > > Cheers, > Andre > > > > > Best > > Etienne > > > >> > >> Jason > > > > _______________________________________________ > > openwrt-devel mailing list > > openwrt-devel@lists.openwrt.org > > https://lists.openwrt.org/mailman/listinfo/openwrt-devel >
diff --git a/package/system/urandom-seed/Makefile b/package/system/urandom-seed/Makefile index 7c5524a9db..f890c0b10a 100644 --- a/package/system/urandom-seed/Makefile +++ b/package/system/urandom-seed/Makefile @@ -9,7 +9,6 @@ include $(INCLUDE_DIR)/package.mk define Package/urandom-seed SECTION:=base CATEGORY:=Base system - DEPENDS:=+getrandom TITLE:=/etc/urandom.seed handling for OpenWrt URL:=https://openwrt.org/ endef @@ -19,11 +18,14 @@ define Build/Prepare endef define Build/Compile/Default + $(TARGET_CC) $(TARGET_CFLAGS) $(TARGET_CPPFLAGS) $(TARGET_LDFLAGS) \ + -std=gnu99 -o $(PKG_BUILD_DIR)/seedrng seedrng.c endef Build/Compile = $(Build/Compile/Default) define Package/urandom-seed/install $(CP) ./files/* $(1)/ + $(CP) $(PKG_BUILD_DIR)/seedrng $(1)/sbin/ endef $(eval $(call BuildPackage,urandom-seed)) diff --git a/package/system/urandom-seed/files/etc/init.d/urandom_seed b/package/system/urandom-seed/files/etc/init.d/urandom_seed index 17d9c13400..d6e81c6079 100755 --- a/package/system/urandom-seed/files/etc/init.d/urandom_seed +++ b/package/system/urandom-seed/files/etc/init.d/urandom_seed @@ -5,7 +5,7 @@ USE_PROCD=1 start_service() { procd_open_instance "urandom_seed" - procd_set_param command "/sbin/urandom_seed" + procd_set_param command "/sbin/seedrng" procd_set_param stdout 1 procd_set_param stderr 1 procd_close_instance diff --git a/package/system/urandom-seed/files/lib/preinit/81_urandom_seed b/package/system/urandom-seed/files/lib/preinit/81_urandom_seed index 2adc6c47f0..b3014daeaf 100644 --- a/package/system/urandom-seed/files/lib/preinit/81_urandom_seed +++ b/package/system/urandom-seed/files/lib/preinit/81_urandom_seed @@ -2,21 +2,11 @@ log_urandom_seed() { echo "urandom-seed: $1" > /dev/kmsg } -_do_urandom_seed() { - [ -f "$1" ] || { log_urandom_seed "Seed file not found ($1)"; return; } - [ -O "$1" -a -G "$1" -a ! -x "$1" ] || { log_urandom_seed "Wrong owner / permissions for $1"; return; } - - log_urandom_seed "Seeding with $1" - cat "$1" > /dev/urandom -} - do_urandom_seed() { [ -c /dev/urandom ] || { log_urandom_seed "Something is wrong with /dev/urandom"; return; } - - _do_urandom_seed "/etc/urandom.seed" - - SEED="$(uci -q get system.@system[0].urandom_seed)" - [ "${SEED:0:1}" = "/" -a "$SEED" != "/etc/urandom.seed" ] && _do_urandom_seed "$SEED" + seedrng 2>&1 | while read -r line; do + log_urandom_seed "$line" + done } boot_hook_add preinit_main do_urandom_seed diff --git a/package/system/urandom-seed/files/sbin/urandom_seed b/package/system/urandom-seed/files/sbin/urandom_seed deleted file mode 100755 index 7043e8af4e..0000000000 --- a/package/system/urandom-seed/files/sbin/urandom_seed +++ /dev/null @@ -1,20 +0,0 @@ -#!/bin/sh -set -e - -trap '[ "$?" -eq 0 ] || echo "An error occured" >&2' EXIT - -save() { - touch "$1.tmp" - chown root:root "$1.tmp" - chmod 600 "$1.tmp" - getrandom 512 > "$1.tmp" - mv "$1.tmp" "$1" - echo "Seed saved ($1)" -} - -SEED="$(uci -q get system.@system[0].urandom_seed || true)" -[ "${SEED:0:1}" = "/" ] && save "$SEED" - -SEED=/etc/urandom.seed -[ ! -f $SEED ] && save "$SEED" -true diff --git a/package/system/urandom-seed/seedrng.c b/package/system/urandom-seed/seedrng.c new file mode 100644 index 0000000000..9a2cb10f55 --- /dev/null +++ b/package/system/urandom-seed/seedrng.c @@ -0,0 +1,434 @@ +// SPDX-License-Identifier: (GPL-2.0 OR MIT OR Apache-2.0) +/* + * Copyright (C) 2022 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved. + */ + +#include <linux/random.h> +#include <sys/random.h> +#include <sys/ioctl.h> +#include <sys/file.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <fcntl.h> +#include <poll.h> +#include <unistd.h> +#include <time.h> +#include <errno.h> +#include <endian.h> +#include <stdbool.h> +#include <stdint.h> +#include <string.h> +#include <stdio.h> +#include <stdlib.h> + +#define SEED_DIR "/etc/seedrng" +#define CREDITABLE_SEED SEED_DIR "/seed.credit" +#define NON_CREDITABLE_SEED SEED_DIR "/seed.no-credit" +#define LOCK_FILE "/tmp/run/seedrng.lock" + +enum blake2s_lengths { + BLAKE2S_BLOCK_LEN = 64, + BLAKE2S_HASH_LEN = 32, + BLAKE2S_KEY_LEN = 32 +}; + +enum seedrng_lengths { + MAX_SEED_LEN = 512, + MIN_SEED_LEN = BLAKE2S_HASH_LEN +}; + +struct blake2s_state { + uint32_t h[8]; + uint32_t t[2]; + uint32_t f[2]; + uint8_t buf[BLAKE2S_BLOCK_LEN]; + unsigned int buflen; + unsigned int outlen; +}; + +#define le32_to_cpup(a) le32toh(*(a)) +#define cpu_to_le32(a) htole32(a) +#ifndef ARRAY_SIZE +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) +#endif +#ifndef DIV_ROUND_UP +#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d)) +#endif + +static inline void cpu_to_le32_array(uint32_t *buf, unsigned int words) +{ + while (words--) { + *buf = cpu_to_le32(*buf); + ++buf; + } +} + +static inline void le32_to_cpu_array(uint32_t *buf, unsigned int words) +{ + while (words--) { + *buf = le32_to_cpup(buf); + ++buf; + } +} + +static inline uint32_t ror32(uint32_t word, unsigned int shift) +{ + return (word >> (shift & 31)) | (word << ((-shift) & 31)); +} + +static const uint32_t blake2s_iv[8] = { + 0x6A09E667UL, 0xBB67AE85UL, 0x3C6EF372UL, 0xA54FF53AUL, + 0x510E527FUL, 0x9B05688CUL, 0x1F83D9ABUL, 0x5BE0CD19UL +}; + +static const uint8_t blake2s_sigma[10][16] = { + { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 }, + { 14, 10, 4, 8, 9, 15, 13, 6, 1, 12, 0, 2, 11, 7, 5, 3 }, + { 11, 8, 12, 0, 5, 2, 15, 13, 10, 14, 3, 6, 7, 1, 9, 4 }, + { 7, 9, 3, 1, 13, 12, 11, 14, 2, 6, 5, 10, 4, 0, 15, 8 }, + { 9, 0, 5, 7, 2, 4, 10, 15, 14, 1, 11, 12, 6, 8, 3, 13 }, + { 2, 12, 6, 10, 0, 11, 8, 3, 4, 13, 7, 5, 15, 14, 1, 9 }, + { 12, 5, 1, 15, 14, 13, 4, 10, 0, 7, 6, 3, 9, 2, 8, 11 }, + { 13, 11, 7, 14, 12, 1, 3, 9, 5, 0, 15, 4, 8, 6, 2, 10 }, + { 6, 15, 14, 9, 11, 3, 0, 8, 12, 2, 13, 7, 1, 4, 10, 5 }, + { 10, 2, 8, 4, 7, 6, 1, 5, 15, 11, 9, 14, 3, 12, 13, 0 }, +}; + +static void blake2s_set_lastblock(struct blake2s_state *state) +{ + state->f[0] = -1; +} + +static void blake2s_increment_counter(struct blake2s_state *state, const uint32_t inc) +{ + state->t[0] += inc; + state->t[1] += (state->t[0] < inc); +} + +static void blake2s_init_param(struct blake2s_state *state, const uint32_t param) +{ + int i; + + memset(state, 0, sizeof(*state)); + for (i = 0; i < 8; ++i) + state->h[i] = blake2s_iv[i]; + state->h[0] ^= param; +} + +static void blake2s_init(struct blake2s_state *state, const size_t outlen) +{ + blake2s_init_param(state, 0x01010000 | outlen); + state->outlen = outlen; +} + +static void blake2s_compress(struct blake2s_state *state, const uint8_t *block, size_t nblocks, const uint32_t inc) +{ + uint32_t m[16]; + uint32_t v[16]; + int i; + + while (nblocks > 0) { + blake2s_increment_counter(state, inc); + memcpy(m, block, BLAKE2S_BLOCK_LEN); + le32_to_cpu_array(m, ARRAY_SIZE(m)); + memcpy(v, state->h, 32); + v[ 8] = blake2s_iv[0]; + v[ 9] = blake2s_iv[1]; + v[10] = blake2s_iv[2]; + v[11] = blake2s_iv[3]; + v[12] = blake2s_iv[4] ^ state->t[0]; + v[13] = blake2s_iv[5] ^ state->t[1]; + v[14] = blake2s_iv[6] ^ state->f[0]; + v[15] = blake2s_iv[7] ^ state->f[1]; + +#define G(r, i, a, b, c, d) do { \ + a += b + m[blake2s_sigma[r][2 * i + 0]]; \ + d = ror32(d ^ a, 16); \ + c += d; \ + b = ror32(b ^ c, 12); \ + a += b + m[blake2s_sigma[r][2 * i + 1]]; \ + d = ror32(d ^ a, 8); \ + c += d; \ + b = ror32(b ^ c, 7); \ +} while (0) + +#define ROUND(r) do { \ + G(r, 0, v[0], v[ 4], v[ 8], v[12]); \ + G(r, 1, v[1], v[ 5], v[ 9], v[13]); \ + G(r, 2, v[2], v[ 6], v[10], v[14]); \ + G(r, 3, v[3], v[ 7], v[11], v[15]); \ + G(r, 4, v[0], v[ 5], v[10], v[15]); \ + G(r, 5, v[1], v[ 6], v[11], v[12]); \ + G(r, 6, v[2], v[ 7], v[ 8], v[13]); \ + G(r, 7, v[3], v[ 4], v[ 9], v[14]); \ +} while (0) + ROUND(0); + ROUND(1); + ROUND(2); + ROUND(3); + ROUND(4); + ROUND(5); + ROUND(6); + ROUND(7); + ROUND(8); + ROUND(9); + +#undef G +#undef ROUND + + for (i = 0; i < 8; ++i) + state->h[i] ^= v[i] ^ v[i + 8]; + + block += BLAKE2S_BLOCK_LEN; + --nblocks; + } +} + +static void blake2s_update(struct blake2s_state *state, const void *inp, size_t inlen) +{ + const size_t fill = BLAKE2S_BLOCK_LEN - state->buflen; + const uint8_t *in = inp; + + if (!inlen) + return; + if (inlen > fill) { + memcpy(state->buf + state->buflen, in, fill); + blake2s_compress(state, state->buf, 1, BLAKE2S_BLOCK_LEN); + state->buflen = 0; + in += fill; + inlen -= fill; + } + if (inlen > BLAKE2S_BLOCK_LEN) { + const size_t nblocks = DIV_ROUND_UP(inlen, BLAKE2S_BLOCK_LEN); + blake2s_compress(state, in, nblocks - 1, BLAKE2S_BLOCK_LEN); + in += BLAKE2S_BLOCK_LEN * (nblocks - 1); + inlen -= BLAKE2S_BLOCK_LEN * (nblocks - 1); + } + memcpy(state->buf + state->buflen, in, inlen); + state->buflen += inlen; +} + +static void blake2s_final(struct blake2s_state *state, uint8_t *out) +{ + blake2s_set_lastblock(state); + memset(state->buf + state->buflen, 0, BLAKE2S_BLOCK_LEN - state->buflen); + blake2s_compress(state, state->buf, 1, state->buflen); + cpu_to_le32_array(state->h, ARRAY_SIZE(state->h)); + memcpy(out, state->h, state->outlen); +} + +static size_t determine_optimal_seed_len(void) +{ + size_t ret = 0; + char poolsize_str[11] = { 0 }; + int fd = open("/proc/sys/kernel/random/poolsize", O_RDONLY); + + if (fd < 0 || read(fd, poolsize_str, sizeof(poolsize_str) - 1) < 0) { + fprintf(stderr, "WARNING: Unable to determine pool size, falling back to %u bits: %s\n", MIN_SEED_LEN * 8, strerror(errno)); + ret = MIN_SEED_LEN; + } else + ret = DIV_ROUND_UP(strtoul(poolsize_str, NULL, 10), 8); + if (fd >= 0) + close(fd); + if (ret < MIN_SEED_LEN) + ret = MIN_SEED_LEN; + else if (ret > MAX_SEED_LEN) + ret = MAX_SEED_LEN; + return ret; +} + +static int read_new_seed(uint8_t *seed, size_t len, bool *is_creditable) +{ + ssize_t ret; + int urandom_fd; + + *is_creditable = false; + ret = getrandom(seed, len, GRND_NONBLOCK); + if (ret == (ssize_t)len) { + *is_creditable = true; + return 0; + } else if (ret < 0 && errno == ENOSYS) { + struct pollfd random_fd = { + .fd = open("/dev/random", O_RDONLY), + .events = POLLIN + }; + if (random_fd.fd < 0) + return -errno; + *is_creditable = poll(&random_fd, 1, 0) == 1; + close(random_fd.fd); + } else if (getrandom(seed, len, GRND_INSECURE) == (ssize_t)len) + return 0; + urandom_fd = open("/dev/urandom", O_RDONLY); + if (urandom_fd < 0) + return -errno; + ret = read(urandom_fd, seed, len); + if (ret == (ssize_t)len) + ret = 0; + else + ret = -errno ? -errno : -EIO; + close(urandom_fd); + return ret; +} + +static int seed_rng(uint8_t *seed, size_t len, bool credit) +{ + struct { + int entropy_count; + int buf_size; + uint8_t buffer[MAX_SEED_LEN]; + } req = { + .entropy_count = credit ? len * 8 : 0, + .buf_size = len + }; + int random_fd, ret; + + if (len > sizeof(req.buffer)) + return -EFBIG; + memcpy(req.buffer, seed, len); + + random_fd = open("/dev/random", O_RDWR); + if (random_fd < 0) + return -errno; + ret = ioctl(random_fd, RNDADDENTROPY, &req); + if (ret) + ret = -errno ? -errno : -EIO; + close(random_fd); + return ret; +} + +static int seed_from_file_if_exists(const char *filename, bool credit, struct blake2s_state *hash) +{ + uint8_t seed[MAX_SEED_LEN]; + ssize_t seed_len; + int fd, dfd, ret = 0; + + fd = open(filename, O_RDONLY); + if (fd < 0 && errno == ENOENT) + return 0; + else if (fd < 0) { + ret = -errno; + fprintf(stderr, "ERROR: Unable to open seed file: %s\n", strerror(errno)); + return ret; + } + dfd = open(SEED_DIR, O_DIRECTORY | O_RDONLY); + if (dfd < 0) { + ret = -errno; + close(fd); + fprintf(stderr, "ERROR: Unable to open seed directory: %s\n", strerror(errno)); + return ret; + } + seed_len = read(fd, seed, sizeof(seed)); + if (seed_len < 0) { + ret = -errno; + fprintf(stderr, "ERROR: Unable to read seed file: %s\n", strerror(errno)); + } + close(fd); + if (ret) { + close(dfd); + return ret; + } + if ((unlink(filename) < 0 || fsync(dfd) < 0) && seed_len) { + ret = -errno; + fprintf(stderr, "ERROR: Unable to remove seed after reading, so not seeding: %s\n", strerror(errno)); + } + close(dfd); + if (ret) + return ret; + if (!seed_len) + return 0; + + blake2s_update(hash, &seed_len, sizeof(seed_len)); + blake2s_update(hash, seed, seed_len); + + fprintf(stdout, "Seeding %zd bits %s crediting\n", seed_len * 8, credit ? "and" : "without"); + ret = seed_rng(seed, seed_len, credit); + if (ret < 0) + fprintf(stderr, "ERROR: Unable to seed: %s\n", strerror(-ret)); + return ret; +} + +static bool skip_credit(void) +{ + const char *skip = getenv("SEEDRNG_SKIP_CREDIT"); + return skip && (!strcmp(skip, "1") || !strcasecmp(skip, "true") || + !strcasecmp(skip, "yes") || !strcasecmp(skip, "y")); +} + +int main(int argc __attribute__((unused)), char *argv[] __attribute__((unused))) +{ + static const char seedrng_prefix[] = "SeedRNG v1 Old+New Prefix"; + static const char seedrng_failure[] = "SeedRNG v1 No New Seed Failure"; + int ret, fd = -1, lock, program_ret = 0; + uint8_t new_seed[MAX_SEED_LEN]; + size_t new_seed_len; + bool new_seed_creditable; + struct timespec realtime = { 0 }, boottime = { 0 }; + struct blake2s_state hash; + + umask(0077); + if (getuid()) { + fprintf(stderr, "ERROR: This program requires root\n"); + return 1; + } + + blake2s_init(&hash, BLAKE2S_HASH_LEN); + blake2s_update(&hash, seedrng_prefix, strlen(seedrng_prefix)); + clock_gettime(CLOCK_REALTIME, &realtime); + clock_gettime(CLOCK_BOOTTIME, &boottime); + blake2s_update(&hash, &realtime, sizeof(realtime)); + blake2s_update(&hash, &boottime, sizeof(boottime)); + + if (mkdir(SEED_DIR, 0700) < 0 && errno != EEXIST) { + fprintf(stderr, "ERROR: Unable to create \"%s\" directory: %s\n", SEED_DIR, strerror(errno)); + return 1; + } + + lock = open(LOCK_FILE, O_WRONLY | O_CREAT, 0000); + if (lock < 0 || flock(lock, LOCK_EX) < 0) { + fprintf(stderr, "ERROR: Unable to open lock file: %s\n", strerror(errno)); + program_ret = 1; + goto out; + } + + ret = seed_from_file_if_exists(NON_CREDITABLE_SEED, false, &hash); + if (ret < 0) + program_ret |= 1 << 1; + ret = seed_from_file_if_exists(CREDITABLE_SEED, !skip_credit(), &hash); + if (ret < 0) + program_ret |= 1 << 2; + + new_seed_len = determine_optimal_seed_len(); + ret = read_new_seed(new_seed, new_seed_len, &new_seed_creditable); + if (ret < 0) { + fprintf(stderr, "ERROR: Unable to read new seed: %s\n", strerror(-ret)); + new_seed_len = BLAKE2S_HASH_LEN; + strncpy((char *)new_seed, seedrng_failure, new_seed_len); + program_ret |= 1 << 3; + } + blake2s_update(&hash, &new_seed_len, sizeof(new_seed_len)); + blake2s_update(&hash, new_seed, new_seed_len); + blake2s_final(&hash, new_seed + new_seed_len - BLAKE2S_HASH_LEN); + + fprintf(stdout, "Saving %zu bits of %s seed for next boot\n", new_seed_len * 8, new_seed_creditable ? "creditable" : "non-creditable"); + fd = open(NON_CREDITABLE_SEED, O_WRONLY | O_CREAT | O_TRUNC, 0400); + if (fd < 0) { + fprintf(stderr, "ERROR: Unable to open seed file for writing: %s\n", strerror(errno)); + program_ret |= 1 << 4; + goto out; + } + if (write(fd, new_seed, new_seed_len) != (ssize_t)new_seed_len || fsync(fd) < 0) { + fprintf(stderr, "ERROR: Unable to write seed file: %s\n", strerror(errno)); + program_ret |= 1 << 5; + goto out; + } + if (new_seed_creditable && rename(NON_CREDITABLE_SEED, CREDITABLE_SEED) < 0) { + fprintf(stderr, "WARNING: Unable to make new seed creditable: %s\n", strerror(errno)); + program_ret |= 1 << 6; + } +out: + if (fd >= 0) + close(fd); + if (lock >= 0) + close(lock); + return program_ret; +}
The RNG can't actually be seeded from a shell script, due to the reliance on ioctls. For this reason, the seedrng project provides a basic script meant to be copy and pasted into projects like OpenWRT and tweaked as needed: <https://git.zx2c4.com/seedrng/about/>. This commit imports it into the urandom-seed package and wires up the init scripts to call it. This also is a significant improvement over the current init script, which does not robustly handle cleaning up of seeds and syncing to prevent reuse. Additionally, the existing script creates a new seed immediately after writing an old one, which means that the amount of entropy might actually regress, due to failing to credit the old seed. Closes: https://github.com/openwrt/openwrt/issues/9570 Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- package/system/urandom-seed/Makefile | 4 +- .../files/etc/init.d/urandom_seed | 2 +- .../files/lib/preinit/81_urandom_seed | 16 +- .../urandom-seed/files/sbin/urandom_seed | 20 - package/system/urandom-seed/seedrng.c | 434 ++++++++++++++++++ 5 files changed, 441 insertions(+), 35 deletions(-) delete mode 100755 package/system/urandom-seed/files/sbin/urandom_seed create mode 100644 package/system/urandom-seed/seedrng.c