Patch posted upstream:

https://www.redhat.com/archives/libvir-list/2012-September/msg01031.html

Second version:

https://www.redhat.com/archives/libvir-list/2012-September/msg01037.html

Third version:

https://www.redhat.com/archives/libvir-list/2012-September/msg01478.html

Fixed upstream:
http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=db2aff6adaa405f0bc998c7bd3158fe43805ee60
http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=f8b08d0e961c7e10d87ee011ec7bf1f8fe7b1fbb

This bug is against upstream and we just close such bugs when patches are pushed to upstream git repository. Therefore I'm closing this bug.

Created attachment 617530 [details]
libvirtd.log

I have a very strange case where this bug still manifests
itself, even though libvirt and selinux-policy have all been
updated to the latest versions.

Attached is libvirtd.log.

The error is:

libguestfs: error: could not create appliance through libvirt: internal error process exited while connecting to monitor: 2012-09-26 12:04:27.080+0000: 8047: debug : virFileClose:72 : Closed fd 21
2012-09-26 12:04:27.080+0000: 8047: debug : virFileClose:72 : Closed fd 26
2012-09-26 12:04:27.087+0000: 8047: debug : virFileClose:72 : Closed fd 3
connect(unix:/home/rjones/d/libguestfs/tmp/libguestfsH249y9/console.sock): Permission denied
chardev: opening backend "socket" failed
 [code=1 domain=10]

The AVC is:

type=AVC msg=audit(1348661067.174:1181): avc:  denied  { connectto } for  pid=8048 comm="qemu-kvm" path="/home/rjones/d/libguestfs/tmp/libguestfsH249y9/console.sock" scontext=unconfined_u:unconfined_r:svirt_t:s0:c29,c783 tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=unix_stream_socket
type=SYSCALL msg=audit(1348661067.174:1181): arch=c000003e syscall=42 success=no exit=-13 a0=4 a1=7fff38bb3540 a2=6e a3=3b items=0 ppid=1 pid=8048 auid=500 uid=500 gid=500 euid=500 suid=500 fsuid=500 egid=500 sgid=500 fsgid=500 tty=(none) ses=1 comm="qemu-kvm" exe="/usr/bin/qemu-kvm" subj=unconfined_u:unconfined_r:svirt_t:s0:c29,c783 key=(null)

Note that according to libvirtd.log, this socket should have been
labelled before qemu runs, so either it's not being labelled, there
is an error which libvirtd is ignoring, or something else in the
system is changing the label in the brief moment before qemu runs.

libvirt-0.10.2-3.fc18.x86_64
selinux-policy-3.11.1-25.fc18.noarch

Also fails with libvirt from git.

However my other Fedora 18 machine has all the same packages
and does NOT show this problem, which indicates (I guess?)
labelling or something similar.

The libvirtd.log shows the socket had its label set for sure:

2012-09-26 12:04:27.100+0000: 7925: info : virSecuritySELinuxSetFileconHelper:754 : Setting SELinux context on '/home/rjones/d/libguestfs/tmp/libguestfsH249y9/console.sock' to 'unconfined_u:object_r:svirt_image_t:s0:c29,c783'

And we don't execve() QEMU until after this point, so damned if I understand why it is showing 'unconfined_t' as the target type in the AVC.

Pulling Dan Walsh into the discussion

@DanW: the sequene of steps we have is

 1. We have libguestfs running in an unconfined process.
 2. libguestfs opens, binds & listens on a UNIX socket "/home/rjones/d/libguestfs/tmp/libguestfsH249y9/console.sock"
 3. libguestfs starts a QEMU guest, with a console configured to connect to the UNIX socket "/home/rjones/d/libguestfs/tmp/libguestfsH249y9/console.sock"
 4. libvirt sets the label on "/home/rjones/d/libguestfs/tmp/libguestfsH249y9/console.sock" to "svirt_image_t:s0:c29,c783" and then starts QEMU
 5. QEMU tries connect() and gets an AVC showing "unconfined_t:s0-s0:c0.c1023"

I'm vaguely remembering there are some odd things with UNIX sockets, with there being 2 labels in play, one of the socket file on disk and one for the socket FD. I'm thinking that the server socket file desriptor libguests uses in (2), does not have its label changed when libvirt sets label on the UNIX socket in (3) ?

Yes, libvirt does a setsockcreate() call before creating sockets that it will allow the container to connect to.

