Re: [PATCH] nbd/server: Add --selinux-label option

2021-07-22 Thread Daniel P . Berrangé
On Thu, Jul 22, 2021 at 05:32:40PM +0100, Richard W.M. Jones wrote:
> Under SELinux, Unix domain sockets have two labels.  One is on the
> disk and can be set with commands such as chcon(1).  There is a
> different label stored in memory (called the process label).  This can
> only be set by the process creating the socket.  When using SELinux +
> SVirt and wanting qemu to be able to connect to a qemu-nbd instance,
> you must set both labels correctly first.
> 
> For qemu-nbd the options to set the second label are awkward.  You can
> create the socket in a wrapper program and then exec into qemu-nbd.
> Or you could try something with LD_PRELOAD.
> 
> This commit adds the ability to set the label straightforwardly on the
> command line, via the new --selinux-label flag.  (The name of the flag
> is the same as the equivalent nbdkit option.)
> 
> A worked example showing how to use the new option can be found in
> this bug: https://bugzilla.redhat.com/show_bug.cgi?id=1984938
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1984938
> Signed-off-by: Richard W.M. Jones 
> ---
>  configure |  9 -
>  meson.build   | 10 +-
>  meson_options.txt |  3 +++
>  qemu-nbd.c| 33 +
>  4 files changed, 53 insertions(+), 2 deletions(-)


> diff --git a/meson.build b/meson.build
> index 2f377098d7..2d7206233e 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1064,6 +1064,11 @@ keyutils = dependency('libkeyutils', required: false,
>  
>  has_gettid = cc.has_function('gettid')
>  
> +# libselinux
> +selinux = dependency('libselinux',
> + required: get_option('selinux'),
> + method: 'pkg-config', kwargs: static_kwargs)
> +

For the new build dep we'll need updated package lists in
tests/docker/dockerfiles. For centos, fedra ,opensuse
add libselinux-devel and for ubuntu 18/20 add libselinux-dev

That'll make sure this new code gets tested in CI.


The rest looks ok to me

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




[PATCH] nbd/server: Add --selinux-label option

2021-07-22 Thread Richard W.M. Jones
Under SELinux, Unix domain sockets have two labels.  One is on the
disk and can be set with commands such as chcon(1).  There is a
different label stored in memory (called the process label).  This can
only be set by the process creating the socket.  When using SELinux +
SVirt and wanting qemu to be able to connect to a qemu-nbd instance,
you must set both labels correctly first.

For qemu-nbd the options to set the second label are awkward.  You can
create the socket in a wrapper program and then exec into qemu-nbd.
Or you could try something with LD_PRELOAD.

This commit adds the ability to set the label straightforwardly on the
command line, via the new --selinux-label flag.  (The name of the flag
is the same as the equivalent nbdkit option.)

A worked example showing how to use the new option can be found in
this bug: https://bugzilla.redhat.com/show_bug.cgi?id=1984938

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1984938
Signed-off-by: Richard W.M. Jones 
---
 configure |  9 -
 meson.build   | 10 +-
 meson_options.txt |  3 +++
 qemu-nbd.c| 33 +
 4 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index b5965b159f..7e04bd485f 100755
--- a/configure
+++ b/configure
@@ -443,6 +443,7 @@ fuse="auto"
 fuse_lseek="auto"
 multiprocess="auto"
 slirp_smbd="$default_feature"
+selinux="auto"
 
 malloc_trim="auto"
 gio="$default_feature"
@@ -1578,6 +1579,10 @@ for opt do
   ;;
   --disable-slirp-smbd) slirp_smbd=no
   ;;
+  --enable-selinux) selinux="enabled"
+  ;;
+  --disable-selinux) selinux="disabled"
+  ;;
   *)
   echo "ERROR: unknown option $opt"
   echo "Try '$0 --help' for more information"
@@ -1965,6 +1970,7 @@ disabled with --disable-FEATURE, default is enabled if 
available
   multiprocessOut of process device emulation support
   gio libgio support
   slirp-smbd  use smbd (at path --smbd=*) in slirp networking
