linux-kernel - Re: [PATCH v4 2/5] random: Add and use arch_get_rng

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 22 Jul 2014 13:44:36 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	"Theodore Ts'o" <tytso@....edu>,
	Andy Lutomirski <luto@...capital.net>,
	kvm list <kvm@...r.kernel.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Kees Cook <keescook@...omium.org>, X86 ML <x86@...nel.org>,
	Daniel Borkmann <dborkman@...hat.com>,
	Srivatsa Vaddagiri <vatsa@...ux.vnet.ibm.com>,
	Raghavendra K T <raghavendra.kt@...ux.vnet.ibm.com>,
	Gleb Natapov <gleb@...nel.org>,
	Paolo Bonzini <pbonzini@...hat.com>,
	Bandan Das <bsd@...hat.com>, Andrew Honig <ahonig@...gle.com>
Subject: Re: [PATCH v4 2/5] random: Add and use arch_get_rng_seed

On Tue, Jul 22, 2014 at 6:59 AM, Theodore Ts'o <tytso@....edu> wrote:
> On Thu, Jul 17, 2014 at 11:22:17AM -0700, Andy Lutomirski wrote:
>> Currently, init_std_data contains its own logic for using arch
>> random sources.  This logic is a bit strange: it reads one long of
>> arch random data per byte of internal state.
>
> This isn't true.  Check out the init_std_data() a bit more closely.
>
>         unsigned long rv;
>
>         ...
>
>         for (i = r->poolinfo->poolbytes; i > 0; i -= sizeof(rv)) {
>             ...
>
> In particular, note the "i -= sizeof(rv)".  We are reading one bit per
> bit of internal state beeing seeded.

Whoops, my bad.

>
>> Assuming the arch sources are perfect, this is the right thing to
>> do.  They're not, though, so the followup patch attempts to
>> implement the correct logic on x86.
>
> ... and that's not a problem because we aren't giving any entropy
> credit --- and this is deliberate, because we don't want to trust
> un-auditable hardware.  We are deliberately trying to be conservative
> here.

True.

But, if you Intel's hardware does, in fact, work as documented, then
the current code will collect very little entropy on RDSEED-less
hardware.  I see no great reason that we should do something weaker
than following Intel's explicit recommendation for how to seed a PRNG
from RDRAND.

>
> So I don't think either this patch or the next one is needed.  It adds
> far more complexity than is warranted.

The real reason I did this is because I didn't want to pollute the
kernel with yet more arch_get_random_xyz functions.  In the previous
iteration of this patchset, init_std_data had to deal with no less
than three arch random sources.  If Xen adds something (which, IMO,
they should), then either it'll be up to four, or one of them will
have to multiplex.

Another benefit of this split is that it will potentially allow
arch_get_rng_seed to be made to work before alternatives are run.
There's no fundamental reason that it couldn't work *extremely* early
in boot.  (The KASLR code is an example of how this might work.)  On
the other hand, making arch_get_random_long work very early in boot
would either slow down all the other callers or add a considerable
amount of extra complexity.

So I think that this patch is a slight improvement in RNG
initialization and will actually result in simpler code.  (And yes, if
I submit a new version of it, I'll fix the changelog.)

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/