[PATCH v2 1/2] lib: uuid: use RNG device if present

[Simon Glass]

Hi Matthias,

On Wed, 20 Jan 2021 at 04:17, Matthias Brugger <mbrugger at suse.com> wrote:
>
>
>
> On 29/12/2020 04:32, Simon Glass wrote:
> > Hi guys,
> >
> > On Wed, 23 Dec 2020 at 03:24, Torsten Duwe <duwe at lst.de> wrote:
> >>
> >> On Sun, 20 Dec 2020 11:17:50 -0700
> >> Simon Glass <sjg at chromium.org> wrote:
> >>
> >>> Hi Torsten,
> >>>
> >>> On Sun, 20 Dec 2020 at 10:00, Torsten Duwe <duwe at lst.de> wrote:
> >>>>
> >>>> On Fri, 18 Dec 2020 19:29:12 -0700
> >>>> Simon Glass <sjg at chromium.org> wrote:
> >>>>
> >>>>>> -       int i;
> >>>>>> -
> >>>>>> -       srand(get_ticks() + rand());
> >>>>>> +       int i, ret;
> >>>>>> +       struct udevice *devp;
> >>>>>> +       u8 randv = 0;
> >>>>>> +
> >>>>>> +#if defined(CONFIG_DM_RNG)
> >>>>>
> >>>>> This seems a little backwards to me. The caller should request a
> >>>>> RNG device, getting either a hardware one or a software one, and
> >>>>> then call the uclass method to get the uuid.
> >>>>
> >>>> Strictly speaking, there's no such thing as a "software RNG". The
> >>>> term "DRBG" was coined for accurateness, "deterministic random bit
> >>>> generator". The oxymoron "deterministic random" pretty much nails
> >>>> it. Alternatively, it can be called "pseudo" RNG.
> >>>> rand() and srand() exactly implement such a mechanism already, with
> >>>> low coding overhead. U-Boot runs fine with them most of the time,
> >>>> but there are rare cases where real entropy would be needed. This
> >>>> is what these two patches are about. In case there's more, I
> >>>> already speculated about a centralised entity in my response to the
> >>>> v1 cover letter, but for now these two changes should do.
> >>>
> >>> I am used to the term pseudo-random, but it doesn't much matter what
> >>> kind of random number is used. It is still covered by the RNG class.
> >>
> >> Well, in these 2 cases, it _does_ matter. And besides, as I wrote
> >> above, pseudo randomness is produced by the rand() function, and RNG
> >> devices provide _real_ entropy to a system.
> >>
> >> So, while every other entity in U-Boot is happy with a DRBG, a UUID and
> >> a BOOTP delay need this real entropy, hence the different code, for a
> >> start.
> >>
> >>> You are currently burying device access in a utility function. That
> >>> really isn't the right way to do it. See my comment above. There is no
> >>> way to control which RNG device is used and no visibility that this is
> >>> happening at all, outside this function.
> >>
> >> The code looks a bit odd to me, too, as I mentioned. I imagined
> >> something not so full blown as in the Linux kernel, but still some
> >> central mechanism to get entropy from, for those who really need it (in
> >> the current situation, just these 2 cases). This alternative would
> >> result in a real /dev/random in U-Boot, which would yield a cleaner
> >> structure, but would require more code to be produced and more code
> >> needing change. That given, I'd agree to these 2 hacks, especially
> >> because there are security implications.
> >>
> >> What's your opinion, how would you like to create really unique UUIDs?
> >> How should BOOTP clients wait randomly (esp. in a large group)?
> >> Shall we create some U-Boot version of /dev/random and haveged?
> >>
> >> I'm really open to suggestions.
> >>
> >
> > I really don't mind too much about that side of it. But I do worry
> > when I see code that buries hard-coded DM access in a lib/ function.
> >
>
> v3 of this series got merged, which should not stop us from making the code better.
>
>
> > If boot wants a random device it should get one, at the top level, not
> > in the bowels of U-Boot. It can pass that device down to other
> > functions as needed.
> >
>
> So you mean we should get the RNG udevice at the caller of gen_rand_uuid_str and
> then pass it down to gen_rand_uuid? And if no device has been found, we will
> call srand(get_ticks() + rand())?

Yes. I suppose you could call it with NULL if there is no device, but
I think it would be much better to have a software RNG device that can
be enabled on platforms that don't have hardware support. Then your
get_ticks() + rand() can be in that.

Also the command stuff in uuid.c should move to cmd/

>
> I'm not quite sure how we would need to implement this in the case of
> srand_mac() though.

You can put it in struct eth_uclass_priv and provide a function in
eth-uclass.c to return it? Then call that from bootp.c

At least then the caller is in control.

>
> Regards,
> Matthias
>
> > To my reading, the current definition of the RNG uclass looks similar
> > to a /dev/random
> >
> > Let me know if I am missing something.
> >
>