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

[Eric Blake <eblake@redhat.com>]

On Fri, Jul 23, 2021 at 11:33:03AM +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 <rjones@redhat.com>
> ---

I'm making one tweak to your patch before sending the pull request:

> +++ b/qemu-nbd.c
> @@ -64,6 +68,7 @@
>  #define QEMU_NBD_OPT_FORK          263
>  #define QEMU_NBD_OPT_TLSAUTHZ      264
>  #define QEMU_NBD_OPT_PID_FILE      265
> +#define QEMU_NBD_OPT_SELINUX_LABEL 266
>  
>  #define MBR_SIZE 512
>  
> @@ -116,6 +121,9 @@ static void usage(const char *name)
>  "  --fork                    fork off the server process and exit the parent\n"
>  "                            once the server is running\n"
>  "  --pid-file=PATH           store the server's process ID in the given file\n"
> +#ifdef CONFIG_SELINUX
> +"  --selinux-label=LABEL     set SELinux process label on listening socket\n"
> +#endif

The new option is only conditionally advertised under --help (qemu-nbd
lacks a stable machine-parseable output, so scraping --help output
will have to do for now)...

>  #if HAVE_NBD_DEVICE
>  "\n"
>  "Kernel NBD client support:\n"
> @@ -532,6 +540,8 @@ int main(int argc, char **argv)
>          { "trace", required_argument, NULL, 'T' },
>          { "fork", no_argument, NULL, QEMU_NBD_OPT_FORK },
>          { "pid-file", required_argument, NULL, QEMU_NBD_OPT_PID_FILE },
> +        { "selinux-label", required_argument, NULL,
> +          QEMU_NBD_OPT_SELINUX_LABEL },

...but is unconditionally supported as a long option even when support
was not compiled in...

>          { NULL, 0, NULL, 0 }
>      };
>      int ch;
> @@ -558,6 +568,7 @@ int main(int argc, char **argv)
>      int old_stderr = -1;
>      unsigned socket_activation;
>      const char *pid_file_name = NULL;
> +    const char *selinux_label = NULL;
>      BlockExportOptions *export_opts;
>  
>  #ifdef CONFIG_POSIX
> @@ -747,6 +758,9 @@ int main(int argc, char **argv)
>          case QEMU_NBD_OPT_PID_FILE:
>              pid_file_name = optarg;
>              break;
> +        case QEMU_NBD_OPT_SELINUX_LABEL:
> +            selinux_label = optarg;
> +            break;
>          }
>      }
>  
> @@ -938,6 +952,16 @@ int main(int argc, char **argv)
>          } else {
>              backlog = MIN(shared, SOMAXCONN);
>          }
> +        if (sockpath && selinux_label) {
> +#ifdef CONFIG_SELINUX
> +            if (setsockcreatecon_raw(selinux_label) == -1) {
> +                error_report("Cannot set SELinux socket create context "
> +                             "to %s: %s",
> +                             selinux_label, strerror(errno));
> +                exit(EXIT_FAILURE);
> +            }
> +#endif

...but here we silently ignore it if support is not compiled in.
Better is to issue an error message about using an unsupported option,
so I'll squash this in:

diff --git i/qemu-nbd.c w/qemu-nbd.c
index 5dc82c419255..94f8ec07c064 100644
--- i/qemu-nbd.c
+++ w/qemu-nbd.c
@@ -962,6 +962,9 @@ int main(int argc, char **argv)
                              selinux_label, strerror(errno));
                 exit(EXIT_FAILURE);
             }
+#else
+            error_report("SELinux support not enabled in this binary");
+            exit(EXIT_FAILURE);
 #endif
         }
         saddr = nbd_build_socket_address(sockpath, bindto, port);
@@ -978,6 +981,9 @@ int main(int argc, char **argv)
                              strerror(errno));
                 exit(EXIT_FAILURE);
             }
+#else
+            error_report("SELinux support not enabled in this binary");
+            exit(EXIT_FAILURE);
 #endif
         }
     } else {


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org