[libvirt-users] [libvirt] virRandomBits - not very random

Fri Jun 1 13:03:19 UTC 2018

On Fri, Jun 01, 2018 at 01:25:26PM +0100, Daniel P. Berrangé wrote:
>On Fri, Jun 01, 2018 at 02:01:03PM +0200, Martin Kletzander wrote:
>> On Fri, Jun 01, 2018 at 11:17:44AM +0100, Daniel P. Berrangé wrote:
>> > On Wed, May 30, 2018 at 10:21:54PM +0200, Martin Kletzander wrote:
>> > > On Tue, May 29, 2018 at 10:06:25AM -0400, John Ferlan wrote:
>> > > >
>> > > >
>> > > > On 05/29/2018 09:44 AM, Michal Privoznik wrote:
>> > > > > On 05/29/2018 03:38 PM, Martin Kletzander wrote:
>> > > > > > On Fri, May 25, 2018 at 09:37:44AM -0500, Eric Blake wrote:
>> > > > > > > On 05/25/2018 09:17 AM, Michal Privoznik wrote:
>> > > > > > >
>> > > > > > > > > > We should probably seed it with data from /dev/urandom, and/or the new
>> > > > > > > > > > Linux getrandom() syscall (or BSD equivalent).
>> > > > > > > >
>> > > > > > > > I'm not quite sure that right after reboot there's going to be enough
>> > > > > > > > entropy. Every service that's starting wants some random bits. But it's
>> > > > > > > > probably better than what we have now.
>> > > > > > >
>> > > > > > > Here's where we left things last time it came up:
>> > > > > > >
>> > > > > > > https://www.redhat.com/archives/libvir-list/2014-December/msg00573.html
>> > > > > > >
>> > > > > > > If gnutls has an interface that will give us random bits
>> > > > > > > (gnutls_key_generate() in 3.0, perhaps), we should use THAT for all of
>> > > > > > > our random bits (and forget about a seed), except when we are mocking
>> > > > > > > things in our testsuite, and need a deterministic PRNG from a
>> > > > > > > deterministic seed.
>> > > > > > >
>> > > > > > > If not (including if we are not linked with gnutls), then we should
>> > > > > > > prefer the new Linux syscall but fall back to /dev/urandom for JUST
>> > > > > > > enough bits for a seed; once we're seeded, stick with using our existing
>> > > > > > > PRNG for all future bits (after all, we aren't trying to generate
>> > > > > > > cryptographically secure keys using virRandomBits - and the places where
>> > > > > > > we DO need crypto-strong randomness such as setting up TLS migration is
>> > > > > > > where we are relying on gnutls to provide it rather than virRandomBits).
>> > > > > > >
>> > > > > > > So at this point, it's just a matter of someone writing the patches.
>> > > > > > >
>> > > > > >
>> > > > > > Actually, do we need to have a fallback at all?  Can't we just drop all the
>> > > > > > gross parts of the code the conditionally compile based on GNUTLS
>> > > > > > support?  Why
>> > > > > > don't we have gnutls required?
>> > > > >
>> > > > > That's exactly what I'm suggesting in my patches [1]. gnutls is widely
>> > > > > available (including Linux, Windows, *BSD, Mac Os X). However, before
>> > > > > doing that we need to fix virRandomBits() to actually call gnutls_rnd().
>> > > > >
>> > > > > 1: https://www.redhat.com/archives/libvir-list/2018-May/msg02077.html
>> > > > >
>> > > >
>> > > > I have this faint recollection of one of the CI platform builds failing
>> > > > because something in the gnutls* family didn't exist there when I was
>> > > > making the changes to add the domain master secret code....  After a bit
>> > > > of digging, it seems it was a perhaps a CENTOS6 environment:
>> > > >
>> > > > https://www.redhat.com/archives/libvir-list/2016-April/msg00287.html
>> > > >
>> > > > and since IIUC that's not an issue any more....
>> > > >
>> > >
>> > > Oh, cool to know.  Michal also found the patch [1] where Dan switched the gnutls
>> > > from being mandatory to making it optional and there is no explanation for that
>> > > change in the commit message:
>> > >
>> > > [1] f587c27768ee13f5bed6a9262106307b7a124403
>> >
>> > Not all usage scenarios in libvirt have required GNUTLS - only the remote
>> > driver, when using stateful virt drivers. If you're just biulding libvirt
>> > for usage with ESX/HyperV/etc, there's no reason you'd want GNUTLS historically.
>> >
>> > Also note when building the setuid libvirt pieces we must never use GNUTLS
>> > because its library constructors do very bad things leading to CVEs.
>> >
>>
>> Good to know, thanks for the info.  I'll need to read up on the gnutls
>> constructors (is it just gnutls or some other libs as well?) even though I
>> remember researching something related to it some time ago.  However that
>> doesn't mean it has to be optional.
>>
>> Do you think there are people who would be bothered by the requirement of
>> gnutls?  I'm sure most of them have gnutls anyway.  Some of them even need to
>> have, for example if you have a look at virCryptoHashBuf() its stub returns -1
>> and it is used in ESX.  If you compile without GNUTLS, all the functions that
>> require it will just fail and it's not something that is used rarely.
>>
>> We can we make it required only if you are building with qemu or remote or esx
>> or basically something that requires it.  Encryption of RAW volumes for example.
>> Or lock driver.  But I don't think there are some who run the builds themselves
>> and doesn't want gnutls on their system.  I can't think of a reason for having
>> it and not linking libvirt with it.
>
>I should clarify - I've no objection to making GNUTLS mandatory at configure
>time - just that we need to continue to have WITH_GNUTLS conditionals in some
>parts of the code in order to support the setuid build without GNUTLS. So we
>might be able to remove some of the WITH_GNUTLS bits, but not all of them.
>

Oh, OK.  Great.

>Regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvirt-users/attachments/20180601/585e0ff5/attachment.sig>