From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from youngberry.canonical.com ([91.189.89.112]:51610 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726027AbeJBE0w (ORCPT ); Tue, 2 Oct 2018 00:26:52 -0400 Subject: Re: [PATCH security-next v3 18/29] LSM: Introduce lsm.enable= and lsm.disable= References: <20180925001832.18322-1-keescook@chromium.org> <20180925001832.18322-19-keescook@chromium.org> From: John Johansen Message-ID: <68e4e323-3216-7e77-2807-c3207126ae68@canonical.com> Date: Mon, 1 Oct 2018 14:46:54 -0700 MIME-Version: 1.0 In-Reply-To: <20180925001832.18322-19-keescook@chromium.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-arch-owner@vger.kernel.org List-ID: To: Kees Cook , James Morris Cc: Casey Schaufler , Tetsuo Handa , Paul Moore , Stephen Smalley , "Schaufler, Casey" , LSM , Jonathan Corbet , linux-doc@vger.kernel.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org Message-ID: <20181001214654.PxoFDIETFoC3vckbbgxo9x0UsEyoIBt3e73unwfrEC8@z> On 09/24/2018 05:18 PM, Kees Cook wrote: > This introduces the "lsm.enable=..." and "lsm.disable=..." boot parameters > which each can contain a comma-separated list of LSMs to enable or > disable, respectively. The string "all" matches all LSMs. > > This has very similar functionality to the existing per-LSM enable > handling ("apparmor.enabled=...", etc), but provides a centralized > place to perform the changes. These parameters take precedent over any > LSM-specific boot parameters. > > Disabling an LSM means it will not be considered when performing > initializations. Enabling an LSM means either undoing a previous > LSM-specific boot parameter disabling or a undoing a default-disabled > CONFIG setting. > > For example: "lsm.disable=apparmor apparmor.enabled=1" will result in > AppArmor being disabled. "selinux.enabled=0 lsm.enable=selinux" will > result in SELinux being enabled. > > Signed-off-by: Kees Cook I don't like this. It brings about conflicting kernel params that are bound to confuse users. Its pretty easy for a user to understand that when they specify a parameter manually at boot, that it overrides the build time default. But conflicting kernel parameters are a lot harder to deal with. I prefer a plain enabled= list being an override of the default build time value. Where conflicts with LSM-specific configs always result in the LSM being disabled with a complaint about the conflict. Though I have yet to be convinced its worth the cost, I do recognize it is sometimes convenient to disable a single LSM, instead of typing in a whole list of what to enable. If we have to have conflicting kernel parameters I would prefer that the conflict throw up a warning and leaving the LSM with the conflicting config disabled. > --- > .../admin-guide/kernel-parameters.txt | 12 ++++++++++ > security/Kconfig | 4 +++- > security/security.c | 22 +++++++++++++++++++ > 3 files changed, 37 insertions(+), 1 deletion(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index 32d323ee9218..67c90985d2b8 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -2276,6 +2276,18 @@ > > lsm.debug [SECURITY] Enable LSM initialization debugging output. > > + lsm.disable=lsm1,...,lsmN > + [SECURITY] Comma-separated list of LSMs to disable > + at boot time. This overrides "lsm.enable=", > + CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and boot > + parameters. > + > + lsm.enable=lsm1,...,lsmN > + [SECURITY] Comma-separated list of LSMs to enable > + at boot time. This overrides any omissions from > + CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and > + boot parameters. > + > machvec= [IA-64] Force the use of a particular machine-vector > (machvec) in a generic kernel. > Example: machvec=hpzx1_swiotlb > diff --git a/security/Kconfig b/security/Kconfig > index 71306b046270..1a82a006cc62 100644 > --- a/security/Kconfig > +++ b/security/Kconfig > @@ -282,7 +282,9 @@ config LSM_ENABLE > help > A comma-separate list of LSMs to enable by default at boot. The > default is "all", to enable all LSM modules at boot. Any LSMs > - not listed here will be disabled by default. > + not listed here will be disabled by default. This can be > + changed with the "lsm.enable=" and "lsm.disable=" boot > + parameters. > > endmenu > > diff --git a/security/security.c b/security/security.c > index 7ecb9879a863..456a3f73bc36 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -44,6 +44,8 @@ char *lsm_names; > /* Boot-time LSM user choice */ > static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] = > CONFIG_DEFAULT_SECURITY; > +static __initdata const char *chosen_lsm_enable; > +static __initdata const char *chosen_lsm_disable; > > static __initconst const char * const builtin_lsm_enable = CONFIG_LSM_ENABLE; > > @@ -185,6 +187,10 @@ static void __init prepare_lsm_enable(void) > { > /* Prepare defaults. */ > parse_lsm_enable(builtin_lsm_enable, default_enabled, true); > + > + /* Process "lsm.enable=" and "lsm.disable=", if given. */ > + parse_lsm_enable(chosen_lsm_enable, set_enabled, true); > + parse_lsm_enable(chosen_lsm_disable, set_enabled, false); > } > > /** > @@ -240,6 +246,22 @@ static int __init enable_debug(char *str) > } > __setup("lsm.debug", enable_debug); > > +/* Explicitly enable a list of LSMs. */ > +static int __init enable_lsm(char *str) > +{ > + chosen_lsm_enable = str; > + return 1; > +} > +__setup("lsm.enable=", enable_lsm); > + > +/* Explicitly disable a list of LSMs. */ > +static int __init disable_lsm(char *str) > +{ > + chosen_lsm_disable = str; > + return 1; > +} > +__setup("lsm.disable=", disable_lsm); > + > static bool match_last_lsm(const char *list, const char *lsm) > { > const char *last; >