[U-Boot] [PATCH v2 4/4] lib: uuid: Fix unseeded PRNG on RANDOM_UUID=y

[Eugeniu Rosca]

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.

Assuming U-Boot with RANDOM_UUID=y is deployed on a large number of
devices, all those devices would essentially expose the same UUID,
breaking the assumption of system/RFS/application designers who rely
on UUID as being globally unique (e.g. a database using UUID as key
would alias/mix up entries/records due to duplicated UUID).

The root cause seems to be simply _not_ seeding PRNG before generating
a random value. It turns out this belongs to an established class of
PRNG-specific problems, commonly known as "unseeded randomness", for
which I am able to find below bugs/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 first revision [1] of this patch updated the seed based on the
output of get_timer(), similar to [4].

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

The above drawbacks have been addressed in the second version [2].
In its third revision (current), the patch reworded the description
and summary line to emphasize it is a *fix* rather than an improvement.

Testing [3] consisted of running 'gpt write mmc 1 $partitions' in a
loop on R-Car3 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] https://patchwork.ozlabs.org/patch/1092945/
[3] https://gist.github.com/erosca/2820be9d554f76b982edd48474d0e7ca
[4] commit da384a9d7628 ("net: rename and refactor eth_rand_ethaddr() function")

Reported-by: Roman Stratiienko <roman.stratiienko at globallogic.com>
Signed-off-by: Eugeniu Rosca <erosca at de.adit-jv.com>
--
v3:
 - Reworked the patch summary line and description to emphasize this is
   a fix rather than an improvement by precisely pointing out the root
   cause and mentioning related CVE/CWE.
v2:
 - https://patchwork.ozlabs.org/patch/1092945/
 - Replaced get_timer(0) with get_ticks() and added rand() to seed value
 - Performed extensive testing on R-Car3 (ARMv8). See:
   https://gist.github.com/erosca/2820be9d554f76b982edd48474d0e7ca
v1:
 - https://patchwork.ozlabs.org/patch/1092944/
---
 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());
-- 
2.21.0