diff mbox series

[v3,1/1] security: Add CONFIG_LSM_AUTO to handle default LSM stack ordering

Message ID 20210222150608.808146-2-mic@digikod.net (mailing list archive)
State New, archived
Headers show
Series Automatic LSM stack ordering | expand

Commit Message

Mickaël Salaün Feb. 22, 2021, 3:06 p.m. UTC
From: Mickaël Salaün <mic@linux.microsoft.com>

Add a new option CONFIG_LSM_AUTO to enable users to delegate default LSM
stacking order to kernel developers.  This enable to keep a consistent
order of enabled LSM when changing the LSM selection, especially when a
new LSM is added to the kernel.

CONFIG_LSM depends on !CONFIG_LSM_AUTO, which is backward compatible and
gives the opportunity to users to select CONFIG_LSM_AUTO with a make
oldconfig.

CONFIG_LSM and CONFIG_LSM_AUTO depend on CONFIG_SECURITY, which makes
sense because an LSM depends on the security framework.

Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20210222150608.808146-2-mic@digikod.net
---

Changes since v2:
* Revamp without virtual dependencies but a new option to automatically
  enable all selected LSMs.

Changes since v1:
* Add CONFIG_SECURITY as a dependency of CONFIG_LSM.  This prevent an
  error when building without any LSMs.
---
 security/Kconfig    | 19 +++++++++++++++++++
 security/security.c | 26 +++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)

Comments

Casey Schaufler Feb. 22, 2021, 4:51 p.m. UTC | #1
On 2/22/2021 7:06 AM, Mickaël Salaün wrote:
> From: Mickaël Salaün <mic@linux.microsoft.com>
>
> Add a new option CONFIG_LSM_AUTO to enable users to delegate default LSM
> stacking order to kernel developers.  This enable to keep a consistent
> order of enabled LSM when changing the LSM selection, especially when a
> new LSM is added to the kernel.

TL;DR - NAK

Do you think that we might have considered this when stacking was
introduced? Did you even consider the implications before sending
the patch? This only makes any sense if you want to compile in
AppArmor and/or Smack but always use SELinux. The existing Kconfig
model handles that perfectly well. Also, this will break when the
next phase of module stacking comes in, and all of a sudden
systems will automatically get AppArmor in addition to SELinux
or Smack.

I know that the CONFIG_LSM/lsm= mechanism is clumsy. But we spent
about a year discussing, proposing and implementing alternatives,
and if there's a better mechanism, we couldn't find it. Of course
we considered "just use the kernel order". It doesn't work for
generic kernels. I understand that adding a new LSM that you want
to be included by default is a tough problem. I also suggest
that silently adding an LSM to an existing configuration is likely
to violate the principle of least astonishment.

>
> CONFIG_LSM depends on !CONFIG_LSM_AUTO, which is backward compatible and
> gives the opportunity to users to select CONFIG_LSM_AUTO with a make
> oldconfig.
>
> CONFIG_LSM and CONFIG_LSM_AUTO depend on CONFIG_SECURITY, which makes
> sense because an LSM depends on the security framework.
>
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Serge E. Hallyn <serge@hallyn.com>
> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
> Link: https://lore.kernel.org/r/20210222150608.808146-2-mic@digikod.net
> ---
>
> Changes since v2:
> * Revamp without virtual dependencies but a new option to automatically
>   enable all selected LSMs.
>
> Changes since v1:
> * Add CONFIG_SECURITY as a dependency of CONFIG_LSM.  This prevent an
>   error when building without any LSMs.
> ---
>  security/Kconfig    | 19 +++++++++++++++++++
>  security/security.c | 26 +++++++++++++++++++++++++-
>  2 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/security/Kconfig b/security/Kconfig
> index 7561f6f99f1d..fae083e9867d 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -243,6 +243,7 @@ source "security/integrity/Kconfig"
>  
>  choice
>  	prompt "First legacy 'major LSM' to be initialized"
> +	depends on SECURITY
>  	default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX
>  	default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
>  	default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
> @@ -275,8 +276,26 @@ choice
>  
>  endchoice
>  
> +config LSM_AUTO
> +	bool "Automatically enable all selected LSMs at boot"
> +	depends on SECURITY
> +	default y
> +	help
> +	  This automatically configure the build-time selected LSMs to be
> +	  enabled at boot unless the "lsm=" parameter is provided.
> +
> +	  If this option is not selected, it will be required to configure and
> +	  maintained a static list of enabled LSMs that may become inconsistent
> +	  with future user configuration.  Indeed, this list will not be
> +	  automatically upgraded when selecting a new (future) LSM, e.g. with
> +	  make oldconfig.
> +
> +	  If you are unsure how to answer this question, answer Y.
> +
> +# This lists should be synchronized with LSM_ORDER defined in security/security.c .
>  config LSM
>  	string "Ordered list of enabled LSMs"
> +	depends on SECURITY && !LSM_AUTO
>  	default "lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
>  	default "lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
>  	default "lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
> diff --git a/security/security.c b/security/security.c
> index 401663b5b70e..defa1d2c40a3 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -82,7 +82,31 @@ static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
>  static __initdata const char *chosen_lsm_order;
>  static __initdata const char *chosen_major_lsm;
>  
> -static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
> +#ifdef CONFIG_LSM
> +#define LSM_ORDER	CONFIG_LSM
> +#else
> +
> +/*
> + * This lists should be synchronized with the default values of CONFIG_LSM
> + * defined in security/Kconfig .
> + */
> +#define LSM_ORDER_PRE	"lockdown,yama,loadpin,safesetid,integrity,"
> +
> +#if defined(CONFIG_DEFAULT_SECURITY_SMACK)
> +#define LSM_ORDER	LSM_ORDER_PRE "smack,selinux,tomoyo,apparmor,bpf"
> +#elif defined(CONFIG_DEFAULT_SECURITY_APPARMOR)
> +#define LSM_ORDER	LSM_ORDER_PRE "apparmor,selinux,smack,tomoyo,bpf"
> +#elif defined(CONFIG_DEFAULT_SECURITY_TOMOYO)
> +#define LSM_ORDER	LSM_ORDER_PRE "tomoyo,bpf"
> +#elif defined(CONFIG_DEFAULT_SECURITY_DAC)
> +#define LSM_ORDER	LSM_ORDER_PRE "bpf"
> +#else
> +#define LSM_ORDER	LSM_ORDER_PRE "selinux,smack,tomoyo,apparmor,bpf"
> +#endif
> +
> +#endif /* CONFIG_LSM */
> +
> +static __initconst const char * const builtin_lsm_order = LSM_ORDER;
>  
>  /* Ordered list of LSMs to initialize. */
>  static __initdata struct lsm_info **ordered_lsms;
Mickaël Salaün Feb. 22, 2021, 6:31 p.m. UTC | #2
On 22/02/2021 17:51, Casey Schaufler wrote:
> On 2/22/2021 7:06 AM, Mickaël Salaün wrote:
>> From: Mickaël Salaün <mic@linux.microsoft.com>
>>
>> Add a new option CONFIG_LSM_AUTO to enable users to delegate default LSM
>> stacking order to kernel developers.  This enable to keep a consistent
>> order of enabled LSM when changing the LSM selection, especially when a
>> new LSM is added to the kernel.
> 
> TL;DR - NAK
> 
> Do you think that we might have considered this when stacking was
> introduced?

I didn't dig the detailed history of LSM stacking, but you are in Cc
because I know that you know. I may have though that the main goal of
the current LSM stacking implementation was to enable to stack existing
LSMs, which works well with this CONFIG_LSM list, but doesn't work as
well for new LSMs.

> Did you even consider the implications before sending
> the patch?

Yes, and it doesn't change much the current behavior without user
interaction. However, it gives the choice to users to choose how they
want their configuration to evolve.

> This only makes any sense if you want to compile in
> AppArmor and/or Smack but always use SELinux. The existing Kconfig
> model handles that perfectly well.

This patch series doesn't change this behavior if the user doesn't want
it to change.

> Also, this will break when the
> next phase of module stacking comes in, and all of a sudden
> systems will automatically get AppArmor in addition to SELinux
> or Smack.

What is the next phase of module stacking? What would be the consequences?

Systems will only get new LSMs if their configuration said so, either
from Kconfig or from boot arguments. I think we should make easier to
have a working, consistent and secure kernel configuration by default.
If users want to have a non-default configuration, that's fine, fully
supported, and they can do it.

> 
> I know that the CONFIG_LSM/lsm= mechanism is clumsy. But we spent
> about a year discussing, proposing and implementing alternatives,
> and if there's a better mechanism, we couldn't find it. Of course
> we considered "just use the kernel order".

This is indeed the intent of this patch, but this configuration is optional.

> It doesn't work for generic kernels.

Why? Generic kernels can be configured with CONFIG_LSM or with
CONFIG_LSM_AUTO. I agree that generic distros may want to not enable
major LSMs such as SELinux, AppArmor, Smack and Tomoyo by default but
support them in their generic kernel anyway to let users pick and
configure an LSM thanks to the boot arguments, and that's totally fine.
Moreover, distro maintainers will surely browse most of new options to
identify if it is the best choice for their distro. The *default* choice
(for LSMs enabled at boot) is in the hand of users configuring their
kernel, and they are in the best position to choose if they want to
follow new kernel options and their consequences (e.g. distro kernel
maintainers, whose job is to follow kernel development), or to have an
easier way to maintain an up-to-date kernel (e.g. sysadmins or
hobbyists, who may not have so much time dedicated to follow kernel
developments).

> I understand that adding a new LSM that you want
> to be included by default is a tough problem. I also suggest
> that silently adding an LSM to an existing configuration is likely
> to violate the principle of least astonishment.

Nothing is silently added to the user configuration with this patch. It
is an optional (default) configuration, which I think makes more sense
for users not expert in every kernel toggles.

> 
>>
>> CONFIG_LSM depends on !CONFIG_LSM_AUTO, which is backward compatible and
>> gives the opportunity to users to select CONFIG_LSM_AUTO with a make
>> oldconfig.
>>
>> CONFIG_LSM and CONFIG_LSM_AUTO depend on CONFIG_SECURITY, which makes
>> sense because an LSM depends on the security framework.
>>
>> Cc: Casey Schaufler <casey@schaufler-ca.com>
>> Cc: James Morris <jmorris@namei.org>
>> Cc: Kees Cook <keescook@chromium.org>
>> Cc: Serge E. Hallyn <serge@hallyn.com>
>> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
>> Link: https://lore.kernel.org/r/20210222150608.808146-2-mic@digikod.net
>> ---
>>
>> Changes since v2:
>> * Revamp without virtual dependencies but a new option to automatically
>>   enable all selected LSMs.
>>
>> Changes since v1:
>> * Add CONFIG_SECURITY as a dependency of CONFIG_LSM.  This prevent an
>>   error when building without any LSMs.
>> ---
>>  security/Kconfig    | 19 +++++++++++++++++++
>>  security/security.c | 26 +++++++++++++++++++++++++-
>>  2 files changed, 44 insertions(+), 1 deletion(-)
>>
>> diff --git a/security/Kconfig b/security/Kconfig
>> index 7561f6f99f1d..fae083e9867d 100644
>> --- a/security/Kconfig
>> +++ b/security/Kconfig
>> @@ -243,6 +243,7 @@ source "security/integrity/Kconfig"
>>  
>>  choice
>>  	prompt "First legacy 'major LSM' to be initialized"
>> +	depends on SECURITY
>>  	default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX
>>  	default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
>>  	default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
>> @@ -275,8 +276,26 @@ choice
>>  
>>  endchoice
>>  
>> +config LSM_AUTO
>> +	bool "Automatically enable all selected LSMs at boot"
>> +	depends on SECURITY
>> +	default y
>> +	help
>> +	  This automatically configure the build-time selected LSMs to be
>> +	  enabled at boot unless the "lsm=" parameter is provided.
>> +
>> +	  If this option is not selected, it will be required to configure and
>> +	  maintained a static list of enabled LSMs that may become inconsistent
>> +	  with future user configuration.  Indeed, this list will not be
>> +	  automatically upgraded when selecting a new (future) LSM, e.g. with
>> +	  make oldconfig.
>> +
>> +	  If you are unsure how to answer this question, answer Y.
>> +
>> +# This lists should be synchronized with LSM_ORDER defined in security/security.c .
>>  config LSM
>>  	string "Ordered list of enabled LSMs"
>> +	depends on SECURITY && !LSM_AUTO
>>  	default "lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
>>  	default "lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
>>  	default "lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
>> diff --git a/security/security.c b/security/security.c
>> index 401663b5b70e..defa1d2c40a3 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -82,7 +82,31 @@ static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
>>  static __initdata const char *chosen_lsm_order;
>>  static __initdata const char *chosen_major_lsm;
>>  
>> -static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
>> +#ifdef CONFIG_LSM
>> +#define LSM_ORDER	CONFIG_LSM
>> +#else
>> +
>> +/*
>> + * This lists should be synchronized with the default values of CONFIG_LSM
>> + * defined in security/Kconfig .
>> + */
>> +#define LSM_ORDER_PRE	"lockdown,yama,loadpin,safesetid,integrity,"
>> +
>> +#if defined(CONFIG_DEFAULT_SECURITY_SMACK)
>> +#define LSM_ORDER	LSM_ORDER_PRE "smack,selinux,tomoyo,apparmor,bpf"
>> +#elif defined(CONFIG_DEFAULT_SECURITY_APPARMOR)
>> +#define LSM_ORDER	LSM_ORDER_PRE "apparmor,selinux,smack,tomoyo,bpf"
>> +#elif defined(CONFIG_DEFAULT_SECURITY_TOMOYO)
>> +#define LSM_ORDER	LSM_ORDER_PRE "tomoyo,bpf"
>> +#elif defined(CONFIG_DEFAULT_SECURITY_DAC)
>> +#define LSM_ORDER	LSM_ORDER_PRE "bpf"
>> +#else
>> +#define LSM_ORDER	LSM_ORDER_PRE "selinux,smack,tomoyo,apparmor,bpf"
>> +#endif
>> +
>> +#endif /* CONFIG_LSM */
>> +
>> +static __initconst const char * const builtin_lsm_order = LSM_ORDER;
>>  
>>  /* Ordered list of LSMs to initialize. */
>>  static __initdata struct lsm_info **ordered_lsms;
>
Casey Schaufler Feb. 22, 2021, 8:31 p.m. UTC | #3
On 2/22/2021 10:31 AM, Mickaël Salaün wrote:
> On 22/02/2021 17:51, Casey Schaufler wrote:
>> On 2/22/2021 7:06 AM, Mickaël Salaün wrote:
>>> From: Mickaël Salaün <mic@linux.microsoft.com>
>>>
>>> Add a new option CONFIG_LSM_AUTO to enable users to delegate default LSM
>>> stacking order to kernel developers.  This enable to keep a consistent
>>> order of enabled LSM when changing the LSM selection, especially when a
>>> new LSM is added to the kernel.
>> TL;DR - NAK
>>
>> Do you think that we might have considered this when stacking was
>> introduced?
> I didn't dig the detailed history of LSM stacking, but you are in Cc
> because I know that you know. I may have though that the main goal of
> the current LSM stacking implementation was to enable to stack existing
> LSMs, which works well with this CONFIG_LSM list, but doesn't work as
> well for new LSMs.

It works just fine for new LSMs if you treat them as significant
features which may have significant impact on the behavior of the
system.

>> Did you even consider the implications before sending
>> the patch?
> Yes, and it doesn't change much the current behavior without user
> interaction. However, it gives the choice to users to choose how they
> want their configuration to evolve.

Automatic inclusions of new LSMs would be counter to existing practice.
It won't work for "major" LSMs.


>> This only makes any sense if you want to compile in
>> AppArmor and/or Smack but always use SELinux. The existing Kconfig
>> model handles that perfectly well.
> This patch series doesn't change this behavior if the user doesn't want
> it to change.

Well, there's the question. If a distribution/system uses the new scheme
"users" are going to get new LSMs spontaniously. If they don't it's up to
the "user". Unsophisticated users won't want this, and the others don't
need it.

>> Also, this will break when the
>> next phase of module stacking comes in, and all of a sudden
>> systems will automatically get AppArmor in addition to SELinux
>> or Smack.
> What is the next phase of module stacking? What would be the consequences?

The next phase ( coming real soon now :) ) allows AppArmor and SELinux/Smack
at the same time. More generically, the number of interfaces that can't be
used by multiple LSMs are reduced.

> Systems will only get new LSMs if their configuration said so, either
> from Kconfig or from boot arguments. I think we should make easier to
> have a working, consistent and secure kernel configuration by default.
> If users want to have a non-default configuration, that's fine, fully
> supported, and they can do it.

That really only matters for distribution or product kernels.
Neither of those should use CONFIG_LSM_AUTO.


>> I know that the CONFIG_LSM/lsm= mechanism is clumsy. But we spent
>> about a year discussing, proposing and implementing alternatives,
>> and if there's a better mechanism, we couldn't find it. Of course
>> we considered "just use the kernel order".
> This is indeed the intent of this patch, but this configuration is optional.

Anyone who should rationally chose this option doesn't need it.


>> It doesn't work for generic kernels.
> Why? 

Because you only ever get the first compiled in Major LSM, which
will always be SELinux. If you only ever get a particular LSM,
why compile in the others? OK, I'm ignoring boot options.

> Generic kernels can be configured with CONFIG_LSM or with
> CONFIG_LSM_AUTO. 

Sure, if they use SELinux. But why would they use CONFIG_LSM_AUTO?
They loss of control over system behavior is unreasonable for a
distribution or a product kernel.

> I agree that generic distros may want to not enable
> major LSMs such as SELinux, AppArmor, Smack and Tomoyo by default but
> support them in their generic kernel anyway to let users pick and
> configure an LSM thanks to the boot arguments, and that's totally fine.
> Moreover, distro maintainers will surely browse most of new options to
> identify if it is the best choice for their distro. The *default* choice
> (for LSMs enabled at boot) is in the hand of users configuring their
> kernel, and they are in the best position to choose if they want to
> follow new kernel options and their consequences (e.g. distro kernel
> maintainers, whose job is to follow kernel development), or to have an
> easier way to maintain an up-to-date kernel (e.g. sysadmins or
> hobbyists, who may not have so much time dedicated to follow kernel
> developments).

Users who configure their kernels don't need CONFIG_LSM_AUTO.
Users who don't configure their kernels shouldn't have it.


>> I understand that adding a new LSM that you want
>> to be included by default is a tough problem. I also suggest
>> that silently adding an LSM to an existing configuration is likely
>> to violate the principle of least astonishment.
> Nothing is silently added to the user configuration with this patch. It
> is an optional (default) configuration, which I think makes more sense
> for users not expert in every kernel toggles.

That is exactly wrong. Users who are not expert on kernel configuration
should not get LSMs added to their configuration without their knowledge.

>>> CONFIG_LSM depends on !CONFIG_LSM_AUTO, which is backward compatible and
>>> gives the opportunity to users to select CONFIG_LSM_AUTO with a make
>>> oldconfig.
>>>
>>> CONFIG_LSM and CONFIG_LSM_AUTO depend on CONFIG_SECURITY, which makes
>>> sense because an LSM depends on the security framework.
>>>
>>> Cc: Casey Schaufler <casey@schaufler-ca.com>
>>> Cc: James Morris <jmorris@namei.org>
>>> Cc: Kees Cook <keescook@chromium.org>
>>> Cc: Serge E. Hallyn <serge@hallyn.com>
>>> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
>>> Link: https://lore.kernel.org/r/20210222150608.808146-2-mic@digikod.net
>>> ---
>>>
>>> Changes since v2:
>>> * Revamp without virtual dependencies but a new option to automatically
>>>   enable all selected LSMs.
>>>
>>> Changes since v1:
>>> * Add CONFIG_SECURITY as a dependency of CONFIG_LSM.  This prevent an
>>>   error when building without any LSMs.
>>> ---
>>>  security/Kconfig    | 19 +++++++++++++++++++
>>>  security/security.c | 26 +++++++++++++++++++++++++-
>>>  2 files changed, 44 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/security/Kconfig b/security/Kconfig
>>> index 7561f6f99f1d..fae083e9867d 100644
>>> --- a/security/Kconfig
>>> +++ b/security/Kconfig
>>> @@ -243,6 +243,7 @@ source "security/integrity/Kconfig"
>>>  
>>>  choice
>>>  	prompt "First legacy 'major LSM' to be initialized"
>>> +	depends on SECURITY
>>>  	default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX
>>>  	default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
>>>  	default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
>>> @@ -275,8 +276,26 @@ choice
>>>  
>>>  endchoice
>>>  
>>> +config LSM_AUTO
>>> +	bool "Automatically enable all selected LSMs at boot"
>>> +	depends on SECURITY
>>> +	default y
>>> +	help
>>> +	  This automatically configure the build-time selected LSMs to be
>>> +	  enabled at boot unless the "lsm=" parameter is provided.
>>> +
>>> +	  If this option is not selected, it will be required to configure and
>>> +	  maintained a static list of enabled LSMs that may become inconsistent
>>> +	  with future user configuration.  Indeed, this list will not be
>>> +	  automatically upgraded when selecting a new (future) LSM, e.g. with
>>> +	  make oldconfig.
>>> +
>>> +	  If you are unsure how to answer this question, answer Y.
>>> +
>>> +# This lists should be synchronized with LSM_ORDER defined in security/security.c .
>>>  config LSM
>>>  	string "Ordered list of enabled LSMs"
>>> +	depends on SECURITY && !LSM_AUTO
>>>  	default "lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
>>>  	default "lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
>>>  	default "lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
>>> diff --git a/security/security.c b/security/security.c
>>> index 401663b5b70e..defa1d2c40a3 100644
>>> --- a/security/security.c
>>> +++ b/security/security.c
>>> @@ -82,7 +82,31 @@ static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
>>>  static __initdata const char *chosen_lsm_order;
>>>  static __initdata const char *chosen_major_lsm;
>>>  
>>> -static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
>>> +#ifdef CONFIG_LSM
>>> +#define LSM_ORDER	CONFIG_LSM
>>> +#else
>>> +
>>> +/*
>>> + * This lists should be synchronized with the default values of CONFIG_LSM
>>> + * defined in security/Kconfig .
>>> + */
>>> +#define LSM_ORDER_PRE	"lockdown,yama,loadpin,safesetid,integrity,"
>>> +
>>> +#if defined(CONFIG_DEFAULT_SECURITY_SMACK)
>>> +#define LSM_ORDER	LSM_ORDER_PRE "smack,selinux,tomoyo,apparmor,bpf"
>>> +#elif defined(CONFIG_DEFAULT_SECURITY_APPARMOR)
>>> +#define LSM_ORDER	LSM_ORDER_PRE "apparmor,selinux,smack,tomoyo,bpf"
>>> +#elif defined(CONFIG_DEFAULT_SECURITY_TOMOYO)
>>> +#define LSM_ORDER	LSM_ORDER_PRE "tomoyo,bpf"
>>> +#elif defined(CONFIG_DEFAULT_SECURITY_DAC)
>>> +#define LSM_ORDER	LSM_ORDER_PRE "bpf"
>>> +#else
>>> +#define LSM_ORDER	LSM_ORDER_PRE "selinux,smack,tomoyo,apparmor,bpf"
>>> +#endif
>>> +
>>> +#endif /* CONFIG_LSM */
>>> +
>>> +static __initconst const char * const builtin_lsm_order = LSM_ORDER;
>>>  
>>>  /* Ordered list of LSMs to initialize. */
>>>  static __initdata struct lsm_info **ordered_lsms;
Nicolas Iooss Feb. 22, 2021, 9:12 p.m. UTC | #4
On Mon, Feb 22, 2021 at 9:32 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 2/22/2021 10:31 AM, Mickaël Salaün wrote:
> > On 22/02/2021 17:51, Casey Schaufler wrote:
> >> On 2/22/2021 7:06 AM, Mickaël Salaün wrote:
> >>> From: Mickaël Salaün <mic@linux.microsoft.com>
> >>>
> >>> Add a new option CONFIG_LSM_AUTO to enable users to delegate default LSM
> >>> stacking order to kernel developers.  This enable to keep a consistent
> >>> order of enabled LSM when changing the LSM selection, especially when a
> >>> new LSM is added to the kernel.
> >> TL;DR - NAK
> >>
> >> Do you think that we might have considered this when stacking was
> >> introduced?
> > I didn't dig the detailed history of LSM stacking, but you are in Cc
> > because I know that you know. I may have though that the main goal of
> > the current LSM stacking implementation was to enable to stack existing
> > LSMs, which works well with this CONFIG_LSM list, but doesn't work as
> > well for new LSMs.
>
> It works just fine for new LSMs if you treat them as significant
> features which may have significant impact on the behavior of the
> system.
>
> >> Did you even consider the implications before sending
> >> the patch?
> > Yes, and it doesn't change much the current behavior without user
> > interaction. However, it gives the choice to users to choose how they
> > want their configuration to evolve.
>
> Automatic inclusions of new LSMs would be counter to existing practice.
> It won't work for "major" LSMs.
>
>
> >> This only makes any sense if you want to compile in
> >> AppArmor and/or Smack but always use SELinux. The existing Kconfig
> >> model handles that perfectly well.
> > This patch series doesn't change this behavior if the user doesn't want
> > it to change.
>
> Well, there's the question. If a distribution/system uses the new scheme
> "users" are going to get new LSMs spontaniously. If they don't it's up to
> the "user". Unsophisticated users won't want this, and the others don't
> need it.

