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

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 08/06/16 13:11, Daniel P. Berrange wrote:
> 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.

Interesting -- I had actually been thinking that if there were something
along these lines it would be quite useful. :-)  I was actually
pondering whether in the qemu case it would be possible to add this kind
of limit to logging / serial output files, but having the kernel do it
is as you say a lot more secure.

From a Xen perspective: At the moment qemu running in dom0 cannot be
properly isolated from the rest of the system anyway; if you manage to
break into qemu in dom0 on a default system, you can map any guest's ram
you wish (including dom0's), which (we assume) allows you to get control
of the entire system.  The only way at the moment to be secure against
this is to run qemu in a separate domain (stubdoms).

(XenServer has had deprivileged qemu running in dom0 for years, but
their solution isn't directly upstreamable, since it relies on the Xen /
Linux / qemu interface version being tightly linked.  There is an effort
being made to do this in an upstreamable manner, but it's not there yet.)

All that to say: At the moment restricting qemu running in a Xen dom0
with the kernel is sort of pointless, but hopefully in the
not-too-distant future it will actually become useful again. :-)

 -George



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