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

[Marc-André Lureau]

Hi

On Thu, Apr 25, 2019 at 3:22 PM Michal Privoznik <mprivozn at redhat.com> wrote:
>
> On 4/18/19 3:24 PM, marcandre.lureau at redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau at redhat.com>
> >
> > 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?

"user" networking (<interface type='user'>) is not broken, it's
guestfwd that is not working for a while apparently.

Various projects use slirp forks, that's why we decided to make it
again a standalone project / library that can be shared. slirp4netns
(used by podman afaik), is perhaps the most worrisome to me.


> 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 at redhat.com>
> > ---
> >   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.
>

Well, that's what I propose to discuss with this proposal. Make
current libslirp usage safer by using it as a subprocess, continue to
improve/fix it, and allow competing implementation to emerge.

thanks for the feedback