Hello, sorry if I missed something simple but I did not understand
what "Automatic inclusions of new LSMs " and "get new LSMs
spontaniously" is about. If I understood the kernel practice
development correctly, when a new LSM will be included, it will have a
dedicated "config SECURITY_MYNEWLSM" which will be default to "n" in
order to respect the "principle of least astonishment". How could such
a new LSM be automatically/spontaneously added to the LSM list?

I understand that this is a tough issue and that the subject might
have been discussed a few years ago, and if that's the case, it would
be nice to have pointers to some clear documentation or past emails
(and it would be very very nice if the kernel documentation was
updated to document the current state of LSM stacking: for example
https://www.kernel.org/doc/html/v5.11/admin-guide/LSM/index.html still
documents the "security=" kernel parameter even though it conflicts
with CONFIG_LSM and can be ignored by the kernel in practise).

Thanks,
Nicolas
Casey Schaufler Feb. 22, 2021, 10:46 p.m. UTC | #5
On 2/22/2021 1:12 PM, Nicolas Iooss wrote:
> On Mon, Feb 22, 2021 at 9:32 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 2/22/2021 10:31 AM, Mickaël Salaün wrote:
>>> On 22/02/2021 17:51, Casey Schaufler wrote:
>>>> On 2/22/2021 7:06 AM, Mickaël Salaün wrote:
>>>>> From: Mickaël Salaün <mic@linux.microsoft.com>
>>>>>
>>>>> Add a new option CONFIG_LSM_AUTO to enable users to delegate default LSM
>>>>> stacking order to kernel developers.  This enable to keep a consistent
>>>>> order of enabled LSM when changing the LSM selection, especially when a
>>>>> new LSM is added to the kernel.
>>>> TL;DR - NAK
>>>>
>>>> Do you think that we might have considered this when stacking was
>>>> introduced?
>>> I didn't dig the detailed history of LSM stacking, but you are in Cc
>>> because I know that you know. I may have though that the main goal of
>>> the current LSM stacking implementation was to enable to stack existing
>>> LSMs, which works well with this CONFIG_LSM list, but doesn't work as
>>> well for new LSMs.
>> It works just fine for new LSMs if you treat them as significant
>> features which may have significant impact on the behavior of the
>> system.
>>
>>>> Did you even consider the implications before sending
>>>> the patch?
>>> Yes, and it doesn't change much the current behavior without user
>>> interaction. However, it gives the choice to users to choose how they
>>> want their configuration to evolve.
>> Automatic inclusions of new LSMs would be counter to existing practice.
>> It won't work for "major" LSMs.
>>
>>
>>>> This only makes any sense if you want to compile in
>>>> AppArmor and/or Smack but always use SELinux. The existing Kconfig
>>>> model handles that perfectly well.
>>> This patch series doesn't change this behavior if the user doesn't want
>>> it to change.
>> Well, there's the question. If a distribution/system uses the new scheme
>> "users" are going to get new LSMs spontaniously. If they don't it's up to
>> the "user". Unsophisticated users won't want this, and the others don't
>> need it.
> Hello, sorry if I missed something simple but I did not understand
> what "Automatic inclusions of new LSMs " and "get new LSMs
> spontaniously" is about. If I understood the kernel practice
> development correctly, when a new LSM will be included, it will have a
> dedicated "config SECURITY_MYNEWLSM" which will be default to "n" in
> order to respect the "principle of least astonishment". How could such
> a new LSM be automatically/spontaneously added to the LSM list?

It wouldn't. But compiling the new LSM mynewlsm doesn't add it to
the list, either. Today no one should expect a LSM to be active if
it hasn't been added to the CONFIG_LSM list. The proposed addition
of CONFIG_LSM_AUTO would change that. "make oldconfig" would add
security modules that are built to the list. This is unnecessary
since whoever changed CONFIG_SECURITY_MYNEWLSM to "y" could easily
have added it to CONFIG_LSM. In the right place.

> I understand that this is a tough issue and that the subject might
> have been discussed a few years ago, and if that's the case, it would
> be nice to have pointers to some clear documentation or past emails
> (and it would be very very nice if the kernel documentation was
> updated to document the current state of LSM stacking:

I'm not going to argue against that.

>  for example
> https://www.kernel.org/doc/html/v5.11/admin-guide/LSM/index.html still
> documents the "security=" kernel parameter even though it conflicts
> with CONFIG_LSM and can be ignored by the kernel in practise).

You can still select one "major" module using security= if you
don't use lsm= to specify a full list. We put real effort into
being backward compatible. 

