public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Tom Rini <trini@konsulko.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 4/4] lib: uuid: Improve randomness of uuid values on RANDOM_UUID=y
Date: Wed, 1 May 2019 15:51:49 -0400	[thread overview]
Message-ID: <20190501195149.GE31207@bill-the-cat> (raw)
In-Reply-To: <20190501190831.GA23797@x230>

On Wed, May 01, 2019 at 09:08:31PM +0200, Eugeniu Rosca wrote:
> Hi Heinrich,
> 
> Thank you for reviewing this series.
> 
> On Tue, Apr 30, 2019 at 09:07:09PM +0200, Heinrich Schuchardt wrote:
> [..]
> > This patch may ameliorate the situation for GUIDs a bit. But I dislike:
> 
> In general, we can find reasons to dislike anything, since there is room
> for improvement in virtually anything.
> 
> > 
> > - This patch is a uuid only solution to introduce time ticks as source
> >   of entropy.
> 
> I would like to clarify once again what this patch is about. It _fixes_
> (hence I will rewrite the summary line in the next patch revision) a
> concrete real-life problem of providing repeatable (not predictable -
> which implies some effort in my mind - but literally repeatable) uuid
> values on enabling CONFIG_RANDOM_UUID.
> 
> It is my understanding that CONFIG_RANDOM_UUID (based on its name
> and help message) does promise random uuids to the user. If so, then
> U-Boot simply breaks this promise.
> 
> While doing additional research on PRNG, it looks to me that there is
> an established class of PRNG-specific problems, commonly known as
> "unseeded randomness" for which I am also able to find below CVE/CWE:
>  - https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2015-0285
>    ("CVE-2015-0285 openssl: handshake with unseeded PRNG")
>  - https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2015-9019
>    ("CVE-2015-9019 libxslt: math.random() in xslt uses unseeded randomness")
>  - https://cwe.mitre.org/data/definitions/336.html
>    ("CWE-336: Same Seed in Pseudo-Random Number Generator (PRNG)")
> 
> The above tells me that using the same seed yields the same sequence
> of random numbers. That's precisely the topic of this patch: simply
> switching from unseeded PRNG to seeded PRNG.
> 
> And yes, this patch is deliberately limited to UUID naming function,
> since it is lib/uuid's responsibility to seed the PRNG. The same is
> true for other callers of rand() and rand_r(). All of them seed the
> PRNG prior to getting a random value from it.
> 
> > - With timer ticks you possibly introduce very little entropy.
> 
> Theoretically, yes. Practically, the improvement to the current state
> of affairs is huge and this has been testified by the test results
> linked in the description. 
> 
> Again, this patch is not about improving the random pattern of the UUID
> values (sorry for the misleading title). It is really about _enabling_
> any kind of randomness in those values at all.
> 
> > - Our random number generator with only 32 state bits remains
> >   sub-standard.
> 
> I believe this is orthogonal to my patch and can be improved
> independently. In addition, whatever is the bit-width of U-Boot PRNG
> (I can find shootouts between various 64/128/256-bit PRNG) its essence
> will not change. It will remain a deterministic number generator,
> whose randomness will still be dictated by the seed.
> 
> > 
> > This is the current situation:
> > 
> > net/bootp.c uses the MAC address to seed the random number generator and
> > uses random numbers for defining waits.
> > 
> > lib/uuid.c is using it for UUID generation.
> 
> I can see that rfc4122 suggests including MAC address in the UUID, but
> it also warns that MAC address could be unavailable:
> 
>  -----------
>    For systems with no IEEE address, a randomly or pseudo-randomly
>    generated value may be used;
>  -----------
> 
> The guess is right. Some SoCs like R-Car3 (in contrast to e.g. i.MX6)
> don't provide any OTP memory/register containing their unique MAC
> address. Needless to say some machines would never connect to IEEE
> network. So, it looks to me that MAC address cannot be taken as
> consistent source of entropy for UUID in U-Boot.

Indeed, we have a lot of platforms where the MAC is not a great source.

> > In the UEFI sub-system I would like to implement the EFI_RNG_PROTOCOL.
> > Linux uses it for randomizing memory layout. iPXE needs it for secure
> > network connections. This requires a good random number generator with
> > sufficient entropy.
> > 
> > We already have implemented a single hardware random number generator in
> > drivers/crypto/ace_sha.c (CONFIG_EXYNOS_ACE_SHA).
> > 
> > Many other CPUs come with a hardware random number generator. In Linux's
> > drivers/char/hw_random/ I found, e.g.
> > 
> > - meson-rng.c (Amlogic)
> > - mtk-rng.c (MediaTek)
> > - st-rng.c (STMicroelectronics)
> > - imx-rng.c (Freescale)
> 
> To the best of my knowledge, there is no HW RNG on R-Car3, so our only
> option is currently U-Boot PRNG.

And we have a lot of systems without HW RNG.

> > I think we should have a u-class for hardware RNGs as one source of entropy.
> > 
> > I would like a random number generator with a high number of state bits
> > (> 127) that we initialize with hardware RNG bits and other sources of
> > entropy. A nice discussion of how Linux does it can be found in [1].
> 
> All sound like future topics to me, orthogonal to this patch.
> Let me know if I misunderstood anything. TIA!

Agreed, this patch sounds like it addresses a number of problems today
that are real problems (I await someone filing a CVE now for our PRNG
problem) and can be iteratively improved on, once merged rather than
having a fundamental problem that needs to be addressed.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190501/af9985f4/attachment.sig>

  reply	other threads:[~2019-05-01 19:51 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-30  2:53 [U-Boot] [PATCH 0/4] Misc EFI/GPT/UUID fixes Eugeniu Rosca
2019-04-30  2:53 ` [U-Boot] [PATCH 1/4] disk: efi: Fix memory leak on 'gpt guid' Eugeniu Rosca
2019-04-30 17:56   ` Heinrich Schuchardt
2019-04-30  2:53 ` [U-Boot] [PATCH 2/4] disk: efi: Fix memory leak on 'gpt verify' Eugeniu Rosca
2019-04-30 18:01   ` Heinrich Schuchardt
2019-04-30  2:53 ` [U-Boot] [PATCH 3/4] cmd: gpt: fix and tidy up help message Eugeniu Rosca
2019-04-30 18:10   ` Heinrich Schuchardt
2019-04-30  2:53 ` [U-Boot] [PATCH 4/4] lib: uuid: Improve randomness of uuid values on RANDOM_UUID=y Eugeniu Rosca
2019-04-30 19:07   ` Heinrich Schuchardt
2019-05-01 19:08     ` Eugeniu Rosca
2019-05-01 19:51       ` Tom Rini [this message]
2019-05-01 22:32         ` Eugeniu Rosca
2019-05-03 16:09           ` Eugeniu Rosca
2019-05-07 12:25             ` Eugeniu Rosca
2019-05-16 15:13     ` Matthias Brugger
2019-05-16 15:54       ` Eugeniu Rosca

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190501195149.GE31207@bill-the-cat \
    --to=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox