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

[Vladimir Sementsov-Ogievskiy]

9/30/21 11:47, 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>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>

this should be Reviewed-by?

> ---
>   configure                                     |  8 +++-
>   meson.build                                   | 10 ++++-
>   meson_options.txt                             |  3 ++
>   qemu-nbd.c                                    | 39 +++++++++++++++++++

[..]

>       }
>   
> @@ -938,6 +952,19 @@ int main(int argc, char **argv)
>           } else {
>               backlog = MIN(shared, SOMAXCONN);
>           }
> +        if (sockpath && selinux_label) {

1. Why only for unix sockets?

2. If [1] is intentional, why silently ignore the new option for ip sockets, 
shouldn't error-out instead?

> +#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);
> +            }
> +#else
> +            error_report("SELinux support not enabled in this binary");
> +            exit(EXIT_FAILURE);
> +#endif
> +        }
>           saddr = nbd_build_socket_address(sockpath, bindto, port);
>           if (qio_net_listener_open_sync(server, saddr, backlog,
>                                          &local_err) < 0) {
> @@ -945,6 +972,18 @@ int main(int argc, char **argv)
>               error_report_err(local_err);
>               exit(EXIT_FAILURE);
>           }
> +        if (sockpath && selinux_label) {
> +#ifdef CONFIG_SELINUX
> +            if (setsockcreatecon_raw(NULL) == -1) {
> +                error_report("Cannot clear SELinux socket create context: %s",
> +                             strerror(errno));
> +                exit(EXIT_FAILURE);
> +            }
> +#else
> +            error_report("SELinux support not enabled in this binary");
> +            exit(EXIT_FAILURE);
> +#endif
> +        }
>       } else {
>           size_t i;
>           /* See comment in check_socket_activation above. */

[..]


--
Best regards,
Vladimir