Re: [Xen-devel] [libvirt] Questions about virtlogd

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

On Wed, Jun 08, 2016 at 11:57:45AM +0100, George Dunlap wrote:
> 
> Well we definitely want to make it possible for people to use xl while
> still avoiding DoSes.  But at the simplest level this could be done by
> having qemu's stderr/stdout piped to /dev/null by default, and allowing
> an option for the admin to enable piping it to a file on a per-guest
> basis when necessary.
> 
> This would effectively be declaring a "proper solution" out-of-scope,
> while not opening up our users to security issues.

I hadn't thought of /dev/null as an option, that's a nice idea.


BTW, I want to raise another item as more general food for thought
which is somewhat related to this topic, albeit not useful as an
solution to use today.

If you consider the risk to be a compromised QEMU inflicting DoS
on the host filesystem, then the stderr/out logging and serial
port file writing is not the only attack vector. If you give QEMU
any kind of file based disk, then (unless you have quotas in place)
it can expand those disk files to arbitrary size - this is by design
of course, since QEMU needs to save snapshots inside qcow2, and has
the ability to resize virtual disks, etc.

At the same time, it would be nice to be able to have a possibility
too have a locked down solution. Conceptually it would be nice to be
able to place size limits on individual files that were open. It turns
out Linux introduced just such a facility under the concept "file sealing"

The motivation for this was kDBus which wanted a way to share tmpfs backed
file handles between processes with certain policy rules. This concept was
implemented using a new fcntl() call F_ADD_SEAL which allows a number of
flags to be set against a file descriptor:

  - SHRINK: If SEAL_SHRINK is set, the file in question cannot be reduced
            in size. This affects ftruncate() and open(O_TRUNC).
  - GROW: If SEAL_GROW is set, the file in question cannot be increased
          in size. This affects ftruncate(), fallocate() and write().
  - WRITE: If SEAL_WRITE is set, no write operations (besides resizing)
           are possible. This affects fallocate(PUNCH_HOLE), mmap() and
           write().
  - SEAL: If SEAL_SEAL is set, no further seals can be added to a file.
          This basically prevents the F_ADD_SEAL operation on a file and
          can be set to prevent others from adding further seals that you
          don't want.

The SEAL_GROW flag seems like exactly the kind of thing QEMU could make
use of. First of all we would have to assume that the QEMU disk image
is fully pre-allocated, ie a non-sparse raw file, or a qcow2 file which
has had its extents fully allocated. This is not unreasonable for many
usage scenarios. I could imagine -drive gaining a new parameter
'growable=yes|no'. eg

  $QEMU  -drive file=/some/image/vm.qcow2,growable=no

would cause QEMU to set SEAL_GROW when it open the disk image. After
that point it is not possible for a compromised QEMU to cause further
disk allocation on the /some/image filesystem. This of course relies
on SELinux/AppArmour/other-MAC to prevent QEMU opening/creating other
arbitrary files.

Hotplug is not so simple though. At the time we hotplug the disk we
have to assume QEMU is already hostile, so we can't really rely on
QEMU to honour the request to set SEAL_GROW. To deal with this we
would have to deny QEMU any "open" permission using MAC. Instead
libvirt (or equiv) would have to be responsible for opening disk
images, setting SEAL_GROW and passing the file descriptor onto the
running QEMU to use.

This all feels remarkably simple and useful, with one huge glaring
issue....

....the kernel only implemented support for F_ADD_SEAL on the tmpfs
filesystem, since that's all kDBus needs it for :-) To be useful for
QEMU/libxl/libvirt/etc we'd at least need it supported on ext4 and xfs,
and preferrably NFS too. I've no clue how hard this would be since I'm
not at all familiar with the kernel code.

So clearly this isn't something we can use at time in the forseeable
future, but if people are interested in attacking the more general
problem, it might be an approach worth investigating to see if it
really is viable for the future.

How it would relate to the bug we're talking about here, is that I
could imagine pre-allocating the logfile at say 128 KB in size, opening
it, setting the SEAL_GROW flag and then attaching it to QEMU stdout/err.
This would let QEMU write straight to the file as normal, but not let
it exceed 128 KB.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel