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

[Stephen Smalley <sds@tycho.nsa.gov>]

On 2/15/19 2:36 PM, Dominick Grift wrote:
> 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.

Oh, I see: scripts/selinux/install_policy.sh just invokes checkpolicy 
without specifying -U / --handle-unknown, so the policy defaults to 
deny, and that would indeed render dbus-daemon and systemd broken with 
that policy.  Might be as simple to fix as passing -U allow.

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