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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
>  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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux