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

[libvirt] Re: PATCH: Mark <seclabel> as dynamic generated, or statically pre-defined



On Tue, Mar 03, 2009 at 11:01:34AM -0500, Daniel J Walsh wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Daniel P. Berrange wrote:
> > This patch implements the behaviour I was refering to earlier, whereby
> > the domain XML explicitly says whether the security label is a statically
> > pre-defined one, or dynamically generated on VM boot by libvirtd
> > 
> > So when creating a new guest, apps like virt-install have 2 options:
> > 
> >  - Leave out the <seclabel> tag completely
> >     -> If no security driver is active, just works as normal unconfined VM
>
> I want to make sure of terminology.  The VM will be confined, but will
> not be separated.  IE the host machine will be protected from the VM,
> while other VM's will not be.  svirt is concerned with protecting VM's
> from each other.  Normal SELinux protects the Apache server from rogue
> VMs.  If we want to have a truly unconfined vm, we need additional
> policy.  I would suggest permissive mode or a permissive domain.

This was my bad terminolgy. There are two scenarios where a security
driver is not active

 - Admin turned it off in /etc/libvirt/qemu.conf, but SELinux is still
   active on the host in general. The VM will still be confined here,
   but not isolated from each other
 - Admin turned off SELinux for the entire host. The VM will be unconfined


> >     -> If a security driver is active, a dynamic seclabel is generated
> > 
> >       <seclabel type='dynamic' model='selinux'>
> >         <label>system_u:system_r:qemu_t:s0:c424,c719</label>
> >         <imagelabel>system_u:object_r:virt_image_t:s0:c424,c719</imagelabel>
> >       </seclabel>
> > 
> >  - Add an explicit <seclabel> tag with type='static' attribute
> >     -> Security driver uses the defined label & imagelabel
> > 
> >       <seclabel type='static' model='selinux'>
> >         <label>system_u:system_r:qemu_t:s0:c25,c100</label>
> >         <imagelabel>system_u:system_r:virt_image_t:s0:c25,c100</imagelabel>
> >       </seclabel>
> > 
> >
> Not sure imagelabel is required in the second case since, libvirt will
> not change the label.
> 
> This is the biggest difference between static and dynamic, static will
> never run setfilecon, while dynamic runs it at startup and shutdown.
> 
> DYNAMIC:
> 	start
> 	setfilecon(image, imagelabel)
> 	setexeccon(label)
> 	exec()
> 
> 	Finish
> 	ChildExit()
> 	setfilecon(image, DEFAULT_LABEL)
>  	Exit()
> 
> STATIC:
> 	start
> 	setexecon(label)
> 	exec()
> 
> 	Finish:
> 	ChildExit()
> 	Exit()

This new patch implements this logic now. It only parses the <imagelabel>
element from the XML, if parsing the 'live' XML config for a dynamic
label - this is basically just used for libvirt's state file in /var/run,
so it can remember config across libvirtd restarts.

The <imagelabel> is now never used for static labels, and thus no setfilecon()
will take place for them.


Index: src/domain_conf.c
===================================================================
RCS file: /data/cvs/libvirt/src/domain_conf.c,v
retrieving revision 1.70
diff -u -p -r1.70 domain_conf.c
--- src/domain_conf.c	3 Mar 2009 09:44:42 -0000	1.70
+++ src/domain_conf.c	3 Mar 2009 16:36:17 -0000
@@ -168,6 +168,10 @@ VIR_ENUM_IMPL(virDomainState, VIR_DOMAIN
               "shutoff",
               "crashed")
 
+VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOMAIN_SECLABEL_LAST,
+              "dynamic",
+              "static")
+
 #define virDomainReportError(conn, code, fmt...)                           \
         virReportErrorHelper(conn, VIR_FROM_DOMAIN, code, __FILE__,        \
                                __FUNCTION__, __LINE__, fmt)
@@ -1847,24 +1851,49 @@ static int virDomainLifecycleParseXML(vi
 static int
 virSecurityLabelDefParseXML(virConnectPtr conn,
                             const virDomainDefPtr def,
-                            xmlXPathContextPtr ctxt)
+                            xmlXPathContextPtr ctxt,
+                            int flags)
 {
     char *p;
 
     if (virXPathNode(conn, "./seclabel", ctxt) == NULL)
         return 0;
 
-    p = virXPathStringLimit(conn, "string(./seclabel/label[1])",
+    p = virXPathStringLimit(conn, "string(./seclabel/@type)",
                             VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
     if (p == NULL)
         goto error;
-    def->seclabel.label = p;
-
-    p = virXPathStringLimit(conn, "string(./seclabel/@model)",
-                            VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
-    if (p == NULL)
+    if ((def->seclabel.type = virDomainSeclabelTypeFromString(p)) < 0)
         goto error;
-    def->seclabel.model = p;
+    VIR_FREE(p);
+
+    /* Only parse details, if using static labels, or
+     * if the 'live' VM XML is requested
+     */
+    if (def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC ||
+        !(flags & VIR_DOMAIN_XML_INACTIVE)) {
+        p = virXPathStringLimit(conn, "string(./seclabel/@model)",
+                                VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
+        if (p == NULL)
+            goto error;
+        def->seclabel.model = p;
+
+        p = virXPathStringLimit(conn, "string(./seclabel/label[1])",
+                                VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
+        if (p == NULL)
+            goto error;
+        def->seclabel.label = p;
+    }
+
+    /* Only parse imagelabel, if requested live XML for dynamic label */
+    if (def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
+        !(flags & VIR_DOMAIN_XML_INACTIVE)) {
+        p = virXPathStringLimit(conn, "string(./seclabel/imagelabel[1])",
+                                VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
+        if (p == NULL)
+            goto error;
+        def->seclabel.imagelabel = p;
+    }
 
     return 0;
 
@@ -2458,7 +2487,7 @@ static virDomainDefPtr virDomainDefParse
     VIR_FREE(nodes);
 
     /* analysis of security label */
-    if (virSecurityLabelDefParseXML(conn, def, ctxt) == -1)
+    if (virSecurityLabelDefParseXML(conn, def, ctxt, flags) == -1)
         goto error;
 
     return def;
@@ -3480,9 +3509,25 @@ char *virDomainDefFormat(virConnectPtr c
     virBufferAddLit(&buf, "  </devices>\n");
 
     if (def->seclabel.model) {
-        virBufferEscapeString(&buf, "  <seclabel model='%s'>\n", def->seclabel.model);
-        virBufferEscapeString(&buf, "    <label>%s</label>\n", def->seclabel.label);
-        virBufferAddLit(&buf, "  </seclabel>\n");
+        const char *sectype = virDomainSeclabelTypeToString(def->seclabel.type);
+        if (!sectype)
+            goto cleanup;
+        if (!def->seclabel.label ||
+            (def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
+             (flags & VIR_DOMAIN_XML_INACTIVE))) {
+            virBufferVSprintf(&buf, "  <seclabel type='%s' model='%s'/>\n",
+                              sectype, def->seclabel.model);
+        } else {
+            virBufferVSprintf(&buf, "  <seclabel type='%s' model='%s'>\n",
+                                  sectype, def->seclabel.model);
+            virBufferEscapeString(&buf, "    <label>%s</label>\n",
+                                  def->seclabel.label);
+            if (def->seclabel.imagelabel &&
+                def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC)
+                virBufferEscapeString(&buf, "    <imagelabel>%s</imagelabel>\n",
+                                      def->seclabel.imagelabel);
+            virBufferAddLit(&buf, "  </seclabel>\n");
+        }
     }
 
     virBufferAddLit(&buf, "</domain>\n");
Index: src/domain_conf.h
===================================================================
RCS file: /data/cvs/libvirt/src/domain_conf.h,v
retrieving revision 1.39
diff -u -p -r1.39 domain_conf.h
--- src/domain_conf.h	3 Mar 2009 09:44:42 -0000	1.39
+++ src/domain_conf.h	3 Mar 2009 16:36:17 -0000
@@ -410,6 +410,13 @@ struct _virDomainOSDef {
     char *bootloaderArgs;
 };
 
+enum virDomainSeclabelType {
+    VIR_DOMAIN_SECLABEL_DYNAMIC,
+    VIR_DOMAIN_SECLABEL_STATIC,
+
+    VIR_DOMAIN_SECLABEL_LAST,
+};
+
 /* Security configuration for domain */
 typedef struct _virSecurityLabelDef virSecurityLabelDef;
 typedef virSecurityLabelDef *virSecurityLabelDefPtr;
@@ -417,6 +424,7 @@ struct _virSecurityLabelDef {
     char *model;        /* name of security model */
     char *label;        /* security label string */
     char *imagelabel;   /* security image label string */
+    int type;
 };
 
 #define VIR_DOMAIN_CPUMASK_LEN 1024
@@ -650,5 +658,6 @@ VIR_ENUM_DECL(virDomainInputBus)
 VIR_ENUM_DECL(virDomainGraphics)
 /* from libvirt.h */
 VIR_ENUM_DECL(virDomainState)
+VIR_ENUM_DECL(virDomainSeclabel)
 
 #endif /* __DOMAIN_CONF_H */
Index: src/qemu_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/qemu_driver.c,v
retrieving revision 1.213
diff -u -p -r1.213 qemu_driver.c
--- src/qemu_driver.c	3 Mar 2009 15:18:24 -0000	1.213
+++ src/qemu_driver.c	3 Mar 2009 16:36:18 -0000
@@ -1314,9 +1314,9 @@ static int qemudStartVMDaemon(virConnect
     hookData.vm = vm;
     hookData.driver = driver;
 
-   /* If you are using a SecurityDriver and there was no security label in
-      database, then generate a security label for isolation */
-    if (vm->def->seclabel.label == NULL &&
+   /* If you are using a SecurityDriver with dynamic labelling, 
+      then generate a security label for isolation */
+    if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
         driver->securityDriver &&
         driver->securityDriver->domainGenSecurityLabel &&
         driver->securityDriver->domainGenSecurityLabel(conn, vm) < 0)
@@ -1525,6 +1525,13 @@ static void qemudShutdownVMDaemon(virCon
     if (driver->securityDriver)
         driver->securityDriver->domainRestoreSecurityLabel(conn, vm);
 
+    /* Clear out dynamically assigned labels */
+    if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
+        VIR_FREE(vm->def->seclabel.model);
+        VIR_FREE(vm->def->seclabel.label);
+        VIR_FREE(vm->def->seclabel.imagelabel);
+    }
+
     if (qemudRemoveDomainStatus(conn, driver, vm) < 0) {
         VIR_WARN(_("Failed to remove domain status for %s"),
                  vm->def->name);

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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