[PATCH] LSM: Ignore "security=" when "lsm=" is specified

[PATCH] LSM: Ignore "security=" when "lsm=" is specified

[Kees Cook]

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");
+			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++)
-- 
2.17.1


-- 
Kees Cook

Re: [PATCH] LSM: Ignore "security=" when "lsm=" is specified

[Casey Schaufler]

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++)

Re: [PATCH] LSM: Ignore "security=" when "lsm=" is specified

[Kees Cook]

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?

-- 
Kees Cook

Re: [PATCH] LSM: Ignore "security=" when "lsm=" is specified

[James Morris]

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= ?


-- 
James Morris
<jmorris@namei.org>

Re: [PATCH] LSM: Ignore "security=" when "lsm=" is specified

[Tetsuo Handa]

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);
-- 
1.8.3.1

Re: [PATCH] LSM: Ignore "security=" when "lsm=" is specified

[Kees Cook]

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

Re: [PATCH] LSM: Ignore "security=" when "lsm=" is specified

[Tetsuo Handa]

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.

Re: [PATCH] LSM: Ignore "security=" when "lsm=" is specified

[Kees Cook]

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.

-- 
Kees Cook