[PATCH] urandom-seed: use seedrng for seeding the random number generator

[Etienne Champetier]

Le mer. 1 févr. 2023 à 10:29, Andre Heider <a.heider at 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 at zx2c4.com> a écrit :
> >>
> >> Hey Etienne,
> >>
> >> On Tue, Mar 29, 2022 at 7:21 AM Jason A. Donenfeld <Jason at zx2c4.com> wrote:
> >>>
> >>> Hi Etienne,
> >>>
> >>> On Tue, Mar 29, 2022 at 1:06 AM Etienne Champetier
> >>> <champetier.etienne at 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 at lists.openwrt.org
> > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>