Hi!
On Fri, Feb 26, 2016 at 08:28:32PM +0000, Jason Cooper wrote:
> Hi Steffen,
>
> On Fri, Feb 26, 2016 at 01:04:44PM +0100, Steffen Trumtrar wrote:
> > Instead of generating pseudo random numbers, get random bytes
> > from an optional HW generator, if enabled and registered.
> >
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@xxxxxxxxxxxxxx>
> > ---
> > lib/random.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
>
> I have a long interest in KASLR on arm/arm64, so I'm glad to see this
> series! :-)
>
That is not what I had in mind when writing this ;-)
> > diff --git a/lib/random.c b/lib/random.c
> > index 210fea99466e..ff82dd1bed85 100644
> > --- a/lib/random.c
> > +++ b/lib/random.c
> > @@ -1,5 +1,6 @@
> > #include <common.h>
> > #include <stdlib.h>
> > +#include <linux/hw_random.h>
> >
> > static unsigned int random_seed;
> >
> > @@ -22,6 +23,24 @@ void get_random_bytes(void *_buf, int len)
> > {
> > char *buf = _buf;
> >
> > + if (IS_ENABLED(CONFIG_HWRNG)) {
> > + struct hwrng *rng;
> > + int bytes;
> > +
> > + rng = hwrng_get_first();
> > + if (!PTR_ERR(rng)) {
> > + while (len) {
> > + bytes = hwrng_get_data(rng, _buf, len, 0);
> > + if (!bytes)
> > + goto sw_fallback;
> > + len -= bytes;
> > + }
> > +
> > + return;
> > + }
> > + }
> > +
> > +sw_fallback:
> > while (len--)
> > *buf++ = rand() % 256;
> > }
>
> However, I disagree with this approach. One of the main problems we've
> had over the years with random number generation is not being clear
> about *how* the numbers were generated. Something designed for placing
> characters in a game is not suitable for creating long-term crypto keys,
> stack canaries, and ASLR offsets. See the paper on iOS bootloader and
> KASLR [1].
>
> With that in mind, I'd like to suggest that we preserve the current
> functionality of get_random_bytes() as a non-strong source of entropy.
> After all, all current calls to srand() feed in a time...
>
> We can then add get_hwrng_bytes() which makes it clear where the bytes
> are coming from. Users down the line can more easily make an assessment
> of whether the SoC hwrng is trusted or not for their usecase.
>
I see your point. So, is get_random_bytes() in the kernel always a
non-strong source? I was under the assumption, that the hwrng fills the
same pool that get_random_bytes() sources. The kernel however has
get_random_bytes_arch(). Shouldn't we also use this nomenclature here then?
As long as a user is not left under the assumption, that enabling HWRNG
somehow makes the randomness stronger, I think it may be a good idea to
make the code clearer via a different function call. No problem for me.
> My goal here is to have the bootloader grab some strong random numbers
> to hand off to the kernel. The decompressor would consume a bit of it
> to initialize KASLR, and the rest would be consumed seeding the kernel's
> entropy pools.
>
> A second goal is to have the OS write a seed file somewhere the
> bootloader can access. Then, on reboot, the bootloader can read it in
> and hand it off to the kernel for KASLR, etc. But that is orthogonal to
> this patch series...
>
This is out of my comfort zone, so: send patches :-)
Thanks,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
