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

[Matthias Brugger]

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())?

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

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.
>