Re: [Qemu-devel] [RFC Patch 0/3] Accept passed in socket 'fd' open from outside for unix socket

[Michal Privoznik]

On 01.06.2016 18:16, Wei Xu wrote:
> On 2016年06月01日 00:44, Daniel P. Berrange wrote:
>> On Wed, Jun 01, 2016 at 12:30:44AM +0800, address@hidden wrote:
>>> From: Wei Xu <address@hidden>
>>>
>>> Recently I'm working on a fd passing issue, selinux forbids qemu to
>>> create a unix socket for a chardev when managing VMs with libvirt,
>>> because qemu don't have sufficient permissions in this case, and
>>> proposal from libvirt team is opening the 'fd' in libvirt and merely
>>> passing it to qemu.
>>
>> This sounds like a bug in libvirt, or selinux, or a mistaken
>> configuration
>> of the guest. It is entirely possible for QEMU to create a unix socket
>> - not
>> least because that is exactly what QEMU uses for its QMP monitor backend.
>> Looking at your example command line, I think the issue is simply that
>> you
>> should be putting the sockets in a different location. ie at
>> /var/lib/libvirt/qemu/$guest-vhost-user1.sock where QEMU has
>> permission to
>> create sockets already.
> ah.. adjusting permission or file location can solve this problem, i'm
> guessing maybe this is a more security concern, the socket is used as a
> network interface for a vm, similar as the qcow image file, thus should
> prevent it to be arbitrarily accessed.
> 
> Michael, do you have any comment on this?

I haven't seen the patches. But in libvirt we allow users to create a
vhostuser interface and even specify where the socket should be placed:

    <interface type='vhostuser'>
      <mac address='52:54:00:ee:96:6c'/>
      <source type='unix' path='/tmp/vhost1.sock' mode='server'/>
      <model type='virtio'/>
    </interface>

The following cmd line is generated by libvirt then:

-chardev socket,id=charnet1,path=/tmp/vhost1.sock,server \
-netdev type=vhost-user,id=hostnet1,chardev=charnet1 \
-device
virtio-net-pci,netdev=hostnet1,id=net1,mac=52:54:00:ee:96:6c,bus=pci.0,\

Now, if we accept only /var/run/openvwitch path in
/interface/source/@path (or whatever path to OVS is), we don't need this
and have users manually label the dir (unless already labeled). But
since we accept just any path in there, we should make sure that qemu is
then able to create the socket. One possible fix would be to allow qemu
create sockets just anywhere in the system. This, however, brings huge
security risks and it's not acceptable IMO. The other option would be
that libvirt would create the socket, and pass its FD to qemu (since
libvirt already is allowed to create sockets anywhere).

> 
>>
>> Alternatively you could enhance the SELinux policy to grant svirt_t the
>> permission to create sockets under /var/run/openvswitch too.

Nah, the point of using libvirt is that you don't have to configure
anything (or just bare minimum) and libvirt makes sure your domains have
all the permissions needed - that's why we relabel domain's disks when
starting it up.

BTW: Is there any reason why the patches have not been sent to the list?

>>
>>> I finished a RFC patch for Unix socket after a glance of the code,
>>> and not sure if this is right or there maybe other side-effects,
>>> please point me out.
>>>
>>> I tested it for both server and client mode 'PF_UNIX' socket with a VM
>>> running vhost-user.
>>>
>>> Old command line:
>>> -chardev socket,id=char0,path=/var/run/openvswitch/vhost-user1,server
>>>
>>> New command line:
>>> -chardev
>>> socket,id=char0,path=/var/run/openvswitch/vhost-user1,server,sockfd=$n
>>>
>>> because unix socket is bundled with a path, so it should be kept even
>>> with the
>>> 'fd' is indicated, this looks odd, any comments?
>>
>> Yes, this syntax doesn't really make sense. The passed in sockfd may not
>> even be a UNIX socket - it could be a TCP socket. As such, the 'sockfd'
>> option should be mutually exclusive with the 'path' and 'host' options.
>> ie you can only supply one out of 'sockfd', 'path', or 'host'.
> Currently i just add it for unix socket, and the connect/listen syscall
> must have a path name, an inet socket doesn't need this param at all,
> should it be supported also?
> 
>>
>>
>> FWIW, I think the ability to pass a pre-opened socket FD with the
>> -chardev socket backend is a useful enhancement, even though I don't
>> think you need this in order to fix the problem you have.

Isn't this the ideal goal? I mean, in theory (and far far future)
libvirt would open all the files for qemu and then pass all the FDs
rather than what we have now?

Michal