>
> Thanks,
> Nicolas
>
Nicolas Iooss Feb. 23, 2021, 6:21 a.m. UTC | #6
On Mon, Feb 22, 2021 at 11:46 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
>
> On 2/22/2021 1:12 PM, Nicolas Iooss wrote:
> > On Mon, Feb 22, 2021 at 9:32 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> On 2/22/2021 10:31 AM, Mickaël Salaün wrote:
> >>> On 22/02/2021 17:51, Casey Schaufler wrote:
> >>>> On 2/22/2021 7:06 AM, Mickaël Salaün wrote:
> >>>>> From: Mickaël Salaün <mic@linux.microsoft.com>
> >>>>>
> >>>>> Add a new option CONFIG_LSM_AUTO to enable users to delegate default LSM
> >>>>> stacking order to kernel developers.  This enable to keep a consistent
> >>>>> order of enabled LSM when changing the LSM selection, especially when a
> >>>>> new LSM is added to the kernel.
> >>>> TL;DR - NAK
> >>>>
> >>>> Do you think that we might have considered this when stacking was
> >>>> introduced?
> >>> I didn't dig the detailed history of LSM stacking, but you are in Cc
> >>> because I know that you know. I may have though that the main goal of
> >>> the current LSM stacking implementation was to enable to stack existing
> >>> LSMs, which works well with this CONFIG_LSM list, but doesn't work as
> >>> well for new LSMs.
> >> It works just fine for new LSMs if you treat them as significant
> >> features which may have significant impact on the behavior of the
> >> system.
> >>
> >>>> Did you even consider the implications before sending
> >>>> the patch?
> >>> Yes, and it doesn't change much the current behavior without user
> >>> interaction. However, it gives the choice to users to choose how they
> >>> want their configuration to evolve.
> >> Automatic inclusions of new LSMs would be counter to existing practice.
> >> It won't work for "major" LSMs.
> >>
> >>
> >>>> This only makes any sense if you want to compile in
> >>>> AppArmor and/or Smack but always use SELinux. The existing Kconfig
> >>>> model handles that perfectly well.
> >>> This patch series doesn't change this behavior if the user doesn't want
> >>> it to change.
> >> Well, there's the question. If a distribution/system uses the new scheme
> >> "users" are going to get new LSMs spontaniously. If they don't it's up to
> >> the "user". Unsophisticated users won't want this, and the others don't
> >> need it.
> > Hello, sorry if I missed something simple but I did not understand
> > what "Automatic inclusions of new LSMs " and "get new LSMs
> > spontaniously" is about. If I understood the kernel practice
> > development correctly, when a new LSM will be included, it will have a
> > dedicated "config SECURITY_MYNEWLSM" which will be default to "n" in
> > order to respect the "principle of least astonishment". How could such
> > a new LSM be automatically/spontaneously added to the LSM list?
>
> It wouldn't. But compiling the new LSM mynewlsm doesn't add it to
> the list, either. Today no one should expect a LSM to be active if
> it hasn't been added to the CONFIG_LSM list. The proposed addition
> of CONFIG_LSM_AUTO would change that. "make oldconfig" would add
> security modules that are built to the list. This is unnecessary
> since whoever changed CONFIG_SECURITY_MYNEWLSM to "y" could easily
> have added it to CONFIG_LSM. In the right place.
>
> > I understand that this is a tough issue and that the subject might
> > have been discussed a few years ago, and if that's the case, it would
> > be nice to have pointers to some clear documentation or past emails
> > (and it would be very very nice if the kernel documentation was
> > updated to document the current state of LSM stacking:
>
> I'm not going to argue against that.
>
> >  for example
> > https://www.kernel.org/doc/html/v5.11/admin-guide/LSM/index.html still
> > documents the "security=" kernel parameter even though it conflicts
> > with CONFIG_LSM and can be ignored by the kernel in practise).
>
> You can still select one "major" module using security= if you
> don't use lsm= to specify a full list. We put real effort into
> being backward compatible.

No, this is not true. If CONFIG_LSM is defined to "lockdown,yama,bpf"
and if the kernel command line contains "security=selinux" without any
"lsm" parameter, then SELinux is not enabled properly.

This broke the configuration of several Arch Linux users (cf.
https://bbs.archlinux.org/viewtopic.php?id=263360 and
https://github.com/archlinuxhardened/selinux/issues/81) and I reported
this on some kernel mailing lists a few days ago
(https://lore.kernel.org/linux-security-module/CAJfZ7=nWJisw2RRW2AvFgpYKQK_PghudeBqiTQXNfedS2idP-Q@mail.gmail.com/).
Your answer to this issue was very clear (and thank you for explaining
this):

« You can't (currently) use SELinux and BPF at the same time. This is
because the infrastructure does not support multiple secid<->secctx
translation hooks. You get the first one in the list. BPF provides all
hooks, so the SELinux hooks aren't reached and the secid to secctx
translation fails in the "bpf,selinux" case. »

Anyway, this means that using "security=..." does not work if
CONFIG_LSM contains the BPF LSM module, so no: you *cannot* select one
major module using security=, when the kernel is compiled with
CONFIG_LSM="lockdown,yama,bpf".

Backward compatibility was broken and Arch Linux users were required
to switch to lsm= in order to use AppArmor, SELinux, etc. (and the
documentation of this distribution got updated:
https://wiki.archlinux.org/index.php/AppArmor,
https://wiki.archlinux.org/index.php/SELinux, etc.).

Nicolas
Kees Cook Oct. 17, 2022, 7:25 p.m. UTC | #7
[*double thread necromancy*]

On Mon, Feb 22, 2021 at 02:46:24PM -0800, Casey Schaufler wrote:
> It wouldn't. But compiling the new LSM mynewlsm doesn't add it to
> the list, either. Today no one should expect a LSM to be active if
> it hasn't been added to the CONFIG_LSM list. The proposed addition
> of CONFIG_LSM_AUTO would change that. "make oldconfig" would add
> security modules that are built to the list. This is unnecessary
> since whoever changed CONFIG_SECURITY_MYNEWLSM to "y" could easily
> have added it to CONFIG_LSM. In the right place.

Having CONFIG_LSM/lsm= is to support the migration away from having a
"default major LSM", but still provide a way to separate "built" vs
"enabled". As such, it needs to provide ordering. (So we have three
concepts here: "built" at all, "enabled" by default, and in a specific
"order".) And not being listed in CONFIG_LSM/lsm= means an LSM is
disabled.

I don't disagree about "anyone who enables a new LSM config can add it to
CONFIG_LSM", but really I think the question is why require an _ordering_
choice. Most distros and builders don't care beyond having the current
"default major LSM" come first, which leaves only the "enabled by
default" choice.

To review, security= currently only enables/disables apparmor, selinux,
smack, and tomoyo. It will go away once the full implementation of
stacking is finished.

I personally think it's reasonable that a "built" LSM be "enabled" by
default, however, this is not universally held to be true. :) The need
remains that enablement be configurable. The current solution here is
to add/remove it from CONFIG_LSM/lsm=. What remains problematic, though,
is a mismatch between lack of ordering causing disabling, but enabling
doesn't specify ordering. Ordering only matters for the legacy major
LSMs, which is controlled by CONFIG_DEFAULT_SECURITY_*.

Here is a reasonable overview of the main "lsm=" thread...
https://lore.kernel.org/all/CAGXu5jKqXNbEvPr1axQtGCCnWsGhDgjynW5u326mcx4vZ1oH8g@mail.gmail.com/
https://lore.kernel.org/all/abe03d09-4dcb-2b02-4102-5e108d617a42@canonical.com/
https://lore.kernel.org/all/CAGXu5jJtC1gkJ0ZKDFroL8UzvjiPfmC+6EsrzyB0j0oETdSQQg@mail.gmail.com/
https://lore.kernel.org/all/7741e4c1-cc54-4d04-a064-cb5388817058@canonical.com/
https://lore.kernel.org/all/CAGXu5jLKgrdhah-5TtAXDL-odbLGeyAUH2=PkAU769AkEnZFfQ@mail.gmail.com/
https://lore.kernel.org/all/5955f5ce-b803-4f58-8b07-54c291e33da5@canonical.com/
https://lore.kernel.org/all/CAGXu5jLBHN=YSs3Uh49bBJ1SRA1Km2UUD4j37GJJXiKhQq+KPA@mail.gmail.com/
https://lore.kernel.org/all/CAGXu5jJJit8bDNvgXaFkuvFPy7NWtJW2oRWFbG-6iWk0+A1qng@mail.gmail.com/
https://lore.kernel.org/all/88b0cc69-cd42-1798-6ce4-d3cfbbc79d83@canonical.com/
https://lore.kernel.org/all/alpine.LRH.2.21.1810051449110.2590@namei.org/

I *still* think there should be a way to leave ordering alone and have
separate enable/disable control. And I think the growth of additional
LSMs that need explicit ordering supports this proposal.

What has become clear is that allowing _ordering_ to be generically
mutable is a mistake (and we had hints of this due to the standing
exceptions for "capability"). How about making these changes:

1) make ordering be source/"built"-controlled (i.e. similar to what
   CONFIG_LSM_AUTO proposes)
2) have CONFIG_LSM/lsm= control only enable/disable and NOT ordering except
   for the "major" LSMs.
