[libvirt] [RFC PATCH V2] Add proxy FS support to libvirt

M. Mohan Kumar mohan at in.ibm.com
Mon Feb 6 17:47:29 UTC 2012


Hi Daniel,

Any comment on this patch?

M. Mohan Kumar wrote:
> From: "M. Mohan Kumar"<mohan at in.ibm.com>
>
> A new FS driver type 'proxy' is added to QEMU 9p server. This patch adds
> support for using proxy FS driver from libvirt.
>
> QEMU proxy FS driver uses socket for communicating between helper and qemu
> proxy FS driver. Proxy helper (a stand alone binary part of qemu) is invoked
> with one of the descriptors created using socketpair call and the share path.
> Similarly QEMU is invoked with another descriptor created using the same
> socketpair system call and with other required FS driver parameters.
>
> Need for proxy FS driver
> ========================
> Pass through security model in QEMU 9p server has following issues:
> 1) TOCTTOU vulnerability: Following symbolic links in the server could
> provide access to files beyond 9p export path.
>
> 2) Running QEMU with root privilege could be a security issue (pass
> through security model needs root privilege).
>
> Proxy FS driver is implemented to solve these issues.
>
> Proxy FS uses chroot + socket combination for securing the vulnerability
> known with following symbolic links. Intention of adding a new filesystem
> type is to allow qemu to run in non-root mode, but doing privileged
> operations in a chroot environment using socket IO.
>
> Proxy helper is invoked with root privileges and chroots into 9p export path.
> QEMU proxy fs driver sends filesystem request to proxy helper and receives the
> response from it.
>
> Proxy helper is designed such a way that it needs only few capabilities related
> to filesystem operations (such as CAP_DAC_OVERRIDE, CAP_FOWNER, etc) and all
> other capabilities are dropped (CAP_SYS_CHROOT, etc)
>
> Proxy patches
>      http://permalink.gmane.org/gmane.comp.emulators.qemu/128735
>
> Signed-off-by: M. Mohan Kumar<mohan at in.ibm.com>
>
> ---
> Changes from previous version
> * Remove the xml node for specifying the virtfs-proxy-helper, now it is
> determined from qemu binary.
>
>   docs/formatdomain.html.in    |    3 +-
>   src/conf/domain_conf.c       |    3 +-
>   src/conf/domain_conf.h       |    2 +-
>   src/qemu/qemu_capabilities.c |    3 ++
>   src/qemu/qemu_capabilities.h |    1 +
>   src/qemu/qemu_command.c      |   71 ++++++++++++++++++++++++++++++++++++++++--
>   src/qemu/qemu_command.h      |    3 +-
>   tests/qemuhelptest.c         |    3 +-
>   8 files changed, 81 insertions(+), 8 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 18b7e22..e398779 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1336,7 +1336,8 @@
>           This mode also has an optional
>           sub-element<code>driver</code>, with an
>           attribute<code>type='path'</code>
> -        or<code>type='handle'</code>  <span class="since">(since
> +        or<code>type='handle'</code>
> +        or<code>type='proxy'</code>  <span class="since">(since
>           0.9.7)</span>.
>           </dd>
>           <dt><code>type='template'</code></dt>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 0190a81..2d45324 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -250,7 +250,8 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
>   VIR_ENUM_IMPL(virDomainFSDriverType, VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
>                 "default",
>                 "path",
> -              "handle")
> +              "handle",
> +              "proxy")
>
>   VIR_ENUM_IMPL(virDomainFSAccessMode, VIR_DOMAIN_FS_ACCESSMODE_LAST,
>                 "passthrough",
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 03aa5b6..3796da4 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -466,6 +466,7 @@ enum virDomainFSDriverType {
>       VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT = 0,
>       VIR_DOMAIN_FS_DRIVER_TYPE_PATH,
>       VIR_DOMAIN_FS_DRIVER_TYPE_HANDLE,
> +    VIR_DOMAIN_FS_DRIVER_TYPE_PROXY,
>
>       VIR_DOMAIN_FS_DRIVER_TYPE_LAST
>   };
> @@ -491,7 +492,6 @@ struct _virDomainFSDef {
>       virDomainDeviceInfo info;
>   };
>
> -
>   /* 5 different types of networking config */
>   enum virDomainNetType {
>       VIR_DOMAIN_NET_TYPE_USER,
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 43c7578..9c01a3b 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -144,6 +144,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
>                 "ich9-ahci",
>                 "no-acpi",
>                 "fsdev-readonly",
> +              "fsdev-proxy",
>       );
>
>   struct qemu_feature_flags {
> @@ -1083,6 +1084,8 @@ qemuCapsComputeCmdFlags(const char *help,
>           qemuCapsSet(flags, QEMU_CAPS_FSDEV);
>           if (strstr(fsdev, "readonly"))
>               qemuCapsSet(flags, QEMU_CAPS_FSDEV_READONLY);
> +        if (strstr(fsdev, "sock_fd"))
> +            qemuCapsSet(flags, QEMU_CAPS_FSDEV_PROXY);
>       }
>       if (strstr(help, "-smbios type"))
>           qemuCapsSet(flags, QEMU_CAPS_SMBIOS_TYPE);
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index c759baf..e129e35 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -117,6 +117,7 @@ enum qemuCapsFlags {
>       QEMU_CAPS_ICH9_AHCI         = 77, /* -device ich9-ahci */
>       QEMU_CAPS_NO_ACPI		= 78, /* -no-acpi */
>       QEMU_CAPS_FSDEV_READONLY    =79, /* -fsdev readonly supported */
> +    QEMU_CAPS_FSDEV_PROXY      = 80, /* -fsdev proxy supported */
>
>       QEMU_CAPS_LAST,                   /* this must always be the last item */
>   };
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index f2e9cfa..0c79340 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -44,6 +44,7 @@
>   #include<sys/utsname.h>
>   #include<sys/stat.h>
>   #include<fcntl.h>
> +#include<libgen.h>
>
>   #define VIR_FROM_THIS VIR_FROM_QEMU
>
> @@ -106,7 +107,8 @@ VIR_ENUM_DECL(qemuDomainFSDriver)
>   VIR_ENUM_IMPL(qemuDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
>                 "local",
>                 "local",
> -              "handle");
> +              "handle",
> +              "proxy");
>
>
>   static void
> @@ -2080,9 +2082,43 @@ error:
>       return NULL;
>   }
>
> +/*
> + * Invokes the Proxy Helper with one of the socketpair as its parameter
> + *
> + */
> +static int qemuInvokeProxyHelper(const char *emulator, int sock, const char *path)
> +{
> +#define HELPER "virtfs-proxy-helper"
> +    int ret_val, status;
> +    virCommandPtr cmd;
> +    char *helper, *dname;
> +
> +    dname = dirname(strdup(emulator));
> +    if (virAsprintf(&helper, "%s/%s", dname, HELPER)<  0) {
> +        VIR_FREE(dname);
> +        virReportOOMError();
> +        return -1;
> +    }
> +
> +    cmd = virCommandNewArgList(helper, NULL);
> +    virCommandAddArg(cmd, "-f");
> +    virCommandAddArgFormat(cmd, "%d", sock);
> +    virCommandAddArg(cmd, "-p");
> +    virCommandAddArgFormat(cmd, "%s", path);
> +    virCommandTransferFD(cmd, sock);
> +    virCommandDaemonize(cmd);
> +    ret_val = virCommandRun(cmd,&status);
> +    if (ret_val<  0)
> +        qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                            _("%s can't execute"), helper);
> +    virCommandFree(cmd);
> +    VIR_FREE(helper);
> +    VIR_FREE(dname);
> +    return ret_val;
> +}
>
>   char *qemuBuildFSStr(virDomainFSDefPtr fs,
> -                     virBitmapPtr qemuCaps ATTRIBUTE_UNUSED)
> +                     virBitmapPtr qemuCaps ATTRIBUTE_UNUSED, int qemuSocket)
>   {
>       virBuffer opt = VIR_BUFFER_INITIALIZER;
>       const char *driver = qemuDomainFSDriverTypeToString(fs->fsdriver);
> @@ -2108,6 +2144,10 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs,
>           virBufferAddLit(&opt, ",security_model=none");
>       }
>       virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias);
> +
> +    if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PROXY)
> +        virBufferAsprintf(&opt, ",sock_fd=%d", qemuSocket);
> +
>       virBufferAsprintf(&opt, ",path=%s", fs->src);
>
>       if (fs->readonly) {
> @@ -4426,10 +4466,35 @@ qemuBuildCommandLine(virConnectPtr conn,
>       if (qemuCapsGet(qemuCaps, QEMU_CAPS_FSDEV)) {
>           for (i = 0 ; i<  def->nfss ; i++) {
>               char *optstr;
> +            int sockets[2] = {-1, -1};
>               virDomainFSDefPtr fs = def->fss[i];
>
> +            /*
> +             * If its a proxy FS, we need to create a socket pair
> +             * and invoke proxy_helper
> +             */
> +            if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PROXY) {
> +                if (qemuCapsGet(qemuCaps, QEMU_CAPS_FSDEV_PROXY)<  0) {
> +                    qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                        _("proxy helper not supported"));
> +                    goto error;
> +                }
> +                /* create a socket pair */
> +                if (socketpair(PF_UNIX, SOCK_STREAM, 0, sockets)<  0) {
> +                    qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                        _("socketpair failed"));
> +                    goto error;
> +                }
> +                virCommandTransferFD(cmd, sockets[1]);
> +                if (qemuInvokeProxyHelper(def->emulator, sockets[0],
> +                                    fs->src)<  0) {
> +                    VIR_FORCE_CLOSE(sockets[0]);
> +                    VIR_FORCE_CLOSE(sockets[1]);
> +                    goto error;
> +                }
> +            }
>               virCommandAddArg(cmd, "-fsdev");
> -            if (!(optstr = qemuBuildFSStr(fs, qemuCaps)))
> +            if (!(optstr = qemuBuildFSStr(fs, qemuCaps, sockets[1])))
>                   goto error;
>               virCommandAddArg(cmd, optstr);
>               VIR_FREE(optstr);
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index de61cf3..8e4f335 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -87,7 +87,8 @@ char *qemuBuildDriveStr(virConnectPtr conn,
>                           bool bootable,
>                           virBitmapPtr qemuCaps);
>   char *qemuBuildFSStr(virDomainFSDefPtr fs,
> -                     virBitmapPtr qemuCaps);
> +                     virBitmapPtr qemuCaps,
> +                     int qemuSocket);
>
>   /* Current, best practice */
>   char * qemuBuildDriveDevStr(virDomainDiskDefPtr disk,
> diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
> index 60155e7..455a7e1 100644
> --- a/tests/qemuhelptest.c
> +++ b/tests/qemuhelptest.c
> @@ -648,7 +648,8 @@ mymain(void)
>               QEMU_CAPS_PCI_ROMBAR,
>               QEMU_CAPS_ICH9_AHCI,
>               QEMU_CAPS_NO_ACPI,
> -            QEMU_CAPS_FSDEV_READONLY);
> +            QEMU_CAPS_FSDEV_READONLY,
> +            QEMU_CAPS_FSDEV_PROXY);
>
>       return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
>   }
>    




More information about the libvir-list mailing list