Libguestfs - May 2019 - [PATCH] Use proper label for nbdkit sockets

While svirt_t can be used for sockets it does not always guarantee that it will
be accessible from a virtual machine.  The VM might be running under svirt_tcg_t
context which will need a svirt_tcg_t label on the socket in order to access it.

There is, however, another label, svirt_socket_t, which is accessible from
virt_domain:

  # sesearch -A -s svirt_t -c unix_stream_socket -p connectto
  ...
  allow virt_domain svirt_socket_t:unix_stream_socket { ... connectto ... };
  ...

And virt_domain is a type attribute of both svirt_t and svirt_tcg_t:

  # seinfo -x -a virt_domain
  Type Attributes: 1
     attribute virt_domain;
          svirt_t
          svirt_tcg_t

Resolves: https://bugzilla.redhat.com/1698437

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 v2v/input_libvirt_vddk.ml | 2 +-
 v2v/output_rhv_upload.ml  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/v2v/input_libvirt_vddk.ml b/v2v/input_libvirt_vddk.ml
index e2efef842e8e..4e36ff71f783 100644
--- a/v2v/input_libvirt_vddk.ml
+++ b/v2v/input_libvirt_vddk.ml
@@ -286,7 +286,7 @@ object
       add_arg "--newstyle";         (* use newstyle NBD protocol *)
       add_arg "--exportname"; add_arg "/";
       if have_selinux then (        (* label the socket so qemu can open it *)
-        add_arg "--selinux-label"; add_arg
"system_u:object_r:svirt_t:s0"
+        add_arg "--selinux-label"; add_arg
"system_u:object_r:svirt_socket_t:s0"
       );
 
       (* Name of the plugin.  Everything following is a plugin parameter. *)
diff --git a/v2v/output_rhv_upload.ml b/v2v/output_rhv_upload.ml
index 0709c8152c4e..5bc6a400725b 100644
--- a/v2v/output_rhv_upload.ml
+++ b/v2v/output_rhv_upload.ml
@@ -213,7 +213,7 @@ See also the virt-v2v-output-rhv(1) manual.")
     let args        (* label the socket so qemu can open it *)
       if have_selinux then
-        args @ ["--selinux-label";
"system_u:object_r:svirt_t:s0"]
+        args @ ["--selinux-label";
"system_u:object_r:svirt_socket_t:s0"]
       else args in
     args in
 
-- 
2.21.0

On Mon, May 27, 2019 at 01:30:05PM +0200, Martin Kletzander
wrote:
> While svirt_t can be used for sockets it does not always guarantee that it
will
> be accessible from a virtual machine.  The VM might be running under
svirt_tcg_t
> context which will need a svirt_tcg_t label on the socket in order to
access it.
I don't really know enough about SELinux or the sVirt policy to
comment on this, but it's plausible so I'll push it soon, thanks.

Rich.
> There is, however, another label, svirt_socket_t, which is accessible from
> virt_domain:
> 
>   # sesearch -A -s svirt_t -c unix_stream_socket -p connectto
>   ...
>   allow virt_domain svirt_socket_t:unix_stream_socket { ... connectto ...
};
>   ...
> 
> And virt_domain is a type attribute of both svirt_t and svirt_tcg_t:
> 
>   # seinfo -x -a virt_domain
>   Type Attributes: 1
>      attribute virt_domain;
>           svirt_t
>           svirt_tcg_t
> 
> Resolves: https://bugzilla.redhat.com/1698437
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  v2v/input_libvirt_vddk.ml | 2 +-
>  v2v/output_rhv_upload.ml  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/v2v/input_libvirt_vddk.ml b/v2v/input_libvirt_vddk.ml
> index e2efef842e8e..4e36ff71f783 100644
> --- a/v2v/input_libvirt_vddk.ml
> +++ b/v2v/input_libvirt_vddk.ml
> @@ -286,7 +286,7 @@ object
>        add_arg "--newstyle";         (* use newstyle NBD protocol
*)
>        add_arg "--exportname"; add_arg "/";
>        if have_selinux then (        (* label the socket so qemu can open
it *)
> -        add_arg "--selinux-label"; add_arg
"system_u:object_r:svirt_t:s0"
> +        add_arg "--selinux-label"; add_arg
"system_u:object_r:svirt_socket_t:s0"
>        );
>  
>        (* Name of the plugin.  Everything following is a plugin parameter.
*)
> diff --git a/v2v/output_rhv_upload.ml b/v2v/output_rhv_upload.ml
> index 0709c8152c4e..5bc6a400725b 100644
> --- a/v2v/output_rhv_upload.ml
> +++ b/v2v/output_rhv_upload.ml
> @@ -213,7 +213,7 @@ See also the virt-v2v-output-rhv(1) manual.")
>      let args >        (* label the socket so qemu can open it *)
>        if have_selinux then
> -        args @ ["--selinux-label";
"system_u:object_r:svirt_t:s0"]
> +        args @ ["--selinux-label";
"system_u:object_r:svirt_socket_t:s0"]
>        else args in
>      args in
>  
> -- 
> 2.21.0
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On Tue, May 28, 2019 at 10:14:01AM +0100, Richard W.M. Jones
wrote:
>On Mon, May 27, 2019 at 01:30:05PM +0200, Martin Kletzander wrote:
>> While svirt_t can be used for sockets it does not always guarantee that
it will
>> be accessible from a virtual machine.  The VM might be running under
svirt_tcg_t
>> context which will need a svirt_tcg_t label on the socket in order to
access it.
>
>I don't really know enough about SELinux or the sVirt policy to
>comment on this, but it's plausible so I'll push it soon, thanks.
>
I dug through this for quite a while, but I still cannot see why is there a
difference between svirt_t and svirt_tcg_t (except maybe /dev/kvm context).

