diff mbox series

[v2,3/3] security: Add LSMs dependencies to CONFIG_LSM

Message ID 20210215181511.2840674-4-mic@digikod.net (mailing list archive)
State New, archived
Headers show
Series Kconfig oldconfig string update | expand

Commit Message

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

Thanks to the previous commit, this gives the opportunity to users, when
running make oldconfig, to update the list of enabled LSMs at boot time
if an LSM has just been enabled or disabled in the build.  Moreover,
this list only makes sense if at least one LSM is enabled.

Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: James Morris <jmorris@namei.org>
Cc: Masahiro Yamada <masahiroy@kernel.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/20210215181511.2840674-4-mic@digikod.net
---

Changes since v1:
* Add CONFIG_SECURITY as a dependency of CONFIG_LSM.  This prevent an
  error when building without any LSMs.
---
 security/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Ondrej Mosnacek Feb. 15, 2021, 7:03 p.m. UTC | #1
On Mon, Feb 15, 2021 at 7:17 PM Mickaël Salaün <mic@digikod.net> wrote:
> From: Mickaël Salaün <mic@linux.microsoft.com>
>
> Thanks to the previous commit, this gives the opportunity to users, when
> running make oldconfig, to update the list of enabled LSMs at boot time
> if an LSM has just been enabled or disabled in the build.  Moreover,
> this list only makes sense if at least one LSM is enabled.
>
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Masahiro Yamada <masahiroy@kernel.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/20210215181511.2840674-4-mic@digikod.net
> ---
>
> Changes since v1:
> * Add CONFIG_SECURITY as a dependency of CONFIG_LSM.  This prevent an
>   error when building without any LSMs.
> ---
>  security/Kconfig | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/security/Kconfig b/security/Kconfig
> index 7561f6f99f1d..addcc1c04701 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -277,6 +277,10 @@ endchoice
>
>  config LSM
>         string "Ordered list of enabled LSMs"
> +       depends on SECURITY || SECURITY_LOCKDOWN_LSM || SECURITY_YAMA || \
> +               SECURITY_LOADPIN || SECURITY_SAFESETID || INTEGRITY || \
> +               SECURITY_SELINUX || SECURITY_SMACK || SECURITY_TOMOYO || \
> +               SECURITY_APPARMOR || BPF_LSM