setsockcreate("system_u:service_r:svirt_t:s0:c1,c2)
sock = socket()
...

Then both ends of the socket will be labeled correctly.

(In reply to comment #11)
> Yes, libvirt does a setsockcreate() call before creating sockets that it
> will allow the container to connect to.
> 
> setsockcreate("system_u:service_r:svirt_t:s0:c1,c2)
> sock = socket()
> ...
> 
> Then both ends of the socket will be labeled correctly.

setsockcreatecon?

I can't see how we can make this work with libguestfs, since
libguestfs doesn't know what MLS context (ie. c1,c2) will be
assigned to the guest.  Unless libguestfs chooses a random
c1,c2 pair and sets it in the libvirt XML (seclabel I think).

Also, is setsockcreatecon thread-safe?

I'm also quite confused about why this should work fine on
one machine but not on another (both SELinux enforcing, both
have the same libvirt & selinux-policy packages, and are
generally similarly configured).

eparis confirmed that setsockcreatecon is thread safe.

We discussed this on IRC.  My summary:

The problem is that libguestfs creates the socket and holds open
a file descriptor for that socket.  The label on the libguestfs end
is unconfined_t, even if libvirt relabels the underlying file.

In general we want to prevent svirt_t from connecting to unconfined_t.

One possible solution would be for libguestfs to label the socket
as something else, eg. guestfs_t (or whatever), by doing:

setsockcreatecon ("unconfined_u:unconfined_r:guestfs_t:s0");
sock = socket (...);

and then we'd allow svirt_t to connect to guestfs_t.  Although
this is similar to allowing svirt_t to connect to unconfined_t
(since the libguestfs process is effectively still unconfined)
it does at least prevent a rogue sVirt process from connecting
to random unconfined_t sockets.

*** Bug 872511 has been marked as a duplicate of this bug. ***

One potential fix for this would be create a new type guestfs_socket_t for the socket and then allow svirt_t to connect to it.

guestfs could do something like the following pseudo code.

getcon(&scon)
con=context_new(scon)
context_set_type(con,"guestfs_socket_t")
setsockcreatecon(context_str(con))
sock = socket()
context_free(con)
freecon(scon)

# cat mypol.te
policy_module(mypol,1.0)
gen_require(`
type svirt_t;
role unconfined_r;
')
type guestfs_socket_t;
role unconfined_r types guestfs_socket_t;

allow svirt_t guestfs_socket_t:unix_stream_socket connectto;

# make -f /usr/share/selinux/devel/Makefile
# semodule -i mypol.pp

Of course we would have to formalize how guestfs gets the name of its socket, if this works...

(In reply to comment #16)
> One potential fix for this would be create a new type guestfs_socket_t for
> the socket and then allow svirt_t to connect to it.
> 
> guestfs could do something like the following pseudo code.
> 
> getcon(&scon)
> con=context_new(scon)
> context_set_type(con,"guestfs_socket_t")
> setsockcreatecon(context_str(con))
> sock = socket()
> context_free(con)
> freecon(scon)
> 
> # cat mypol.te
> policy_module(mypol,1.0)
> gen_require(`
> type svirt_t;
> role unconfined_r;
> ')
> type guestfs_socket_t;
> role unconfined_r types guestfs_socket_t;
> 
> allow svirt_t guestfs_socket_t:unix_stream_socket connectto;
> 
> # make -f /usr/share/selinux/devel/Makefile
> # semodule -i mypol.pp

I added all of this to libguestfs.  The setsockcreatecon
call is fine, but the subsequent socket call fails with -EPERM:

libguestfs: error: socket: Permission denied

and this AVC:

type=SYSCALL msg=audit(11/12/12 10:43:55.237:2060) : arch=x86_64 syscall=socket success=no exit=-13(Permission denied) a0=local a1=SOCK_STREAM|SOCK_CLOEXEC a2=ip a3=0x7fff6702d380 items=0 ppid=26899 pid=26957 auid=rjones uid=rjones gid=rjones euid=rjones suid=rjones fsuid=rjones egid=rjones sgid=rjones fsgid=rjones tty=pts3 ses=2 comm=lt-libguestfs-t exe=/home/rjones/d/libguestfs/test-tool/.libs/lt-libguestfs-test-tool subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null) 
type=AVC msg=audit(11/12/12 10:43:55.237:2060) : avc:  denied  { create } for  pid=26957 comm=lt-libguestfs-t scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tcontext=unconfined_u:unconfined_r:guestfs_socket_t:s0-s0:c0.c1023 tclass=unix_stream_socket 

To cut a long story short, I updated the policy until I finally
got everything working.  Here is my updated policy:

---------------------------------------------
policy_module(mypol,1.0)
gen_require(`
type svirt_t;
type unconfined_t;
type svirt_image_t;
role unconfined_r;
')
type guestfs_socket_t;
role unconfined_r types guestfs_socket_t;

allow svirt_t guestfs_socket_t:unix_stream_socket connectto;

# necessary, but why??? - maybe a bug in libvirt labelling code?
allow svirt_t svirt_image_t:sock_file write;

allow unconfined_t guestfs_socket_t:unix_stream_socket { create bind listen accept write read };
---------------------------------------------

(In reply to comment #17)
> Of course we would have to formalize how guestfs gets the name of its
> socket, if this works...

Why can't we just create sockets in our tmpdir and label them?  I'm quite
happy to add this setsockcreatecon call to libguestfs, and continuing
to use /tmp means (a) we don't have to make invasive changes to the
code to create a third temporary location, (b) we don't have to worry
about naming issues, and (c) the sockets will get cleaned up automatically
(if we fail to) by the tmp cleaner.  This seems a much better solution
all round.

Created attachment 661371 [details]
0001-launch-libvirt-Label-sockets-with-guestfs_socket_t-R.patch

Proposed labelling patch for libguestfs.

I changed to use svirt_socket_t rather then guestfs_socket_t so this solution could be generalized.  selinux-policy-3.11.1-63.fc18 will include this policy.

Way back when we created svirt we created virtual_domain_context without using named value pairs.

cat /etc/selinux/targeted/contexts/virtual_domain_context 
system_u:system_r:svirt_t:s0

for lxc we have fixed this.

cat /etc/selinux/targeted/contexts/lxc_contexts 
process = "system_u:system_r:svirt_lxc_net_t:s0"
file = "system_u:object_r:svirt_lxc_file_t:s0"
content = "system_u:object_r:virt_var_lib_t:s0"


So currently we have no good way of allowing the polciy writer to establish the svirt_socket_t so that libguest could discover it. I am not thrilled with hard coding it, although it will probably always be there.

Can we hard code it for now, and kick the real solution into
a more distant release?  I'm pretty keen to get this working
in Fedora 18 and what we have now does work.

What did you think about this?

  # necessary, but why??? - maybe a bug in libvirt labelling code?
  allow svirt_t svirt_image_t:sock_file write;

The corresponding AVC was:

type=SYSCALL msg=audit(11/12/12 10:54:06.775:2125) : arch=x86_64 syscall=connect
 success=no exit=-13(Permission denied) a0=0x4 a1=0x7fffc86beb60 a2=0x6e a3=0x3b
 items=0 ppid=1 pid=31709 auid=rjones uid=rjones gid=rjones euid=rjones suid=rjo
nes fsuid=rjones egid=rjones sgid=rjones fsgid=rjones tty=(none) ses=2 comm=qemu
-kvm exe=/usr/bin/qemu-kvm subj=unconfined_u:system_r:svirt_t:s0:c334,c877 key=(
null) 
type=AVC msg=audit(11/12/12 10:54:06.775:2125) : avc:  denied  { write } for  pi
d=31709 comm=qemu-kvm name=console.sock dev="dm-3" ino=806792 scontext=unconfine
d_u:system_r:svirt_t:s0:c334,c877 tcontext=unconfined_u:object_r:svirt_image_t:s
0:c334,c877 tclass=sock_file

I added it.  Basically the way he qemu (svirt_t) process communicates with guestfs is through console.sock, which libvirt has labeled svirt_image_t.

So svirt_t need to be able to read/write a sock_file labeled svirt_image_t and connectto the socket of the guestfs process.  Originally this was unconfined_t, with your fix it will be svirt_socket_t.


It is fine if you hard code it.  I am actually hoping Dan B, comments on this,  Whether or not we can change the contents of the virtual_domain_contexts file.

(In reply to comment #20)
> I changed to use svirt_socket_t rather then guestfs_socket_t so this
> solution could be generalized.  selinux-policy-3.11.1-63.fc18 will include
> this policy.

Looks like that change missed the -63 package?

Sorry, ignore that.  It does look like it's there, it
just wasn't mentioned in the changelog.  Now testing ...

I tested this on my machine (after removing 'mypol'), and
it all seems to work.  Therefore I'm closing this bug.