But I created a build with this patch in for the reporter of the bug and they
tested it and it worked. I also took some time to look at the policies (and had
to learn what a "type attribute" is, see below) to make sure this is
as close to
how it should work as I, myself, can get.

One thing I would like to get some feedback on, although I believe it is not
required for this patch's justification, is some information about what each
context is supposed to mean or be used for. I was pretty confident about what
svirt_t is supposed to be used for, but when I have seen that svirt_t is defined
for the unix_stream_socket target class as well, while being defined for
virt_domain type attribute as well, does not really make sense to me.

Anyway, thanks.
Martin
>Rich.
>
>> There is, however, another label, svirt_socket_t, which is accessible
from
>> virt_domain:
>>
>>   # sesearch -A -s svirt_t -c unix_stream_socket -p connectto
>>   ...
>>   allow virt_domain svirt_socket_t:unix_stream_socket { ... connectto
... };
>>   ...
>>
>> And virt_domain is a type attribute of both svirt_t and svirt_tcg_t:
>>
>>   # seinfo -x -a virt_domain
>>   Type Attributes: 1
>>      attribute virt_domain;
>>           svirt_t
>>           svirt_tcg_t
>>
>> Resolves: https://bugzilla.redhat.com/1698437
>>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>>  v2v/input_libvirt_vddk.ml | 2 +-
>>  v2v/output_rhv_upload.ml  | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/v2v/input_libvirt_vddk.ml b/v2v/input_libvirt_vddk.ml
>> index e2efef842e8e..4e36ff71f783 100644
>> --- a/v2v/input_libvirt_vddk.ml
>> +++ b/v2v/input_libvirt_vddk.ml
>> @@ -286,7 +286,7 @@ object
>>        add_arg "--newstyle";         (* use newstyle NBD
protocol *)
>>        add_arg "--exportname"; add_arg "/";
>>        if have_selinux then (        (* label the socket so qemu can
open it *)
>> -        add_arg "--selinux-label"; add_arg
"system_u:object_r:svirt_t:s0"
>> +        add_arg "--selinux-label"; add_arg
"system_u:object_r:svirt_socket_t:s0"
>>        );
>>
>>        (* Name of the plugin.  Everything following is a plugin
parameter. *)
>> diff --git a/v2v/output_rhv_upload.ml b/v2v/output_rhv_upload.ml
>> index 0709c8152c4e..5bc6a400725b 100644
>> --- a/v2v/output_rhv_upload.ml
>> +++ b/v2v/output_rhv_upload.ml
>> @@ -213,7 +213,7 @@ See also the virt-v2v-output-rhv(1) manual.")
>>      let args >>        (* label the socket so qemu can open it
*)
>>        if have_selinux then
>> -        args @ ["--selinux-label";
"system_u:object_r:svirt_t:s0"]
>> +        args @ ["--selinux-label";
"system_u:object_r:svirt_socket_t:s0"]
>>        else args in
>>      args in
>>
>> --
>> 2.21.0
>
>-- 
>Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
>Read my programming and virtualization blog: http://rwmj.wordpress.com
>libguestfs lets you edit virtual machines.  Supports shell scripting,
>bindings from many languages.  http://libguestfs.org