+  selinux SELinux support in qemu-nbd
 
 NOTE: The object files are built at the place where configure is launched
 EOF
@@ -5220,7 +5226,8 @@ if test "$skip_meson" = no; then
 -Dattr=$attr -Ddefault_devices=$default_devices 
-Dvirglrenderer=$virglrenderer \
 -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
 -Dvhost_user_blk_server=$vhost_user_blk_server 
-Dmultiprocess=$multiprocess \
--Dfuse=$fuse -Dfuse_lseek=$fuse_lseek 
-Dguest_agent_msi=$guest_agent_msi -Dbpf=$bpf\
+-Dfuse=$fuse -Dfuse_lseek=$fuse_lseek 
-Dguest_agent_msi=$guest_agent_msi -Dbpf=$bpf \
+-Dselinux=$selinux \
 $(if test "$default_features" = no; then echo 
"-Dauto_features=disabled"; fi) \
-Dtcg_interpreter=$tcg_interpreter \
 $cross_arg \
diff --git a/meson.build b/meson.build
index 2f377098d7..2d7206233e 100644
--- a/meson.build
+++ b/meson.build
@@ -1064,6 +1064,11 @@ keyutils = dependency('libkeyutils', required: false,
 
 has_gettid = cc.has_function('gettid')
 
+# libselinux
+selinux = dependency('libselinux',
+ required: get_option('selinux'),
+ method: 'pkg-config', kwargs: static_kwargs)
+
 # Malloc tests
 
 malloc = []
@@ -1291,6 +1296,7 @@ config_host_data.set('CONFIG_FUSE', fuse.found())
 config_host_data.set('CONFIG_FUSE_LSEEK', fuse_lseek.found())
 config_host_data.set('CONFIG_X11', x11.found())
 config_host_data.set('CONFIG_CFI', get_option('cfi'))
+config_host_data.set('CONFIG_SELINUX', selinux.found())
 config_host_data.set('QEMU_VERSION', '"@0@"'.format(meson.project_version()))
 config_host_data.set('QEMU_VERSION_MAJOR', 
meson.project_version().split('.')[0])
 config_host_data.set('QEMU_VERSION_MINOR', 
meson.project_version().split('.')[1])
@@ -2739,7 +2745,8 @@ if have_tools
   qemu_io = executable('qemu-io', files('qemu-io.c'),
  dependencies: [block, qemuutil], install: true)
   qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'),
-   dependencies: [blockdev, qemuutil, gnutls], install: true)
+   dependencies: [blockdev, qemuutil, gnutls, selinux],
+   install: true)
 
   subdir('storage-daemon')
   subdir('contrib/rdmacm-mux')
@@ -3104,6 +3111,7 @@ summary_info += {'libpmem support':   libpmem.found()}
 summary_info += {'libdaxctl support': libdaxctl.found()}
 summary_info += {'libudev':   libudev.found()}
 summary_info += {'FUSE lseek':fuse_lseek.found()}
+summary_info += {'selinux':   selinux.found()}
 summary(summary_info, bool_yn: true, section: 'Dependencies')
 
 if not supported_cpus.contains(cpu)
diff --git a/meson_options.txt b/meson_options.txt
index a9a9b8f4c6..a5938500a3 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -155,3 +155,6 @@ option('slirp', type: 'combo', value: 'auto',
 option('fdt', type: 'combo', value: 'auto',
choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
description: 'Whether and how to 

[PATCH] nbd/server: Add --selinux-label option

2021-07-22 Thread Richard W.M. Jones
https://bugzilla.redhat.com/show_bug.cgi?id=1984938

The purpose of the patch is explained in the commit message / bug.  In
the cover I want to explain a couple of design choices.

If libselinux isn't available at build time then the --selinux-label
option is still present.  It does not appear in the qemu-nbd --help
output.  If you still use it, it is ignored.  (By contrast nbdkit will
give an error if you try to use the option without having SELinux
support.  It's not clear which is better.)

We give an error if setsockcreatecon_raw fails.  In theory we could
ignore this error (warning?) and keep going.  Either SELinux would
later reject clients or it wouldn't.

Rich.