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

On Sun, Feb 21, 2021 at 8:11 PM Mickaël Salaün [off-list ref] wrote:

On 21/02/2021 09:50, Masahiro Yamada wrote:

quoted

On Tue, Feb 16, 2021 at 4:03 AM Ondrej Mosnacek [off-list ref] wrote:

quoted

On Mon, Feb 15, 2021 at 7:17 PM Mickaël Salaün [off-list ref] wrote:

quoted

From: Mickaël Salaün <redacted>

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 <redacted>
Link: https://lore.kernel.org/r/20210215181511.2840674-4-mic@digikod.net (local)
---

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.

quoted

quoted

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.

quoted

quoted

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

quoted

        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