On 4/30/19 4:53 AM, Eugeniu Rosca wrote:
The random uuid values (enabled via CONFIG_RANDOM_UUID=y) on our
platform are always the same. Below is consistent on each cold boot:

  => ### interrupt autoboot
  => env default -a; gpt write mmc 1 $partitions; print uuid_gpt_misc
  ...
  uuid_gpt_misc=d117f98e-6f2c-d04b-a5b2-331a19f91cb2
  => env default -a; gpt write mmc 1 $partitions; print uuid_gpt_misc
  ...
  uuid_gpt_misc=ad5ec4b6-2d9f-8544-9417-fe3bd1c9b1b3
  => env default -a; gpt write mmc 1 $partitions; print uuid_gpt_misc
  ...
  uuid_gpt_misc=cceb0b18-39cb-d547-9db7-03b405fa77d4
  => env default -a; gpt write mmc 1 $partitions; print uuid_gpt_misc
  ...
  uuid_gpt_misc=d4981a2b-0478-544e-9607-7fd3c651068d
  => env default -a; gpt write mmc 1 $partitions; print uuid_gpt_misc
  ...
  uuid_gpt_misc=6d6c9a36-e919-264d-a9ee-bd00379686c7

While the uuids do change on every 'gpt write' command, the values
appear to be taken from the same pool, in the same order.

As a user, I expect a trully random uuid value in the above example.
Otherwise, system/RFS designers and OS people might assume they have
a reliable/consistent uuid passed by the bootloader, while the truth
is U-Boot simply lacks entropy to generate a random string.

In its first attempt [1] to improve the uuid randomness, this patch
updated the seed based on the output of get_timer(), similar to [2].

There are two problems with this approach:
  - get_timer() has a poor _ms_ resolution
  - when gen_rand_uuid() is called in a loop, get_timer() returns the
    same result, leading to the same seed being passed to srand(),
    leading to the same uuid being generated for several partitions
    with different names

This second patch addresses both drawbacks.

My R-Car3 testing [3] consists of running 'gpt write mmc 1 $partitions'
in a loop for several minutes collecting 8844 randomly generated UUIDS.
Two consecutive cold boots are concatenated in the log. As a result,
all uuid values are unique (scripted check).

Thanks to Roman, who reported the issue and provided support in fixing.

[1] https://patchwork.ozlabs.org/patch/1091802/
[2] commit da384a9d7628 ("net: rename and refactor eth_rand_ethaddr() function")
[3] https://gist.github.com/erosca/2820be9d554f76b982edd48474d0e7ca
  => while true; do \
     env default -a; \
     gpt write mmc 1 $partitions; \
     print; done

Reported-by: Roman Stratiienko <roman.stratiie...@globallogic.com>
Signed-off-by: Eugeniu Rosca <ero...@de.adit-jv.com>

This patch may ameliorate the situation for GUIDs a bit. But I dislike:

- This patch is a uuid only solution to introduce time ticks as source
  of entropy.
- With timer ticks you possibly introduce very little entropy.
- Our random number generator with only 32 state bits remains
  sub-standard.

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.

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)

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

Best regards

Heinrich

[1]
https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/Studies/LinuxRNG/LinuxRNG_EN.pdf


---
v2:
  - Replaced get_timer(0) with get_ticks() and added rand() to seed value
  - Performed extensive testing on R-Car3 (ARMv8)
v1:
  - https://patchwork.ozlabs.org/patch/1091802/
---
  lib/uuid.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/lib/uuid.c b/lib/uuid.c
index fa20ee39fc32..2d4d6ef7e461 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -238,6 +238,8 @@ void gen_rand_uuid(unsigned char *uuid_bin)
        unsigned int *ptr = (unsigned int *)&uuid;
        int i;

+       srand(get_ticks() + rand());
+
        /* Set all fields randomly */
        for (i = 0; i < sizeof(struct uuid) / sizeof(*ptr); i++)
                *(ptr + i) = cpu_to_be32(rand());


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to