Re: [kernel-hardening] Re: [RFC PATCH 2/4] security: mark nf ops in SELinux and Smack as __ro_after_init

From: Casey Schaufler <casey@schaufler-ca.com>
To: Kees Cook <keescook@chromium.org>
Cc: Stephen Smalley <sds@tycho.nsa.gov>,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	James Morris <jmorris@namei.org>,
	linux-security-module <linux-security-module@vger.kernel.org>,
	"kernel-hardening@lists.openwall.com"
	<kernel-hardening@lists.openwall.com>,
	Paul Moore <paul@paul-moore.com>
Subject: Re: [kernel-hardening] Re: [RFC PATCH 2/4] security: mark nf ops in SELinux and Smack as __ro_after_init
Date: Mon, 13 Feb 2017 14:01:13 -0800	[thread overview]
Message-ID: <2395a11c-3583-d98a-5e63-ff1b6fe44d43@schaufler-ca.com> (raw)
In-Reply-To: <CAGXu5j+BoPiy_YNgS4MC9vue8UDWZ5bwhpCO=bnvbgucsNGeqQ@mail.gmail.com>

On 2/13/2017 1:49 PM, Kees Cook wrote:
> On Mon, Feb 13, 2017 at 1:32 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 2/13/2017 1:03 PM, Stephen Smalley wrote:
>>> On Mon, 2017-02-13 at 09:29 -0800, Kees Cook wrote:
>>>> On Mon, Feb 13, 2017 at 3:29 AM, Tetsuo Handa
>>>> <penguin-kernel@i-love.sakura.ne.jp> wrote:
>>>>> James Morris wrote:
>>>>>> Both SELinux and Smack register Netfilter operations during init,
>>>>>> which then don't change.  Mark these ops as __ro_after_init.
>>>>>>
>>>>>> Signed-off-by: James Morris <james.l.morris@oracle.com>
>>>>> This patch breaks CONFIG_SECURITY_SELINUX_DISABLE=y +
>>>>> SELINUX=disabled in /etc/selinux/config case,
>>>>> doesn't it? Although I heard that SELinux is planning to remove
>>>>> CONFIG_SECURITY_SELINUX_DISABLE,
>>>>> CONFIG_SECURITY_SELINUX_DISABLE is valid as of current linux-
>>>>> security.git#next .
>>>> We could fold that removal into this series?
>>> I'm personally in favor of removing it, but that support was originally
>>> requested by the Fedora distro folks on the grounds that it is too
>>> painful to manage kernel boot parameters on some platforms, and
>>> therefore they needed an alternative to booting with selinux=0 on the
>>> kernel command line.  The documented way to disable SELinux on such
>>> distros is through the use of /etc/selinux/config SELINUX=disabled,
>>> which relies on this mechanism.  So you'd have to work through removal
>>> with the distro folks.
>>>
>>> Maybe in the interim we could just wrap the ro-after-init markings
>>> under a conditional dependent on !CONFIG_SECURITY_SELINUX_DISABLE so
>>> that systems (e.g. Android) that do not rely on this feature could
>>> benefit.
>> If we changed CONFIG_SECURITY_SELINUX_DISABLE to
>> CONFIG_SECURITY_DYNAMIC_MODULES and put the __ro_after_init
>> under !CONFIG_SECURITY_DYNAMIC_MODULES we solve both the
>> current and potential future issues.
> Something like...
>
> #ifdef CONFIG_SECURITY_DYNAMIC_LSM
> # define lsm_ro_after_init __ro_after_init
> # define lsm_const         const
> #else
> # define lsm_ro_after_init
> # define lsm_const
> #endif
>
> ?

That's more elaborate than I was thinking, but sure.
I was going to put ifdefs in the respective declarations,
but this may be more in line with modern tastes.

>
> -Kees
>