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

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 05/15/2015 05:39 AM, Ian Campbell wrote:
> On Thu, 2015-05-14 at 19:09 -0400, Daniel De Graaf wrote:
>> 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.
>
> OK, so I think this rules out using unlabelled_t as a default.
>
>>> 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.
>
> That sounds good.
>
>>From the PoV of the code in libxl this would be done by writing ssidref
> as a literal number rather than translating a string (judging from your
> example patch). That works for me.
>
> While looking into this I noticed the existing code is
>     if (ssid_label)
>         ssid_ref = translate(ssid_label)
>
> So my first patch has another issue which is that it will override a
> users attempt to use ssid_ref themselves (which they are entitled to
> do).
>
> Is ssidref==0 "unused"? i.e. could we use it to differentiate whether
> the field had been filled in by the user or not? If not is there some
> other number? Or should we reserve one using the same technique you
> suggest here for domU?

Yes, a value of zero is "not populated"; no valid context resolves to a
SID of zero (including unlabeled_t, which is properly 5 or so).

>> 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.
>
> I was about to ask about this. Short of a pretty serious change to the
> build a BUILD_BUG_ON seems like a reasonable approach.
>
>>>> 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.
>
> I'd be inclined to go the other way and either have a default ssid for
> the DM or to fail if one isn't given (the latter would probably happen
> anyway due to enforcement?).

Yes, it would probably fail at xc_domain_set_target in enforcing mode.

> Sounds like the default ssidref should be either ~= domU_t of domHVM_t
> depending on the type of domain? (domU_t is really domPV_t?)

The domU_t type also works for HVM domains with the device model in dom0.

Looking at the problem again, I think a second initial SID for the device
model would be preferable, removing domHVM_t completely.  There are already
other example types in the policy for domains that do not use a device model
(isolated_domU_t is probably the best example), and the result more closely
matches the permissions used in the hypervisor without XSM enabled.

-- 
Daniel De Graaf
National Security Agency