[libvirt] [PATCH] Added new attribute security_model to filesystem element
This patch introduces new attribute to filesystem element to support customizable security_model for mount type. Valid security_model are: passthrough, mapped and none. Usage: <filesystem type='mount' security_model='passthrough'> <source dir='/export/to/guest'/> <target dir='mount_tag'/> </filesystem> Note: This patch is based on Daniel's patch to support 9pfs. It shall be applied after applying Daniel's patch to support 9pfs. Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> --- docs/schemas/domain.rng | 7 +++++++ src/conf/domain_conf.c | 30 ++++++++++++++++++++++++++++-- src/conf/domain_conf.h | 10 ++++++++++ src/qemu/qemu_conf.c | 11 +++++++++-- 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index ccb8cf3..43a292d 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -761,6 +761,13 @@ </choice> <optional> <ref name="address"/> + <attribute name="security_model"> + <choice> + <value>passthrough</value> + <value>mapped</value> + <value>none</value> + </choice> + </attribute> </optional> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e05d5d7..a9881d1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -161,6 +161,12 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, "file", "template") +VIR_ENUM_IMPL(virDomainFSSecurityModel, VIR_DOMAIN_FS_SECURITY_LAST, + "passthrough", + "mapped", + "none") + + VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "user", "ethernet", @@ -1847,6 +1853,7 @@ virDomainFSDefParseXML(xmlNodePtr node, char *type = NULL; char *source = NULL; char *target = NULL; + char *security_model; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -1864,6 +1871,17 @@ virDomainFSDefParseXML(xmlNodePtr node, def->type = VIR_DOMAIN_FS_TYPE_MOUNT; } + security_model = virXMLPropString(node, "security_model"); + if (security_model) { + if ((def->security_model = virDomainFSSecurityModelTypeFromString(security_model)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown security model '%s'"), security_model); + goto error; + } + } else { + def->security_model = VIR_DOMAIN_FS_SECURITY_PASSTHROUGH; + } + cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { @@ -5602,6 +5620,7 @@ virDomainFSDefFormat(virBufferPtr buf, int flags) { const char *type = virDomainFSTypeToString(def->type); + const char *sec_model = virDomainFSSecurityModelTypeToString(def->security_model); if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -5609,9 +5628,16 @@ virDomainFSDefFormat(virBufferPtr buf, return -1; } + if (!sec_model) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected security model %d"), def->security_model); + return -1; + } + + virBufferVSprintf(buf, - " <filesystem type='%s'>\n", - type); + " <filesystem type='%s' security_model='%s'>\n", + type, sec_model); if (def->src) { switch (def->type) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7195c04..6adf027 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -236,10 +236,20 @@ enum virDomainFSType { VIR_DOMAIN_FS_TYPE_LAST }; +/* Filesystem mount security model */ +enum virDomainFSSecurityModel { + VIR_DOMAIN_FS_SECURITY_PASSTHROUGH, + VIR_DOMAIN_FS_SECURITY_MAPPED, + VIR_DOMAIN_FS_SECURITY_NONE, + + VIR_DOMAIN_FS_SECURITY_LAST +}; + typedef struct _virDomainFSDef virDomainFSDef; typedef virDomainFSDef *virDomainFSDefPtr; struct _virDomainFSDef { int type; + int security_model; char *src; char *dst; unsigned int readonly : 1; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 18a302a..6b96d2f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2014,6 +2014,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, unsigned long long qemuCmdFlags) if (virAsprintf(&def->fss[i]->info.alias, "fs%d", i) < 0) goto no_memory; } + for (i = 0; i < def->nsounds ; i++) { if (virAsprintf(&def->sounds[i]->info.alias, "sound%d", i) < 0) goto no_memory; @@ -2783,11 +2784,17 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("can only passthrough directories")); + _("only supports mount filesystem type")); goto error; } - virBufferAddLit(&opt, "local,security_model=passthrough"); + virBufferAddLit(&opt, "local"); + if (fs->security_model == VIR_DOMAIN_FS_SECURITY_PASSTHROUGH) + virBufferAddLit(&opt, ",security_model=passthrough"); + else if (fs->security_model == VIR_DOMAIN_FS_SECURITY_MAPPED) + virBufferAddLit(&opt, ",security_model=mapped"); + else if (fs->security_model == VIR_DOMAIN_FS_SECURITY_NONE) + virBufferAddLit(&opt, ",security_model=none"); virBufferVSprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); virBufferVSprintf(&opt, ",path=%s", fs->src); -- 1.7.1.1
On Sat, Sep 25, 2010 at 12:04:11AM +0530, Harsh Prateek Bora wrote:
This patch introduces new attribute to filesystem element to support customizable security_model for mount type. Valid security_model are: passthrough, mapped and none.
Usage: <filesystem type='mount' security_model='passthrough'>
I'd like to think of a different name for this, because 'security_model' is already used in libvirt in the context of sVirt and I think it'd be better to avoid the same terminology. I've not got any ideas just yet, but I'll think of some....
<source dir='/export/to/guest'/> <target dir='mount_tag'/> </filesystem>
Note: This patch is based on Daniel's patch to support 9pfs. It shall be applied after applying Daniel's patch to support 9pfs.
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com> --- docs/schemas/domain.rng | 7 +++++++ src/conf/domain_conf.c | 30 ++++++++++++++++++++++++++++-- src/conf/domain_conf.h | 10 ++++++++++ src/qemu/qemu_conf.c | 11 +++++++++-- 4 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index ccb8cf3..43a292d 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -761,6 +761,13 @@ </choice> <optional> <ref name="address"/> + <attribute name="security_model"> + <choice> + <value>passthrough</value> + <value>mapped</value> + <value>none</value> + </choice> + </attribute> </optional> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e05d5d7..a9881d1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -161,6 +161,12 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, "file", "template")
+VIR_ENUM_IMPL(virDomainFSSecurityModel, VIR_DOMAIN_FS_SECURITY_LAST, + "passthrough", + "mapped", + "none") + + VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "user", "ethernet", @@ -1847,6 +1853,7 @@ virDomainFSDefParseXML(xmlNodePtr node, char *type = NULL; char *source = NULL; char *target = NULL; + char *security_model;
if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -1864,6 +1871,17 @@ virDomainFSDefParseXML(xmlNodePtr node, def->type = VIR_DOMAIN_FS_TYPE_MOUNT; }
+ security_model = virXMLPropString(node, "security_model"); + if (security_model) { + if ((def->security_model = virDomainFSSecurityModelTypeFromString(security_model)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown security model '%s'"), security_model); + goto error; + } + } else { + def->security_model = VIR_DOMAIN_FS_SECURITY_PASSTHROUGH; + } + cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { @@ -5602,6 +5620,7 @@ virDomainFSDefFormat(virBufferPtr buf, int flags) { const char *type = virDomainFSTypeToString(def->type); + const char *sec_model = virDomainFSSecurityModelTypeToString(def->security_model);
if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -5609,9 +5628,16 @@ virDomainFSDefFormat(virBufferPtr buf, return -1; }
+ if (!sec_model) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected security model %d"), def->security_model); + return -1; + } + + virBufferVSprintf(buf, - " <filesystem type='%s'>\n", - type); + " <filesystem type='%s' security_model='%s'>\n", + type, sec_model);
if (def->src) { switch (def->type) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7195c04..6adf027 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -236,10 +236,20 @@ enum virDomainFSType { VIR_DOMAIN_FS_TYPE_LAST };
+/* Filesystem mount security model */ +enum virDomainFSSecurityModel { + VIR_DOMAIN_FS_SECURITY_PASSTHROUGH, + VIR_DOMAIN_FS_SECURITY_MAPPED, + VIR_DOMAIN_FS_SECURITY_NONE, + + VIR_DOMAIN_FS_SECURITY_LAST +};
What is the difference between 'PASSTHROUGH' mode and 'NONE' ? IIUC, 'PASSTHROUGH' just lets the uid/gid and mode appear in the guest unchanged, which seems to be just what 'NONE' would do too. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 10/07/2010 10:17 PM, Daniel P. Berrange wrote:
On Sat, Sep 25, 2010 at 12:04:11AM +0530, Harsh Prateek Bora wrote:
This patch introduces new attribute to filesystem element to support customizable security_model for mount type. Valid security_model are: passthrough, mapped and none.
Usage: <filesystem type='mount' security_model='passthrough'>
I'd like to think of a different name for this, because 'security_model' is already used in libvirt in the context of sVirt and I think it'd be better to avoid the same terminology. I've not got any ideas just yet, but I'll think of some....
"mount_security" "mount_security_type" "mount_security_mode" ("mode" sounds too similar to "model" tho maybe) ?
On 10/7/2010 5:13 AM, Justin Clift wrote:
On 10/07/2010 10:17 PM, Daniel P. Berrange wrote:
On Sat, Sep 25, 2010 at 12:04:11AM +0530, Harsh Prateek Bora wrote:
This patch introduces new attribute to filesystem element to support customizable security_model for mount type. Valid security_model are: passthrough, mapped and none.
Usage: <filesystem type='mount' security_model='passthrough'>
I'd like to think of a different name for this, because 'security_model' is already used in libvirt in the context of sVirt and I think it'd be better to avoid the same terminology. I've not got any ideas just yet, but I'll think of some....
"mount_security" "mount_security_type" "mount_security_mode" ("mode" sounds too similar to "model" tho maybe)
Actually all three sound good.. mount_security is good enough if no objections. - JV
?
On Sat, Sep 25, 2010 at 12:04:11AM +0530, Harsh Prateek Bora wrote:
This patch introduces new attribute to filesystem element to support customizable security_model for mount type. Valid security_model are: passthrough, mapped and none.
Usage: <filesystem type='mount' security_model='passthrough'> <source dir='/export/to/guest'/> <target dir='mount_tag'/> </filesystem>
Well, there is a missing documentation part in the patch. In what way does having security_model='passthrough', security_model='mapped' or security_model='none' change the behaviour of the library compared to not having the attribute at all. Any patch which modifies the API (and the XML is part of the API) must have a clearly defined semantic, and that definition (and if possible an example) must be provided in the set of patches for the documentation. We cannot accept a patch whose only semantic is "add a given QEmu command line argument" because it makes impossible to try to support the option on other hypervisor or if QEmu end up changing the command line. What is your patch providing in term of API ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 10/7/2010 7:20 AM, Daniel Veillard wrote:
On Sat, Sep 25, 2010 at 12:04:11AM +0530, Harsh Prateek Bora wrote:
This patch introduces new attribute to filesystem element to support customizable security_model for mount type. Valid security_model are: passthrough, mapped and none.
Usage: <filesystem type='mount' security_model='passthrough'> <source dir='/export/to/guest'/> <target dir='mount_tag'/> </filesystem>
Well, there is a missing documentation part in the patch. In what way does having security_model='passthrough', security_model='mapped' or security_model='none' change the behaviour of the library compared to not having the attribute at all.
I think we can ignore "none" and stick to mapped and passthrough for now. The security model is a mandatory argument for QEMU to start 9p/virtfs exports Here is the detailed explanation on these security models: Security model: mapped ---------------------- Fileserver intercepts and maps all the file object create requests. Files on the fileserver will be created with Fileserver's user credentials and the client-user's credentials are stored in extended attributes. During getattr() server extracts the client-user's credentials from extended attributes and sends to the client. This adds a great deal of security in the cloud environments where the guest's(client) user space is kept completely isolated from host's user space. Security model : passthrough ---------------------------- In this security model, Fileserver passes down all requests to the underlying filesystem. File system objects on the fileserver will be created with client-user's credentials. This is done by setting setuid()/setgid() during creation or chmod/chown after file creation. At the end of create protocol request, files on the fileserver will be owned by cleint-user's uid/gid. This model mimic's current NFSv3 level of security.
Any patch which modifies the API (and the XML is part of the API) must have a clearly defined semantic, and that definition (and if possible an example) must be provided in the set of patches for the documentation. We cannot accept a patch whose only semantic is "add a given QEmu command line argument" because it makes impossible to try to support the option on other hypervisor or if QEmu end up changing the command line.
Our need for different security models is because of the different domains we or any other paravirtual filesystem may need to serve. "mapped" model is targeted for domains which demands a complete isolation of guest user domain from that of the hosts hence practically eliminating any security issues related to setuid/setgid and root. This is a very practical use case for certain classes of cloud workloads where multi-tenancy is a requirement. A complete isolation of user domain can practically make two competing customers share the same file system. The "passthrough" model is to play along with the traditional network file-serving methodologies like NFS and CIFS by sharing user domains of the host and guest. This method edges out by offering flexibility to export the same file system through other network file systems along with VirtFS.
What is your patch providing in term of API ?
Does my explanation above answers your questions? If not, I guess I am not very clear about this question. The code is already in the QEMU mainline and I don't expect the options to change. I also assume that other hypervisors may have to have some level of security models defined to serve different needs. Thanks, JV
Daniel
participants (5)
-
Daniel P. Berrange -
Daniel Veillard -
Harsh Prateek Bora -
Justin Clift -
Venkateswararao Jujjuri (JV)