Stephen Smalley <sds@xxxxxxxxxxxxx> writes: > On 5/16/19 11:07 AM, Petr Lautrbach wrote: >> >> Stephen Smalley <sds@xxxxxxxxxxxxx> 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@xxxxxxxxxx> >>>> --- > <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/
