[PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR

[Ard Biesheuvel]

On Thu, 13 Jan 2022 at 14:41, Jason A. Donenfeld <Jason at zx2c4.com> wrote:
>
> Hi Ard,
>
> Wow, didn't expect for this to come so fast. Excellent.
>
> On Thu, Jan 13, 2022 at 02:12:39PM +0100, Ard Biesheuvel wrote:
> > - map arch_get_random_int/_long() onto RNDR, which returns the output of
> >   a DRBG that is reseeded at an implemented defined rate;
>
> implemented -> implementation?
>

Ack

> >  static inline bool __must_check arch_get_random_long(unsigned long *v)
> >  {
> > +     /*
> > +      * Only support the generic interface after we have detected
> > +      * the system wide capability, avoiding complexity with the
> > +      * cpufeature code and with potential scheduling between CPUs
> > +      * with and without the feature.
> > +      */
> > +     if (cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndr(v))
> > +             return true;
> >       return false;
> >  }
>
> Can't this just become:
>
>   return cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndr(v);
>

Sure, but I just retained the original style.

> >
> >  static inline bool __must_check arch_get_random_int(unsigned int *v)
> >  {
> > +     if (cpus_have_const_cap(ARM64_HAS_RNG)) {
> > +             unsigned long val;
> > +
> > +             if (__arm64_rndr(&val)) {
> > +                     *v = val;
> > +                     return true;
> > +             }
> > +     }
> >       return false;
> >  }
>
> Why not implement arch_get_random_int with the same type of flow as
> arch_get_random_long?
>
> static inline bool __must_check arch_get_random_int(unsigned int *v)
> {
>         unsigned long val;
>         if (cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndr(&val))) {
>                 *v = val;
>                 return true;
>         }
>         return false;
> }
>
> Or, even better, just define arch_get_random_int in terms of
> arch_get_random_long:
>
> static inline bool __must_check arch_get_random_int(unsigned int *v)
> {
>         unsigned long val;
>         if (arch_get_random_long(&val)) {
>                 *v = val;
>                 return true;
>         }
>         return false;
> }
>
>

If I was interested in rewriting this header file, I might consider
all these options. For now, I am just trying to focus the change on
the parts that actually matter.

> > @@ -71,12 +105,11 @@ static inline bool __must_check arch_get_random_seed_long(unsigned long *v)
> >       }
> >
> >       /*
> > -      * Only support the generic interface after we have detected
> > -      * the system wide capability, avoiding complexity with the
> > -      * cpufeature code and with potential scheduling between CPUs
> > -      * with and without the feature.
> > +      * RNDRRS is not backed by an entropy source but by a DRBG that is
> > +      * reseeded after each invocation. This is not a 100% fit but good
> > +      * enough to implement this API if no other entropy source exists.
>
> The docs are actually a bit more optimistic than that:
>
> https://developer.arm.com/documentation/ddi0595/2021-03/AArch64-Registers/RNDRRS--Reseeded-Random-Number
>
>  ~ Reseeded Random Number. Returns a 64-bit random number which is reseeded
>  ~ from the True Random Number source immediately before the read of the
>  ~ random number.
>
> If I'm reading that correctly, it looks like the reseeding happens
> *before* the read, and it looks like it comes from a TRNG. In
> other words, it sounds to me like it's just doing something like
> HASH(READ_TRNG()). That would be pretty darn good.
>

No it does not. RNDR and RNDRRS both return the output of a DRBG, the
only difference is the reseed interval.

Specifically, this means that, even though the ARM ARM references NIST
SP800-90B directly, the RNDRRS construction is a black box containing
a entropy source + DRBG, and so we shouldn't pretend that RNDRRS
itself can be treated as a source of true entropy. This is especially
relevant when it comes to seeding a DRBG of a certain security
strength N >= the security strength of the hidden DRBG, as
concatenating multiple RNDRRS results does not satisfy the
requirements for seeding a DRBG of security strength N.

> >        */
> > -     if (cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndr(v))
> > +     if (cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndrrs(v))
> >               return true;
> >
> >       return false;
> > @@ -96,7 +129,7 @@ static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
> >       }
> >
> >       if (cpus_have_const_cap(ARM64_HAS_RNG)) {
> > -             if (__arm64_rndr(&val)) {
> > +             if (__arm64_rndrrs(&val)) {
> >                       *v = val;
> >                       return true;
> >               }
>
> I suppose the same control flow simplification stuff mentioned above
> could be done here too, if you feel like what I mentioned earlier is
> worthwhile.
>
> From a randomness perspective:
>
> Acked-by: Jason A. Donenfeld <Jason at zx2c4.com>
>

Thanks,