Re: [libvirt] [PATCH] RFC: use a slirp helper process

On 4/18/19 3:24 PM, marcandre.lureau(a)redhat.com wrote: > From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; > > I am throwing this away for discussions, and early feedback. > > With the upcoming release of libslirp[1], we have an opportunity to > run SLIRP networking in a separate process. This will allow for > stricter security policies for both qemu & slirp, as slirp is > notoriously not very safe (discussed on ML, various CVEs, and even the > code says so explicitly in the comments), yet people rely on it regularly. Do they? I mean, SLIRP is broken in libvirt for quite some time and we haven't noticed nor seen a bug about it. What is the typical use anyway?

I can see some potential in combining -netdev user + -chardev where one could see/inject packets into a guest (if I got that correctly). But that can't work currently, because libvirt doesn't allow setting all the interesting bits (like subnet mask). > > For network type "user", libvirt could spawn an helper process (an > experimental version is [2]). It would setup a socket pair between > qemu and the helper and use -net socket. qemu could then be built > without libslirp! (imho, qemu should deprecate built-in -net user > support, and doesn't need to grow its own sub-process handling) > > This libvirt patch is a bit rough, I am not sure where things should > go. In particular, how to manage the subprocesses properly (security > aspects, and lifecycle etc), and how to deal with migration (prevent > migrating from non-helper to helper version etc). > > It seems guestfwd has been non-functional for a while. This is also > something that the current experimental helper doesn't support > atm. Doing all the various "channel" types that libvirt/qemu support > would be tedious, and probably unnecessary. But the underlying > libslirp library support redirections the same way qemu does today, so > it could be added if necessary. > > At this point, the slirp-helper doesn't handle migration. But is it > really worth it? From a guest POV, it would look like packet lost or > disconnection. Adding migration handling is possible, to avoid a > disconnection event. How should it be done? via a libvirt provided > socket for storage? via its own file transferred by libvirt? via qemu > somehow (qemu wouldn't really know about the external process but > would copy around the migration bits?). > > Does the helper need to have a "standard" & capabilities, similar to > what is proposed for vhost-user [3]? This would allow for easier > competing implementation to emerge (I have plans in mind, not using > libslirp). > > Thanks for the feedback! > > [1] https://gitlab.freedesktop.org/slirp/libslirp > [2] https://github.com/elmarco/libslirp-rs > [3] https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/vhost-user.json > > Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; > --- > m4/virt-driver-qemu.m4 | 5 ++ > src/qemu/libvirtd_qemu.aug | 1 + > src/qemu/qemu.conf | 3 ++ > src/qemu/qemu_capabilities.c | 6 +++ > src/qemu/qemu_capabilities.h | 1 + > src/qemu/qemu_command.c | 38 +++++++++++--- > src/qemu/qemu_command.h | 3 +- > src/qemu/qemu_conf.c | 7 ++- > src/qemu/qemu_conf.h | 1 + > src/qemu/qemu_domain.c | 11 ++++ > src/qemu/qemu_domain.h | 3 ++ > src/qemu/qemu_hotplug.c | 5 +- > src/qemu/qemu_interface.c | 80 ++++++++++++++++++++++++++++++ > src/qemu/qemu_interface.h | 6 +++ > src/qemu/test_libvirtd_qemu.aug.in | 1 + > 15 files changed, 161 insertions(+), 10 deletions(-) The code looks more or less okaysh, but I'd rather discuss the future of slirp for now.