On Fri, Dec 13, 2019 at 9:07 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > On Thu, Dec 12, 2019 at 8:01 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > On Thu, Dec 12, 2019 at 1:48 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: > > > On 12/12/19 1:18 PM, Paul Moore wrote: > > > > On Thu, Dec 12, 2019 at 1:10 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: > > > >> > > > >> On 12/12/19 1:09 PM, Paul Moore wrote: > > > >>> On Thu, Dec 12, 2019 at 12:57 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: > > > >>>> On 12/12/19 12:54 PM, Paul Moore wrote: > > > >>>>> On Thu, Dec 12, 2019 at 8:14 AM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: > > > >>>>>> On 12/12/19 6:49 AM, Ondrej Mosnacek wrote: > > > >>>>>>> On Wed, Dec 11, 2019 at 8:12 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: > > > >>>>>>>> On 12/11/19 1:35 PM, Casey Schaufler wrote: > > > >>>>>>>>> On 12/11/2019 8:42 AM, Kees Cook wrote: > > > >>>>>>>>>> On Wed, Dec 11, 2019 at 09:29:10AM -0500, Stephen Smalley wrote: > > > >>>>>>>>>>> On 12/11/19 9:08 AM, Ondrej Mosnacek wrote: > > > >>>>> > > > >>>>> ... > > > >>>>> > > > >>>>>>>> selinux_state.initialized reflects whether a policy has > > > >>>>>>>> been loaded. With a few exceptions in certain hook functions, it is > > > >>>>>>>> only checked by the security server service functions > > > >>>>>>>> (security/selinux/ss/services.c) prior to accessing the policydb. So > > > >>>>>>>> there is a lot of SELinux processing that would still occur in that > > > >>>>>>>> situation unless we added if (!selinux_state.initialized) return 0; > > > >>>>>>>> checks to all the hook functions, which would create the same exposure > > > >>>>>>>> and would further break the SELinux-enabled case (we need to perform > > > >>>>>>>> some SELinux processing pre-policy-load to allocate blobs and track what > > > >>>>>>>> tasks and objects require delayed security initialization when policy > > > >>>>>>>> load finally occurs). > > > >>>>>>> > > > >>>>>>> I think what Casey was suggesting is to add another flag that would > > > >>>>>>> switch from "no policy loaded, but we expect it to be loaded > > > >>>>>>> eventually" to "no policy loaded and we don't expect/allow it to be > > > >>>>>>> loaded any more", which is essentially equivalent to checking > > > >>>>>>> selinux_enabled in each hook, which you had already brought up. > > > >>>>>> > > > >>>>>> Yep. if (!selinux_enabled) return 0; or if (selinux_state.disabled) > > > >>>>>> return 0; under #ifdef CONFIG_SECURITY_SELINUX_DISABLE in every hook > > > >>>>>> might be the best option until it can be removed altogether; avoids > > > >>>>>> impacting the LSM framework or any other security module, preserves the > > > >>>>>> existing functionality, fairly low overhead on the SELinux-disabled case. > > > >>>>> > > > >>>>> Just so I'm understanding this thread correctly, the above change > > > >>>>> (adding enabled checks to each SELinux hook implementation) is only > > > >>>>> until Fedora can figure out a way to deprecate and remove the runtime > > > >>>>> disable? > > > >>>> > > > >>>> That's my understanding. In the interim, Android kernels should already > > > >>>> be disabling CONFIG_SECURITY_SELINUX_DISABLE and other distros may > > > >>>> choose to disable it as long as they don't care about supporting SELinux > > > >>>> runtime disable. > > > >>> > > > >>> Okay, I just wanted to make sure I wasn't missing something. > > > >>> Honestly, I'd rather Fedora just go ahead and do whatever it is they > > > >>> need to do to turn off CONFIG_SECURITY_SELINUX_DISABLE (it sounds like > > > >>> they have a plan and are working on it), I'm not overly excited about > > > >>> temporarily cluttering up the code with additional "enabled" checks > > > >>> when the status quo works, even if it is less than ideal. > > > >> > > > >> The status quo is producing kernel crashes, courtesy of LSM stacking > > > >> changes... > > > > > > > > How prevalent are these crashes? > > > > > > > > This also only happens when disabling SELinux at runtime, yes? > > > > Something we've advised against for some time now and are working to > > > > eliminate? Let's just get rid of the runtime disable *soon*, and if > > > > we need a stop-gap fix let's just go with the hook reordering since > > > > that seems to minimize the impact, if not resolve it. > > > > > > Not optimistic given that the original bug was opened 2.5+ years ago, > > > commenters identified fairly significant challenges to removing the > > > support, and no visible progress was ever made. I suppose the hook > > > reordering will do but seems fragile and hacky. Whatever. > > > > Based on Ondrej's comments in this thread it sounded as if there was > > some renewed interest in actually eliminating it from Fedora this > > time. If that's not the case, perhaps it's time to start that effort > > back up, and if we can't ever get away from the runtime disable then > > we can take the step of admitting that everything is terrible and we > > can consider some of the options presented in this thread. > > Yes, we are quite determined to do what it takes to remove it. It may > go more smoothly than expected, or it may get unexpectedly > complicated. It's hard to tell at this point. That's good to hear, I know it is going to be difficult, but I think we can all agree it's the right thing to do. Any idea when you might have a better idea of the time involved? Next week I think I'm going to put together a RFC patch that marks CONFIG_SECURITY_SELINUX_DISABLE as deprecated, and displays a warning when it is used at runtime. Later on when we have a better idea of the removal date, we can start adding delays when it is used to help get people to migrate to the cmdline approach. -- paul moore www.paul-moore.com
