This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 853393 - libvirt doesn't label console, serial sockets
libvirt doesn't label console, serial sockets
Status: CLOSED CURRENTRELEASE
Product: Virtualization Tools
Classification: Community
Component: libguestfs (Show other bugs)
unspecified
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Richard W.M. Jones
: Reopened
: 872511 (view as bug list)
Depends On:
Blocks: 857453 888502
  Show dependency treegraph
 
Reported: 2012-08-31 07:38 EDT by Richard W.M. Jones
Modified: 2012-12-18 14:31 EST (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 857453 888502 (view as bug list)
Environment:
Last Closed: 2012-12-12 12:32:52 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
libvirtd.log (1.46 MB, text/x-log)
2012-09-26 08:21 EDT, Richard W.M. Jones
no flags Details
0001-launch-libvirt-Label-sockets-with-guestfs_socket_t-R.patch (3.82 KB, patch)
2012-12-11 06:04 EST, Richard W.M. Jones
no flags Details | Diff

  None (edit)
Description Richard W.M. Jones 2012-08-31 07:38:30 EDT
Description of problem:

In libguestfs with the libvirt backend we create sockets
to talk to the daemon and console like this:

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

where "/tmp/libguestfs3vK8dN" is a temporary directory created
by libguestfs.

Note that these socket files are created by libguestfs.  Qemu has
to connect to them.

libvirt labels most things -- disks, kernel etc -- but it doesn't
label the console socket (and possibly not the virtio socket either,
but see below).  Therefore when SELinux is Enforcing, qemu
cannot connect to the console socket.

If SELinux is Permissive then we get the following AVC:

type=AVC msg=audit(1346412361.391:3414): avc:  denied  { connectto } for  pid=24102 comm="qemu-kvm" path="/tmp/libguestfs3vK8dN/console.sock" scontext=unconfined_u:unconfined_r:svirt_t:s0:c471,c928 tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=unix_stream_socket

(By the way, no AVC is shown for the virtio-serial socket
'guestfsd.sock'.  I'm not clear why that is, because I can't
see any evidence that libvirt is labelling it.)

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

libvirt-0.10.0-1.fc19.x86_64

How reproducible:

100%

Steps to Reproduce:
1. Run 'libguestfs-test-tool' with and without SELinux Enforcing.
Comment 1 Richard W.M. Jones 2012-09-14 09:32:23 EDT
Patch posted upstream:

https://www.redhat.com/archives/libvir-list/2012-September/msg01031.html
Comment 2 Richard W.M. Jones 2012-09-14 10:09:08 EDT
Second version:

https://www.redhat.com/archives/libvir-list/2012-September/msg01037.html
Comment 3 Richard W.M. Jones 2012-09-20 11:30:58 EDT
Third version:

https://www.redhat.com/archives/libvir-list/2012-September/msg01478.html
Comment 5 Jiri Denemark 2012-09-21 15:53:47 EDT
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.
Comment 6 Richard W.M. Jones 2012-09-26 08:21:07 EDT
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.
Comment 7 Richard W.M. Jones 2012-09-26 08:27:32 EDT
libvirt-0.10.2-3.fc18.x86_64
selinux-policy-3.11.1-25.fc18.noarch
Comment 8 Richard W.M. Jones 2012-09-26 08:40:06 EDT
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.
Comment 9 Daniel Berrange 2012-09-26 09:56:20 EDT
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.
Comment 10 Daniel Berrange 2012-09-26 10:04:20 EDT
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) ?
Comment 11 Daniel Walsh 2012-09-26 17:40:38 EDT
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.
Comment 12 Richard W.M. Jones 2012-09-26 17:47:16 EDT
(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).
Comment 13 Richard W.M. Jones 2012-12-07 12:01:44 EST
eparis confirmed that setsockcreatecon is thread safe.
Comment 14 Richard W.M. Jones 2012-12-07 12:29:18 EST
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.
Comment 15 Richard W.M. Jones 2012-12-07 12:30:02 EST
*** Bug 872511 has been marked as a duplicate of this bug. ***
Comment 16 Daniel Walsh 2012-12-07 12:38:37 EST
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
Comment 17 Daniel Walsh 2012-12-07 12:39:17 EST
Of course we would have to formalize how guestfs gets the name of its socket, if this works...
Comment 18 Richard W.M. Jones 2012-12-11 06:03:44 EST
(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.
Comment 19 Richard W.M. Jones 2012-12-11 06:04:47 EST
Created attachment 661371 [details]
0001-launch-libvirt-Label-sockets-with-guestfs_socket_t-R.patch

Proposed labelling patch for libguestfs.
Comment 20 Daniel Walsh 2012-12-11 14:42:10 EST
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.
Comment 21 Richard W.M. Jones 2012-12-11 15:30:33 EST
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
Comment 22 Daniel Walsh 2012-12-11 15:50:16 EST
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.
Comment 23 Richard W.M. Jones 2012-12-12 12:11:20 EST
(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?
Comment 24 Richard W.M. Jones 2012-12-12 12:12:18 EST
Sorry, ignore that.  It does look like it's there, it
just wasn't mentioned in the changelog.  Now testing ...
Comment 25 Richard W.M. Jones 2012-12-12 12:32:52 EST
I tested this on my machine (after removing 'mypol'), and
it all seems to work.  Therefore I'm closing this bug.

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