On Fri, Jun 01, 2018 at 02:10:56PM +0200, Martin Kletzander wrote: > 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. Ah that's good to know - I guess they added the symlink due to software having assumptions from linux world :-) 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
