| Message ID | 20190211225403.GA7769@beast (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | LSM: Ignore "security=" when "lsm=" is specified | expand |
On 2/11/2019 2:54 PM, Kees Cook wrote: > To avoid potential confusion, explicitly ignore "security=" when "lsm=" is > used on the command line, and report that it is happening. > > Suggested-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > security/security.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/security/security.c b/security/security.c > index 3147785e20d7..e6153ed54361 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -288,9 +288,13 @@ static void __init ordered_lsm_init(void) > ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms), > GFP_KERNEL); > > - if (chosen_lsm_order) > + if (chosen_lsm_order) { > + if (chosen_major_lsm) { > + pr_info("security= is ignored because of lsm=\n"); This is a little awkward. How about "lsm= supersedes security=". > + chosen_major_lsm = NULL; > + } > ordered_lsm_parse(chosen_lsm_order, "cmdline"); > - else > + } else > ordered_lsm_parse(builtin_lsm_order, "builtin"); > > for (lsm = ordered_lsms; *lsm; lsm++)
On Mon, Feb 11, 2019 at 3:10 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > On 2/11/2019 2:54 PM, Kees Cook wrote: > > To avoid potential confusion, explicitly ignore "security=" when "lsm=" is > > used on the command line, and report that it is happening. > > > > Suggested-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> > > Signed-off-by: Kees Cook <keescook@chromium.org> > > --- > > security/security.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/security/security.c b/security/security.c > > index 3147785e20d7..e6153ed54361 100644 > > --- a/security/security.c > > +++ b/security/security.c > > @@ -288,9 +288,13 @@ static void __init ordered_lsm_init(void) > > ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms), > > GFP_KERNEL); > > > > - if (chosen_lsm_order) > > + if (chosen_lsm_order) { > > + if (chosen_major_lsm) { > > + pr_info("security= is ignored because of lsm=\n"); > > This is a little awkward. How about "lsm= supersedes security=". Fine by me. James? What would you like here?
On Mon, 11 Feb 2019, Kees Cook wrote: > On Mon, Feb 11, 2019 at 3:10 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > > > On 2/11/2019 2:54 PM, Kees Cook wrote: > > > To avoid potential confusion, explicitly ignore "security=" when "lsm=" is > > > used on the command line, and report that it is happening. > > > > > > Suggested-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > > --- > > > security/security.c | 8 ++++++-- > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > diff --git a/security/security.c b/security/security.c > > > index 3147785e20d7..e6153ed54361 100644 > > > --- a/security/security.c > > > +++ b/security/security.c > > > @@ -288,9 +288,13 @@ static void __init ordered_lsm_init(void) > > > ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms), > > > GFP_KERNEL); > > > > > > - if (chosen_lsm_order) > > > + if (chosen_lsm_order) { > > > + if (chosen_major_lsm) { > > > + pr_info("security= is ignored because of lsm=\n"); > > > > This is a little awkward. How about "lsm= supersedes security=". > > Fine by me. James? What would you like here? How about security= is ignored because it is superseded by lsm= ?
Kees Cook wrote: > To avoid potential confusion, explicitly ignore "security=" when "lsm=" is > used on the command line, and report that it is happening. To maintain the existing behavior of CONFIG_DEFAULT_SECURITY, I also suggest this change. This saves e.g. Ubuntu users who are using only AppArmor from explicitly specifying security=apparmor when they don't want to enable other LSM_FLAG_LEGACY_MAJOR modules. --- security/Kconfig | 37 +++++++++++++++++++++++++++++++++++++ security/security.c | 5 ++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/security/Kconfig b/security/Kconfig index 9555f49..6a40995 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -250,5 +250,42 @@ config LSM If unsure, leave this as the default. +choice + prompt "Default exclusive security module" + default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX + default DEFAULT_SECURITY_SMACK if SECURITY_SMACK + default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO + default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR + default DEFAULT_SECURITY_DAC + + help + The security module where only one of these modules should be enabled if + neither the "security=" parameter nor the "lsm=" parameter is specified. + + config DEFAULT_SECURITY_SELINUX + bool "SELinux" if SECURITY_SELINUX=y + + config DEFAULT_SECURITY_SMACK + bool "Simplified Mandatory Access Control" if SECURITY_SMACK=y + + config DEFAULT_SECURITY_TOMOYO + bool "TOMOYO" if SECURITY_TOMOYO=y + + config DEFAULT_SECURITY_APPARMOR + bool "AppArmor" if SECURITY_APPARMOR=y + + config DEFAULT_SECURITY_DAC + bool "Unix Discretionary Access Controls" + +endchoice + +config DEFAULT_SECURITY + string + default "selinux" if DEFAULT_SECURITY_SELINUX + default "smack" if DEFAULT_SECURITY_SMACK + default "tomoyo" if DEFAULT_SECURITY_TOMOYO + default "apparmor" if DEFAULT_SECURITY_APPARMOR + default "" if DEFAULT_SECURITY_DAC + endmenu diff --git a/security/security.c b/security/security.c index e6153ed..c44e3cd 100644 --- a/security/security.c +++ b/security/security.c @@ -294,8 +294,11 @@ static void __init ordered_lsm_init(void) chosen_major_lsm = NULL; } ordered_lsm_parse(chosen_lsm_order, "cmdline"); - } else + } else { + if (!chosen_major_lsm) + chosen_major_lsm = CONFIG_DEFAULT_SECURITY; ordered_lsm_parse(builtin_lsm_order, "builtin"); + } for (lsm = ordered_lsms; *lsm; lsm++) prepare_lsm(*lsm);
On Mon, Feb 11, 2019 at 4:21 PM Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > > Kees Cook wrote: > > To avoid potential confusion, explicitly ignore "security=" when "lsm=" is > > used on the command line, and report that it is happening. > > To maintain the existing behavior of CONFIG_DEFAULT_SECURITY, I also suggest this change. > This saves e.g. Ubuntu users who are using only AppArmor from explicitly specifying > security=apparmor when they don't want to enable other LSM_FLAG_LEGACY_MAJOR modules. No, this completely disables the purpose of lsm= I don't understand the use-case you're concerned about? -Kees > > --- > security/Kconfig | 37 +++++++++++++++++++++++++++++++++++++ > security/security.c | 5 ++++- > 2 files changed, 41 insertions(+), 1 deletion(-) > > diff --git a/security/Kconfig b/security/Kconfig > index 9555f49..6a40995 100644 > --- a/security/Kconfig > +++ b/security/Kconfig > @@ -250,5 +250,42 @@ config LSM > > If unsure, leave this as the default. > > +choice > + prompt "Default exclusive security module" > + default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX > + default DEFAULT_SECURITY_SMACK if SECURITY_SMACK > + default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO > + default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR > + default DEFAULT_SECURITY_DAC > + > + help > + The security module where only one of these modules should be enabled if > + neither the "security=" parameter nor the "lsm=" parameter is specified. > + > + config DEFAULT_SECURITY_SELINUX > + bool "SELinux" if SECURITY_SELINUX=y > + > + config DEFAULT_SECURITY_SMACK > + bool "Simplified Mandatory Access Control" if SECURITY_SMACK=y > + > + config DEFAULT_SECURITY_TOMOYO > + bool "TOMOYO" if SECURITY_TOMOYO=y > + > + config DEFAULT_SECURITY_APPARMOR > + bool "AppArmor" if SECURITY_APPARMOR=y > + > + config DEFAULT_SECURITY_DAC > + bool "Unix Discretionary Access Controls" > + > +endchoice > + > +config DEFAULT_SECURITY > + string > + default "selinux" if DEFAULT_SECURITY_SELINUX > + default "smack" if DEFAULT_SECURITY_SMACK > + default "tomoyo" if DEFAULT_SECURITY_TOMOYO > + default "apparmor" if DEFAULT_SECURITY_APPARMOR > + default "" if DEFAULT_SECURITY_DAC > + > endmenu > > diff --git a/security/security.c b/security/security.c > index e6153ed..c44e3cd 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -294,8 +294,11 @@ static void __init ordered_lsm_init(void) > chosen_major_lsm = NULL; > } > ordered_lsm_parse(chosen_lsm_order, "cmdline"); > - } else > + } else { > + if (!chosen_major_lsm) > + chosen_major_lsm = CONFIG_DEFAULT_SECURITY; > ordered_lsm_parse(builtin_lsm_order, "builtin"); > + } > > for (lsm = ordered_lsms; *lsm; lsm++) > prepare_lsm(*lsm); > -- > 1.8.3.1 >
Kees Cook wrote: > On Mon, Feb 11, 2019 at 4:21 PM Tetsuo Handa > <penguin-kernel@i-love.sakura.ne.jp> wrote: > > > > Kees Cook wrote: > > > To avoid potential confusion, explicitly ignore "security=" when "lsm=" is > > > used on the command line, and report that it is happening. > > > > To maintain the existing behavior of CONFIG_DEFAULT_SECURITY, I also suggest this change. > > This saves e.g. Ubuntu users who are using only AppArmor from explicitly specifying > > security=apparmor when they don't want to enable other LSM_FLAG_LEGACY_MAJOR modules. > > No, this completely disables the purpose of lsm= > > I don't understand the use-case you're concerned about? The purpose of lsm= remains. I worry that distro users who don't explicitly specify security= parameter suddenly find TOMOYO messages because TOMOYO is no longer exclusive. There are two ways for avoiding it. One is to explicitly specify security= parameter. The other is to remove tomoyo from CONFIG_LSM. This change adds the third way; preserve current security= behavior until they start explicitly specifying lsm= parameter.
On Mon, Feb 11, 2019 at 4:59 PM Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > Kees Cook wrote: > > On Mon, Feb 11, 2019 at 4:21 PM Tetsuo Handa > > <penguin-kernel@i-love.sakura.ne.jp> wrote: > > > > > > Kees Cook wrote: > > > > To avoid potential confusion, explicitly ignore "security=" when "lsm=" is > > > > used on the command line, and report that it is happening. > > > > > > To maintain the existing behavior of CONFIG_DEFAULT_SECURITY, I also suggest this change. > > > This saves e.g. Ubuntu users who are using only AppArmor from explicitly specifying > > > security=apparmor when they don't want to enable other LSM_FLAG_LEGACY_MAJOR modules. > > > > No, this completely disables the purpose of lsm= > > > > I don't understand the use-case you're concerned about? > > The purpose of lsm= remains. > > I worry that distro users who don't explicitly specify security= parameter > suddenly find TOMOYO messages because TOMOYO is no longer exclusive. What's wrong with that? TOMOYO will start, see there is no policy, and not do anything else. > There are two ways for avoiding it. One is to explicitly specify security= > parameter. The other is to remove tomoyo from CONFIG_LSM. This change adds > the third way; preserve current security= behavior until they start explicitly > specifying lsm= parameter. "security=" has been deprecated due to the many many threads about how it won't work moving forward. Leaving the CONFIG settings confuses the situation and needlessly drags out the transition for no real gain. But yes, if someone selects a "legacy major" LSM, the others (including TOMOYO) will be disabled, which matches the old behavior. Also, yes, removing it from CONFIG_LSM works; this is up to the distro to decide.
diff --git a/security/security.c b/security/security.c index 3147785e20d7..e6153ed54361 100644 --- a/security/security.c +++ b/security/security.c @@ -288,9 +288,13 @@ static void __init ordered_lsm_init(void) ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms), GFP_KERNEL); - if (chosen_lsm_order) + if (chosen_lsm_order) { + if (chosen_major_lsm) { + pr_info("security= is ignored because of lsm=\n"); + chosen_major_lsm = NULL; + } ordered_lsm_parse(chosen_lsm_order, "cmdline"); - else + } else ordered_lsm_parse(builtin_lsm_order, "builtin"); for (lsm = ordered_lsms; *lsm; lsm++)
To avoid potential confusion, explicitly ignore "security=" when "lsm=" is used on the command line, and report that it is happening. Suggested-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> Signed-off-by: Kees Cook <keescook@chromium.org> --- security/security.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)