From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D088FC433DB for ; Mon, 22 Feb 2021 15:09:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8977164E61 for ; Mon, 22 Feb 2021 15:09:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230489AbhBVPI5 (ORCPT ); Mon, 22 Feb 2021 10:08:57 -0500 Received: from smtp-190f.mail.infomaniak.ch ([185.125.25.15]:54099 "EHLO smtp-190f.mail.infomaniak.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230008AbhBVPIM (ORCPT ); Mon, 22 Feb 2021 10:08:12 -0500 Received: from smtp-2-0001.mail.infomaniak.ch (unknown [10.5.36.108]) by smtp-2-3000.mail.infomaniak.ch (Postfix) with ESMTPS id 4DklsQ6vvXzMqXJk; Mon, 22 Feb 2021 16:07:22 +0100 (CET) Received: from ns3096276.ip-94-23-54.eu (unknown [23.97.221.149]) by smtp-2-0001.mail.infomaniak.ch (Postfix) with ESMTPA id 4DklsP6SCPzlh8Tg; Mon, 22 Feb 2021 16:07:21 +0100 (CET) Subject: Re: [PATCH v2 3/3] security: Add LSMs dependencies to CONFIG_LSM From: =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= To: Masahiro Yamada Cc: Ondrej Mosnacek , James Morris , "Serge E . Hallyn" , Casey Schaufler , Nicolas Iooss , Linux Kbuild mailing list , Linux kernel mailing list , Linux Security Module list , Kees Cook References: <20210215181511.2840674-1-mic@digikod.net> <20210215181511.2840674-4-mic@digikod.net> <8809a929-980a-95d1-42dc-576ff54e2923@digikod.net> <12b27829-5db0-e9a4-0c74-896c53445da4@digikod.net> Message-ID: Date: Mon, 22 Feb 2021 16:08:39 +0100 User-Agent: MIME-Version: 1.0 In-Reply-To: <12b27829-5db0-e9a4-0c74-896c53445da4@digikod.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 wrote: >>> >>> >>> On 21/02/2021 09:50, Masahiro Yamada wrote: >>>> On Tue, Feb 16, 2021 at 4:03 AM Ondrej Mosnacek wrote: >>>>> >>>>> On Mon, Feb 15, 2021 at 7:17 PM Mickaël Salaün wrote: >>>>>> From: Mickaël Salaün >>>>>> >>>>>> 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 >>>>>> Cc: James Morris >>>>>> Cc: Masahiro Yamada >>>>>> Cc: Serge E. Hallyn >>>>>> Signed-off-by: Mickaël Salaün >>>>>> 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 >>