| Message ID | 20210723103303.1731437-2-rjones@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] nbd/server: Add --selinux-label option | expand |
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> > --- > configure | 9 ++++- > meson.build | 10 +++++- > meson_options.txt | 3 ++ > qemu-nbd.c | 33 +++++++++++++++++++ > tests/docker/dockerfiles/centos8.docker | 1 + > tests/docker/dockerfiles/fedora.docker | 1 + > tests/docker/dockerfiles/opensuse-leap.docker | 1 + > tests/docker/dockerfiles/ubuntu1804.docker | 1 + > tests/docker/dockerfiles/ubuntu2004.docker | 1 + > 9 files changed, 58 insertions(+), 2 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel
Am 23.07.2021 um 12:33 hat Richard W.M. Jones geschrieben: > 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 suppose this would also be relevant for the built-in NBD server, especially in the context of qemu-storage-daemon? If so, is this something specific to NBD sockets, or would it actually make sense to have it as a generic option in UnixSocketAddress? Kevin
On Fri, Jul 23, 2021 at 06:18:55PM +0200, Kevin Wolf wrote: > Am 23.07.2021 um 12:33 hat Richard W.M. Jones geschrieben: > > 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 suppose this would also be relevant for the built-in NBD server, > especially in the context of qemu-storage-daemon? > > If so, is this something specific to NBD sockets, or would it actually > make sense to have it as a generic option in UnixSocketAddress? For other NBD sockets, most likely. I'm not sure about Unix sockets in general (as in: I know they also have the two label thing, but I don't know if there's a situation where SVirt protects other sockets apart from NBD sockets). I'm sure Dan will know ... By the way although it appears that setsockcreatecon_raw is setting a global flag, it seems to actually use a thread-local variable, so implementing this (although still ugly) would not require locks. https://github.com/SELinuxProject/selinux/blob/32611aea6543e3a8f32635857e37b4332b0b5c99/libselinux/src/procattr.c#L347 Rich.
On Fri, Jul 23, 2021 at 06:18:55PM +0200, Kevin Wolf wrote: > Am 23.07.2021 um 12:33 hat Richard W.M. Jones geschrieben: > > 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 suppose this would also be relevant for the built-in NBD server, > especially in the context of qemu-storage-daemon? It depends on the usage scenario really. nbdkit / qemu-nbd are not commonly run under any SELinux policy, so then end up being unconfined_t. A QEMU NBD client can't connect to an unconfined_t socket, so we need to override it with this arg. In the case of qemu system emulator, under libvirt, it will already have a svirt_t type, so in that case there is no need to override the type for the socket. For qsd there's not really any strong practice established but i expect most current usage is unconfined_t too and would benefit from setting label. > If so, is this something specific to NBD sockets, or would it actually > make sense to have it as a generic option in UnixSocketAddress? It is applicable to inet sockets too in fact. Regards, Daniel
On Fri, Jul 23, 2021 at 11:47:51AM +0100, Daniel P. Berrangé wrote: > 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> > > --- > > configure | 9 ++++- > > meson.build | 10 +++++- > > meson_options.txt | 3 ++ > > qemu-nbd.c | 33 +++++++++++++++++++ > > tests/docker/dockerfiles/centos8.docker | 1 + > > tests/docker/dockerfiles/fedora.docker | 1 + > > tests/docker/dockerfiles/opensuse-leap.docker | 1 + > > tests/docker/dockerfiles/ubuntu1804.docker | 1 + > > tests/docker/dockerfiles/ubuntu2004.docker | 1 + > > 9 files changed, 58 insertions(+), 2 deletions(-) > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Thanks. This is a new feature, so it doesn't qualify for inclusion in 6.1, but I'm queuing it through my NBD tree to go in as soon as upstream reopens for 6.2.
On Fri, Jul 23, 2021 at 05:38:06PM +0100, Daniel P. Berrangé wrote: > On Fri, Jul 23, 2021 at 06:18:55PM +0200, Kevin Wolf wrote: > > Am 23.07.2021 um 12:33 hat Richard W.M. Jones geschrieben: > > > 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 suppose this would also be relevant for the built-in NBD server, > > especially in the context of qemu-storage-daemon? > > It depends on the usage scenario really. nbdkit / qemu-nbd are > not commonly run under any SELinux policy, so then end up being > unconfined_t. A QEMU NBD client can't connect to an unconfined_t > socket, so we need to override it with this arg. > > In the case of qemu system emulator, under libvirt, it will > already have a svirt_t type, so in that case there is no need > to override the type for the socket. > > For qsd there's not really any strong practice established > but i expect most current usage is unconfined_t too and > would benefit from setting label. > > > If so, is this something specific to NBD sockets, or would it actually > > make sense to have it as a generic option in UnixSocketAddress? > > It is applicable to inet sockets too in fact. So now that 6.2 is open, should I queue the patch as is, or wait for a v3 that makes the option more generic to all socket usage?
Ping On Wed, Aug 25, 2021 at 02:35:04PM -0500, Eric Blake wrote: > On Fri, Jul 23, 2021 at 05:38:06PM +0100, Daniel P. Berrangé wrote: > > On Fri, Jul 23, 2021 at 06:18:55PM +0200, Kevin Wolf wrote: > > > Am 23.07.2021 um 12:33 hat Richard W.M. Jones geschrieben: > > > > 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 suppose this would also be relevant for the built-in NBD server, > > > especially in the context of qemu-storage-daemon? > > > > It depends on the usage scenario really. nbdkit / qemu-nbd are > > not commonly run under any SELinux policy, so then end up being > > unconfined_t. A QEMU NBD client can't connect to an unconfined_t > > socket, so we need to override it with this arg. > > > > In the case of qemu system emulator, under libvirt, it will > > already have a svirt_t type, so in that case there is no need > > to override the type for the socket. > > > > For qsd there's not really any strong practice established > > but i expect most current usage is unconfined_t too and > > would benefit from setting label. > > > > > If so, is this something specific to NBD sockets, or would it actually > > > make sense to have it as a generic option in UnixSocketAddress? > > > > It is applicable to inet sockets too in fact. > > So now that 6.2 is open, should I queue the patch as is, or wait for a > v3 that makes the option more generic to all socket usage? >
24.09.2021 22:23, Eric Blake wrote: > Ping > > On Wed, Aug 25, 2021 at 02:35:04PM -0500, Eric Blake wrote: >> On Fri, Jul 23, 2021 at 05:38:06PM +0100, Daniel P. Berrangé wrote: >>> On Fri, Jul 23, 2021 at 06:18:55PM +0200, Kevin Wolf wrote: >>>> Am 23.07.2021 um 12:33 hat Richard W.M. Jones geschrieben: >>>>> 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 suppose this would also be relevant for the built-in NBD server, >>>> especially in the context of qemu-storage-daemon? >>> >>> It depends on the usage scenario really. nbdkit / qemu-nbd are >>> not commonly run under any SELinux policy, so then end up being >>> unconfined_t. A QEMU NBD client can't connect to an unconfined_t >>> socket, so we need to override it with this arg. >>> >>> In the case of qemu system emulator, under libvirt, it will >>> already have a svirt_t type, so in that case there is no need >>> to override the type for the socket. >>> >>> For qsd there's not really any strong practice established >>> but i expect most current usage is unconfined_t too and >>> would benefit from setting label. >>> >>>> If so, is this something specific to NBD sockets, or would it actually >>>> make sense to have it as a generic option in UnixSocketAddress? >>> >>> It is applicable to inet sockets too in fact. If so, should patch at least be changed to call setsockcreatecon_raw() for inet sockets as well? With current code selinux_label is silently ignored in that case. >> >> So now that 6.2 is open, should I queue the patch as is, or wait for a >> v3 that makes the option more generic to all socket usage? >> >
On Wed, Aug 25, 2021 at 02:35:04PM -0500, Eric Blake wrote: > On Fri, Jul 23, 2021 at 05:38:06PM +0100, Daniel P. Berrangé wrote: > > On Fri, Jul 23, 2021 at 06:18:55PM +0200, Kevin Wolf wrote: > > > Am 23.07.2021 um 12:33 hat Richard W.M. Jones geschrieben: > > > > 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 suppose this would also be relevant for the built-in NBD server, > > > especially in the context of qemu-storage-daemon? > > > > It depends on the usage scenario really. nbdkit / qemu-nbd are > > not commonly run under any SELinux policy, so then end up being > > unconfined_t. A QEMU NBD client can't connect to an unconfined_t > > socket, so we need to override it with this arg. > > > > In the case of qemu system emulator, under libvirt, it will > > already have a svirt_t type, so in that case there is no need > > to override the type for the socket. > > > > For qsd there's not really any strong practice established > > but i expect most current usage is unconfined_t too and > > would benefit from setting label. > > > > > If so, is this something specific to NBD sockets, or would it actually > > > make sense to have it as a generic option in UnixSocketAddress? > > > > It is applicable to inet sockets too in fact. > > So now that 6.2 is open, should I queue the patch as is, or wait for a > v3 that makes the option more generic to all socket usage? My gut feeling is that it makes sense to have a more generic option, with the selinux label specified in the "SocketAddress" QAPI type, and then have util/qemu-sockets.h be setting the context in socket_listen(). I don't think this invalidates the patch that Richard proprosed here, as we'll still need the command line argument he's added. All that will differ is that the setsockcreatecon_raw will get moved down. So from that POV, I don't think we need the general solution to be a blocker, it can be additive. Regards, Daniel
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 {
On Mon, Sep 27, 2021 at 04:18:34PM -0500, Eric Blake wrote: > 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 { > Good idea, thanks. Rich.
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 multiprocess Out 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 find the libfdt library') + +option('selinux', type: 'feature', value: 'auto', + description: 'SELinux support in qemu-nbd') diff --git a/qemu-nbd.c b/qemu-nbd.c index 26ffbf15af..003ba2492e 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -47,6 +47,10 @@ #include "trace/control.h" #include "qemu-version.h" +#ifdef CONFIG_SELINUX +#include <selinux/selinux.h> +#endif + #ifdef __linux__ #define HAVE_NBD_DEVICE 1 #else @@ -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 #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 }, { 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 + } saddr = nbd_build_socket_address(sockpath, bindto, port); if (qio_net_listener_open_sync(server, saddr, backlog, &local_err) < 0) { @@ -945,6 +969,15 @@ 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); + } +#endif + } } else { size_t i; /* See comment in check_socket_activation above. */ diff --git a/tests/docker/dockerfiles/centos8.docker b/tests/docker/dockerfiles/centos8.docker index 46398c61ee..7f135f8e8c 100644 --- a/tests/docker/dockerfiles/centos8.docker +++ b/tests/docker/dockerfiles/centos8.docker @@ -51,6 +51,7 @@ ENV PACKAGES \ libpng-devel \ librbd-devel \ libseccomp-devel \ + libselinux-devel \ libslirp-devel \ libssh-devel \ libtasn1-devel \ diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker index eec1add7f6..c6fd7e1113 100644 --- a/tests/docker/dockerfiles/fedora.docker +++ b/tests/docker/dockerfiles/fedora.docker @@ -53,6 +53,7 @@ ENV PACKAGES \ libpng-devel \ librbd-devel \ libseccomp-devel \ + libselinux-devel \ libslirp-devel \ libssh-devel \ libtasn1-devel \ diff --git a/tests/docker/dockerfiles/opensuse-leap.docker b/tests/docker/dockerfiles/opensuse-leap.docker index 5a8bee0289..3bbdb67f4f 100644 --- a/tests/docker/dockerfiles/opensuse-leap.docker +++ b/tests/docker/dockerfiles/opensuse-leap.docker @@ -55,6 +55,7 @@ ENV PACKAGES \ libpulse-devel \ librbd-devel \ libseccomp-devel \ + libselinux-devel \ libspice-server-devel \ libssh-devel \ libtasn1-devel \ diff --git a/tests/docker/dockerfiles/ubuntu1804.docker b/tests/docker/dockerfiles/ubuntu1804.docker index 0880bf3e29..450fd06d0d 100644 --- a/tests/docker/dockerfiles/ubuntu1804.docker +++ b/tests/docker/dockerfiles/ubuntu1804.docker @@ -60,6 +60,7 @@ ENV PACKAGES \ libsdl2-dev \ libsdl2-image-dev \ libseccomp-dev \ + libselinux-dev \ libsnappy-dev \ libspice-protocol-dev \ libspice-server-dev \ diff --git a/tests/docker/dockerfiles/ubuntu2004.docker b/tests/docker/dockerfiles/ubuntu2004.docker index 39de63d012..15a026be09 100644 --- a/tests/docker/dockerfiles/ubuntu2004.docker +++ b/tests/docker/dockerfiles/ubuntu2004.docker @@ -60,6 +60,7 @@ ENV PACKAGES \ libsdl2-dev \ libsdl2-image-dev \ libseccomp-dev \ + libselinux-dev \ libslirp-dev \ libsnappy-dev \ libspice-protocol-dev \
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> --- configure | 9 ++++- meson.build | 10 +++++- meson_options.txt | 3 ++ qemu-nbd.c | 33 +++++++++++++++++++ tests/docker/dockerfiles/centos8.docker | 1 + tests/docker/dockerfiles/fedora.docker | 1 + tests/docker/dockerfiles/opensuse-leap.docker | 1 + tests/docker/dockerfiles/ubuntu1804.docker | 1 + tests/docker/dockerfiles/ubuntu2004.docker | 1 + 9 files changed, 58 insertions(+), 2 deletions(-)