On Fri, Jun 01, 2018 at 11:25:35AM +0100, Daniel P. Berrangé wrote:
On Wed, May 30, 2018 at 03:43:31PM +0200, Martin Kletzander wrote:On Wed, May 30, 2018 at 02:16:08PM +0200, Michal Privoznik wrote: > On 05/29/2018 03:44 PM, Martin Kletzander wrote: > > On Tue, May 29, 2018 at 10:24:44AM +0200, Michal Privoznik wrote: > > > Now that we have strong PRNG generator implemented in > > > virRandomBytes() let's use that instead of gnulib's random_r. > > > > > > Problem with the latter is in way we seed it: current UNIX time > > > and libvirtd's PID are not that random as one might think. > > > Imagine two hosts booting at the same time. There's a fair chance > > > that those hosts spawn libvirtds at the same time and with the > > > same PID. This will result in both daemons generating the same > > > sequence of say MAC addresses [1]. > > > > > > 1: https://www.redhat.com/archives/libvirt-users/2018-May/msg00097.html > > > > > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > > > --- > > > src/util/virrandom.c | 63 > > > ++-------------------------------------------------- > > > 1 file changed, 2 insertions(+), 61 deletions(-) > > > > > > > ACK to patches 1-7. But for this one I'm "concerned" about few things. > > > > First of all, just so I don't forget it, random_r can be removed from > > bootstrap.conf after this patch, right? > > Yes, I was meaning to make that change but then I forgot. > > > > > Before this patch, and without gnutls, we wouldn't deplete the entropy > > of the > > kernel, (even though we're just using /dev/urandom and not /dev/random), > > but now > > we'd read everything from /dev/urandom. > > Unless we are built with gnutls. But I don't see much problem with that. > Yeah, it's not that big of a deal, just an extra point for the next thing I mentioned below. > > > > Last but not least, if we completely drop the non-gnutls variants of > > everything, > > wouldn't everything be easier anyway? Like the worrying about entropy > > pool in > > previous point? > > Sure. But requiring gnutls (like I'm suggesting in the cover letter) is > orthogonal to these patches IMO. > My point was that the fixes might be could be cleaner and shorter, but that "not that big of a deal" above would be fixed. It also makes it kind of relevant. Since /dev/urandom cannot be really exhausted in newer Linux kernels (not sure for FreeBSD and others), I don't think that's a problem. We should ensure, however, that it is seeded properly. It might not be when it's early during the boot for Linux (although systemd and others seed it explicitly early enough), that's what getrandom() is for as it ensures the seeding was done properly. But that's Linux-specific. FreeBSD will apparently not give you any data until it's seeded properly./dev/urandom does not even exist on non-Linux hosts AFAIK, so this has the effect of breaking libvirt on non-Linux hosts when gnutls is disabled.
On FreeBSD it is a symlink to /dev/random (they both behave like getrandom() on Linux) and I guess on MacOS it is the same. Random search showed it exists there.
IMHO we should used be using getrandom() as the first fallback, and only then try /dev/urandom or /dev/random if the former doesn't exist
Sure, we can do that. It's just some crust (more configure checks and conditional compilations, etc.) in case libvirt would run so early that /dev/urandom was not properly seeded. Is there a modern distribution that doesn't seed /dev/urandom during boot before starting any services?
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 :|
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