This looks really awkward, since all of these already depend on
SECURITY (if not, it's a bug)... I guarantee you that after some time
someone will come, see that the weird boolean expression is equivalent
to just SECURITY, and simplify it.

I assume the new mechanism wouldn't work as intended if there is just
SECURITY? If not, then maybe you should rather specify this value
dependency via some new  field rather than abusing "depends on" (say,
"value depends on"?). The fact that a seemingly innocent change to the
config definition breaks your mechanism suggests that the design is
flawed.

I do think this would be a useful feature, but IMHO shouldn't be
implemented like this.

>         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
> --
> 2.30.0
>

--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
Masahiro Yamada Feb. 21, 2021, 8:50 a.m. UTC | #2
On Tue, Feb 16, 2021 at 4:03 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> On Mon, Feb 15, 2021 at 7:17 PM Mickaël Salaün <mic@digikod.net> wrote:
> > From: Mickaël Salaün <mic@linux.microsoft.com>
> >
> > Thanks to the previous commit, this gives the opportunity to users, when
> > running make oldconfig, to update the list of enabled LSMs at boot time
> > if an LSM has just been enabled or disabled in the build.  Moreover,
> > this list only makes sense if at least one LSM is enabled.
> >
> > Cc: Casey Schaufler <casey@schaufler-ca.com>
> > Cc: James Morris <jmorris@namei.org>
> > Cc: Masahiro Yamada <masahiroy@kernel.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/20210215181511.2840674-4-mic@digikod.net
> > ---
> >
> > Changes since v1:
> > * Add CONFIG_SECURITY as a dependency of CONFIG_LSM.  This prevent an
> >   error when building without any LSMs.
> > ---
> >  security/Kconfig | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/security/Kconfig b/security/Kconfig
> > index 7561f6f99f1d..addcc1c04701 100644
> > --- a/security/Kconfig
> > +++ b/security/Kconfig
> > @@ -277,6 +277,10 @@ endchoice
> >
> >  config LSM
> >         string "Ordered list of enabled LSMs"
> > +       depends on SECURITY || SECURITY_LOCKDOWN_LSM || SECURITY_YAMA || \
> > +               SECURITY_LOADPIN || SECURITY_SAFESETID || INTEGRITY || \
> > +               SECURITY_SELINUX || SECURITY_SMACK || SECURITY_TOMOYO || \
> > +               SECURITY_APPARMOR || BPF_LSM
>
> This looks really awkward, since all of these already depend on
> SECURITY (if not, it's a bug)... I guarantee you that after some time
> someone will come, see that the weird boolean expression is equivalent
> to just SECURITY, and simplify it.


Currently, LSM does not depend on SECURITY.
So you can always define LSM irrespective of SECURITY,
which seems a bug.

So, I agree with adding 'depends on SECURITY'.

What he is trying to achieve in this series
seems wrong, of course.










> I assume the new mechanism wouldn't work as intended if there is just
> SECURITY? If not, then maybe you should rather specify this value
> dependency via some new  field rather than abusing "depends on" (say,
> "value depends on"?). The fact that a seemingly innocent change to the
> config definition breaks your mechanism suggests that the design is
> flawed.
>
> I do think this would be a useful feature, but IMHO shouldn't be
> implemented like this.
>
> >         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
> > --
> > 2.30.0
> >
>
> --
> Ondrej Mosnacek
> Software Engineer, Linux Security - SELinux kernel
> Red Hat, Inc.
>
Mickaël Salaün Feb. 21, 2021, 11:12 a.m. UTC | #3
On 21/02/2021 09:50, Masahiro Yamada wrote:
> On Tue, Feb 16, 2021 at 4:03 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>>
>> On Mon, Feb 15, 2021 at 7:17 PM Mickaël Salaün <mic@digikod.net> wrote:
>>> From: Mickaël Salaün <mic@linux.microsoft.com>
>>>
>>> Thanks to the previous commit, this gives the opportunity to users, when
>>> running make oldconfig, to update the list of enabled LSMs at boot time
>>> if an LSM has just been enabled or disabled in the build.  Moreover,
>>> this list only makes sense if at least one LSM is enabled.
>>>
>>> Cc: Casey Schaufler <casey@schaufler-ca.com>
>>> Cc: James Morris <jmorris@namei.org>
>>> Cc: Masahiro Yamada <masahiroy@kernel.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/20210215181511.2840674-4-mic@digikod.net
>>> ---
>>>
>>> Changes since v1:
>>> * Add CONFIG_SECURITY as a dependency of CONFIG_LSM.  This prevent an
>>>   error when building without any LSMs.
>>> ---
>>>  security/Kconfig | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/security/Kconfig b/security/Kconfig
>>> index 7561f6f99f1d..addcc1c04701 100644
>>> --- a/security/Kconfig
>>> +++ b/security/Kconfig
>>> @@ -277,6 +277,10 @@ endchoice
>>>
>>>  config LSM
>>>         string "Ordered list of enabled LSMs"
>>> +       depends on SECURITY || SECURITY_LOCKDOWN_LSM || SECURITY_YAMA || \
>>> +               SECURITY_LOADPIN || SECURITY_SAFESETID || INTEGRITY || \
>>> +               SECURITY_SELINUX || SECURITY_SMACK || SECURITY_TOMOYO || \
>>> +               SECURITY_APPARMOR || BPF_LSM
>>
>> This looks really awkward, since all of these already depend on
>> SECURITY (if not, it's a bug)... I guarantee you that after some time
>> someone will come, see that the weird boolean expression is equivalent
>> to just SECURITY, and simplify it.
> 
> 
> Currently, LSM does not depend on SECURITY.
> So you can always define LSM irrespective of SECURITY,
> which seems a bug.
> 
> So, I agree with adding 'depends on SECURITY'.
> 
> What he is trying to achieve in this series
> seems wrong, of course.

This may be wrong in the general case, but not for CONFIG_LSM.

> 
> 
>> I assume the new mechanism wouldn't work as intended if there is just
>> SECURITY? If not, then maybe you should rather specify this value
>> dependency via some new  field rather than abusing "depends on" (say,
>> "value depends on"?). The fact that a seemingly innocent change to the
>> config definition breaks your mechanism suggests that the design is
>> flawed.

Masahiro, what do you think about this suggested "value depends on"?


>>
>> I do think this would be a useful feature, but IMHO shouldn't be
>> implemented like this.
>>
>>>         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
>>> --
>>> 2.30.0
>>>
>>
>> --
>> Ondrej Mosnacek
>> Software Engineer, Linux Security - SELinux kernel
>> Red Hat, Inc.
>>
> 
>
Masahiro Yamada Feb. 21, 2021, 2:45 p.m. UTC | #4
On Sun, Feb 21, 2021 at 8:11 PM Mickaël Salaün <mic@digikod.net> wrote:
>
>
> On 21/02/2021 09:50, Masahiro Yamada wrote:
> > On Tue, Feb 16, 2021 at 4:03 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >>
> >> On Mon, Feb 15, 2021 at 7:17 PM Mickaël Salaün <mic@digikod.net> wrote:
> >>> From: Mickaël Salaün <mic@linux.microsoft.com>
> >>>
> >>> Thanks to the previous commit, this gives the opportunity to users, when
> >>> running make oldconfig, to update the list of enabled LSMs at boot time
> >>> if an LSM has just been enabled or disabled in the build.  Moreover,
> >>> this list only makes sense if at least one LSM is enabled.
> >>>
> >>> Cc: Casey Schaufler <casey@schaufler-ca.com>
> >>> Cc: James Morris <jmorris@namei.org>
> >>> Cc: Masahiro Yamada <masahiroy@kernel.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/20210215181511.2840674-4-mic@digikod.net
> >>> ---
> >>>
> >>> Changes since v1:
> >>> * Add CONFIG_SECURITY as a dependency of CONFIG_LSM.  This prevent an
> >>>   error when building without any LSMs.
> >>> ---
> >>>  security/Kconfig | 4 ++++
> >>>  1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/security/Kconfig b/security/Kconfig
> >>> index 7561f6f99f1d..addcc1c04701 100644
> >>> --- a/security/Kconfig
> >>> +++ b/security/Kconfig
> >>> @@ -277,6 +277,10 @@ endchoice
> >>>
> >>>  config LSM
> >>>         string "Ordered list of enabled LSMs"
> >>> +       depends on SECURITY || SECURITY_LOCKDOWN_LSM || SECURITY_YAMA || \
> >>> +               SECURITY_LOADPIN || SECURITY_SAFESETID || INTEGRITY || \
> >>> +               SECURITY_SELINUX || SECURITY_SMACK || SECURITY_TOMOYO || \
> >>> +               SECURITY_APPARMOR || BPF_LSM
> >>
> >> This looks really awkward, since all of these already depend on
> >> SECURITY (if not, it's a bug)... I guarantee you that after some time
> >> someone will come, see that the weird boolean expression is equivalent
> >> to just SECURITY, and simplify it.
> >
> >
> > Currently, LSM does not depend on SECURITY.
> > So you can always define LSM irrespective of SECURITY,
> > which seems a bug.
> >
> > So, I agree with adding 'depends on SECURITY'.
> >
> > What he is trying to achieve in this series
> > seems wrong, of course.
>
> This may be wrong in the general case, but not for CONFIG_LSM.
>
> >
> >
> >> I assume the new mechanism wouldn't work as intended if there is just
> >> SECURITY? If not, then maybe you should rather specify this value
> >> dependency via some new  field rather than abusing "depends on" (say,
> >> "value depends on"?). The fact that a seemingly innocent change to the
> >> config definition breaks your mechanism suggests that the design is
> >> flawed.
>
> Masahiro, what do you think about this suggested "value depends on"?


Of course, no.


See the help text in init/Kconfig:

          This choice is there only for converting CONFIG_DEFAULT_SECURITY
          in old kernel configs to CONFIG_LSM in new kernel configs. Don't
          change this choice unless you are creating a fresh kernel config,
          for this choice will be ignored after CONFIG_LSM has been set.


When CONFIG_LSM is already set in the .config,
this choice is just ignored.
So, oldconfig is working as the help message says.

If you think 2623c4fbe2ad1341ff2d1e12410d0afdae2490ca
is a pointless commit, you should ask Kees about it.






> >>
> >> I do think this would be a useful feature, but IMHO shouldn't be
> >> implemented like this.
> >>
> >>>         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
> >>> --
> >>> 2.30.0
> >>>
> >>
> >> --
> >> Ondrej Mosnacek
> >> Software Engineer, Linux Security - SELinux kernel
> >> Red Hat, Inc.
> >>
> >
> >
--
Best Regards
Masahiro Yamada
Mickaël Salaün Feb. 22, 2021, 10:47 a.m. UTC | #5
On 21/02/2021 15:45, Masahiro Yamada wrote:
> On Sun, Feb 21, 2021 at 8:11 PM Mickaël Salaün <mic@digikod.net> wrote:
>>
>>
>> On 21/02/2021 09:50, Masahiro Yamada wrote:
>>> On Tue, Feb 16, 2021 at 4:03 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>>>>
>>>> On Mon, Feb 15, 2021 at 7:17 PM Mickaël Salaün <mic@digikod.net> wrote:
>>>>> From: Mickaël Salaün <mic@linux.microsoft.com>
>>>>>
>>>>> Thanks to the previous commit, this gives the opportunity to users, when
>>>>> running make oldconfig, to update the list of enabled LSMs at boot time
>>>>> if an LSM has just been enabled or disabled in the build.  Moreover,
>>>>> this list only makes sense if at least one LSM is enabled.
>>>>>
>>>>> Cc: Casey Schaufler <casey@schaufler-ca.com>
>>>>> Cc: James Morris <jmorris@namei.org>
>>>>> Cc: Masahiro Yamada <masahiroy@kernel.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/20210215181511.2840674-4-mic@digikod.net
>>>>> ---
>>>>>
>>>>> Changes since v1:
>>>>> * Add CONFIG_SECURITY as a dependency of CONFIG_LSM.  This prevent an
>>>>>   error when building without any LSMs.
>>>>> ---
>>>>>  security/Kconfig | 4 ++++
>>>>>  1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/security/Kconfig b/security/Kconfig
>>>>> index 7561f6f99f1d..addcc1c04701 100644
>>>>> --- a/security/Kconfig
>>>>> +++ b/security/Kconfig
>>>>> @@ -277,6 +277,10 @@ endchoice
>>>>>
>>>>>  config LSM
>>>>>         string "Ordered list of enabled LSMs"
>>>>> +       depends on SECURITY || SECURITY_LOCKDOWN_LSM || SECURITY_YAMA || \
>>>>> +               SECURITY_LOADPIN || SECURITY_SAFESETID || INTEGRITY || \
>>>>> +               SECURITY_SELINUX || SECURITY_SMACK || SECURITY_TOMOYO || \
>>>>> +               SECURITY_APPARMOR || BPF_LSM
>>>>
>>>> This looks really awkward, since all of these already depend on
>>>> SECURITY (if not, it's a bug)... I guarantee you that after some time
>>>> someone will come, see that the weird boolean expression is equivalent
>>>> to just SECURITY, and simplify it.
>>>
>>>
>>> Currently, LSM does not depend on SECURITY.
>>> So you can always define LSM irrespective of SECURITY,
>>> which seems a bug.
>>>
>>> So, I agree with adding 'depends on SECURITY'.
>>>
>>> What he is trying to achieve in this series
>>> seems wrong, of course.
>>
>> This may be wrong in the general case, but not for CONFIG_LSM.
>>
>>>
>>>
>>>> I assume the new mechanism wouldn't work as intended if there is just
>>>> SECURITY? If not, then maybe you should rather specify this value
>>>> dependency via some new  field rather than abusing "depends on" (say,
>>>> "value depends on"?). The fact that a seemingly innocent change to the
>>>> config definition breaks your mechanism suggests that the design is
>>>> flawed.
>>
>> Masahiro, what do you think about this suggested "value depends on"?
> 
> 
> Of course, no.
> 
> 
> See the help text in init/Kconfig:
> 
>           This choice is there only for converting CONFIG_DEFAULT_SECURITY
>           in old kernel configs to CONFIG_LSM in new kernel configs. Don't
>           change this choice unless you are creating a fresh kernel config,
>           for this choice will be ignored after CONFIG_LSM has been set.
> 
> 
> When CONFIG_LSM is already set in the .config,
> this choice is just ignored.
> So, oldconfig is working as the help message says.
> 
> If you think 2623c4fbe2ad1341ff2d1e12410d0afdae2490ca
> is a pointless commit, you should ask Kees about it.

This commit was for backward compatibility to not change the configured
system behavior because of a new default configuration.
Here I want to address a forward compatibility issue: when users want to
enable an LSM, give them the opportunity to enable it at boot time too
instead of silently ignoring this new configuration at boot time.
Indeed, there is two kind of configurations: built time configuration
with Kconfig, and boot time configuration with the content of
CONFIG_LSM. However, there is no direct dependency between LSM toggles
and CONFIG_LSM once it is set.

I think a better solution would be to add a new CONFIG_LSM_AUTO boolean
to automatically generate the content of CONFIG_LSM according to the
(build/kconfig) enabled LSMs, while letting users the ability to
manually configure CONFIG_LSM otherwise. What do you think?

> 
>>>>
>>>> I do think this would be a useful feature, but IMHO shouldn't be
>>>> implemented like this.
>>>>
>>>>>         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
>>>>> --
>>>>> 2.30.0
>>>>>
>>>>
>>>> --
>>>> Ondrej Mosnacek
>>>> Software Engineer, Linux Security - SELinux kernel
>>>> Red Hat, Inc.
>>>>
>>>
>>>
> --
> Best Regards
> Masahiro Yamada
>
Mickaël Salaün Feb. 22, 2021, 3:08 p.m. UTC | #6
On 22/02/2021 11:47, Mickaël Salaün wrote:
> 
> On 21/02/2021 15:45, Masahiro Yamada wrote:
>> On Sun, Feb 21, 2021 at 8:11 PM Mickaël Salaün <mic@digikod.net> wrote:
>>>
>>>
>>> On 21/02/2021 09:50, Masahiro Yamada wrote:
>>>> On Tue, Feb 16, 2021 at 4:03 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>>>>>
>>>>> On Mon, Feb 15, 2021 at 7:17 PM Mickaël Salaün <mic@digikod.net> wrote:
>>>>>> From: Mickaël Salaün <mic@linux.microsoft.com>
>>>>>>
>>>>>> Thanks to the previous commit, this gives the opportunity to users, when
>>>>>> running make oldconfig, to update the list of enabled LSMs at boot time
>>>>>> if an LSM has just been enabled or disabled in the build.  Moreover,
>>>>>> this list only makes sense if at least one LSM is enabled.
>>>>>>
>>>>>> Cc: Casey Schaufler <casey@schaufler-ca.com>
>>>>>> Cc: James Morris <jmorris@namei.org>
>>>>>> Cc: Masahiro Yamada <masahiroy@kernel.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/20210215181511.2840674-4-mic@digikod.net
>>>>>> ---
>>>>>>
>>>>>> Changes since v1:
>>>>>> * Add CONFIG_SECURITY as a dependency of CONFIG_LSM.  This prevent an
>>>>>>   error when building without any LSMs.
>>>>>> ---
>>>>>>  security/Kconfig | 4 ++++
>>>>>>  1 file changed, 4 insertions(+)
>>>>>>
>>>>>> diff --git a/security/Kconfig b/security/Kconfig
>>>>>> index 7561f6f99f1d..addcc1c04701 100644
>>>>>> --- a/security/Kconfig
>>>>>> +++ b/security/Kconfig
>>>>>> @@ -277,6 +277,10 @@ endchoice
>>>>>>
>>>>>>  config LSM
>>>>>>         string "Ordered list of enabled LSMs"
>>>>>> +       depends on SECURITY || SECURITY_LOCKDOWN_LSM || SECURITY_YAMA || \
>>>>>> +               SECURITY_LOADPIN || SECURITY_SAFESETID || INTEGRITY || \
>>>>>> +               SECURITY_SELINUX || SECURITY_SMACK || SECURITY_TOMOYO || \
>>>>>> +               SECURITY_APPARMOR || BPF_LSM
>>>>>
>>>>> This looks really awkward, since all of these already depend on
>>>>> SECURITY (if not, it's a bug)... I guarantee you that after some time
>>>>> someone will come, see that the weird boolean expression is equivalent
>>>>> to just SECURITY, and simplify it.
>>>>
>>>>
>>>> Currently, LSM does not depend on SECURITY.
>>>> So you can always define LSM irrespective of SECURITY,
>>>> which seems a bug.
>>>>
>>>> So, I agree with adding 'depends on SECURITY'.
>>>>
>>>> What he is trying to achieve in this series
>>>> seems wrong, of course.
>>>
>>> This may be wrong in the general case, but not for CONFIG_LSM.
>>>
>>>>
>>>>
>>>>> I assume the new mechanism wouldn't work as intended if there is just
>>>>> SECURITY? If not, then maybe you should rather specify this value
>>>>> dependency via some new  field rather than abusing "depends on" (say,
>>>>> "value depends on"?). The fact that a seemingly innocent change to the
>>>>> config definition breaks your mechanism suggests that the design is
>>>>> flawed.
>>>
>>> Masahiro, what do you think about this suggested "value depends on"?
>>
>>
>> Of course, no.
>>
>>
>> See the help text in init/Kconfig:
>>
>>           This choice is there only for converting CONFIG_DEFAULT_SECURITY
>>           in old kernel configs to CONFIG_LSM in new kernel configs. Don't
>>           change this choice unless you are creating a fresh kernel config,
>>           for this choice will be ignored after CONFIG_LSM has been set.
>>
>>
>> When CONFIG_LSM is already set in the .config,
>> this choice is just ignored.
>> So, oldconfig is working as the help message says.
>>
>> If you think 2623c4fbe2ad1341ff2d1e12410d0afdae2490ca
>> is a pointless commit, you should ask Kees about it.
> 
> This commit was for backward compatibility to not change the configured
> system behavior because of a new default configuration.
> Here I want to address a forward compatibility issue: when users want to
> enable an LSM, give them the opportunity to enable it at boot time too
> instead of silently ignoring this new configuration at boot time.
> Indeed, there is two kind of configurations: built time configuration
> with Kconfig, and boot time configuration with the content of
> CONFIG_LSM. However, there is no direct dependency between LSM toggles
> and CONFIG_LSM once it is set.
> 
> I think a better solution would be to add a new CONFIG_LSM_AUTO boolean
> to automatically generate the content of CONFIG_LSM according to the
> (build/kconfig) enabled LSMs, while letting users the ability to
> manually configure CONFIG_LSM otherwise. What do you think?

I sent a new patch series dedicated to the LSM issue:
https://lore.kernel.org/linux-security-module/20210222150608.808146-1-mic@digikod.net/

> 
>>
>>>>>
>>>>> I do think this would be a useful feature, but IMHO shouldn't be
>>>>> implemented like this.
>>>>>
>>>>>>         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
>>>>>> --
>>>>>> 2.30.0
>>>>>>
>>>>>
>>>>> --
>>>>> Ondrej Mosnacek
>>>>> Software Engineer, Linux Security - SELinux kernel
>>>>> Red Hat, Inc.
>>>>>
>>>>
>>>>
>> --
>> Best Regards
>> Masahiro Yamada
>>
diff mbox series

Patch

diff --git a/security/Kconfig b/security/Kconfig
index 7561f6f99f1d..addcc1c04701 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -277,6 +277,10 @@  endchoice
 
 config LSM
 	string "Ordered list of enabled LSMs"
+	depends on SECURITY || SECURITY_LOCKDOWN_LSM || SECURITY_YAMA || \
+		SECURITY_LOADPIN || SECURITY_SAFESETID || INTEGRITY || \
+		SECURITY_SELINUX || SECURITY_SMACK || SECURITY_TOMOYO || \
+		SECURITY_APPARMOR || BPF_LSM
 	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