Re: [PATCH] libselinux: Eliminate use of security_compute_user()

[Petr Lautrbach <plautrba@redhat.com>]

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

> On 5/16/19 11:07 AM, Petr Lautrbach wrote:
>>
>> Stephen Smalley <sds@tycho.nsa.gov> writes:
>>
>>> On 5/9/19 4:42 AM, Petr Lautrbach wrote:
>>>> get_ordered_context_list() code used to ask the kernel to compute the
>>>> complete
>>>> set of reachable contexts using /sys/fs/selinux/user aka
>>>> security_compute_user(). This set can be so huge so that it doesn't fit into
>>>> a
>>>> kernel page and security_compute_user() fails. Even if it doesn't fail,
>>>> get_ordered_context_list() throws away the vast majority of the returned
>>>> contexts because they don't match anything in
>>>> /etc/selinux/targeted/contexts/default_contexts or
>>>> /etc/selinux/targeted/contexts/users/
>>>>
>>>> get_ordered_context_list() is rewritten to compute set of contexts based on
>>>> /etc/selinux/targeted/contexts/users/ and
>>>> /etc/selinux/targeted/contexts/default_contexts files and to return only
>>>> valid
>>>> contexts, using security_check_context(), from this set.
>>>>
>>>> Fixes: https://github.com/SELinuxProject/selinux/issues/28
>>>>
>>>> Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
>>>> ---
> <snip>
>>> I think the main potential stumbling block here is the MLS range component.
>>> The
>>> kernel policy defines the default level and allowed range for the (SELinux)
>>> user, and uses this information in the kernel function
>>> mls_setup_user_range(),
>>> https://elixir.bootlin.com/linux/latest/source/security/selinux/ss/mls.c#L402, 
>>>
>>> to determine the most suitable MLS range for the user session, based on both
>>> the
>>> from-context and the user default and range from the kernel policy.  Just
>>> using
>>> the level from the from-context could fail if the user isn't authorized to
>>> operate at that level, and even if the user is authorized to operate at that
>>> level, it could introduce a change in the default behavior if the user's
>>> default
>>> level differs. I think when we have discussed this in the past on the list,
>>> we
>>> were going to either export the user's default level and range information
>>> from
>>> the kernel via selinuxfs and replicate the mls_setup_user_ranges() logic in
>>> userspace, or have it automatically extracted from the kernel policy during
>>> policy build into a userspace configuration file that could be used directly
>>> by
>>> userspace.  Or something like that.  This gets a bit tricky though in that
>>> the
>>> logic involves comparing MLS levels, which is intrinsically policy-specific
>>> logic, and thus if we wanted to truly replicate it in userspace, we'd
>>> probably
>>> need to use libsepol.  Ugh. Maybe the kernel could just provide a simple
>>> selinuxfs interface for computing the result of mls_setup_user_range() and
>>> return that piece.
>>>
>>> That said, I don't know to what extent anyone is relying on this logic and to
>>> what extent it is obsoleted by the use of the level/range from seusers.  It
>>> looks like today we are replacing the level/range in the original
>>> from-context
>>> with the one from seusers before calling this code, in which case the
>>> fromlevel
>>> is in fact the one we ultimately want to use.  So perhaps this doesn't matter
>>> and we can just go with your approach.
>>
>> The problem is much complicated than I originally thought and this
>> patch changes the behavior of get_ordered_context_list what is probably
>> not acceptable.
>>
>> I'll do more tests and think about it the light of new (for me)
>> information.
>>
>> Thanks all for reviews and inputs.
>
> I would like to re-visit this patch again.  I did some looking at how
> get_ordered_context_list() and its variant interfaces are currently being used
> by callers, and at the internal logic of get_ordered_context_list() in userspace
> and mls_setup_user_ranges() in the kernel.  Since we are already substituting
> the range/level from seusers into the from-context before calling
> security_compute_user(), and since the only sensible configuration of seusers
> would be to use a range/level that falls within (or is identical to) the SELinux
> user's authorized range, I don't think your patch is likely to break anything.
> There are corner cases where it could yield a different result but I would be
> surprised if such corner cases are in real use and arguably they would be
> configuration errors.  Consequently, I think we should refresh your patch,
> address any comments made on it previously, and submit it for merging and try it
> out.  If we encounter any real world breakage from it, we can consider adding a
> new selinuxfs node that exports the kernel's mls_setup_user_ranges() logic and
> rework get_ordered_context_list() to use that to obtain the MLS portion of the
> context, but I don't think it is worth doing that without a real example where
> simply applying your patch breaks something.  Thoughts?


No objection at the moment. But it'll take me few days, we're kind of busy
when it's about https://www.devconf.info/cz/