Bug 842307 - RFE: Need help designing and implementing selinux policy for libguestfs/sVirt
Summary: RFE: Need help designing and implementing selinux policy for libguestfs/sVirt
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: selinux-policy
Version: 18
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Miroslav Grepl
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-07-23 12:30 UTC by Richard W.M. Jones
Modified: 2014-04-16 18:57 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-09-18 13:32:34 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Richard W.M. Jones 2012-07-23 12:30:51 UTC
Description of problem:

libguestfs now has an optional back-end which uses libvirt to
launch the appliance.  It needs to create two communication
sockets.  In libvirt XML this is expressed like this:

    <serial type="unix">
      <source mode="connect" path="???/console.sock"/>
      <target port="0"/>
    </serial>
    <channel type="unix">
      <source mode="connect" path="???/guestfsd.sock"/>
      <target type="virtio" name="org.libguestfs.channel.0"/>
    </channel>

We need to (a) put these sockets in a suitable place and
(b) ensure they are labelled so that qemu can connect to them.

NB:

  (1) libguestfs creates the sockets and qemu connects to them.

  (2) libguestfs can run as non-root or as root, and it can connect
  to libvirt running non-root or root, in any combination.  This needs
  to work in all combinations, particularly the non-root + non-root case.

Dan Berrange suggested[i][ii] that we create a socket in either
  $HOME/.libguestfs/libvirt/
or
  /var/lib/libguestfs/libvirt/
depending on whether libguestfs is running as non-root or root.  (I
have changed the names from the ones he suggested).  And that we set
up SELinux to label these paths so that sockets created by libguestfs
under here are 

   [i] https://www.redhat.com/archives/libvir-list/2012-July/msg01188.html
  [ii] https://www.redhat.com/archives/libvir-list/2012-July/msg01201.html

Will this work?  Can we get this added to SELinux policy so
that it'll just work?

libguestfs is a library, not a program.  Since libguestfs creates the
sockets, do programs that use libguestfs need to be labelled to make
this work?

Version-Release number of selected component (if applicable):

  libguestfs 1.19.23
  selinux-policy-3.11.0-8.fc18.noarch

Additional info:

  https://www.redhat.com/archives/libvir-list/2012-July/thread.html#01142

Comment 1 Richard W.M. Jones 2012-07-23 12:52:03 UTC
(In reply to comment #0)
> up SELinux to label these paths so that sockets created by libguestfs
> under here are 

... labeled automatically.

Comment 2 Daniel Walsh 2012-07-23 14:56:56 UTC
Why not the other way around?

/var/lib/libvirt/libguest and ~/.libvirt/libguestfs?

This would just work with current labeling, Any reason these directories should be owned by libguest?  By the way it would be easy to add the alternative, but I 
would label them the same.

Comment 3 Daniel Walsh 2012-07-23 14:59:07 UTC
Also I think the desktop team wants to move away from creating new content in ~/ and move it to places like /run/user/3267 and maybe /run/libvirt/libguest

Comment 4 Richard W.M. Jones 2012-07-23 15:14:30 UTC
libguestfs

I don't think I really care where the directory is located.  danpb?

If we add /var/lib/libvirt/libguestfs then that would require a
trivial change to libvirt.spec.

I'm less clear about ~/.libvirt (or ~/.cache/libvirt as it now
appears to be called).  Do I just mkdir a directory under
here and go?

Furthermore I still don't think that SELinux is the whole story.
We've still got the problem that qemu runs as qemu.qemu.  In the
case where non-root libguestfs talks to root libvirtd how do I know
which path to use?  Can non-root libguestfs create the socket in
/var?

Comment 5 Daniel Berrangé 2012-07-23 15:25:04 UTC
My suggestion is that libguest use its own directory, so it doesn't have to deal with the fact that libvirt has moved its own directories.

IMHO, non-root libguestfs shouldn't talk to a root libvirtd - it should always use the session instance, thus avoiding this problem.

Comment 6 Daniel Walsh 2012-07-23 15:41:23 UTC
If these are just temporary sock files they should be created in /run, no reason to have them stick around in the homedir or /var/lib

Comment 7 Richard W.M. Jones 2012-07-23 15:42:03 UTC
Yes, they are temporary in all cases.

Comment 8 Daniel Walsh 2012-07-23 15:47:20 UTC
Ok lets go with

$XDG_RUNTIME_DIR/libguestfs
and
/var/run/libguestfs

And I will setup SELinux labeling for these.

Comment 9 Richard W.M. Jones 2012-07-23 15:50:11 UTC
Thanks.

Should that be /run or /var/run?  (I know /var/run is a symlink
to /run, just not clear which one I should be writing to)

Also this change only needs to be made in Rawhide.

Comment 10 Daniel Walsh 2012-07-23 15:52:35 UTC
If your code is only going to run on systemd machines, then /run should be fine.  I am adding

/var/run/libguestfs(/.*)?      gen_context(system_u:object_r:virt_var_run_t,s0)

/var/run/user/[^/]*/libguestfs(/.*)?   gen_context(system_u:object_r:virt_home_t,s0)


Since upstream policy requires /var/run and we have a rule in policy that states everything under /run == /var/run

Comment 11 Richard W.M. Jones 2012-07-24 13:07:02 UTC
I've pushed this upstream into libguestfs:
https://github.com/libguestfs/libguestfs/commit/493e80ab4e7199bdbb44b16535448d65944eca66

Note that sVirt confinement is not yet enabled (waiting for
this change to go into selinux-policy + further testing).

Comment 12 Richard W.M. Jones 2012-08-15 22:01:00 UTC
Did this change get into selinux-policy yet?

Since F18 has branched now, we'd really like it in
F18 & Rawhide.

Comment 13 Daniel Walsh 2012-08-16 13:36:38 UTC
The current package has the labels.

matchpathcon /run/libguestfs
/run/libguestfs	system_u:object_r:virt_var_run_t:s0

 matchpathcon /run/user/1234/libguestfs
/run/user/1234/libguestfs	system_u:object_r:virt_home_t:s0

selinux-policy-3.11.1-7.fc18.noarch

Comment 14 Richard W.M. Jones 2012-08-21 15:45:16 UTC
So now we have two problems.

Running libguestfs as non-root gives this error:

libguestfs: [01268ms] create libvirt XML
libguestfs: libvirt XML:\n<?xml version="1.0"?>\n<domain type="kvm" xmlns:qemu="http://libvirt.org/schemas/domain/qemu/1.0">\n  <name>guestfs-7zf7a2nxn6o1tufh</name>\n  <memory unit="MiB">500</memory>\n  <currentMemory unit="MiB">500</currentMemory>\n  <cpu model="host-model"/>\n  <vcpu>1</vcpu>\n  <clock offset="utc"/>\n  <os>\n    <type>hvm</type>\n    <kernel>/home/rjones/d/libguestfs/.guestfs-1000/kernel.17570</kernel>\n    <initrd>/home/rjones/d/libguestfs/.guestfs-1000/initrd.17570</initrd>\n    <cmdline>panic=1 console=ttyS0 udevtimeout=600 no_timer_check acpi=off printk.time=1 cgroup_disable=memory root=/dev/sdb selinux=0 guestfs_verbose=1 TERM=xterm </cmdline>\n  </os>\n  <on_reboot>destroy</on_reboot>\n  <devices>\n    <controller type="scsi" index="0" model="virtio-scsi"/>\n    <disk device="disk" type="file">\n      <source file="/tmp/libguestfs-test-tool-sda-RJeljg"/>\n      <target dev="sda" bus="scsi"/>\n      <driver name="qemu" type="raw"/>\n      <address type="drive" controller="0" bus="0" target="0" unit="0"/>\n    </disk>\n    <disk type="file" device="disk">\n      <source file="/home/rjones/d/libguestfs/.guestfs-1000/root.17570"/>\n      <target dev="sdb" bus="scsi"/>\n      <driver name="qemu" type="raw" cache="unsafe"/>\n      <address type="drive" controller="0" bus="0" target="1" unit="0"/>\n    </disk>\n    <serial type="unix">\n      <source mode="connect" path="/home/rjones/d/libguestfs/libguestfsNGj89u/console.sock"/>\n      <target port="0"/>\n    </serial>\n    <channel type="unix">\n      <source mode="connect" path="/home/rjones/d/libguestfs/libguestfsNGj89u/guestfsd.sock"/>\n      <target type="virtio" name="org.libguestfs.channel.0"/>\n    </channel>\n  </devices>\n  <qemu:commandline>\n    <qemu:arg value="-set"/>\n    <qemu:arg value="drive.drive-scsi0-0-1-0.snapshot=on"/>\n  </qemu:commandline>\n</domain>\n
libguestfs: [01268ms] launch libvirt guest
libguestfs: error: could not create appliance through libvirt: unable to set security context 'system_u:object_r:virt_content_t:s0' on '/home/rjones/d/libguestfs/.guestfs-1000/kernel.17570': Operation not permitted [code=38 domain=24]

