On Thu, 2 May 2019 14:27:06 +0200 Eugeniu Rosca <ero...@de.adit-jv.com> 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. > > 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.stratiie...@globallogic.com> > Signed-off-by: Eugeniu Rosca <ero...@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()); Reviewed-by: Lukasz Majewski <lu...@denx.de> Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lu...@denx.de
pgpozua77Aa9K.pgp
Description: OpenPGP digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
