On Wed, Jan 20, 2021 at 12:50:56PM -0700, Simon Glass wrote: > Hi Matthias, > > On Wed, 20 Jan 2021 at 04:17, Matthias Brugger <mbrug...@suse.com> wrote: > > > > > > > > On 29/12/2020 04:32, Simon Glass wrote: > > > Hi guys, > > > > > > On Wed, 23 Dec 2020 at 03:24, Torsten Duwe <d...@lst.de> wrote: > > >> > > >> On Sun, 20 Dec 2020 11:17:50 -0700 > > >> Simon Glass <s...@chromium.org> wrote: > > >> > > >>> Hi Torsten, > > >>> > > >>> On Sun, 20 Dec 2020 at 10:00, Torsten Duwe <d...@lst.de> wrote: > > >>>> > > >>>> On Fri, 18 Dec 2020 19:29:12 -0700 > > >>>> Simon Glass <s...@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.
Well the thing is that up to now the SW RNG implementation uses different ways in uuid and bootp to create the seeds. > > Also the command stuff in uuid.c should move to cmd/ > Yes, realized that as well :) > > > > 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. > I'll have a look on that. Regards, Matthias > > > > 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. > > > > >