3) introduce "lsm=+foo,-bar" that will enable/disable the given LSMs without
   changing relative order.

I think of it like this. LSMs declare their ordering position (btw,
capability remains an exception to the existing logic, and this change
would begin to regularize it, IMO):

first:   capability (cannot be disabled)
early:   landlock,lockdown,yama,loadpin,safesetid,integrity
mutable: selinux,apparmor,smack,tomoyo
late:    bpf
last:    ...empty...

And "lsm=" can only change the order of the "mutable" ordering LSMs.

As an example:

Assuming The "built" order for all LSMs was defined as:
  capability,landlock,lockdown,yama,loadpin,safesetid,integrity,selinux,apparmor,smack,tomoyo,bfp

If CONFIG_LSM was:      yama,integrity,apparmor,selinux,bpf,lockdown

a) without boot param
   result would be: lsm=capability,yama,integrity,lockdown,apparmor,selinux,bpf

b) with boot param: lsm=selinux,lockdown,yama
   result would be: lsm=capability,yama,lockdown,selinux

c) with boot param: lsm=-lockdown
   result would be: lsm=capability,yama,lockdown,integrity,apparmor,selinux,bpf

d) with boot param: lsm=+setsetid
   result would be: lsm=capability,yama,safesetid,integrity,lockdown,apparmor,selinux,bpf

Thoughts?
Paul Moore Oct. 18, 2022, 1:45 a.m. UTC | #8
On Mon, Oct 17, 2022 at 3:29 PM Kees Cook <keescook@chromium.org> wrote:
>
> [*double thread necromancy*]

Yikes!  This is some pretty serious mail thread crime ...

> On Mon, Feb 22, 2021 at 02:46:24PM -0800, Casey Schaufler wrote:
> > It wouldn't. But compiling the new LSM mynewlsm doesn't add it to
> > the list, either. Today no one should expect a LSM to be active if
> > it hasn't been added to the CONFIG_LSM list. The proposed addition
> > of CONFIG_LSM_AUTO would change that. "make oldconfig" would add
> > security modules that are built to the list. This is unnecessary
> > since whoever changed CONFIG_SECURITY_MYNEWLSM to "y" could easily
> > have added it to CONFIG_LSM. In the right place.
>
> Having CONFIG_LSM/lsm= is to support the migration away from having a
> "default major LSM", but still provide a way to separate "built" vs
> "enabled". As such, it needs to provide ordering. (So we have three
> concepts here: "built" at all, "enabled" by default, and in a specific
> "order".) And not being listed in CONFIG_LSM/lsm= means an LSM is
> disabled.
>
> I don't disagree about "anyone who enables a new LSM config can add it to
> CONFIG_LSM", but really I think the question is why require an _ordering_
> choice. Most distros and builders don't care beyond having the current
> "default major LSM" come first, which leaves only the "enabled by
> default" choice.

The code sorta cares about ordering, at least to the extent that the
LSMs will behave differently depending on the ordering, e.g. a LSM
lower in the priority order might never "see" an operation if a higher
priority LSM rejects the operation.  Yes, it's an implementation
quirk, but I'm not sure I want to start messing with the
fail-on-first-error logic right now.  Maybe once we've got the LSM
layer fully stackable and we've gotten past the initial support
nightmare of that we can revisit this idea.

> I personally think it's reasonable that a "built" LSM be "enabled" by
> default, however, this is not universally held to be true. :)

I personally would like to preserve the existing concept where "built"
does *not* equate to "enabled" by default.

> I *still* think there should be a way to leave ordering alone and have
> separate enable/disable control.

My current opinion is that enabling a LSM and specifying its place in
an ordered list are one in the same.  The way LSM stacking is
currently done almost requires the ability to specify an order if an
admin is trying to meet an security relevant operation visibility
goal.

We can have defaults, like we do know, but I'm in no hurry to remove
the ability to allow admins to change the ordering at boot time.
Kees Cook Oct. 18, 2022, 5:55 a.m. UTC | #9
On Mon, Oct 17, 2022 at 09:45:21PM -0400, Paul Moore wrote:
> The code sorta cares about ordering, at least to the extent that the
> LSMs will behave differently depending on the ordering, e.g. a LSM

Right -- this is why I've been so uncomfortable with allowing
arbitrarily reordering of the LSM list from lsm=. There are orderings we
know work, and others may have undesirable side-effects. I'd much rather
the kernel be specific about the order.

> I personally would like to preserve the existing concept where "built"
> does *not* equate to "enabled" by default.

Yup, understood. I didn't think I was going to win over anyone on that
one, but figured I'd just point it out again. ;)

> > I *still* think there should be a way to leave ordering alone and have
> > separate enable/disable control.
> 
> My current opinion is that enabling a LSM and specifying its place in
> an ordered list are one in the same.  The way LSM stacking as
> currently done almost requires the ability to specify an order if an
> admin is trying to meet an security relevant operation visibility
> goal.

As in an admin wants to see selinux rejections instead of loadpin
rejections for a blocked module loading?

Hmmm. Is this a realistic need?

> We can have defaults, like we do know, but I'm in no hurry to remove
> the ability to allow admins to change the ordering at boot time.

My concern is with new LSMs vs the build system. A system builder will
be prompted for a new CONFIG_SECURITY_SHINY, but won't be prompted
about making changes to CONFIG_LSM to include it.

Even booting with "lsm.debug" isn't entirely helpful to helping someone
construct the "lsm=" option they actually want... I guess I can fix that
part, at least. :)
Paul Moore Oct. 18, 2022, 7:31 p.m. UTC | #10
On Tue, Oct 18, 2022 at 1:55 AM Kees Cook <keescook@chromium.org> wrote:
> On Mon, Oct 17, 2022 at 09:45:21PM -0400, Paul Moore wrote:
> > The code sorta cares about ordering, at least to the extent that the
> > LSMs will behave differently depending on the ordering, e.g. a LSM
>
> Right -- this is why I've been so uncomfortable with allowing
> arbitrarily reordering of the LSM list from lsm=. There are orderings we
> know work, and others may have undesirable side-effects. I'd much rather
> the kernel be specific about the order.

At this point in the ongoing process of LSM stacking I'd much rather
we focus on documenting what we know to work and what is known to be
problematic without placing any restrictions on the ordering.  I
believe there are going to be too many "good" combinations to settle
on one single supported ordering; perhaps at some point in the future
we can agree on what that ordering should be, but I think there are
still too many changes in the foreseeable future to settle on an
ordering now.

> > I personally would like to preserve the existing concept where "built"
> > does *not* equate to "enabled" by default.
>
> Yup, understood. I didn't think I was going to win over anyone on that
> one, but figured I'd just point it out again. ;)

Fair enough.

> > > I *still* think there should be a way to leave ordering alone and have
> > > separate enable/disable control.
> >
> > My current opinion is that enabling a LSM and specifying its place in
> > an ordered list are one in the same.  The way LSM stacking as
> > currently done almost requires the ability to specify an order if an
> > admin is trying to meet an security relevant operation visibility
> > goal.
>
> As in an admin wants to see selinux rejections instead of loadpin
> rejections for a blocked module loading?

When I wrote my response I was thinking more of the BPF LSM, any
future LSMs that might be merged upstream, and the myriad of security
requirements admins must meet both now and in the future.

> > We can have defaults, like we do know, but I'm in no hurry to remove
> > the ability to allow admins to change the ordering at boot time.
>
> My concern is with new LSMs vs the build system. A system builder will
> be prompted for a new CONFIG_SECURITY_SHINY, but won't be prompted
> about making changes to CONFIG_LSM to include it.

I would argue that if an admin/builder doesn't understand what a shiny
new LSM does, they shouldn't be enabling that shiny new LSM.  Adding
new, potentially restrictive, controls to your kernel build without a
basic understanding of those controls is a recipe for disaster and I
try to avoid recommending disaster as a planned course of action :)
Casey Schaufler Oct. 20, 2022, 4 p.m. UTC | #11
On 10/17/2022 10:55 PM, Kees Cook wrote:
> On Mon, Oct 17, 2022 at 09:45:21PM -0400, Paul Moore wrote:
>> The code sorta cares about ordering, at least to the extent that the
>> LSMs will behave differently depending on the ordering, e.g. a LSM
> Right -- this is why I've been so uncomfortable with allowing
> arbitrarily reordering of the LSM list from lsm=. There are orderings we
> know work, and others may have undesirable side-effects. I'd much rather
> the kernel be specific about the order.
>
>> I personally would like to preserve the existing concept where "built"
>> does *not* equate to "enabled" by default.
> Yup, understood. I didn't think I was going to win over anyone on that
> one, but figured I'd just point it out again. ;)
>
>>> I *still* think there should be a way to leave ordering alone and have
>>> separate enable/disable control.
>> My current opinion is that enabling a LSM and specifying its place in
>> an ordered list are one in the same.  The way LSM stacking as
>> currently done almost requires the ability to specify an order if an
>> admin is trying to meet an security relevant operation visibility
>> goal.
> As in an admin wants to see selinux rejections instead of loadpin
> rejections for a blocked module loading?
>
> Hmmm. Is this a realistic need?

One of the security modules I hope to write someday will provide controls
based on multiple successful accesses to particular resources. The old
school example is a program that scans /tmp constantly. No one access will
be denied, but the fact that it does stat() on /tmp/foo a thousand times
a second is suspicious. If my system is running any of SELinux, Smack or
AppArmor I may get different results depending on whether it is loaded
before or after the "primary" module. The user may care which result is
obtained. In truth, the user may use the module both ways as a mechanism
to measure the effectiveness of the "primary" module.

The current set of new security modules is diverging from the MAC model
that LSM was invented to support. I don't see that restricting its use
to make the infrastructure easier to deal with (as much as I'd like to
do that from time to time) would be the Right Thing.

>
>> We can have defaults, like we do know, but I'm in no hurry to remove
>> the ability to allow admins to change the ordering at boot time.
> My concern is with new LSMs vs the build system. A system builder will
> be prompted for a new CONFIG_SECURITY_SHINY, but won't be prompted
> about making changes to CONFIG_LSM to include it.
>
> Even booting with "lsm.debug" isn't entirely helpful to helping someone
> construct the "lsm=" option they actually want... I guess I can fix that
> part, at least. :)
>
Mickaël Salaün Nov. 4, 2022, 4:29 p.m. UTC | #12
On 18/10/2022 21:31, Paul Moore wrote:
> On Tue, Oct 18, 2022 at 1:55 AM Kees Cook <keescook@chromium.org> wrote:
>> On Mon, Oct 17, 2022 at 09:45:21PM -0400, Paul Moore wrote:

[...]

>>> We can have defaults, like we do know, but I'm in no hurry to remove
>>> the ability to allow admins to change the ordering at boot time.
>>
>> My concern is with new LSMs vs the build system. A system builder will
>> be prompted for a new CONFIG_SECURITY_SHINY, but won't be prompted
>> about making changes to CONFIG_LSM to include it.
> 
> I would argue that if an admin/builder doesn't understand what a shiny
> new LSM does, they shouldn't be enabling that shiny new LSM.  Adding
> new, potentially restrictive, controls to your kernel build without a
> basic understanding of those controls is a recipe for disaster and I
> try to avoid recommending disaster as a planned course of action :)

It depends on what this shiny new LSMs do *by default*. In the case of 
Landlock, it do nothing unless a process does specific system calls 
(same as for most new kernel features: sysfs entries, syscall flags…). I 
guess this is the same for most LSMs.
Casey Schaufler Nov. 4, 2022, 5:20 p.m. UTC | #13
On 11/4/2022 9:29 AM, Mickaël Salaün wrote:
>
> On 18/10/2022 21:31, Paul Moore wrote:
>> On Tue, Oct 18, 2022 at 1:55 AM Kees Cook <keescook@chromium.org> wrote:
>>> On Mon, Oct 17, 2022 at 09:45:21PM -0400, Paul Moore wrote:
>
> [...]
>
>>>> We can have defaults, like we do know, but I'm in no hurry to remove
>>>> the ability to allow admins to change the ordering at boot time.
>>>
>>> My concern is with new LSMs vs the build system. A system builder will
>>> be prompted for a new CONFIG_SECURITY_SHINY, but won't be prompted
>>> about making changes to CONFIG_LSM to include it.
>>
>> I would argue that if an admin/builder doesn't understand what a shiny
>> new LSM does, they shouldn't be enabling that shiny new LSM.  Adding
>> new, potentially restrictive, controls to your kernel build without a
>> basic understanding of those controls is a recipe for disaster and I
>> try to avoid recommending disaster as a planned course of action :)
>
> It depends on what this shiny new LSMs do *by default*. In the case of
> Landlock, it do nothing unless a process does specific system calls
> (same as for most new kernel features: sysfs entries, syscall flags…).
> I guess this is the same for most LSMs.

"By default" is somewhat ambiguous. Smack will always enforce its
basic policy. If files aren't labeled and the Smack process label
isn't explicitly set there won't be any problems. However, if files
have somehow gotten labels assigned and there are no rules defined
things can go sideways.
Mickaël Salaün Nov. 7, 2022, 12:35 p.m. UTC | #14
On 04/11/2022 18:20, Casey Schaufler wrote:
> On 11/4/2022 9:29 AM, Mickaël Salaün wrote:
>>
>> On 18/10/2022 21:31, Paul Moore wrote:
>>> On Tue, Oct 18, 2022 at 1:55 AM Kees Cook <keescook@chromium.org> wrote:
>>>> On Mon, Oct 17, 2022 at 09:45:21PM -0400, Paul Moore wrote:
>>
>> [...]
>>
>>>>> We can have defaults, like we do know, but I'm in no hurry to remove
>>>>> the ability to allow admins to change the ordering at boot time.
>>>>
>>>> My concern is with new LSMs vs the build system. A system builder will
>>>> be prompted for a new CONFIG_SECURITY_SHINY, but won't be prompted
>>>> about making changes to CONFIG_LSM to include it.
>>>
>>> I would argue that if an admin/builder doesn't understand what a shiny
>>> new LSM does, they shouldn't be enabling that shiny new LSM.  Adding
>>> new, potentially restrictive, controls to your kernel build without a
>>> basic understanding of those controls is a recipe for disaster and I
>>> try to avoid recommending disaster as a planned course of action :)
>>
>> It depends on what this shiny new LSMs do *by default*. In the case of
>> Landlock, it do nothing unless a process does specific system calls
>> (same as for most new kernel features: sysfs entries, syscall flags…).
>> I guess this is the same for most LSMs.
> 
> "By default" is somewhat ambiguous. Smack will always enforce its
> basic policy. If files aren't labeled and the Smack process label
> isn't explicitly set there won't be any problems. However, if files
> have somehow gotten labels assigned and there are no rules defined
> things can go sideways.

