Re: [PATCH v3] scripts/selinux: add basic mls support to mdp

[Dominick Grift <dac.override@gmail.com>]

Stephen Smalley <sds@tycho.nsa.gov> writes:

> On 2/15/19 2:21 PM, Dominick Grift wrote:
>> Paul Moore <paul@paul-moore.com> writes:
>>
>>> On Fri, Feb 15, 2019 at 12:24 PM Dominick Grift <dac.override@gmail.com> wrote:
>>>> Dominick Grift <dac.override@gmail.com> writes:
>>>>> Stephen Smalley <sds@tycho.nsa.gov> writes:
>>>>>
>>>>>> On 2/15/19 10:25 AM, Stephen Smalley wrote:
>>>>>>> On 2/15/19 10:05 AM, Stephen Smalley wrote:
>>>>>>>> On 2/15/19 10:03 AM, Stephen Smalley wrote:
>>>>>>>>> On 2/15/19 10:00 AM, Paul Moore wrote:
>>>>>>>>>> On Fri, Feb 15, 2019 at 9:51 AM Stephen Smalley
>>>>>>>>>> <sds@tycho.nsa.gov> wrote:
>>>>>>>>>>> Add basic MLS policy support to mdp.  Declares
>>>>>>>>>>> two sensitivities and two categories, defines
>>>>>>>>>>> mls constraints for all permissions requiring
>>>>>>>>>>> dominance (ala MCS), assigns the system-high
>>>>>>>>>>> level to initial SID contexts and the default user
>>>>>>>>>>> level, and assigns system-low level to filesystems.
>>>>>>>>>>>
>>>>>>>>>>> Also reworks the fs_use and genfscon rules to only
>>>>>>>>>>> generate rules for filesystems that are configured
>>>>>>>>>>> in the kernel.  In some cases this depends on a specific
>>>>>>>>>>> config option for security xattrs, in other cases security
>>>>>>>>>>> xattrs are unconditionally supported by a given filesystem
>>>>>>>>>>> if the filesystem is enabled, and in some cases the filesystem
>>>>>>>>>>> is always enabled in the kernel.  Dropped obsolete pseudo
>>>>>>>>>>> filesystems.
>>>>>>>>>>>
>>>>>>>>>>> NB The list of fs_use_* and genfscon rules emitted by mdp
>>>>>>>>>>> is very incomplete compared to refpolicy or Android sepolicy.
>>>>>>>>>>> We should probably expand it.
>>>>>>>>>>>
>>>>>>>>>>> Usage:
>>>>>>>>>>> scripts/selinux/mdp/mdp -m policy.conf file_contexts
>>>>>>>>>>> checkpolicy -M -o policy policy.conf
>>>>>>>>>>>
>>>>>>>>>>> Then install the resulting policy and file_contexts as usual.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
>>>>>>>>>>> ---
>>>>>>>>>>> v3 fixes up the file contexts generation code to also use
>>>>>>>>>>> SYSTEMLOW and
>>>>>>>>>>> collapse down to a single fprintf call per line.
>>>>>>>>>>>    scripts/selinux/mdp/mdp.c | 131
>>>>>>>>>>> ++++++++++++++++++++++++++++++--------
>>>>>>>>>>>    1 file changed, 103 insertions(+), 28 deletions(-)
>>>>>>>>>>
>>>>>>>>>> This is great Stephen, thanks for working on this - and rather quickly
>>>>>>>>>> too!  For those who don't follow the GitHub issues, I just opened an
>>>>>>>>>> issue yesterday mentioning it would be nice to add MLS support to the
>>>>>>>>>> mdp tool.
>>>>>>>>>>
>>>>>>>>>> Are you planning to keep playing with this?  I'm asking not because I
>>>>>>>>>> think it needs more work to be worthwhile, but rather I don't want to
>>>>>>>>>> merge something that you want to continue working on.  If you are
>>>>>>>>>> happy with this latest patch I think it is okay to merge this into
>>>>>>>>>> selinux/next, even at this late stage, simply because it is not part
>>>>>>>>>> of a built kernel, but rather a developer's tool.
>>>>>>>>>
>>>>>>>>> No, I think I'm done for now unless you find a problem with
>>>>>>>>> it. Absent some compelling use case for mdp it is hard to justify
>>>>>>>>> spending any more time on it.
>>>>>>>>
>>>>>>>> Note however that the instructions in
>>>>>>>> Documentation/admin-guide/LSM/SELinux.rst just say to run
>>>>>>>> scripts/selinux/install_policy.sh and since that doesn't pass -m to
>>>>>>>> mdp or -M to checkpolicy, no one will use this support unless they
>>>>>>>> do it all by hand.
>>>>>>>
>>>>>>> FWIW, a Fedora system wouldn't come up cleanly with this policy.
>>>>>>> Partly appears to be due to systemd having embedded security
>>>>>>> contexts specific to Fedora/refpolicy into its own configurations
>>>>>>> and partly due to MLS denials.  I don't even know if it would work
>>>>>>> before this change though...
>>>>>>
>>>>>> Couldn't seem to get a mdp-generated policy to boot on Fedora even in
>>>>>> permissive, before or after this change.  I assume it has to do with
>>>>>> leaking of contexts outside of the policy and/or missing config files
>>>>>> from the dummy policy (e.g. /etc/selinux/targeted/contexts/ has
>>>>>> systemd_contexts and other userspace config files that don't exist in
>>>>>> the mdp policy).  More evidence of the irrelevance of mdp...
>>>>>
>>>>> Oh, right you need a "dbus_contexts" file probably. DBUS refuses to
>>>>> start without it, and these day's without dbus no system
>>>>
>>>> My dssp2-minimal [1] policy is my alternative to mdp.
>>>>
>>>> https://github.com/DefenSec/dssp2-minimal
>>>>
>>>> It is not quite as simple as mpd but it think it is decent balance
>>>> between having something useful and still easy to read.
>>>
>>> While dssp2-minimal is much smaller than reference policy, it's still
>>> an order of magnitude larger than the mdp generated policy.  I'm not
>>> sure if this is something you care about, but if you wanted to work on
>>> getting mdp to the point where it would allow a Fedora system (or any
>>> modern SELinux based system for that matter) to boot, that could be
>>> useful, even if I'm not convinced it needs to be a priority at the
>>> moment.
>>
>> It is also an order of magnitude more useful than mdp.
>>
>> I suppose I could look at what it would take to get it to boot on some
>> rainy afternoon. Should not be hard, but i hesitate to polute mdp with
>> user space access vectors. It feels like setting a precendent somehow.
>
> In theory, if using selinux_check_access() to check permissions and if
> the policy sets allow_unknown=true, then the absence of the userspace
> classes and access vectors should just cause all userspace permission
> checks to pass.
>
> Of course, not all userspace object managers use
> selinux_check_access(), and may not check security_deny_unknown()
> directly.

The two object managers that matter do use selinux_access_check()

I admit that I got a little curious to find out what the issue is.

>
>>
>>>
>>> Besides, haven't you always wanted to get a patch accepted into the
>>> kernel Dominick? ;)
>>
>> Not particularly, no.
>>
>

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift