Re: [Xen-devel] [PATCH] libxl: assigned a default ssid_label (XSM label) to guests

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

On 05/14/2015 07:54 AM, Ian Campbell wrote:
> On Thu, 2015-05-14 at 12:21 +0100, Julien Grall wrote:
> > Hi Ian,
> >
> > On 14/05/15 11:33, Ian Campbell wrote:
> > > system_u:system_r:domU_t is defined in the default policy and makes as
> > > much sense as anything for a default.
> > So you rule out the possibility to run an unlabelled domain? This is
> > possible if the policy explicitly authorized it. That's a significant
> > change in the libxl behavior.
> I didn't realise this was a possibility, wouldn't such a domain be
> system_u:system_r:unlabeled_t> or something?
Yes.  FLASK resolves any numeric SID value that is unused (including zero)
to the unlabeled sid (defined in tools/flask/policy/policy/initial_sids
to be system_u:system_r:unlabeled_t).  Because this could be the result of
an error (in the hypervisor, toolstack, etc), the use of unlabled_t for
real objects is discouraged in SELinux and XSM/FLASK.

> Note that this won't override a label which is just '' (i.e. an empty
> string rather than NULL). I don't know if that results in the behaviour
> you want.
>
> When this was discussed before (in a conversation Wei started, but which
> I can't find, maybe it was IRC rather than email) it seemed that
> consensus was that by default things should Just Work as if XSM weren't
> disabled, which is what I've implemented here.
I agree that this is a useful feature.  It is possible to extend the
initial_sids list with new entries that are used by the toolstack instead
of by the hypervisor, which could be used to define SECINITSID_DOMU as the
default label for a domU created by a toolstack without a label.  This is
better than hard-coding a string that may not be valid in a given security
policy, and it can be associated with a label that better reflects how the
policy wishes to treat domains with an "incomplete" configuration file.

The header file defining these SIDs is buried in the hypervisor source
tree (xen/xsm/flask/include/flask.h) and is only generated during a build
with XSM enabled.  It may be simpler to define the value in a shared header
and add a BUILD_BUG_ON somewhere in the flask code to check for mismatches.

> > IHMO, having a default policy doesn't mean libxl should set a default
> > ssid to make XSM transparent to the user.
> >
> > The explicit ssid makes clear that the guest is using a ssid foo and if
> > it's not provided then it will fail to boot.
> >
> > Setting a default value may hide a bigger issue and take the wrong
> > policy the user forgot to set up an ssid.
> Does domU_t really have so many privileges that this is an issue? I'd
> expect it to be almost totally privilegeless apart from things which any
> domU needs.
>
> The benefits of XSM seem to mainly apply to the various service domains.
>
> Daniel, do you have an opinion here?
In the example policy, domU_t should have the same level of access as a
normal domain (i.e. not device model stubdom) has with XSM disabled.

The only real difference is that the example policy does not allow any
domain to act as a device model to domU_t; it uses domHVM_t and dm_dom_t
for this.  If you want to use configurations with device model stubdoms
that also do not assign labels in the configuration, this distinction
will need to be removed.

> > > This change required moving the call to domain_create_info_setdefault
> > > to be before the ssid_label is translated into ssidref, which also
> > > moves it before some other stuff which consumes things from c_info,
> > > which is correct since setdefault should always be called first. Apart
> > > from the SSID handling there should be no functional change (since
> > > setdefault doesn't actually act on anything which that other stuff
> > > uses).
> > >
> > > There is no need to set exec_ssid_label since the default is to leave
> > > the domain using the ssid_label after build.
> > By setting a ssid label, libxl will print a new warning on Xen not built
> > with XSM which will confuse the user:
> >
> > libxl: warning: libxl_create.c:813:initiate_domain_create: XSM Disabled:
> > init_seclabel not supported
> Ah, I didn't try that case. I'll see if I can work out a way to suppress
> that warning.
I would be fine with removing that warning completely; someone trying to
use XSM without it enabled will likely be able to figure out the problem
without this error, likely by noticing the "-" labels in xl list -v/-Z.

------------->8-----------------
Example patch adding SECINITSID_DOMU, for testing/reference.

---
diff --git a/tools/flask/policy/policy/initial_sids 
b/tools/flask/policy/policy/initial_sids
index 5de0bbf..48aad17 100644
--- a/tools/flask/policy/policy/initial_sids
+++ b/tools/flask/policy/policy/initial_sids
@@ -12,3 +12,4 @@ sid irq gen_context(system_u:object_r:irq_t,s0)
 sid iomem gen_context(system_u:object_r:iomem_t,s0)
 sid ioport gen_context(system_u:object_r:ioport_t,s0)
 sid device gen_context(system_u:object_r:device_t,s0)
+sid domU gen_context(system_u:system_r:domU_t,s0)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index f0da7dc..0c3d4ed 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -815,6 +815,8 @@ static void initiate_domain_create(libxl__egc *egc,
                 goto error_out;
             }
         }
+    } else {
+        d_config->c_info.ssidref = 11; /* SECINITSID_DOMU */
     }
 
     if (d_config->b_info.exec_ssid_label) {
diff --git a/xen/xsm/flask/policy/initial_sids 
b/xen/xsm/flask/policy/initial_sids
index e508bde..a442a38 100644
--- a/xen/xsm/flask/policy/initial_sids
+++ b/xen/xsm/flask/policy/initial_sids
@@ -13,4 +13,5 @@ sid ioport
 sid iomem
 sid irq
 sid device
+sid domU
 # FLASK


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel