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

From: "M. Mohan Kumar" <mohan(a)in.ibm.com&gt; 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(a)in.ibm.com&gt; --- 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