It seems like libvirt tries to relabel files, specifically the
<kernel/> file, but cannot do that because it's not running as root.

There's a second problem that occurs as root, but that might not
be a real problem so I'll leave it for now.

Comment 15 Daniel Berrangé 2012-08-22 15:53:21 UTC
> libguestfs: error: could not create appliance through libvirt: unable to set security context 'system_u:object_r:virt_content_t:s0' on '/home/rjones/d/libguestfs/.guestfs-1000/kernel.17570': Operation not permitted [code=38 domain=24]

> It seems like libvirt tries to relabel files, specifically the
> <kernel/> file, but cannot do that because it's not running as root.

As non-root we're allowed to relabel any files that the user has existing permission for. 

The issue in this case was that the kernel was a symlink to a file in /boot, which is owned by root. This can be dealt with by copying the kernel instead of symlinking.

Comment 16 Richard W.M. Jones 2012-09-14 13:29:50 UTC
(In reply to comment #13)
> The current package has the labels.
> 
> matchpathcon /run/libguestfs
> /run/libguestfs	system_u:object_r:virt_var_run_t:s0
> 
>  matchpathcon /run/user/1234/libguestfs
> /run/user/1234/libguestfs	system_u:object_r:virt_home_t:s0
> 
> selinux-policy-3.11.1-7.fc18.noarch

I think we were wrong about this, and this piece of policy
is not necessary and should be reversed.

I think it's libvirt's job to label these sockets correctly,
just as it already does with disk drives.  It's a bug in
libvirt that it doesn't do so (bug 853393).  I've sent
a patch to libvir-list which fixes this bug.

Now there is one remaining piece of policy missing to make
the whole thing work, but I've opened a separate bug about
that: bug 857453.

Comment 17 Richard W.M. Jones 2012-09-18 13:39:27 UTC
Sorry for the runaround with this bug, but did the
policy added (comment 13) get removed?

Comment 18 Daniel Walsh 2012-09-18 13:47:46 UTC
No but I am not sure how those directories should be labeled.

Comment 19 Richard W.M. Jones 2012-09-18 13:54:39 UTC
We're not going to create those directories at all.

Instead the plan is (bug 857453 - thanks for fixing it so quickly)
to just create ordinary sockets in our regular temp directory,
and have libvirt label them.  This matches how libvirt labels
other things as well, so it seems more natural.

Comment 20 Daniel Walsh 2012-09-18 15:23:09 UTC
Fixed in selinux-policy-3.11.1-23.fc18.noarch

Comment 21 Richard W.M. Jones 2014-04-16 18:57:58 UTC
FYI I noticed that we were not actually using the {/var,}/run/libguestfs
directory anywhere, so I have removed it from the spec file.  I don't
know if it still exists in selinux policy, but likely it can
be removed.


Note You need to log in before you can comment on or make changes to this bug.