[libvirt] [PATCH] security: dac: also label listen UNIX sockets

Erik Skultety eskultet at redhat.com
Mon Oct 1 08:22:59 UTC 2018


On Thu, Sep 27, 2018 at 05:02:13PM +0200, Ján Tomko wrote:
> We switched to opening mode='bind' sockets ourselves:
> commit 30fb2276d88b275dc2aad6ddd28c100d944b59a5
>     qemu: support passing pre-opened UNIX socket listen FD
> in v4.5.0-rc1~251
>
> Then fixed qemuBuildChrChardevStr to change libvirtd's label
> while creating the socket:
> commit b0c6300fc42bbc3e5eb0b236392f7344581c5810
>     qemu: ensure FDs passed to QEMU for chardevs have correct SELinux labels
> v4.5.0-rc1~52
>
> Also add labeling of these sockets to the DAC driver.
> Instead of trying to figure out which one was created by libvirt,
> label it if it exists.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1633389
>
> Signed-off-by: Ján Tomko <jtomko at redhat.com>
> ---
>  src/security/security_dac.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index 62442745dd..da4a6c72fe 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -1308,7 +1308,12 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
>          break;
>
>      case VIR_DOMAIN_CHR_TYPE_UNIX:
> -        if (!dev_source->data.nix.listen) {
> +        if (!dev_source->data.nix.listen ||
> +            (dev_source->data.nix.path &&
> +             virFileExists(dev_source->data.nix.path))) {
> +            /* Also label mode='bind' sockets if they exist,
> +             * e.g. because they were created by libvirt
> +             * and passed via FD */
>              if (virSecurityDACSetOwnership(mgr, NULL,
>                                             dev_source->data.nix.path,
>                                             user, group) < 0)

So we're labeling path even if nix.listen == false, shouldn't we check for the
file's existence too? Or have we already done it and I missed this fact?
Basically what I'm aiming at is that nix.path will always be set at this point,
either explicitly by the user (most cases) or it would have been generated by
us if the target type was either VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN or
VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN. I'm just simply wondering whether the
condition cannot be further simplified to:

if (virFileExists(foo) && virSecurityDACSetOwnership(...) < 0)
    goto done;

ret = 0;
break;


Erik




More information about the libvir-list mailing list