Right, it should then mean without effect whatever kernel-mediated 
persistent data (e.g. FS's xattr), but I agree that the limit with an 
explicit configuration can be blurry. I guess we could explicitly mark 
LSMs with a property that specify if they consider safe (for the system) 
to be implicitly enabled without explicit run time configuration.
Casey Schaufler Nov. 7, 2022, 5:21 p.m. UTC | #15
On 11/7/2022 4:35 AM, Mickaël Salaün wrote:
>
> On 04/11/2022 18:20, Casey Schaufler wrote:
>> On 11/4/2022 9:29 AM, Mickaël Salaün wrote:
>>>
>>> On 18/10/2022 21:31, Paul Moore wrote:
>>>> On Tue, Oct 18, 2022 at 1:55 AM Kees Cook <keescook@chromium.org>
>>>> wrote:
>>>>> On Mon, Oct 17, 2022 at 09:45:21PM -0400, Paul Moore wrote:
>>>
>>> [...]
>>>
>>>>>> We can have defaults, like we do know, but I'm in no hurry to remove
>>>>>> the ability to allow admins to change the ordering at boot time.
>>>>>
>>>>> My concern is with new LSMs vs the build system. A system builder
>>>>> will
>>>>> be prompted for a new CONFIG_SECURITY_SHINY, but won't be prompted
>>>>> about making changes to CONFIG_LSM to include it.
>>>>
>>>> I would argue that if an admin/builder doesn't understand what a shiny
>>>> new LSM does, they shouldn't be enabling that shiny new LSM.  Adding
>>>> new, potentially restrictive, controls to your kernel build without a
>>>> basic understanding of those controls is a recipe for disaster and I
>>>> try to avoid recommending disaster as a planned course of action :)
>>>
>>> It depends on what this shiny new LSMs do *by default*. In the case of
>>> Landlock, it do nothing unless a process does specific system calls
>>> (same as for most new kernel features: sysfs entries, syscall flags…).
>>> I guess this is the same for most LSMs.
>>
>> "By default" is somewhat ambiguous. Smack will always enforce its
>> basic policy. If files aren't labeled and the Smack process label
>> isn't explicitly set there won't be any problems. However, if files
>> have somehow gotten labels assigned and there are no rules defined
>> things can go sideways.
>
> Right, it should then mean without effect whatever kernel-mediated
> persistent data (e.g. FS's xattr), but I agree that the limit with an
> explicit configuration can be blurry. I guess we could explicitly mark
> LSMs with a property that specify if they consider safe (for the
> system) to be implicitly enabled without explicit run time configuration.

In the Smack example, the system would be "safe" from the standpoint
of system security policy. It might not "work", because the enforcement
could prevent expected access. There is no simple way to identify if an
LSM is going to need configuration, and can be counted on having it, at
initialization. It's up to the LSM to decide what to do if it isn't
properly initialized.
Mickaël Salaün Nov. 7, 2022, 7:37 p.m. UTC | #16
On 07/11/2022 18:21, Casey Schaufler wrote:
> On 11/7/2022 4:35 AM, Mickaël Salaün wrote:
>>
>> On 04/11/2022 18:20, Casey Schaufler wrote:
>>> On 11/4/2022 9:29 AM, Mickaël Salaün wrote:
>>>>
>>>> On 18/10/2022 21:31, Paul Moore wrote:
>>>>> On Tue, Oct 18, 2022 at 1:55 AM Kees Cook <keescook@chromium.org>
>>>>> wrote:
>>>>>> On Mon, Oct 17, 2022 at 09:45:21PM -0400, Paul Moore wrote:
>>>>
>>>> [...]
>>>>
>>>>>>> We can have defaults, like we do know, but I'm in no hurry to remove
>>>>>>> the ability to allow admins to change the ordering at boot time.
>>>>>>
>>>>>> My concern is with new LSMs vs the build system. A system builder
>>>>>> will
>>>>>> be prompted for a new CONFIG_SECURITY_SHINY, but won't be prompted
>>>>>> about making changes to CONFIG_LSM to include it.
>>>>>
>>>>> I would argue that if an admin/builder doesn't understand what a shiny
>>>>> new LSM does, they shouldn't be enabling that shiny new LSM.  Adding
>>>>> new, potentially restrictive, controls to your kernel build without a
>>>>> basic understanding of those controls is a recipe for disaster and I
>>>>> try to avoid recommending disaster as a planned course of action :)
>>>>
>>>> It depends on what this shiny new LSMs do *by default*. In the case of
>>>> Landlock, it do nothing unless a process does specific system calls
>>>> (same as for most new kernel features: sysfs entries, syscall flags…).
>>>> I guess this is the same for most LSMs.
>>>
>>> "By default" is somewhat ambiguous. Smack will always enforce its
>>> basic policy. If files aren't labeled and the Smack process label
>>> isn't explicitly set there won't be any problems. However, if files
>>> have somehow gotten labels assigned and there are no rules defined
>>> things can go sideways.
>>
>> Right, it should then mean without effect whatever kernel-mediated
>> persistent data (e.g. FS's xattr), but I agree that the limit with an
>> explicit configuration can be blurry. I guess we could explicitly mark
>> LSMs with a property that specify if they consider safe (for the
>> system) to be implicitly enabled without explicit run time configuration.
> 
> In the Smack example, the system would be "safe" from the standpoint
> of system security policy. It might not "work", because the enforcement
> could prevent expected access. There is no simple way to identify if an
> LSM is going to need configuration, and can be counted on having it, at
> initialization. It's up to the LSM to decide what to do if it isn't
> properly initialized.

I agree, I was thinking about "working" (without specific security 
contract). I was suggesting to create a dedicated field in the 
DEFINE_LSM struct to identify if the LSM needs a proper initialization 
(which is not the case for Yama, Landlock, BPF, and probably others).
diff mbox series

Patch

diff --git a/security/Kconfig b/security/Kconfig
index 7561f6f99f1d..fae083e9867d 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -243,6 +243,7 @@  source "security/integrity/Kconfig"
 
 choice
 	prompt "First legacy 'major LSM' to be initialized"
+	depends on SECURITY
 	default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX
 	default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
 	default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
@@ -275,8 +276,26 @@  choice
 
 endchoice
 
+config LSM_AUTO
+	bool "Automatically enable all selected LSMs at boot"
+	depends on SECURITY
+	default y
+	help
+	  This automatically configure the build-time selected LSMs to be
+	  enabled at boot unless the "lsm=" parameter is provided.
+
+	  If this option is not selected, it will be required to configure and
+	  maintained a static list of enabled LSMs that may become inconsistent
+	  with future user configuration.  Indeed, this list will not be
+	  automatically upgraded when selecting a new (future) LSM, e.g. with
+	  make oldconfig.
+
+	  If you are unsure how to answer this question, answer Y.
+
+# This lists should be synchronized with LSM_ORDER defined in security/security.c .
 config LSM
 	string "Ordered list of enabled LSMs"
+	depends on SECURITY && !LSM_AUTO
 	default "lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
 	default "lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
 	default "lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
diff --git a/security/security.c b/security/security.c
index 401663b5b70e..defa1d2c40a3 100644
--- a/security/security.c
+++ b/security/security.c
@@ -82,7 +82,31 @@  static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
 static __initdata const char *chosen_lsm_order;
 static __initdata const char *chosen_major_lsm;
 
-static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
+#ifdef CONFIG_LSM
+#define LSM_ORDER	CONFIG_LSM
+#else
+
+/*
+ * This lists should be synchronized with the default values of CONFIG_LSM
+ * defined in security/Kconfig .
+ */
+#define LSM_ORDER_PRE	"lockdown,yama,loadpin,safesetid,integrity,"
+
+#if defined(CONFIG_DEFAULT_SECURITY_SMACK)
+#define LSM_ORDER	LSM_ORDER_PRE "smack,selinux,tomoyo,apparmor,bpf"
+#elif defined(CONFIG_DEFAULT_SECURITY_APPARMOR)
+#define LSM_ORDER	LSM_ORDER_PRE "apparmor,selinux,smack,tomoyo,bpf"
+#elif defined(CONFIG_DEFAULT_SECURITY_TOMOYO)
+#define LSM_ORDER	LSM_ORDER_PRE "tomoyo,bpf"
+#elif defined(CONFIG_DEFAULT_SECURITY_DAC)
+#define LSM_ORDER	LSM_ORDER_PRE "bpf"
+#else
+#define LSM_ORDER	LSM_ORDER_PRE "selinux,smack,tomoyo,apparmor,bpf"
+#endif
+
+#endif /* CONFIG_LSM */
+
+static __initconst const char * const builtin_lsm_order = LSM_ORDER;
 
 /* Ordered list of LSMs to initialize. */
 static __initdata struct lsm_info **ordered_lsms;