* [PATCH v2 0/3] Kconfig oldconfig string update
@ 2021-02-15 18:15 Mickaël Salaün
2021-02-15 18:15 ` [PATCH v2 1/3] kconfig: Remove duplicate call to sym_get_string_value() Mickaël Salaün
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Mickaël Salaün @ 2021-02-15 18:15 UTC (permalink / raw)
To: James Morris, Masahiro Yamada, Serge E . Hallyn
Cc: Mickaël Salaün, Casey Schaufler, Nicolas Iooss,
linux-kbuild, linux-kernel, linux-security-module
Hi,
This patch series gives the opportunity to users, when running make
oldconfig, to update configuration strings (e.g. CONFIG_LSM) according
to dependency changes. This helps users keep a consistent up-to-date
kernel configuration.
This second series fixes a build error reported by kernel test robot
when building without any LSM.
This patch series can be applied on v5.11 . Previous version:
https://lore.kernel.org/r/20210215122513.1773897-1-mic@digikod.net
Regards,
Mickaël Salaün (3):
kconfig: Remove duplicate call to sym_get_string_value()
kconfig: Ask user if string needs to be changed when dependency
changed
security: Add LSMs dependencies to CONFIG_LSM
scripts/kconfig/conf.c | 37 ++++++++++++++++++++++++++++++++++---
security/Kconfig | 4 ++++
2 files changed, 38 insertions(+), 3 deletions(-)
base-commit: f40ddce88593482919761f74910f42f4b84c004b
--
2.30.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 1/3] kconfig: Remove duplicate call to sym_get_string_value() 2021-02-15 18:15 [PATCH v2 0/3] Kconfig oldconfig string update Mickaël Salaün @ 2021-02-15 18:15 ` Mickaël Salaün 2021-02-21 8:52 ` Masahiro Yamada 2021-02-15 18:15 ` [PATCH v2 2/3] kconfig: Ask user if string needs to be changed when dependency changed Mickaël Salaün 2021-02-15 18:15 ` [PATCH v2 3/3] security: Add LSMs dependencies to CONFIG_LSM Mickaël Salaün 2 siblings, 1 reply; 14+ messages in thread From: Mickaël Salaün @ 2021-02-15 18:15 UTC (permalink / raw) To: James Morris, Masahiro Yamada, Serge E . Hallyn Cc: Mickaël Salaün, Casey Schaufler, Nicolas Iooss, linux-kbuild, linux-kernel, linux-security-module, Mickaël Salaün From: Mickaël Salaün <mic@linux.microsoft.com> Use the saved returned value of sym_get_string_value() instead of calling it twice. Cc: Masahiro Yamada <masahiroy@kernel.org> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com> Link: https://lore.kernel.org/r/20210215181511.2840674-2-mic@digikod.net --- scripts/kconfig/conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c index db03e2f45de4..18a233d27a8d 100644 --- a/scripts/kconfig/conf.c +++ b/scripts/kconfig/conf.c @@ -137,7 +137,7 @@ static int conf_string(struct menu *menu) printf("%*s%s ", indent - 1, "", menu->prompt->text); printf("(%s) ", sym->name); def = sym_get_string_value(sym); - if (sym_get_string_value(sym)) + if (def) printf("[%s] ", def); if (!conf_askvalue(sym, def)) return 0; -- 2.30.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] kconfig: Remove duplicate call to sym_get_string_value() 2021-02-15 18:15 ` [PATCH v2 1/3] kconfig: Remove duplicate call to sym_get_string_value() Mickaël Salaün @ 2021-02-21 8:52 ` Masahiro Yamada 0 siblings, 0 replies; 14+ messages in thread From: Masahiro Yamada @ 2021-02-21 8:52 UTC (permalink / raw) To: Mickaël Salaün Cc: James Morris, Serge E . Hallyn, Casey Schaufler, Nicolas Iooss, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-security-module, Mickaël Salaün On Tue, Feb 16, 2021 at 3:15 AM Mickaël Salaün <mic@digikod.net> wrote: > > From: Mickaël Salaün <mic@linux.microsoft.com> > > Use the saved returned value of sym_get_string_value() instead of > calling it twice. > > Cc: Masahiro Yamada <masahiroy@kernel.org> > Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com> > Link: https://lore.kernel.org/r/20210215181511.2840674-2-mic@digikod.net > --- Applied to linux-kbuild. Thanks. > scripts/kconfig/conf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c > index db03e2f45de4..18a233d27a8d 100644 > --- a/scripts/kconfig/conf.c > +++ b/scripts/kconfig/conf.c > @@ -137,7 +137,7 @@ static int conf_string(struct menu *menu) > printf("%*s%s ", indent - 1, "", menu->prompt->text); > printf("(%s) ", sym->name); > def = sym_get_string_value(sym); > - if (sym_get_string_value(sym)) > + if (def) > printf("[%s] ", def); > if (!conf_askvalue(sym, def)) > return 0; > -- > 2.30.0 > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/3] kconfig: Ask user if string needs to be changed when dependency changed 2021-02-15 18:15 [PATCH v2 0/3] Kconfig oldconfig string update Mickaël Salaün 2021-02-15 18:15 ` [PATCH v2 1/3] kconfig: Remove duplicate call to sym_get_string_value() Mickaël Salaün @ 2021-02-15 18:15 ` Mickaël Salaün 2021-02-21 8:47 ` Masahiro Yamada 2021-02-15 18:15 ` [PATCH v2 3/3] security: Add LSMs dependencies to CONFIG_LSM Mickaël Salaün 2 siblings, 1 reply; 14+ messages in thread From: Mickaël Salaün @ 2021-02-15 18:15 UTC (permalink / raw) To: James Morris, Masahiro Yamada, Serge E . Hallyn Cc: Mickaël Salaün, Casey Schaufler, Nicolas Iooss, linux-kbuild, linux-kernel, linux-security-module, Mickaël Salaün From: Mickaël Salaün <mic@linux.microsoft.com> Content of string configuration may depend on related kernel configurations. Modify oldconfig and syncconfig to inform users about possible required configuration update and give them the opportunity to update it: * if dependencies of this string has changed (e.g. enabled or disabled), * and if the current value of this string is different than the (new) default one. This is particularly relevant for CONFIG_LSM which contains a list of LSMs enabled at boot, but users will not have a chance to update this list with a make oldconfig. 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-3-mic@digikod.net --- scripts/kconfig/conf.c | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c index 18a233d27a8d..8633dacd39a9 100644 --- a/scripts/kconfig/conf.c +++ b/scripts/kconfig/conf.c @@ -82,6 +82,26 @@ static void xfgets(char *str, int size, FILE *in) printf("%s", str); } +static bool may_need_string_update(struct symbol *sym, const char *def) +{ + const struct symbol *dep_sym; + const struct expr *e; + + if (sym->type != S_STRING) + return false; + if (strcmp(def, sym_get_string_default(sym)) == 0) + return false; + /* + * The user may want to synchronize the content of a string related to + * changed dependencies (e.g. CONFIG_LSM). + */ + expr_list_for_each_sym(sym->dir_dep.expr, e, dep_sym) { + if (dep_sym->flags & SYMBOL_CHANGED) + return true; + } + return false; +} + static int conf_askvalue(struct symbol *sym, const char *def) { enum symbol_type type = sym_get_type(sym); @@ -102,7 +122,7 @@ static int conf_askvalue(struct symbol *sym, const char *def) switch (input_mode) { case oldconfig: case syncconfig: - if (sym_has_value(sym)) { + if (sym_has_value(sym) && !may_need_string_update(sym, def)) { printf("%s\n", def); return 0; } @@ -137,8 +157,19 @@ static int conf_string(struct menu *menu) printf("%*s%s ", indent - 1, "", menu->prompt->text); printf("(%s) ", sym->name); def = sym_get_string_value(sym); - if (def) - printf("[%s] ", def); + if (def) { + if (may_need_string_update(sym, def)) { + indent += 2; + printf("\n%*sDefault value is [%s]\n", + indent - 1, "", + sym_get_string_default(sym)); + printf("%*sCurrent value is [%s] ", + indent - 1, "", def); + indent -= 2; + } else { + printf("[%s] ", def); + } + } if (!conf_askvalue(sym, def)) return 0; switch (line[0]) { -- 2.30.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] kconfig: Ask user if string needs to be changed when dependency changed 2021-02-15 18:15 ` [PATCH v2 2/3] kconfig: Ask user if string needs to be changed when dependency changed Mickaël Salaün @ 2021-02-21 8:47 ` Masahiro Yamada 2021-02-21 11:10 ` Mickaël Salaün 0 siblings, 1 reply; 14+ messages in thread From: Masahiro Yamada @ 2021-02-21 8:47 UTC (permalink / raw) To: Mickaël Salaün Cc: James Morris, Serge E . Hallyn, Casey Schaufler, Nicolas Iooss, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-security-module, Mickaël Salaün On Tue, Feb 16, 2021 at 3:14 AM Mickaël Salaün <mic@digikod.net> wrote: > > From: Mickaël Salaün <mic@linux.microsoft.com> > > Content of string configuration may depend on related kernel > configurations. Modify oldconfig and syncconfig to inform users about > possible required configuration update and give them the opportunity to > update it: > * if dependencies of this string has changed (e.g. enabled or disabled), > * and if the current value of this string is different than the (new) > default one. > > This is particularly relevant for CONFIG_LSM which contains a list of > LSMs enabled at boot, but users will not have a chance to update this > list with a make oldconfig. If CONFIG_LSM already exists in the .config, oldconfig does not show a prompt. This is the expected behavior. You are trying to fix your problem in a wrong way. NACK. > > 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-3-mic@digikod.net > --- > scripts/kconfig/conf.c | 37 ++++++++++++++++++++++++++++++++++--- > 1 file changed, 34 insertions(+), 3 deletions(-) > > diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c > index 18a233d27a8d..8633dacd39a9 100644 > --- a/scripts/kconfig/conf.c > +++ b/scripts/kconfig/conf.c > @@ -82,6 +82,26 @@ static void xfgets(char *str, int size, FILE *in) > printf("%s", str); > } > > +static bool may_need_string_update(struct symbol *sym, const char *def) > +{ > + const struct symbol *dep_sym; > + const struct expr *e; > + > + if (sym->type != S_STRING) > + return false; > + if (strcmp(def, sym_get_string_default(sym)) == 0) > + return false; > + /* > + * The user may want to synchronize the content of a string related to > + * changed dependencies (e.g. CONFIG_LSM). > + */ > + expr_list_for_each_sym(sym->dir_dep.expr, e, dep_sym) { > + if (dep_sym->flags & SYMBOL_CHANGED) > + return true; > + } > + return false; > +} > + > static int conf_askvalue(struct symbol *sym, const char *def) > { > enum symbol_type type = sym_get_type(sym); > @@ -102,7 +122,7 @@ static int conf_askvalue(struct symbol *sym, const char *def) > switch (input_mode) { > case oldconfig: > case syncconfig: > - if (sym_has_value(sym)) { > + if (sym_has_value(sym) && !may_need_string_update(sym, def)) { > printf("%s\n", def); > return 0; > } > @@ -137,8 +157,19 @@ static int conf_string(struct menu *menu) > printf("%*s%s ", indent - 1, "", menu->prompt->text); > printf("(%s) ", sym->name); > def = sym_get_string_value(sym); > - if (def) > - printf("[%s] ", def); > + if (def) { > + if (may_need_string_update(sym, def)) { > + indent += 2; > + printf("\n%*sDefault value is [%s]\n", > + indent - 1, "", > + sym_get_string_default(sym)); > + printf("%*sCurrent value is [%s] ", > + indent - 1, "", def); > + indent -= 2; > + } else { > + printf("[%s] ", def); > + } > + } > if (!conf_askvalue(sym, def)) > return 0; > switch (line[0]) { > -- > 2.30.0 > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] kconfig: Ask user if string needs to be changed when dependency changed 2021-02-21 8:47 ` Masahiro Yamada @ 2021-02-21 11:10 ` Mickaël Salaün 2021-02-21 14:45 ` Masahiro Yamada 0 siblings, 1 reply; 14+ messages in thread From: Mickaël Salaün @ 2021-02-21 11:10 UTC (permalink / raw) To: Masahiro Yamada, Casey Schaufler Cc: James Morris, Serge E . Hallyn, Nicolas Iooss, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-security-module, Mickaël Salaün On 21/02/2021 09:47, Masahiro Yamada wrote: > On Tue, Feb 16, 2021 at 3:14 AM Mickaël Salaün <mic@digikod.net> wrote: >> >> From: Mickaël Salaün <mic@linux.microsoft.com> >> >> Content of string configuration may depend on related kernel >> configurations. Modify oldconfig and syncconfig to inform users about >> possible required configuration update and give them the opportunity to >> update it: >> * if dependencies of this string has changed (e.g. enabled or disabled), >> * and if the current value of this string is different than the (new) >> default one. >> >> This is particularly relevant for CONFIG_LSM which contains a list of >> LSMs enabled at boot, but users will not have a chance to update this >> list with a make oldconfig. > > If CONFIG_LSM already exists in the .config, > oldconfig does not show a prompt. > This is the expected behavior. It is not the behavior wished for LSM stacking. > > You are trying to fix your problem in a wrong way. > NACK. What do you suggest to ensure that users will be asked to update the CONFIG_LSM string if they add or remove an LSM? > > > >> >> 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-3-mic@digikod.net >> --- >> scripts/kconfig/conf.c | 37 ++++++++++++++++++++++++++++++++++--- >> 1 file changed, 34 insertions(+), 3 deletions(-) >> >> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c >> index 18a233d27a8d..8633dacd39a9 100644 >> --- a/scripts/kconfig/conf.c >> +++ b/scripts/kconfig/conf.c >> @@ -82,6 +82,26 @@ static void xfgets(char *str, int size, FILE *in) >> printf("%s", str); >> } >> >> +static bool may_need_string_update(struct symbol *sym, const char *def) >> +{ >> + const struct symbol *dep_sym; >> + const struct expr *e; >> + >> + if (sym->type != S_STRING) >> + return false; >> + if (strcmp(def, sym_get_string_default(sym)) == 0) >> + return false; >> + /* >> + * The user may want to synchronize the content of a string related to >> + * changed dependencies (e.g. CONFIG_LSM). >> + */ >> + expr_list_for_each_sym(sym->dir_dep.expr, e, dep_sym) { >> + if (dep_sym->flags & SYMBOL_CHANGED) >> + return true; >> + } >> + return false; >> +} >> + >> static int conf_askvalue(struct symbol *sym, const char *def) >> { >> enum symbol_type type = sym_get_type(sym); >> @@ -102,7 +122,7 @@ static int conf_askvalue(struct symbol *sym, const char *def) >> switch (input_mode) { >> case oldconfig: >> case syncconfig: >> - if (sym_has_value(sym)) { >> + if (sym_has_value(sym) && !may_need_string_update(sym, def)) { >> printf("%s\n", def); >> return 0; >> } >> @@ -137,8 +157,19 @@ static int conf_string(struct menu *menu) >> printf("%*s%s ", indent - 1, "", menu->prompt->text); >> printf("(%s) ", sym->name); >> def = sym_get_string_value(sym); >> - if (def) >> - printf("[%s] ", def); >> + if (def) { >> + if (may_need_string_update(sym, def)) { >> + indent += 2; >> + printf("\n%*sDefault value is [%s]\n", >> + indent - 1, "", >> + sym_get_string_default(sym)); >> + printf("%*sCurrent value is [%s] ", >> + indent - 1, "", def); >> + indent -= 2; >> + } else { >> + printf("[%s] ", def); >> + } >> + } >> if (!conf_askvalue(sym, def)) >> return 0; >> switch (line[0]) { >> -- >> 2.30.0 >> > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] kconfig: Ask user if string needs to be changed when dependency changed 2021-02-21 11:10 ` Mickaël Salaün @ 2021-02-21 14:45 ` Masahiro Yamada 0 siblings, 0 replies; 14+ messages in thread From: Masahiro Yamada @ 2021-02-21 14:45 UTC (permalink / raw) To: Mickaël Salaün Cc: Casey Schaufler, James Morris, Serge E . Hallyn, Nicolas Iooss, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-security-module, Mickaël Salaün On Sun, Feb 21, 2021 at 8:09 PM Mickaël Salaün <mic@digikod.net> wrote: > > > On 21/02/2021 09:47, Masahiro Yamada wrote: > > On Tue, Feb 16, 2021 at 3:14 AM Mickaël Salaün <mic@digikod.net> wrote: > >> > >> From: Mickaël Salaün <mic@linux.microsoft.com> > >> > >> Content of string configuration may depend on related kernel > >> configurations. Modify oldconfig and syncconfig to inform users about > >> possible required configuration update and give them the opportunity to > >> update it: > >> * if dependencies of this string has changed (e.g. enabled or disabled), > >> * and if the current value of this string is different than the (new) > >> default one. > >> > >> This is particularly relevant for CONFIG_LSM which contains a list of > >> LSMs enabled at boot, but users will not have a chance to update this > >> list with a make oldconfig. > > > > If CONFIG_LSM already exists in the .config, > > oldconfig does not show a prompt. > > This is the expected behavior. > > It is not the behavior wished for LSM stacking. Because LSM is doing wrong. > > > > You are trying to fix your problem in a wrong way. > > NACK. > > What do you suggest to ensure that users will be asked to update the > CONFIG_LSM string if they add or remove an LSM? Fix it in the security subsystem. Hint: See 050e9baa9dc9fbd9ce2b27f0056990fc9e0a08a0 -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/3] security: Add LSMs dependencies to CONFIG_LSM 2021-02-15 18:15 [PATCH v2 0/3] Kconfig oldconfig string update Mickaël Salaün 2021-02-15 18:15 ` [PATCH v2 1/3] kconfig: Remove duplicate call to sym_get_string_value() Mickaël Salaün 2021-02-15 18:15 ` [PATCH v2 2/3] kconfig: Ask user if string needs to be changed when dependency changed Mickaël Salaün @ 2021-02-15 18:15 ` Mickaël Salaün 2021-02-15 19:03 ` Ondrej Mosnacek 2 siblings, 1 reply; 14+ messages in thread From: Mickaël Salaün @ 2021-02-15 18:15 UTC (permalink / raw) To: James Morris, Masahiro Yamada, Serge E . Hallyn Cc: Mickaël Salaün, Casey Schaufler, Nicolas Iooss, linux-kbuild, linux-kernel, linux-security-module, Mickaël Salaün 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 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] security: Add LSMs dependencies to CONFIG_LSM 2021-02-15 18:15 ` [PATCH v2 3/3] security: Add LSMs dependencies to CONFIG_LSM Mickaël Salaün @ 2021-02-15 19:03 ` Ondrej Mosnacek 2021-02-21 8:50 ` Masahiro Yamada 0 siblings, 1 reply; 14+ messages in thread From: Ondrej Mosnacek @ 2021-02-15 19:03 UTC (permalink / raw) To: Mickaël Salaün Cc: James Morris, Masahiro Yamada, Serge E . Hallyn, Casey Schaufler, Nicolas Iooss, linux-kbuild, Linux kernel mailing list, Linux Security Module list, Mickaël Salaün 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] security: Add LSMs dependencies to CONFIG_LSM 2021-02-15 19:03 ` Ondrej Mosnacek @ 2021-02-21 8:50 ` Masahiro Yamada 2021-02-21 11:12 ` Mickaël Salaün 0 siblings, 1 reply; 14+ messages in thread From: Masahiro Yamada @ 2021-02-21 8:50 UTC (permalink / raw) To: Ondrej Mosnacek Cc: Mickaël Salaün, James Morris, Serge E . Hallyn, Casey Schaufler, Nicolas Iooss, Linux Kbuild mailing list, Linux kernel mailing list, Linux Security Module list, Mickaël Salaün 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. > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] security: Add LSMs dependencies to CONFIG_LSM 2021-02-21 8:50 ` Masahiro Yamada @ 2021-02-21 11:12 ` Mickaël Salaün 2021-02-21 14:45 ` Masahiro Yamada 0 siblings, 1 reply; 14+ messages in thread From: Mickaël Salaün @ 2021-02-21 11:12 UTC (permalink / raw) To: Masahiro Yamada, Ondrej Mosnacek Cc: James Morris, Serge E . Hallyn, Casey Schaufler, Nicolas Iooss, Linux Kbuild mailing list, Linux kernel mailing list, Linux Security Module list, Mickaël Salaün 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. >> > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] security: Add LSMs dependencies to CONFIG_LSM 2021-02-21 11:12 ` Mickaël Salaün @ 2021-02-21 14:45 ` Masahiro Yamada 2021-02-22 10:47 ` Mickaël Salaün 0 siblings, 1 reply; 14+ messages in thread From: Masahiro Yamada @ 2021-02-21 14:45 UTC (permalink / raw) To: Mickaël Salaün Cc: Ondrej Mosnacek, James Morris, Serge E . Hallyn, Casey Schaufler, Nicolas Iooss, Linux Kbuild mailing list, Linux kernel mailing list, Linux Security Module list, Mickaël Salaün 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] security: Add LSMs dependencies to CONFIG_LSM 2021-02-21 14:45 ` Masahiro Yamada @ 2021-02-22 10:47 ` Mickaël Salaün 2021-02-22 15:08 ` Mickaël Salaün 0 siblings, 1 reply; 14+ messages in thread From: Mickaël Salaün @ 2021-02-22 10:47 UTC (permalink / raw) 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 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] security: Add LSMs dependencies to CONFIG_LSM 2021-02-22 10:47 ` Mickaël Salaün @ 2021-02-22 15:08 ` Mickaël Salaün 0 siblings, 0 replies; 14+ messages in thread From: Mickaël Salaün @ 2021-02-22 15:08 UTC (permalink / raw) 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 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 >> ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-02-22 15:09 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-15 18:15 [PATCH v2 0/3] Kconfig oldconfig string update Mickaël Salaün 2021-02-15 18:15 ` [PATCH v2 1/3] kconfig: Remove duplicate call to sym_get_string_value() Mickaël Salaün 2021-02-21 8:52 ` Masahiro Yamada 2021-02-15 18:15 ` [PATCH v2 2/3] kconfig: Ask user if string needs to be changed when dependency changed Mickaël Salaün 2021-02-21 8:47 ` Masahiro Yamada 2021-02-21 11:10 ` Mickaël Salaün 2021-02-21 14:45 ` Masahiro Yamada 2021-02-15 18:15 ` [PATCH v2 3/3] security: Add LSMs dependencies to CONFIG_LSM Mickaël Salaün 2021-02-15 19:03 ` Ondrej Mosnacek 2021-02-21 8:50 ` Masahiro Yamada 2021-02-21 11:12 ` Mickaël Salaün 2021-02-21 14:45 ` Masahiro Yamada 2021-02-22 10:47 ` Mickaël Salaün 2021-02-22 15:08 ` Mickaël Salaün
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.