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

Re: [libvirt] [PATCH V3] Add proxy FS support to libvirt



Hi Daniel,

Any comment?

On Thu, 23 Aug 2012 14:36:40 +0530, "M. Mohan Kumar" <mohan in ibm com> wrote:
> From: "M. Mohan Kumar" <mohan 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 in ibm com>
> ---
> Changes from previous version V3
> * Addressed Daniel's review comments
> 
> Changes from previous version V2
> * Rebased on top of current libvirt git level
> 
> Changes from previous version
> * Remove the xml node for specifying the virtfs-proxy-helper path, now it is
> determined from qemu binary path.
> 
>  docs/formatdomain.html.in    |    2 +
>  src/conf/domain_conf.c       |    3 +-
>  src/conf/domain_conf.h       |    2 +-
>  src/qemu/qemu_capabilities.c |    5 ++-
>  src/qemu/qemu_capabilities.h |    1 +
>  src/qemu/qemu_command.c      |   83 ++++++++++++++++++++++++++++++++++++++++--
>  src/qemu/qemu_command.h      |    3 +-
>  tests/qemuhelptest.c         |    7 +++-
>  8 files changed, 96 insertions(+), 10 deletions(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 2c5c456..bf74ed8 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1621,6 +1621,8 @@
>          while the value <code>immediate</code> means that a host writeback
>          is immediately triggered for all pages touched during a guest file
>          write operation <span class="since">(since 0.9.10)</span>.
> +        or <code>type='proxy'</code> <span class="since">(since
> +        1.0)</span>.
>          </dd>
>          <dt><code>type='template'</code></dt>
>          <dd>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 851284a..79ca2b7 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -268,7 +268,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 fd0e89e..a296f49 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -660,6 +660,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
>  };
> @@ -698,7 +699,6 @@ struct _virDomainFSDef {
>      unsigned long long space_soft_limit; /* in bytes */
>  };
> 
> -
>  /* 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 5472267..88b7e87 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -144,7 +144,6 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
>                "ich9-ahci",
>                "no-acpi",
>                "fsdev-readonly",
> -
>                "virtio-blk-pci.scsi", /* 80 */
>                "blk-sg-io",
>                "drive-copy-on-read",
> @@ -172,7 +171,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
>                "bridge", /* 100 */
>                "lsi",
>                "virtio-scsi-pci",
> -
> +              "fsdev-proxy",
>      );
> 
>  struct qemu_feature_flags {
> @@ -1133,6 +1132,8 @@ qemuCapsComputeCmdFlags(const char *help,
>              qemuCapsSet(flags, QEMU_CAPS_FSDEV_READONLY);
>          if (strstr(fsdev, "writeout"))
>              qemuCapsSet(flags, QEMU_CAPS_FSDEV_WRITEOUT);
> +        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 d606890..073a1cd 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -138,6 +138,7 @@ enum qemuCapsFlags {
>      QEMU_CAPS_NETDEV_BRIDGE      = 100, /* bridge helper support */
>      QEMU_CAPS_SCSI_LSI           = 101, /* -device lsi */
>      QEMU_CAPS_VIRTIO_SCSI_PCI    = 102, /* -device virtio-scsi-pci */
> +    QEMU_CAPS_FSDEV_PROXY        = 103, /* -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 4ca3047..3f78635 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -46,6 +46,7 @@
>  #include <sys/utsname.h>
>  #include <sys/stat.h>
>  #include <fcntl.h>
> +#include <libgen.h>
> 
>  #define VIR_FROM_THIS VIR_FROM_QEMU
> 
> @@ -116,7 +117,8 @@ VIR_ENUM_DECL(qemuDomainFSDriver)
>  VIR_ENUM_IMPL(qemuDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
>                "local",
>                "local",
> -              "handle");
> +              "handle",
> +              "proxy");
> 
> 
>  static void
> @@ -2632,9 +2634,59 @@ 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, *dp;
> +
> +    dp = strdup(emulator);
> +    if (!dp) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +    dname = strrchr(dp, '/');
> +    if (!dname) {
> +        VIR_FREE(dp);
> +        VIR_FORCE_CLOSE(sock);
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Unable to get path details from %s"), dp);
> +        return -1;
> +    }
> +    *dname = '\0';
> +    if (virAsprintf(&helper, "%s/%s", dp, HELPER) < 0) {
> +        VIR_FREE(dp);
> +        VIR_FORCE_CLOSE(sock);
> +        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) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                            _("%s can't execute"), helper);
> +        VIR_FORCE_CLOSE(sock);
> +    }
> +    virCommandFree(cmd);
> +    VIR_FREE(helper);
> +    VIR_FREE(dp);
> +    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);
> @@ -2683,6 +2735,10 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs,
>      }
> 
>      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) {
> @@ -5153,10 +5209,31 @@ 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) {
> +                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                        _("proxy helper not supported"));
> +                    goto error;
> +                }
> +                /* create a socket pair */
> +                if (socketpair(PF_UNIX, SOCK_STREAM, 0, sockets) < 0) {
> +                    virReportSystemError(errno, _("socketpair %s"), "failed");
> +                    goto error;
> +                }
> +                virCommandTransferFD(cmd, sockets[1]);
> +                if (qemuInvokeProxyHelper(def->emulator, sockets[0],
> +                                          fs->src) < 0)
> +                    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 e999bc7..2da5ad0 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -89,7 +89,8 @@ char *qemuBuildDriveStr(virConnectPtr conn,
>                          bool bootable,
>                          virBitmapPtr qemuCaps);
>  char *qemuBuildFSStr(virDomainFSDefPtr fs,
> -                     virBitmapPtr qemuCaps);
> +                     virBitmapPtr qemuCaps,
> +                     int qemuSocket);
> 
>  /* Current, best practice */
>  char * qemuBuildDriveDevStr(virDomainDefPtr def,
> diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
> index 2d15c94..6af2e68 100644
> --- a/tests/qemuhelptest.c
> +++ b/tests/qemuhelptest.c
> @@ -679,7 +679,9 @@ mymain(void)
>              QEMU_CAPS_SCSI_BLOCK,
>              QEMU_CAPS_SCSI_CD,
>              QEMU_CAPS_IDE_CD,
> -            QEMU_CAPS_SCSI_LSI);
> +            QEMU_CAPS_SCSI_LSI,
> +            QEMU_CAPS_FSDEV_PROXY);
> +
>      DO_TEST("qemu-1.1.0", 1001000, 0, 0,
>              QEMU_CAPS_VNC_COLON,
>              QEMU_CAPS_NO_REBOOT,
> @@ -759,7 +761,8 @@ mymain(void)
>              QEMU_CAPS_NEC_USB_XHCI,
>              QEMU_CAPS_NETDEV_BRIDGE,
>              QEMU_CAPS_SCSI_LSI,
> -            QEMU_CAPS_VIRTIO_SCSI_PCI);
> +            QEMU_CAPS_VIRTIO_SCSI_PCI,
> +            QEMU_CAPS_FSDEV_PROXY);
> 
>      return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
>  }
> -- 
> 1.7.7.6
> 


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