| Message ID | 20181002005505.6112-24-keescook@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | LSM: Explict LSM ordering | expand |
On Mon, Oct 1, 2018 at 9:04 PM Kees Cook <keescook@chromium.org> wrote: > Since LSM enabling is now centralized with CONFIG_LSM_ENABLE and > "lsm.enable=...", this removes the LSM-specific enabling logic from > SELinux. > > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > .../admin-guide/kernel-parameters.txt | 9 ------ > security/selinux/Kconfig | 29 ------------------- > security/selinux/hooks.c | 15 +--------- > 3 files changed, 1 insertion(+), 52 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index cf963febebb0..0d10ab3d020e 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -4045,15 +4045,6 @@ > loaded. An invalid security module name will be treated > as if no module has been chosen. > > - selinux= [SELINUX] Disable or enable SELinux at boot time. > - Format: { "0" | "1" } > - See security/selinux/Kconfig help text. > - 0 -- disable. > - 1 -- enable. > - Default value is set via kernel config option. > - If enabled at boot time, /selinux/disable can be used > - later to disable prior to initial policy load. No comments yet on the rest of the patchset, but the subject line of this patch caught my eye and I wanted to comment quickly on this one ... Not a fan unfortunately. Much like the SELinux bits under /proc/self/attr, this is a user visible thing which has made its way into a lot of docs, scripts, and minds; I believe removing it would be a big mistake. > serialnumber [BUGS=X86-32] > > shapers= [NET] > diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig > index 8af7a690eb40..86936528a0bb 100644 > --- a/security/selinux/Kconfig > +++ b/security/selinux/Kconfig > @@ -8,35 +8,6 @@ config SECURITY_SELINUX > You will also need a policy configuration and a labeled filesystem. > If you are unsure how to answer this question, answer N. > > -config SECURITY_SELINUX_BOOTPARAM > - bool "NSA SELinux boot parameter" > - depends on SECURITY_SELINUX > - default n > - help > - This option adds a kernel parameter 'selinux', which allows SELinux > - to be disabled at boot. If this option is selected, SELinux > - functionality can be disabled with selinux=0 on the kernel > - command line. The purpose of this option is to allow a single > - kernel image to be distributed with SELinux built in, but not > - necessarily enabled. > - > - If you are unsure how to answer this question, answer N. > - > -config SECURITY_SELINUX_BOOTPARAM_VALUE > - int "NSA SELinux boot parameter default value" > - depends on SECURITY_SELINUX_BOOTPARAM > - range 0 1 > - default 1 > - help > - This option sets the default value for the kernel parameter > - 'selinux', which allows SELinux to be disabled at boot. If this > - option is set to 0 (zero), the SELinux kernel parameter will > - default to 0, disabling SELinux at bootup. If this option is > - set to 1 (one), the SELinux kernel parameter will default to 1, > - enabling SELinux at bootup. > - > - If you are unsure how to answer this question, answer 1. > - > config SECURITY_SELINUX_DISABLE > bool "NSA SELinux runtime disable" > depends on SECURITY_SELINUX > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 71a10fedecb3..8f5eea097612 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -120,20 +120,7 @@ __setup("enforcing=", enforcing_setup); > #define selinux_enforcing_boot 1 > #endif > > -#ifdef CONFIG_SECURITY_SELINUX_BOOTPARAM > -int selinux_enabled = CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE; > - > -static int __init selinux_enabled_setup(char *str) > -{ > - unsigned long enabled; > - if (!kstrtoul(str, 0, &enabled)) > - selinux_enabled = enabled ? 1 : 0; > - return 1; > -} > -__setup("selinux=", selinux_enabled_setup); > -#else > -int selinux_enabled = 1; > -#endif > +int selinux_enabled __lsm_ro_after_init; > > static unsigned int selinux_checkreqprot_boot = > CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE; > -- > 2.17.1 >
On 10/02/2018 08:12 AM, Paul Moore wrote: > On Mon, Oct 1, 2018 at 9:04 PM Kees Cook <keescook@chromium.org> wrote: >> Since LSM enabling is now centralized with CONFIG_LSM_ENABLE and >> "lsm.enable=...", this removes the LSM-specific enabling logic from >> SELinux. >> >> Signed-off-by: Kees Cook <keescook@chromium.org> >> --- >> .../admin-guide/kernel-parameters.txt | 9 ------ >> security/selinux/Kconfig | 29 ------------------- >> security/selinux/hooks.c | 15 +--------- >> 3 files changed, 1 insertion(+), 52 deletions(-) >> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt >> index cf963febebb0..0d10ab3d020e 100644 >> --- a/Documentation/admin-guide/kernel-parameters.txt >> +++ b/Documentation/admin-guide/kernel-parameters.txt >> @@ -4045,15 +4045,6 @@ >> loaded. An invalid security module name will be treated >> as if no module has been chosen. >> >> - selinux= [SELINUX] Disable or enable SELinux at boot time. >> - Format: { "0" | "1" } >> - See security/selinux/Kconfig help text. >> - 0 -- disable. >> - 1 -- enable. >> - Default value is set via kernel config option. >> - If enabled at boot time, /selinux/disable can be used >> - later to disable prior to initial policy load. > > No comments yet on the rest of the patchset, but the subject line of > this patch caught my eye and I wanted to comment quickly on this one > ... > > Not a fan unfortunately. > > Much like the SELinux bits under /proc/self/attr, this is a user > visible thing which has made its way into a lot of docs, scripts, and > minds; I believe removing it would be a big mistake. Yes, we can't suddenly break existing systems that had selinux=0 in their grub config. We have to retain the support. > >> serialnumber [BUGS=X86-32] >> >> shapers= [NET] >> diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig >> index 8af7a690eb40..86936528a0bb 100644 >> --- a/security/selinux/Kconfig >> +++ b/security/selinux/Kconfig >> @@ -8,35 +8,6 @@ config SECURITY_SELINUX >> You will also need a policy configuration and a labeled filesystem. >> If you are unsure how to answer this question, answer N. >> >> -config SECURITY_SELINUX_BOOTPARAM >> - bool "NSA SELinux boot parameter" >> - depends on SECURITY_SELINUX >> - default n >> - help >> - This option adds a kernel parameter 'selinux', which allows SELinux >> - to be disabled at boot. If this option is selected, SELinux >> - functionality can be disabled with selinux=0 on the kernel >> - command line. The purpose of this option is to allow a single >> - kernel image to be distributed with SELinux built in, but not >> - necessarily enabled. >> - >> - If you are unsure how to answer this question, answer N. >> - >> -config SECURITY_SELINUX_BOOTPARAM_VALUE >> - int "NSA SELinux boot parameter default value" >> - depends on SECURITY_SELINUX_BOOTPARAM >> - range 0 1 >> - default 1 >> - help >> - This option sets the default value for the kernel parameter >> - 'selinux', which allows SELinux to be disabled at boot. If this >> - option is set to 0 (zero), the SELinux kernel parameter will >> - default to 0, disabling SELinux at bootup. If this option is >> - set to 1 (one), the SELinux kernel parameter will default to 1, >> - enabling SELinux at bootup. >> - >> - If you are unsure how to answer this question, answer 1. >> - >> config SECURITY_SELINUX_DISABLE >> bool "NSA SELinux runtime disable" >> depends on SECURITY_SELINUX >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index 71a10fedecb3..8f5eea097612 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -120,20 +120,7 @@ __setup("enforcing=", enforcing_setup); >> #define selinux_enforcing_boot 1 >> #endif >> >> -#ifdef CONFIG_SECURITY_SELINUX_BOOTPARAM >> -int selinux_enabled = CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE; >> - >> -static int __init selinux_enabled_setup(char *str) >> -{ >> - unsigned long enabled; >> - if (!kstrtoul(str, 0, &enabled)) >> - selinux_enabled = enabled ? 1 : 0; >> - return 1; >> -} >> -__setup("selinux=", selinux_enabled_setup); >> -#else >> -int selinux_enabled = 1; >> -#endif >> +int selinux_enabled __lsm_ro_after_init; >> >> static unsigned int selinux_checkreqprot_boot = >> CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE; >> -- >> 2.17.1 >> > >
On Tue, Oct 2, 2018 at 6:42 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 10/02/2018 08:12 AM, Paul Moore wrote: >> >> On Mon, Oct 1, 2018 at 9:04 PM Kees Cook <keescook@chromium.org> wrote: >>> >>> Since LSM enabling is now centralized with CONFIG_LSM_ENABLE and >>> "lsm.enable=...", this removes the LSM-specific enabling logic from >>> SELinux. >>> >>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> --- >>> .../admin-guide/kernel-parameters.txt | 9 ------ >>> security/selinux/Kconfig | 29 ------------------- >>> security/selinux/hooks.c | 15 +--------- >>> 3 files changed, 1 insertion(+), 52 deletions(-) >>> >>> diff --git a/Documentation/admin-guide/kernel-parameters.txt >>> b/Documentation/admin-guide/kernel-parameters.txt >>> index cf963febebb0..0d10ab3d020e 100644 >>> --- a/Documentation/admin-guide/kernel-parameters.txt >>> +++ b/Documentation/admin-guide/kernel-parameters.txt >>> @@ -4045,15 +4045,6 @@ >>> loaded. An invalid security module name will be >>> treated >>> as if no module has been chosen. >>> >>> - selinux= [SELINUX] Disable or enable SELinux at boot time. >>> - Format: { "0" | "1" } >>> - See security/selinux/Kconfig help text. >>> - 0 -- disable. >>> - 1 -- enable. >>> - Default value is set via kernel config option. >>> - If enabled at boot time, /selinux/disable can be >>> used >>> - later to disable prior to initial policy load. >> >> >> No comments yet on the rest of the patchset, but the subject line of >> this patch caught my eye and I wanted to comment quickly on this one >> ... >> >> Not a fan unfortunately. >> >> Much like the SELinux bits under /proc/self/attr, this is a user >> visible thing which has made its way into a lot of docs, scripts, and >> minds; I believe removing it would be a big mistake. > > > Yes, we can't suddenly break existing systems that had selinux=0 in their > grub config. We have to retain the support. Is it okay to only support selinux=0 (instead of also selinux=1)? -Kees
On 10/02/2018 10:44 AM, Kees Cook wrote: > On Tue, Oct 2, 2018 at 6:42 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: >> On 10/02/2018 08:12 AM, Paul Moore wrote: >>> >>> On Mon, Oct 1, 2018 at 9:04 PM Kees Cook <keescook@chromium.org> wrote: >>>> >>>> Since LSM enabling is now centralized with CONFIG_LSM_ENABLE and >>>> "lsm.enable=...", this removes the LSM-specific enabling logic from >>>> SELinux. >>>> >>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>>> --- >>>> .../admin-guide/kernel-parameters.txt | 9 ------ >>>> security/selinux/Kconfig | 29 ------------------- >>>> security/selinux/hooks.c | 15 +--------- >>>> 3 files changed, 1 insertion(+), 52 deletions(-) >>>> >>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt >>>> b/Documentation/admin-guide/kernel-parameters.txt >>>> index cf963febebb0..0d10ab3d020e 100644 >>>> --- a/Documentation/admin-guide/kernel-parameters.txt >>>> +++ b/Documentation/admin-guide/kernel-parameters.txt >>>> @@ -4045,15 +4045,6 @@ >>>> loaded. An invalid security module name will be >>>> treated >>>> as if no module has been chosen. >>>> >>>> - selinux= [SELINUX] Disable or enable SELinux at boot time. >>>> - Format: { "0" | "1" } >>>> - See security/selinux/Kconfig help text. >>>> - 0 -- disable. >>>> - 1 -- enable. >>>> - Default value is set via kernel config option. >>>> - If enabled at boot time, /selinux/disable can be >>>> used >>>> - later to disable prior to initial policy load. >>> >>> >>> No comments yet on the rest of the patchset, but the subject line of >>> this patch caught my eye and I wanted to comment quickly on this one >>> ... >>> >>> Not a fan unfortunately. >>> >>> Much like the SELinux bits under /proc/self/attr, this is a user >>> visible thing which has made its way into a lot of docs, scripts, and >>> minds; I believe removing it would be a big mistake. >> >> >> Yes, we can't suddenly break existing systems that had selinux=0 in their >> grub config. We have to retain the support. > > Is it okay to only support selinux=0 (instead of also selinux=1)? For Fedora/RHEL kernels, selinux=1 would be redundant since it is the default. However, in other distros where SELinux is not the default, I think they have documented selinux=1 as the way to enable SELinux. So users may be relying on that as well. I don't think we can safely drop support for either one. Sorry.
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Tuesday, October 2, 2018 4:57 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 10/02/2018 10:44 AM, Kees Cook wrote: > > > On Tue, Oct 2, 2018 at 6:42 AM, Stephen Smalley sds@tycho.nsa.gov wrote: > > > > > On 10/02/2018 08:12 AM, Paul Moore wrote: > > > > > > > On Mon, Oct 1, 2018 at 9:04 PM Kees Cook keescook@chromium.org wrote: > > > > > > > > > Since LSM enabling is now centralized with CONFIG_LSM_ENABLE and > > > > > "lsm.enable=...", this removes the LSM-specific enabling logic from > > > > > SELinux. > > > > > > > > > > Signed-off-by: Kees Cook keescook@chromium.org > > > > > > > > > > ----------------------------------------------- > > > > > > > > > > .../admin-guide/kernel-parameters.txt | 9 ------ > > > > > security/selinux/Kconfig | 29 ------------------- > > > > > security/selinux/hooks.c | 15 +--------- > > > > > 3 files changed, 1 insertion(+), 52 deletions(-) > > > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt > > > > > b/Documentation/admin-guide/kernel-parameters.txt > > > > > index cf963febebb0..0d10ab3d020e 100644 > > > > > --- a/Documentation/admin-guide/kernel-parameters.txt > > > > > +++ b/Documentation/admin-guide/kernel-parameters.txt > > > > > @@ -4045,15 +4045,6 @@ > > > > > loaded. An invalid security module name will be > > > > > treated > > > > > as if no module has been chosen. > > > > > > > > > > - selinux= [SELINUX] Disable or enable SELinux at boot time. > > > > > > > > > > > > > > > - Format: { "0" | "1" } > > > > > > > > > > > > > > > - See security/selinux/Kconfig help text. > > > > > > > > > > > > > > > - 0 -- disable. > > > > > > > > > > > > > > > - 1 -- enable. > > > > > > > > > > > > > > > - Default value is set via kernel config option. > > > > > > > > > > > > > > > - If enabled at boot time, /selinux/disable can be > > > > > > > > > > > > > > > > > > > > used > > > > > > > > > > - later to disable prior to initial policy load. > > > > > > > > > > > > > > > > > > No comments yet on the rest of the patchset, but the subject line of > > > > this patch caught my eye and I wanted to comment quickly on this one > > > > ... > > > > Not a fan unfortunately. > > > > Much like the SELinux bits under /proc/self/attr, this is a user > > > > visible thing which has made its way into a lot of docs, scripts, and > > > > minds; I believe removing it would be a big mistake. > > > > > > Yes, we can't suddenly break existing systems that had selinux=0 in their > > > grub config. We have to retain the support. > > > > Is it okay to only support selinux=0 (instead of also selinux=1)? > > For Fedora/RHEL kernels, selinux=1 would be redundant since it is the > default. However, in other distros where SELinux is not the default, I > think they have documented selinux=1 as the way to enable SELinux. So > users may be relying on that as well. I don't think we can safely drop > support for either one. Sorry. It's always documented as: "selinux=1 security=selinux" so security= should still do the job and selinux=1 become no-op, no? Jordan
On Tue, Oct 2, 2018 at 7:58 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 10/02/2018 10:44 AM, Kees Cook wrote: >> >> On Tue, Oct 2, 2018 at 6:42 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: >>> >>> On 10/02/2018 08:12 AM, Paul Moore wrote: >>>> >>>> >>>> On Mon, Oct 1, 2018 at 9:04 PM Kees Cook <keescook@chromium.org> wrote: >>>>> >>>>> >>>>> Since LSM enabling is now centralized with CONFIG_LSM_ENABLE and >>>>> "lsm.enable=...", this removes the LSM-specific enabling logic from >>>>> SELinux. >>>>> >>>>> Signed-off-by: Kees Cook <keescook@chromium.org> >>>>> --- >>>>> .../admin-guide/kernel-parameters.txt | 9 ------ >>>>> security/selinux/Kconfig | 29 >>>>> ------------------- >>>>> security/selinux/hooks.c | 15 +--------- >>>>> 3 files changed, 1 insertion(+), 52 deletions(-) >>>>> >>>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt >>>>> b/Documentation/admin-guide/kernel-parameters.txt >>>>> index cf963febebb0..0d10ab3d020e 100644 >>>>> --- a/Documentation/admin-guide/kernel-parameters.txt >>>>> +++ b/Documentation/admin-guide/kernel-parameters.txt >>>>> @@ -4045,15 +4045,6 @@ >>>>> loaded. An invalid security module name will >>>>> be >>>>> treated >>>>> as if no module has been chosen. >>>>> >>>>> - selinux= [SELINUX] Disable or enable SELinux at boot >>>>> time. >>>>> - Format: { "0" | "1" } >>>>> - See security/selinux/Kconfig help text. >>>>> - 0 -- disable. >>>>> - 1 -- enable. >>>>> - Default value is set via kernel config option. >>>>> - If enabled at boot time, /selinux/disable can >>>>> be >>>>> used >>>>> - later to disable prior to initial policy load. >>>> >>>> >>>> >>>> No comments yet on the rest of the patchset, but the subject line of >>>> this patch caught my eye and I wanted to comment quickly on this one >>>> ... >>>> >>>> Not a fan unfortunately. >>>> >>>> Much like the SELinux bits under /proc/self/attr, this is a user >>>> visible thing which has made its way into a lot of docs, scripts, and >>>> minds; I believe removing it would be a big mistake. >>> >>> >>> >>> Yes, we can't suddenly break existing systems that had selinux=0 in their >>> grub config. We have to retain the support. >> >> >> Is it okay to only support selinux=0 (instead of also selinux=1)? > > > For Fedora/RHEL kernels, selinux=1 would be redundant since it is the > default. However, in other distros where SELinux is not the default, I > think they have documented selinux=1 as the way to enable SELinux. So users > may be relying on that as well. I don't think we can safely drop support for > either one. Sorry. Okay. How would you like to resolve this? Should SELinux remain "enable special", and AppArmor is okay to remove the LSM-specific enabling? The trouble is with handling CONFIG_LSM_ENABLE vs lsm.enable=... boot param vs the SELinux bootparam. I.e. CONFIG_LSM_ENABLE is redundant to SECURITY_SELINUX_BOOTPARAM_VALUE, and selinux= is redundant to lsm.enable=. Specifically, how should the kernel distinguish between the four settings? -Kees
On Tue, Oct 2, 2018 at 9:33 AM, Jordan Glover <Golden_Miller83@protonmail.ch> wrote: > It's always documented as: "selinux=1 security=selinux" so security= should > still do the job and selinux=1 become no-op, no? The v3 patch set worked this way, yes. (The per-LSM enable defaults were set by the LSM. Only in the case of "lsm.disable=selinux" would the above stop working.) John did not like the separation of having two CONFIG and two bootparams mixing the controls. The v3 resolution rules were: SECURITY_SELINUX_BOOTPARAM_VALUE overrides CONFIG_LSM_ENABLE. SECURITY_APPARMOR_BOOTPARAM_VALUE overrides CONFIG_LSM_ENABLE. selinux= overrides SECURITY_SELINUX_BOOTPARAM_VALUE. apparmor.enabled= overrides SECURITY_APPARMOR_BOOTPARAM_VALUE. apparmor= overrides apparmor.enabled=. lsm.enable= overrides selinux=. lsm.enable= overrides apparmor=. lsm.disable= overrides lsm.enable=. major LSM _omission_ from security= (if present) overrides lsm.enable. v4 removed the per-LSM boot params and CONFIGs at John's request, but Paul and Stephen don't want this for SELinux. The pieces for reducing conflict with CONFIG_LSM_ENABLE and lsm.{enable,disable}= were: 1- Remove SECURITY_APPARMOR_BOOTPARAM_VALUE. 2- Remove apparmor= and apparmor.enabled=. 3- Remove SECURITY_SELINUX_BOOTPARAM_VALUE. 4- Remove selinux=. v4 used all of 1-4 above. SELinux says "4" cannot happen as it's too commonly used. Would 3 be okay for SELinux? John, with 4 not happening, do you prefer to not have 2 happen? With CONFIGs removed, then the boot time defaults are controlled by CONFIG_LSM_ENABLE, but the boot params continue to work as before. Only the use of the new lsm.enable= and lsm.disable= would override the per-LSM boot params. This would clean up the build-time CONFIG weirdness, and leave the existing boot params as before (putting us functionally in between the v3 and v4 series). -Kees
On 10/02/2018 12:54 PM, Kees Cook wrote: > On Tue, Oct 2, 2018 at 9:33 AM, Jordan Glover > <Golden_Miller83@protonmail.ch> wrote: >> It's always documented as: "selinux=1 security=selinux" so security= should >> still do the job and selinux=1 become no-op, no? > > The v3 patch set worked this way, yes. (The per-LSM enable defaults > were set by the LSM. Only in the case of "lsm.disable=selinux" would > the above stop working.) > > John did not like the separation of having two CONFIG and two > bootparams mixing the controls. The v3 resolution rules were: > > SECURITY_SELINUX_BOOTPARAM_VALUE overrides CONFIG_LSM_ENABLE. > SECURITY_APPARMOR_BOOTPARAM_VALUE overrides CONFIG_LSM_ENABLE. > selinux= overrides SECURITY_SELINUX_BOOTPARAM_VALUE. > apparmor.enabled= overrides SECURITY_APPARMOR_BOOTPARAM_VALUE. > apparmor= overrides apparmor.enabled=. > lsm.enable= overrides selinux=. > lsm.enable= overrides apparmor=. > lsm.disable= overrides lsm.enable=. > major LSM _omission_ from security= (if present) overrides lsm.enable. > > v4 removed the per-LSM boot params and CONFIGs at John's request, but > Paul and Stephen don't want this for SELinux. > > The pieces for reducing conflict with CONFIG_LSM_ENABLE and > lsm.{enable,disable}= were: > > 1- Remove SECURITY_APPARMOR_BOOTPARAM_VALUE. > 2- Remove apparmor= and apparmor.enabled=. > 3- Remove SECURITY_SELINUX_BOOTPARAM_VALUE. > 4- Remove selinux=. > > v4 used all of 1-4 above. SELinux says "4" cannot happen as it's too > commonly used. Would 3 be okay for SELinux? Let's say a user/packager/distro has been building kernels for the past 14 years (*) with a config that has SECURITY_SELINUX_BOOTPARAM_VALUE=0, and now they build a new kernel that includes these patches using that same config. Won't SELinux be enabled by default because SECURITY_SELINUX_BOOTPARAM_VALUE is now ignored and LSM_ENABLE defaults to all? Is it ok to require them to specify a new config option to preserve old behavior? (*) how long this config option has been around > > John, with 4 not happening, do you prefer to not have 2 happen? > > With CONFIGs removed, then the boot time defaults are controlled by > CONFIG_LSM_ENABLE, but the boot params continue to work as before. > Only the use of the new lsm.enable= and lsm.disable= would override > the per-LSM boot params. This would clean up the build-time CONFIG > weirdness, and leave the existing boot params as before (putting us > functionally in between the v3 and v4 series). > > -Kees >
On 10/02/2018 09:54 AM, Kees Cook wrote: > On Tue, Oct 2, 2018 at 9:33 AM, Jordan Glover > <Golden_Miller83@protonmail.ch> wrote: >> It's always documented as: "selinux=1 security=selinux" so security= should >> still do the job and selinux=1 become no-op, no? > > The v3 patch set worked this way, yes. (The per-LSM enable defaults > were set by the LSM. Only in the case of "lsm.disable=selinux" would > the above stop working.) > > John did not like the separation of having two CONFIG and two still don't > bootparams mixing the controls. The v3 resolution rules were: > > SECURITY_SELINUX_BOOTPARAM_VALUE overrides CONFIG_LSM_ENABLE. > SECURITY_APPARMOR_BOOTPARAM_VALUE overrides CONFIG_LSM_ENABLE. > selinux= overrides SECURITY_SELINUX_BOOTPARAM_VALUE. > apparmor.enabled= overrides SECURITY_APPARMOR_BOOTPARAM_VALUE. > apparmor= overrides apparmor.enabled=. > lsm.enable= overrides selinux=. > lsm.enable= overrides apparmor=. > lsm.disable= overrides lsm.enable=. > major LSM _omission_ from security= (if present) overrides lsm.enable. > Yeah I would really like to remove the potential confusion for the user. The user now has 4 kernel options to play with, and get confused by LSM= (I'll count apparmor.enabled= here as well) security= lsm.enabled= lsm.disabled= I really don't like this, it will be very confusing for users. I also think an authoritative list of what is enabled is easier for users than mixing a list of whats enabled with kernel config default settings. Under the current scheme lsm.enabled=selinux could actually mean selinux,yama,loadpin,something_else are enabled. If we extend this behavior to when full stacking lands lsm.enabled=selinux,yama might mean selinux,yama,apparmor,loadpin,something_else and what that list is will vary from kernel to kernel, which I think is harder for the user than the lsm.enabled list being what is actually enabled at boot If we have to have multiple kernel parameter, I prefer a behvior where if you hav conflicting kernel parameters specified apparmor=0 lsm.enabled=apparmor that the conflict is logged and the lsm is left disabled, as I think it is easier for users to understand than the overrides scheme of v3, and sans logging of the conflict is effectively what we had in the past apparmor=0 security=apparmor or apparmor=1 security=selinux would result in apparmor being disabed That being said I get we have a mess currently, and there really doesn't seem to be a good way to fix it. I think getting this right for the user is important enough that I am willing to break current apparmor userspace api. While apparmor=0 is documented we have also documented security=X for years and apparmor=0 isn't used too often so I think we can drop it to help clean this mess up abit. I am not going to Nak, or block on v3 behavior if that is considered the best path forward after this discussion/rant. > v4 removed the per-LSM boot params and CONFIGs at John's request, but > Paul and Stephen don't want this for SELinux. > > The pieces for reducing conflict with CONFIG_LSM_ENABLE and > lsm.{enable,disable}= were: > > 1- Remove SECURITY_APPARMOR_BOOTPARAM_VALUE. > 2- Remove apparmor= and apparmor.enabled=. > 3- Remove SECURITY_SELINUX_BOOTPARAM_VALUE. > 4- Remove selinux=. > > v4 used all of 1-4 above. SELinux says "4" cannot happen as it's too > commonly used. Would 3 be okay for SELinux? > > John, with 4 not happening, do you prefer to not have 2 happen? > > With CONFIGs removed, then the boot time defaults are controlled by > CONFIG_LSM_ENABLE, but the boot params continue to work as before. > Only the use of the new lsm.enable= and lsm.disable= would override > the per-LSM boot params. This would clean up the build-time CONFIG > weirdness, and leave the existing boot params as before (putting us > functionally in between the v3 and v4 series). > I'm ambivalent. I really want to clean this up but atm it doesn't really look like 2 is going to provide much of a benefit. If you think it helps clean this up, do it. Regardless 1 is going to happen, and I will start updating documentation and try to get users moving away from using the apparmor= kernel parameter.
On Tue, Oct 2, 2018 at 11:33 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 10/02/2018 12:54 PM, Kees Cook wrote: >> >> On Tue, Oct 2, 2018 at 9:33 AM, Jordan Glover >> <Golden_Miller83@protonmail.ch> wrote: >>> >>> It's always documented as: "selinux=1 security=selinux" so security= >>> should >>> still do the job and selinux=1 become no-op, no? >> >> >> The v3 patch set worked this way, yes. (The per-LSM enable defaults >> were set by the LSM. Only in the case of "lsm.disable=selinux" would >> the above stop working.) >> >> John did not like the separation of having two CONFIG and two >> bootparams mixing the controls. The v3 resolution rules were: >> >> SECURITY_SELINUX_BOOTPARAM_VALUE overrides CONFIG_LSM_ENABLE. >> SECURITY_APPARMOR_BOOTPARAM_VALUE overrides CONFIG_LSM_ENABLE. >> selinux= overrides SECURITY_SELINUX_BOOTPARAM_VALUE. >> apparmor.enabled= overrides SECURITY_APPARMOR_BOOTPARAM_VALUE. >> apparmor= overrides apparmor.enabled=. >> lsm.enable= overrides selinux=. >> lsm.enable= overrides apparmor=. >> lsm.disable= overrides lsm.enable=. >> major LSM _omission_ from security= (if present) overrides lsm.enable. >> >> v4 removed the per-LSM boot params and CONFIGs at John's request, but >> Paul and Stephen don't want this for SELinux. >> >> The pieces for reducing conflict with CONFIG_LSM_ENABLE and >> lsm.{enable,disable}= were: >> >> 1- Remove SECURITY_APPARMOR_BOOTPARAM_VALUE. >> 2- Remove apparmor= and apparmor.enabled=. >> 3- Remove SECURITY_SELINUX_BOOTPARAM_VALUE. >> 4- Remove selinux=. >> >> v4 used all of 1-4 above. SELinux says "4" cannot happen as it's too >> commonly used. Would 3 be okay for SELinux? > > > Let's say a user/packager/distro has been building kernels for the past 14 > years (*) with a config that has SECURITY_SELINUX_BOOTPARAM_VALUE=0, and now > they build a new kernel that includes these patches using that same config. > Won't SELinux be enabled by default because SECURITY_SELINUX_BOOTPARAM_VALUE > is now ignored and LSM_ENABLE defaults to all? Is it ok to require them to > specify a new config option to preserve old behavior? Yes, I think that's fine -- kernel CONFIGs change all the time. System builders are used to examining these changes. -Kees
On Tue, Oct 2, 2018 at 11:57 AM, John Johansen <john.johansen@canonical.com> wrote: > Under the current scheme > > lsm.enabled=selinux > > could actually mean selinux,yama,loadpin,something_else are > enabled. If we extend this behavior to when full stacking lands > > lsm.enabled=selinux,yama > > might mean selinux,yama,apparmor,loadpin,something_else > > and what that list is will vary from kernel to kernel, which I think > is harder for the user than the lsm.enabled list being what is > actually enabled at boot Ah, I think I missed this in your earlier emails. What you don't like here is that "lsm.enable=" is additive. You want it to be explicit. Are you okay with lsm.order= having fallback? The situation we were trying to solve was with new LSMs getting implicitly disabled if someone is booting with an explicit list. For example: lsm.enable=yama,apparmor means when "landlock" gets added to the kernel, it will be implicitly disabled. > If we have to have multiple kernel parameter, I prefer a behvior where > if you hav conflicting kernel parameters specified > > apparmor=0 lsm.enabled=apparmor > > that the conflict is logged and the lsm is left disabled, as I think > it is easier for users to understand than the overrides scheme of v3, > and sans logging of the conflict is effectively what we had in the > past > > apparmor=0 security=apparmor > or > apparmor=1 security=selinux > > would result in apparmor being disabed Okay, so for this part you want per-LSM boot param to have priority (which seems to match SELinux's concerns), possibly logging the conflict, but still accepting the apparmor= and selinux= state. security= would still driving initialization ordering (so I think the behavior I have in the series would be correct). > That being said I get we have a mess currently, and there really > doesn't seem to be a good way to fix it. I think getting this right > for the user is important enough that I am willing to break current > apparmor userspace api. While apparmor=0 is documented we have also > documented security=X for years and apparmor=0 isn't used too often > so I think we can drop it to help clean this mess up abit. > > I am not going to Nak, or block on v3 behavior if that is considered > the best path forward after this discussion/rant. I could define CONFIG_LSM_ENABLE as being "additive" to SECURITY_APPARMOR_BOOTPARAM_VALUE and SECURITY_SELINUX_BOOTPARAM_VALUE? -Kees
On 10/02/2018 12:17 PM, Kees Cook wrote: > On Tue, Oct 2, 2018 at 11:57 AM, John Johansen > <john.johansen@canonical.com> wrote: >> Under the current scheme >> >> lsm.enabled=selinux >> >> could actually mean selinux,yama,loadpin,something_else are >> enabled. If we extend this behavior to when full stacking lands >> >> lsm.enabled=selinux,yama >> >> might mean selinux,yama,apparmor,loadpin,something_else >> >> and what that list is will vary from kernel to kernel, which I think >> is harder for the user than the lsm.enabled list being what is >> actually enabled at boot > > Ah, I think I missed this in your earlier emails. What you don't like > here is that "lsm.enable=" is additive. You want it to be explicit. > > Are you okay with lsm.order= having fallback? > yeah, if we are going to separate order, fallbacks are fine for anything that isn't specified. I am still not convinced that separating order from enablement is right, but its generally something a user should care about so I can live with it. > The situation we were trying to solve was with new LSMs getting > implicitly disabled if someone is booting with an explicit list. For > example: > > lsm.enable=yama,apparmor > > means when "landlock" gets added to the kernel, it will be implicitly disabled. > And here is the point of contention, I wouldn't call that implicitly disabled. The user explicitly selected a set of LSMs to enable. Having other LSMs enable when they aren't specified is confusing to a user, as now they have to consider what is enabled by default in the Kconfig. I think requiring distros/builders to consider Kconfig options is fine, but its a lot higher hurdle for regular users. >> If we have to have multiple kernel parameter, I prefer a behvior where >> if you hav conflicting kernel parameters specified >> >> apparmor=0 lsm.enabled=apparmor >> >> that the conflict is logged and the lsm is left disabled, as I think >> it is easier for users to understand than the overrides scheme of v3, >> and sans logging of the conflict is effectively what we had in the >> past >> >> apparmor=0 security=apparmor >> or >> apparmor=1 security=selinux >> >> would result in apparmor being disabed > > Okay, so for this part you want per-LSM boot param to have priority > (which seems to match SELinux's concerns), possibly logging the hrmmm I wouldn't call it priority :) If you look at the above logic its a boolean AND operation. The LSM is only enabled if $LSM=1 AND security=$LSM all other combinations result in $LSM being disabled > conflict, but still accepting the apparmor= and selinux= state. logging is nice for the user but certainly isn't required and is more than we are doing today > security= would still driving initialization ordering (so I think the > behavior I have in the series would be correct). > >> That being said I get we have a mess currently, and there really >> doesn't seem to be a good way to fix it. I think getting this right >> for the user is important enough that I am willing to break current >> apparmor userspace api. While apparmor=0 is documented we have also >> documented security=X for years and apparmor=0 isn't used too often >> so I think we can drop it to help clean this mess up abit. >> >> I am not going to Nak, or block on v3 behavior if that is considered >> the best path forward after this discussion/rant. > > I could define CONFIG_LSM_ENABLE as being "additive" to > SECURITY_APPARMOR_BOOTPARAM_VALUE and > SECURITY_SELINUX_BOOTPARAM_VALUE? > Oh sure lets deal with my complaint about too many ways to configure this beast by adding yet another config option :P seriously though, please no. That just adds another layer of confusion even if it is only being foisted on the distro/builder
On Tue, Oct 2, 2018 at 12:47 PM, John Johansen <john.johansen@canonical.com> wrote: > On 10/02/2018 12:17 PM, Kees Cook wrote: >> I could define CONFIG_LSM_ENABLE as being "additive" to >> SECURITY_APPARMOR_BOOTPARAM_VALUE and >> SECURITY_SELINUX_BOOTPARAM_VALUE? > > Oh sure lets deal with my complaint about too many ways to configure > this beast by adding yet another config option :P This is what v3 already does: SEC...BOOTPARAM_VALUE trumps ...LSM_ENABLE. > seriously though, please no. That just adds another layer of confusion > even if it is only being foisted on the distro/builder You've already sent a patch removing SECURITY_APPARMOR_BOOTPARAM_VALUE. If SELinux is fine to do that too, then I think we'll be sorted out. I'll just need to make "lsm.enable=" be an explicit list. (Do you have a problem with "lsm.disable=..." ?) -Kees
On 10/02/2018 01:29 PM, Kees Cook wrote: > On Tue, Oct 2, 2018 at 12:47 PM, John Johansen > <john.johansen@canonical.com> wrote: >> On 10/02/2018 12:17 PM, Kees Cook wrote: >>> I could define CONFIG_LSM_ENABLE as being "additive" to >>> SECURITY_APPARMOR_BOOTPARAM_VALUE and >>> SECURITY_SELINUX_BOOTPARAM_VALUE? >> >> Oh sure lets deal with my complaint about too many ways to configure >> this beast by adding yet another config option :P > > This is what v3 already does: SEC...BOOTPARAM_VALUE trumps ...LSM_ENABLE. > sure but I sent in a patch to kill SECURITY_APPARMOR_BOOTPARAM_VALUE because I really dislike the extra levels of config and getting rid of the SEC..BOOTPARAM_VALUE seems to be the easy way to fix it Now if only we can convince Paul and Stephen :) >> seriously though, please no. That just adds another layer of confusion >> even if it is only being foisted on the distro/builder > > You've already sent a patch removing > SECURITY_APPARMOR_BOOTPARAM_VALUE. If SELinux is fine to do that too, > then I think we'll be sorted out. I'll just need to make "lsm.enable=" > be an explicit list. (Do you have a problem with "lsm.disable=..." ?) > why yes, glad you asked If lsm.enabled is an explicit list lsm.disabled isn't required its a convenience option that can introduce confusion and conflicts. If both lsm.enabled and lsm.disabled are being used at the same time. I realize that some times the convenience of specifying lsm.disable=$LSM is easier than specifying an entire list of what should be enabled when you just want to disable a single LSM. I don't think the convenience is worth the potential confusion, but I don't feel strongly about it and realize others feel the other way. tldr: I can live with it, but don't like it if you are asking :)
On Tue, 2 Oct 2018, Kees Cook wrote: > On Tue, Oct 2, 2018 at 11:57 AM, John Johansen > <john.johansen@canonical.com> wrote: > > Under the current scheme > > > > lsm.enabled=selinux > > > > could actually mean selinux,yama,loadpin,something_else are > > enabled. If we extend this behavior to when full stacking lands > > > > lsm.enabled=selinux,yama > > > > might mean selinux,yama,apparmor,loadpin,something_else > > > > and what that list is will vary from kernel to kernel, which I think > > is harder for the user than the lsm.enabled list being what is > > actually enabled at boot > > Ah, I think I missed this in your earlier emails. What you don't like > here is that "lsm.enable=" is additive. You want it to be explicit. > This is a path to madness. How about enable flags set ONLY per LSM: lsm.selinux.enable=x lsm.apparmor.enable=x With no lsm.enable, and removing selinux=x and apparmor=x. Yes this will break existing docs, but they can be updated for newer kernel versions to say "replace selinux=0 with lsm.selinux.enable=0" from kernel X onwards. Surely distro packages and bootloaders are able to cope with changes to kernel parameters? We can either take a one-time hit now, or build new usability debt, which will confuse people forever.
On Tue, Oct 2, 2018 at 3:06 PM, James Morris <jmorris@namei.org> wrote: > On Tue, 2 Oct 2018, Kees Cook wrote: > >> On Tue, Oct 2, 2018 at 11:57 AM, John Johansen >> <john.johansen@canonical.com> wrote: >> > Under the current scheme >> > >> > lsm.enabled=selinux >> > >> > could actually mean selinux,yama,loadpin,something_else are >> > enabled. If we extend this behavior to when full stacking lands >> > >> > lsm.enabled=selinux,yama >> > >> > might mean selinux,yama,apparmor,loadpin,something_else >> > >> > and what that list is will vary from kernel to kernel, which I think >> > is harder for the user than the lsm.enabled list being what is >> > actually enabled at boot >> >> Ah, I think I missed this in your earlier emails. What you don't like >> here is that "lsm.enable=" is additive. You want it to be explicit. >> > > This is a path to madness. > > How about enable flags set ONLY per LSM: > > lsm.selinux.enable=x > lsm.apparmor.enable=x > > With no lsm.enable, and removing selinux=x and apparmor=x. > > Yes this will break existing docs, but they can be updated for newer > kernel versions to say "replace selinux=0 with lsm.selinux.enable=0" from > kernel X onwards. > > Surely distro packages and bootloaders are able to cope with changes to > kernel parameters? > > We can either take a one-time hit now, or build new usability debt, which > will confuse people forever. I'd like to avoid this for a few reasons: - this requires per-LSM plumbing instead of centralized plumbing - each LSM needs to have its own CONFIG flag - each LSM needs to have its own bootparam flag - SELinux has explicited stated they do not want to lose selinux= - this doesn't meet John's goal of having a "single explicit enable list" I think the current proposal (in the other thread) is likely the sanest approach: - Drop CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE - Drop CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE - All enabled LSMs are listed at build-time in CONFIG_LSM_ENABLE - Boot time enabling for selinux= and apparmor= remain - lsm.enable= is explicit: overrides above and omissions are disabled - maybe include lsm.disable= to disable anything -Kees
On 10/02/2018 03:06 PM, James Morris wrote: > On Tue, 2 Oct 2018, Kees Cook wrote: > >> On Tue, Oct 2, 2018 at 11:57 AM, John Johansen >> <john.johansen@canonical.com> wrote: >>> Under the current scheme >>> >>> lsm.enabled=selinux >>> >>> could actually mean selinux,yama,loadpin,something_else are >>> enabled. If we extend this behavior to when full stacking lands >>> >>> lsm.enabled=selinux,yama >>> >>> might mean selinux,yama,apparmor,loadpin,something_else >>> >>> and what that list is will vary from kernel to kernel, which I think >>> is harder for the user than the lsm.enabled list being what is >>> actually enabled at boot >> >> Ah, I think I missed this in your earlier emails. What you don't like >> here is that "lsm.enable=" is additive. You want it to be explicit. >> > > This is a path to madness. > > How about enable flags set ONLY per LSM: > > lsm.selinux.enable=x > lsm.apparmor.enable=x > why add the lsm. prefix? I think if we go this route selinux.enable=x apparmor.enable=x is a little cleaner the question then becomes is this easier for users? Doing something similar to this was discussed earlier but its more tedious to type each of these out, and since they are separate options they can get spread out making it easy to miss one when you are changing your boot options. I honestly don't think we are going to come to a consensus on what is best for users because different sets of users have different priorities. But I do think we need to come up with something cleaner than v3 > With no lsm.enable, and removing selinux=x and apparmor=x. > this will break the user api, not just the distro/builder kernel config. I do think it is probably worth doing, but not everyone agrees. > Yes this will break existing docs, but they can be updated for newer > kernel versions to say "replace selinux=0 with lsm.selinux.enable=0" from > kernel X onwards. > yes docs can be updated but it does take time to propagate out and their are always the dozens of blog, and forum posts etc that google will drag up that won't get updated > Surely distro packages and bootloaders are able to cope with changes to > kernel parameters? > yes, but users who have been taught to add certain incantations to their kernel parameters find it a lot harder > We can either take a one-time hit now, or build new usability debt, which > will confuse people forever. > I'm not opposed to taking a one-time hit for usability in the future.
On 10/02/2018 04:06 PM, Kees Cook wrote: > On Tue, Oct 2, 2018 at 3:06 PM, James Morris <jmorris@namei.org> wrote: >> On Tue, 2 Oct 2018, Kees Cook wrote: >> >>> On Tue, Oct 2, 2018 at 11:57 AM, John Johansen >>> <john.johansen@canonical.com> wrote: >>>> Under the current scheme >>>> >>>> lsm.enabled=selinux >>>> >>>> could actually mean selinux,yama,loadpin,something_else are >>>> enabled. If we extend this behavior to when full stacking lands >>>> >>>> lsm.enabled=selinux,yama >>>> >>>> might mean selinux,yama,apparmor,loadpin,something_else >>>> >>>> and what that list is will vary from kernel to kernel, which I think >>>> is harder for the user than the lsm.enabled list being what is >>>> actually enabled at boot >>> >>> Ah, I think I missed this in your earlier emails. What you don't like >>> here is that "lsm.enable=" is additive. You want it to be explicit. >>> >> >> This is a path to madness. >> >> How about enable flags set ONLY per LSM: >> >> lsm.selinux.enable=x >> lsm.apparmor.enable=x >> >> With no lsm.enable, and removing selinux=x and apparmor=x. >> >> Yes this will break existing docs, but they can be updated for newer >> kernel versions to say "replace selinux=0 with lsm.selinux.enable=0" from >> kernel X onwards. >> >> Surely distro packages and bootloaders are able to cope with changes to >> kernel parameters? >> >> We can either take a one-time hit now, or build new usability debt, which >> will confuse people forever. > > I'd like to avoid this for a few reasons: > > - this requires per-LSM plumbing instead of centralized plumbing > - each LSM needs to have its own CONFIG flag > - each LSM needs to have its own bootparam flag > - SELinux has explicited stated they do not want to lose selinux= > - this doesn't meet John's goal of having a "single explicit enable list" > I can compromise on a single explicit list. My main goal is something that is consistent and easy for the end user, and I would like to avoid potential points of confusion if possible. To me a list like lsm.enable=X,Y,Z is best as a single explicit enable list, and it would be best to avoid lsm.disable as it just introduces confusion. I do think per-LSM bootparams looses the advantages of centralization, and still requires the user to know some Kconfig info but it also gets rid of the lsm.disable confusion. With ordering separated out from being enabled there is a certain cleanness to it. And perhaps most users are looking to enable/disable a single lsm, instead of specifying exactly what security they want on their system. If we were to go this route I would rather drop the lsm. prefix > I think the current proposal (in the other thread) is likely the > sanest approach: > > - Drop CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE > - Drop CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE > - All enabled LSMs are listed at build-time in CONFIG_LSM_ENABLE Hrrmmm isn't this a Kconfig selectable list, with each built-in LSM available to be enabled by default at boot. > - Boot time enabling for selinux= and apparmor= remain > - lsm.enable= is explicit: overrides above and omissions are disabled wfm > - maybe include lsm.disable= to disable anything
On Tue, Oct 2, 2018 at 4:46 PM, John Johansen <john.johansen@canonical.com> wrote: > On 10/02/2018 04:06 PM, Kees Cook wrote: >> I think the current proposal (in the other thread) is likely the >> sanest approach: >> >> - Drop CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE >> - Drop CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE >> - All enabled LSMs are listed at build-time in CONFIG_LSM_ENABLE > > Hrrmmm isn't this a Kconfig selectable list, with each built-in LSM > available to be enabled by default at boot. That's not how I have it currently. It's a comma-separated a string, including the reserved name "all". The default would just be "CONFIG_LSM_ENABLE=all". Casey and I wanted this to have a way to capture new LSMs by default at build-time. >> - Boot time enabling for selinux= and apparmor= remain >> - lsm.enable= is explicit: overrides above and omissions are disabled > wfm Okay, this is closer to v3 than v4. Paul or Stephen, how do you feel about losing the SELinux bootparam CONFIG? (i.e. CONFIG_LSM_ENABLE would be replacing its functionality.) -Kees
On 10/02/2018 04:54 PM, Kees Cook wrote: > On Tue, Oct 2, 2018 at 4:46 PM, John Johansen > <john.johansen@canonical.com> wrote: >> On 10/02/2018 04:06 PM, Kees Cook wrote: >>> I think the current proposal (in the other thread) is likely the >>> sanest approach: >>> >>> - Drop CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE >>> - Drop CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE >>> - All enabled LSMs are listed at build-time in CONFIG_LSM_ENABLE >> >> Hrrmmm isn't this a Kconfig selectable list, with each built-in LSM >> available to be enabled by default at boot. > > That's not how I have it currently. It's a comma-separated a string, > including the reserved name "all". The default would just be > "CONFIG_LSM_ENABLE=all". Casey and I wanted this to have a way to > capture new LSMs by default at build-time. > I understand where you are coming from, but speaking with my distro hat on, that is not going to work. As a distro Ubuntu very much wants to be able to offer all the LSMs built in to the kernel so the user can select them. But very much wants to be able to specify a default supported subset that is enabled at boot. I expect RH and Suse will feel similarily. Speaking for Ubuntu if this isn't available as part of lsm stacking it will get distro patched in. >>> - Boot time enabling for selinux= and apparmor= remain >>> - lsm.enable= is explicit: overrides above and omissions are disabled >> wfm > > Okay, this is closer to v3 than v4. Paul or Stephen, how do you feel > about losing the SELinux bootparam CONFIG? (i.e. CONFIG_LSM_ENABLE > would be replacing its functionality.) > > -Kees >
On Tue, Oct 2, 2018 at 5:05 PM, John Johansen <john.johansen@canonical.com> wrote: > On 10/02/2018 04:54 PM, Kees Cook wrote: >> That's not how I have it currently. It's a comma-separated a string, >> including the reserved name "all". The default would just be >> "CONFIG_LSM_ENABLE=all". Casey and I wanted this to have a way to >> capture new LSMs by default at build-time. >> > > I understand where you are coming from, but speaking with my distro > hat on, that is not going to work. As a distro Ubuntu very much wants > to be able to offer all the LSMs built in to the kernel so the user > can select them. But very much wants to be able to specify a default > supported subset that is enabled at boot. > > I expect RH and Suse will feel similarily. Speaking for Ubuntu if this > isn't available as part of lsm stacking it will get distro patched in. Right. Ubuntu would do something like: CONFIG_LSM_ENABLE=yama,apparmor,integrity And that's why I wanted non-explicit lsm.enable, so that an end user could just do: lsm.enable=loadpin to add loadpin. Perhaps we could have both? "lsm.enable=+loadpin" (add loadpin to build default list) vs "lsm.enable=loadpin" (override build default list with ONLY loadpin). -Kees
On 10/02/2018 05:12 PM, Kees Cook wrote: > On Tue, Oct 2, 2018 at 5:05 PM, John Johansen > <john.johansen@canonical.com> wrote: >> On 10/02/2018 04:54 PM, Kees Cook wrote: >>> That's not how I have it currently. It's a comma-separated a string, >>> including the reserved name "all". The default would just be >>> "CONFIG_LSM_ENABLE=all". Casey and I wanted this to have a way to >>> capture new LSMs by default at build-time. >>> >> >> I understand where you are coming from, but speaking with my distro >> hat on, that is not going to work. As a distro Ubuntu very much wants >> to be able to offer all the LSMs built in to the kernel so the user >> can select them. But very much wants to be able to specify a default >> supported subset that is enabled at boot. >> >> I expect RH and Suse will feel similarily. Speaking for Ubuntu if this >> isn't available as part of lsm stacking it will get distro patched in. > > Right. Ubuntu would do something like: > > CONFIG_LSM_ENABLE=yama,apparmor,integrity > > And that's why I wanted non-explicit lsm.enable, so that an end user > could just do: > > lsm.enable=loadpin > > to add loadpin. > > Perhaps we could have both? "lsm.enable=+loadpin" (add loadpin to > build default list) vs "lsm.enable=loadpin" (override build default > list with ONLY loadpin). > Maybe? I'm not sure what the best option is with all the competing requirements/desires. I need to think about it more and would like to see what others think.
On 10/02/2018 07:54 PM, Kees Cook wrote: > On Tue, Oct 2, 2018 at 4:46 PM, John Johansen > <john.johansen@canonical.com> wrote: >> On 10/02/2018 04:06 PM, Kees Cook wrote: >>> I think the current proposal (in the other thread) is likely the >>> sanest approach: >>> >>> - Drop CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE >>> - Drop CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE >>> - All enabled LSMs are listed at build-time in CONFIG_LSM_ENABLE >> >> Hrrmmm isn't this a Kconfig selectable list, with each built-in LSM >> available to be enabled by default at boot. > > That's not how I have it currently. It's a comma-separated a string, > including the reserved name "all". The default would just be > "CONFIG_LSM_ENABLE=all". Casey and I wanted this to have a way to > capture new LSMs by default at build-time. > >>> - Boot time enabling for selinux= and apparmor= remain >>> - lsm.enable= is explicit: overrides above and omissions are disabled >> wfm > > Okay, this is closer to v3 than v4. Paul or Stephen, how do you feel > about losing the SELinux bootparam CONFIG? (i.e. CONFIG_LSM_ENABLE > would be replacing its functionality.) I'd like to know how distro kernel maintainers feel about it. They would need to understand that if they were previously setting CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE to 0 and want to preserve that behavior, then they must set CONFIG_LSM_ENABLE explicitly to a list of security modules (that does not include selinux, of course). In practice, this means that even the distros that choose to build all security modules into their kernels must explicitly set CONFIG_LSM_ENABLE to a specific list of security modules. So no one would use "all" in practice.
On Wed, Oct 3, 2018 at 6:39 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 10/02/2018 07:54 PM, Kees Cook wrote: >> >> On Tue, Oct 2, 2018 at 4:46 PM, John Johansen >> <john.johansen@canonical.com> wrote: >>> >>> On 10/02/2018 04:06 PM, Kees Cook wrote: >>>> >>>> I think the current proposal (in the other thread) is likely the >>>> sanest approach: >>>> >>>> - Drop CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE >>>> - Drop CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE >>>> - All enabled LSMs are listed at build-time in CONFIG_LSM_ENABLE >>> >>> >>> Hrrmmm isn't this a Kconfig selectable list, with each built-in LSM >>> available to be enabled by default at boot. >> >> >> That's not how I have it currently. It's a comma-separated a string, >> including the reserved name "all". The default would just be >> "CONFIG_LSM_ENABLE=all". Casey and I wanted this to have a way to >> capture new LSMs by default at build-time. >> >>>> - Boot time enabling for selinux= and apparmor= remain >>>> - lsm.enable= is explicit: overrides above and omissions are disabled >>> >>> wfm >> >> >> Okay, this is closer to v3 than v4. Paul or Stephen, how do you feel >> about losing the SELinux bootparam CONFIG? (i.e. CONFIG_LSM_ENABLE >> would be replacing its functionality.) > > > I'd like to know how distro kernel maintainers feel about it. They would > need to understand that if they were previously setting > CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE to 0 and want to preserve that > behavior, then they must set CONFIG_LSM_ENABLE explicitly to a list of > security modules (that does not include selinux, of course). In practice, That's not how it would be done. See below... > this means that even the distros that choose to build all security modules > into their kernels must explicitly set CONFIG_LSM_ENABLE to a specific list > of security modules. So no one would use "all" in practice. This is why I had originally wanted to do CONFIG_LSM_DISABLE. Right now, distro kernel maintainers have two ways to trigger enablement: via the SELinux and AppArmor BOOTPARAM_VALUE _and_ DEFAULT_SECURITY (which is an implicit "enable" for Smack or TOMOYO). All the minors are on-if-built. So, really, the BOOTPARAM_VALUEs were only used for disabling. Distros would build what they wanted, then use DEFAULT_SECURITY for their desired major, and if their DEFAULT_SECURITY wasn't SELinux or AppArmor, they'd _also_ have to set those BOOTPARAM_VALUEs to 0. The goal of the series is to split this more cleanly between "enable" and "order": the way to handle the LSMs is to enable _everything_ and then set the desired init order: the first exclusive "wins". So I *do* think the default would be CONFIG_LSM_ENALBE=all, since it's CONFIG_LSM_ORDER= that effectively replaces CONFIG_DEFAULT_SECURITY. Either a distro builds a very specific subset of LSMs, or they build in all LSMs (for the user to choose from). In both cases, they set an explicit order, which defines which exclusive LSM get selected. AppArmor wants to drop BOOTPARAM_VALUE, which make sense, since it's even now redundant to CONFIG_DEFAULT_SECURITY. I think it makes sense to drop SELinux's BOOTPARAM_VALUE too. The current way to "enable" a major LSM is via CONFIG_DEFAULT_SECURITY. No sane distro kernel is going to set CONFIG_DEFAULT_SECURITY=selinux and CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE=0. If you wanted no major LSM (but still build them all in), you'd set CONFIG_DEFAULT_SECURITY="". -Kees
On Tue, 2 Oct 2018, John Johansen wrote: > To me a list like > lsm.enable=X,Y,Z What about even simpler: lsm=selinux,!apparmor,yama > > is best as a single explicit enable list, and it would be best to avoid > lsm.disable as it just introduces confusion. > > I do think per-LSM bootparams looses the advantages of centralization, > and still requires the user to know some Kconfig info but it also gets > rid of the lsm.disable confusion. > > With ordering separated out from being enabled there is a certain > cleanness to it. And perhaps most users are looking to enable/disable > a single lsm, instead of specifying exactly what security they want > on their system. > > If we were to go this route I would rather drop the lsm. prefix > > > > I think the current proposal (in the other thread) is likely the > > sanest approach: > > > > - Drop CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE > > - Drop CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE > > - All enabled LSMs are listed at build-time in CONFIG_LSM_ENABLE > > Hrrmmm isn't this a Kconfig selectable list, with each built-in LSM > available to be enabled by default at boot. > > > - Boot time enabling for selinux= and apparmor= remain > > - lsm.enable= is explicit: overrides above and omissions are disabled > wfm > > > - maybe include lsm.disable= to disable anything >
On Wed, Oct 3, 2018 at 11:17 AM, James Morris <jmorris@namei.org> wrote: > On Tue, 2 Oct 2018, John Johansen wrote: >> To me a list like >> lsm.enable=X,Y,Z > > What about even simpler: > > lsm=selinux,!apparmor,yama We're going to have lsm.order=, so I'd like to keep it with a dot separator (this makes it more like module parameters, too). You want to mix enable/disable in the same string? That implies you'd want implicit enabling (i.e. it complements the builtin enabling), which is opposite from what John wanted. -Kees
On Wed, 3 Oct 2018, Kees Cook wrote: > On Wed, Oct 3, 2018 at 11:17 AM, James Morris <jmorris@namei.org> wrote: > > On Tue, 2 Oct 2018, John Johansen wrote: > >> To me a list like > >> lsm.enable=X,Y,Z > > > > What about even simpler: > > > > lsm=selinux,!apparmor,yama > > We're going to have lsm.order=, so I'd like to keep it with a dot > separator (this makes it more like module parameters, too). You want > to mix enable/disable in the same string? That implies you'd want > implicit enabling (i.e. it complements the builtin enabling), which is > opposite from what John wanted. > Why can't this be the order as well?
On 10/03/2018 01:26 PM, Kees Cook wrote: > On Wed, Oct 3, 2018 at 6:39 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: >> On 10/02/2018 07:54 PM, Kees Cook wrote: >>> >>> On Tue, Oct 2, 2018 at 4:46 PM, John Johansen >>> <john.johansen@canonical.com> wrote: >>>> >>>> On 10/02/2018 04:06 PM, Kees Cook wrote: >>>>> >>>>> I think the current proposal (in the other thread) is likely the >>>>> sanest approach: >>>>> >>>>> - Drop CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE >>>>> - Drop CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE >>>>> - All enabled LSMs are listed at build-time in CONFIG_LSM_ENABLE >>>> >>>> >>>> Hrrmmm isn't this a Kconfig selectable list, with each built-in LSM >>>> available to be enabled by default at boot. >>> >>> >>> That's not how I have it currently. It's a comma-separated a string, >>> including the reserved name "all". The default would just be >>> "CONFIG_LSM_ENABLE=all". Casey and I wanted this to have a way to >>> capture new LSMs by default at build-time. >>> >>>>> - Boot time enabling for selinux= and apparmor= remain >>>>> - lsm.enable= is explicit: overrides above and omissions are disabled >>>> >>>> wfm >>> >>> >>> Okay, this is closer to v3 than v4. Paul or Stephen, how do you feel >>> about losing the SELinux bootparam CONFIG? (i.e. CONFIG_LSM_ENABLE >>> would be replacing its functionality.) >> >> >> I'd like to know how distro kernel maintainers feel about it. They would >> need to understand that if they were previously setting >> CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE to 0 and want to preserve that >> behavior, then they must set CONFIG_LSM_ENABLE explicitly to a list of >> security modules (that does not include selinux, of course). In practice, > > That's not how it would be done. See below... > >> this means that even the distros that choose to build all security modules >> into their kernels must explicitly set CONFIG_LSM_ENABLE to a specific list >> of security modules. So no one would use "all" in practice. > > This is why I had originally wanted to do CONFIG_LSM_DISABLE. Right > now, distro kernel maintainers have two ways to trigger enablement: > via the SELinux and AppArmor BOOTPARAM_VALUE _and_ DEFAULT_SECURITY > (which is an implicit "enable" for Smack or TOMOYO). All the minors > are on-if-built. So, really, the BOOTPARAM_VALUEs were only used for > disabling. Distros would build what they wanted, then use > DEFAULT_SECURITY for their desired major, and if their > DEFAULT_SECURITY wasn't SELinux or AppArmor, they'd _also_ have to set > those BOOTPARAM_VALUEs to 0. > > The goal of the series is to split this more cleanly between "enable" > and "order": the way to handle the LSMs is to enable _everything_ and > then set the desired init order: the first exclusive "wins". So I *do* > think the default would be CONFIG_LSM_ENALBE=all, since it's > CONFIG_LSM_ORDER= that effectively replaces CONFIG_DEFAULT_SECURITY. > > Either a distro builds a very specific subset of LSMs, or they build > in all LSMs (for the user to choose from). In both cases, they set an > explicit order, which defines which exclusive LSM get selected. > > AppArmor wants to drop BOOTPARAM_VALUE, which make sense, since it's > even now redundant to CONFIG_DEFAULT_SECURITY. I think it makes sense > to drop SELinux's BOOTPARAM_VALUE too. The current way to "enable" a > major LSM is via CONFIG_DEFAULT_SECURITY. No sane distro kernel is > going to set CONFIG_DEFAULT_SECURITY=selinux and > CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE=0. If you wanted no major LSM > (but still build them all in), you'd set CONFIG_DEFAULT_SECURITY="". Ok, then I have no objection to removing BOOTPARAM_VALUE.
On Wed, Oct 3, 2018 at 11:28 AM, James Morris <jmorris@namei.org> wrote: > On Wed, 3 Oct 2018, Kees Cook wrote: > >> On Wed, Oct 3, 2018 at 11:17 AM, James Morris <jmorris@namei.org> wrote: >> > On Tue, 2 Oct 2018, John Johansen wrote: >> >> To me a list like >> >> lsm.enable=X,Y,Z >> > >> > What about even simpler: >> > >> > lsm=selinux,!apparmor,yama >> >> We're going to have lsm.order=, so I'd like to keep it with a dot >> separator (this makes it more like module parameters, too). You want >> to mix enable/disable in the same string? That implies you'd want >> implicit enabling (i.e. it complements the builtin enabling), which is >> opposite from what John wanted. >> > > Why can't this be the order as well? That was covered extensively in the earlier threads. It boils down to making sure we do not create a pattern of leaving LSMs disabled by default when they are added to the kernel. The v1 series used security= like this: + security= [SECURITY] An ordered comma-separated list of + security modules to attempt to enable at boot. If + this boot parameter is not specified, only the + security modules asking for initialization will be + enabled (see CONFIG_DEFAULT_SECURITY). Duplicate + or invalid security modules will be ignored. The + capability module is always loaded first, without + regard to this parameter. This meant booting "security=apparmor" would disable all the other LSMs, which wasn't friendly at all. So "security=" was left alone (to leave it to only select the "major" LSM: all major LSMs not matching "security=" would be disabled). So I proposed "lsm.order=" to specify the order things were going to be initialized in, but to avoid kernels booting with newly added LSMs forced-off due to not being listed in "lsm.order=", it had to have implicit fall-back for unlisted LSMs. (i.e. anything missing from lsm.order would then follow their order in CONFIG_LSM_ORDER, and anything missing there would fall back to link-time ordering.) However, then the objection was raised that this didn't provide a way to explicitly disable an LSM. So I proposed lsm.enable/disable, and John argued for CONFIG_LSM_ENABLE over CONFIG_LSM_DISABLE. -Kees
On Wed, Oct 3, 2018 at 1:10 PM, Kees Cook <keescook@chromium.org> wrote: > On Wed, Oct 3, 2018 at 11:28 AM, James Morris <jmorris@namei.org> wrote: >> On Wed, 3 Oct 2018, Kees Cook wrote: >> >>> On Wed, Oct 3, 2018 at 11:17 AM, James Morris <jmorris@namei.org> wrote: >>> > On Tue, 2 Oct 2018, John Johansen wrote: >>> >> To me a list like >>> >> lsm.enable=X,Y,Z >>> > >>> > What about even simpler: >>> > >>> > lsm=selinux,!apparmor,yama >>> >>> We're going to have lsm.order=, so I'd like to keep it with a dot >>> separator (this makes it more like module parameters, too). You want >>> to mix enable/disable in the same string? That implies you'd want >>> implicit enabling (i.e. it complements the builtin enabling), which is >>> opposite from what John wanted. >>> >> >> Why can't this be the order as well? > > That was covered extensively in the earlier threads. It boils down to > making sure we do not create a pattern of leaving LSMs disabled by > default when they are added to the kernel. The v1 series used > security= like this: > > + security= [SECURITY] An ordered comma-separated list of > + security modules to attempt to enable at boot. If > + this boot parameter is not specified, only the > + security modules asking for initialization will be > + enabled (see CONFIG_DEFAULT_SECURITY). Duplicate > + or invalid security modules will be ignored. The > + capability module is always loaded first, without > + regard to this parameter. > > This meant booting "security=apparmor" would disable all the other > LSMs, which wasn't friendly at all. So "security=" was left alone (to > leave it to only select the "major" LSM: all major LSMs not matching > "security=" would be disabled). So I proposed "lsm.order=" to specify > the order things were going to be initialized in, but to avoid kernels > booting with newly added LSMs forced-off due to not being listed in > "lsm.order=", it had to have implicit fall-back for unlisted LSMs. > (i.e. anything missing from lsm.order would then follow their order in > CONFIG_LSM_ORDER, and anything missing there would fall back to > link-time ordering.) However, then the objection was raised that this > didn't provide a way to explicitly disable an LSM. So I proposed > lsm.enable/disable, and John argued for CONFIG_LSM_ENABLE over > CONFIG_LSM_DISABLE. I still think we should have all built LSMs enabled by default, with CONFIG_LSM_DISABLE available to turn stuff off. CONFIG_LSM_ORDER declares their order, "lsm.order=" works as mentioned, and "lsm.enable/disable=" make changes to what's enabled. (This would be most like the v3 series, swapping CONFIG_LSM_ENABLE for CONFIG_LSM_DISABLE.) It gives us centralized ordering and centralized disabling. Distros wanting specific LSMs are already building them, so _also_ adding them to CONFIG_LSM_ENABLE seems redundant to me. Distros wanting all the LSMs just want to declare the order of initialization, and maybe add some to CONFIG_LSM_DISABLE some day, so they use CONFIG_LSM_ORDER. I should also note that I don't want to leave CONFIG_DEFAULT_SECURITY in, since it's just a way to disable all the other majors. I don't like this because it will force LSMs to be disabled that don't need to be once blob-sharing lands. The whole point of this series is to get us away from fixed ordering and thinking about "major" vs "minor" and towards "exclusive" or not, where we can continue to slowly chip away at exclusivity without breaking anything. -Kees
On Wed, 3 Oct 2018, Kees Cook wrote: > I should also note that I don't want to leave CONFIG_DEFAULT_SECURITY > in, since it's just a way to disable all the other majors. Right, default doesn't make sense anymore.
On Wed, 3 Oct 2018, Kees Cook wrote: > On Wed, Oct 3, 2018 at 11:28 AM, James Morris <jmorris@namei.org> wrote: > > On Wed, 3 Oct 2018, Kees Cook wrote: > > > >> On Wed, Oct 3, 2018 at 11:17 AM, James Morris <jmorris@namei.org> wrote: > >> > On Tue, 2 Oct 2018, John Johansen wrote: > >> >> To me a list like > >> >> lsm.enable=X,Y,Z > >> > > >> > What about even simpler: > >> > > >> > lsm=selinux,!apparmor,yama > >> > >> We're going to have lsm.order=, so I'd like to keep it with a dot > >> separator (this makes it more like module parameters, too). You want > >> to mix enable/disable in the same string? That implies you'd want > >> implicit enabling (i.e. it complements the builtin enabling), which is > >> opposite from what John wanted. > >> > > > > Why can't this be the order as well? > > That was covered extensively in the earlier threads. It boils down to > making sure we do not create a pattern of leaving LSMs disabled by > default when they are added to the kernel. The v1 series used > security= like this: > > + security= [SECURITY] An ordered comma-separated list of > + security modules to attempt to enable at boot. If > + this boot parameter is not specified, only the > + security modules asking for initialization will be > + enabled (see CONFIG_DEFAULT_SECURITY). Duplicate > + or invalid security modules will be ignored. The > + capability module is always loaded first, without > + regard to this parameter. > > This meant booting "security=apparmor" would disable all the other > LSMs, which wasn't friendly at all. So "security=" was left alone (to > leave it to only select the "major" LSM: all major LSMs not matching > "security=" would be disabled). So I proposed "lsm.order=" to specify > the order things were going to be initialized in, but to avoid kernels > booting with newly added LSMs forced-off due to not being listed in > "lsm.order=", it had to have implicit fall-back for unlisted LSMs. > (i.e. anything missing from lsm.order would then follow their order in > CONFIG_LSM_ORDER, and anything missing there would fall back to > link-time ordering.) However, then the objection was raised that this > didn't provide a way to explicitly disable an LSM. So I proposed > lsm.enable/disable, and John argued for CONFIG_LSM_ENABLE over > CONFIG_LSM_DISABLE. Ok, but it may end up being clearer, simpler, and thus more secure to just have a single way to configure LSM. For example: - All LSMs which are built are NOT enabled by default - You specify enablement and order via a Kconfig: CONFIG_LSM="selinux,yama" - This can be entirely overridden by a boot param: lsm="apparmor,landlock" And that's it. Of course, capabilities is always enabled and not be visible to kconfig or boot params.
On Wed, Oct 3, 2018 at 2:34 PM, James Morris <jmorris@namei.org> wrote: > On Wed, 3 Oct 2018, Kees Cook wrote: > >> On Wed, Oct 3, 2018 at 11:28 AM, James Morris <jmorris@namei.org> wrote: >> > On Wed, 3 Oct 2018, Kees Cook wrote: >> > >> >> On Wed, Oct 3, 2018 at 11:17 AM, James Morris <jmorris@namei.org> wrote: >> >> > On Tue, 2 Oct 2018, John Johansen wrote: >> >> >> To me a list like >> >> >> lsm.enable=X,Y,Z >> >> > >> >> > What about even simpler: >> >> > >> >> > lsm=selinux,!apparmor,yama >> >> >> >> We're going to have lsm.order=, so I'd like to keep it with a dot >> >> separator (this makes it more like module parameters, too). You want >> >> to mix enable/disable in the same string? That implies you'd want >> >> implicit enabling (i.e. it complements the builtin enabling), which is >> >> opposite from what John wanted. >> >> >> > >> > Why can't this be the order as well? >> >> That was covered extensively in the earlier threads. It boils down to >> making sure we do not create a pattern of leaving LSMs disabled by >> default when they are added to the kernel. The v1 series used >> security= like this: >> >> + security= [SECURITY] An ordered comma-separated list of >> + security modules to attempt to enable at boot. If >> + this boot parameter is not specified, only the >> + security modules asking for initialization will be >> + enabled (see CONFIG_DEFAULT_SECURITY). Duplicate >> + or invalid security modules will be ignored. The >> + capability module is always loaded first, without >> + regard to this parameter. >> >> This meant booting "security=apparmor" would disable all the other >> LSMs, which wasn't friendly at all. So "security=" was left alone (to >> leave it to only select the "major" LSM: all major LSMs not matching >> "security=" would be disabled). So I proposed "lsm.order=" to specify >> the order things were going to be initialized in, but to avoid kernels >> booting with newly added LSMs forced-off due to not being listed in >> "lsm.order=", it had to have implicit fall-back for unlisted LSMs. >> (i.e. anything missing from lsm.order would then follow their order in >> CONFIG_LSM_ORDER, and anything missing there would fall back to >> link-time ordering.) However, then the objection was raised that this >> didn't provide a way to explicitly disable an LSM. So I proposed >> lsm.enable/disable, and John argued for CONFIG_LSM_ENABLE over >> CONFIG_LSM_DISABLE. > > Ok, but it may end up being clearer, simpler, and thus more secure to just > have a single way to configure LSM. > > For example: > > - All LSMs which are built are NOT enabled by default > > - You specify enablement and order via a Kconfig: > > CONFIG_LSM="selinux,yama" > > - This can be entirely overridden by a boot param: > > lsm="apparmor,landlock" This doesn't work with how SELinux and AppArmor do their bootparams, unfortunately. (And Paul and Stephen have expressed that the documented selinux on/off must continue to work.) For example, let's say you've built an Ubuntu kernel with: CONFIG_SELINUX=y ... CONFIG_LSM="yama,apparmor" (i.e. you want SELinux available, but not enabled, so it's left out of CONFIG_LSM) Then someone boots the system with: selinux=1 security=selinux In what order does selinux get initialized relative to yama? (apparmor, flagged as a "legacy major", would have been disabled by the "security=" not matching it.) The LSM order needs to be defined externally to enablement because something may become enabled when not listed in the order. Now, maybe I misunderstood your earlier suggestion, and what you meant was to do something like: CONFIG_LSM="yama,apparmor,!selinux" to mean "put selinux here in the order, but don't enable it". Then the problem becomes what happens to an LSM that has been built in but not listed in CONFIG_LSM? Related to that, this means that when new LSMs are added, they will need to be added to any custom CONFIG_LSM= or lsm= parameters. If that's really how we have to go, I'll accept it, but I think it's a bit unfriendly. :P Another reason I don't like it is because it requires users to know about all the LSMs to make changes. One LSM can't be added/removed without specifying ALL of the LSMs. (i.e. there is no trivial way to enable/disable a single LSM without it growing its own enable/disable code as in SELinux/AppArmor. I'd hoped to make that easier for both users and developers.) Again, I can live with it, but I think it's unfriendly. I just want to have a direct I can go that meets all the requirements. :) I'm fine to ignore my sense of aesthetics if everyone can agree on the code. > And that's it. > > Of course, capabilities is always enabled and not be visible to kconfig or > boot params. Correct. I've made sure that's true in all the versions. BTW, there doesn't seem to be disagreement about the earlier part of the series, though (patches 1-10). Could these go into -next just so I don't have to keep sending them? :) LSM: Correctly announce start of LSM initialization vmlinux.lds.h: Avoid copy/paste of security_init section LSM: Rename .security_initcall section to .lsm_info LSM: Remove initcall tracing LSM: Convert from initcall to struct lsm_info vmlinux.lds.h: Move LSM_TABLE into INIT_DATA LSM: Convert security_initcall() into DEFINE_LSM() LSM: Record LSM name in struct lsm_info LSM: Provide init debugging infrastructure LSM: Don't ignore initialization failures Thanks! -Kees
On 10/3/18 4:55 PM, Kees Cook wrote: > On Wed, Oct 3, 2018 at 2:34 PM, James Morris <jmorris@namei.org> wrote: >> On Wed, 3 Oct 2018, Kees Cook wrote: >> >>> On Wed, Oct 3, 2018 at 11:28 AM, James Morris <jmorris@namei.org> wrote: >>>> On Wed, 3 Oct 2018, Kees Cook wrote: >>>> >>>>> On Wed, Oct 3, 2018 at 11:17 AM, James Morris <jmorris@namei.org> wrote: >>>>>> On Tue, 2 Oct 2018, John Johansen wrote: >>>>>>> To me a list like >>>>>>> lsm.enable=X,Y,Z >>>>>> >>>>>> What about even simpler: >>>>>> >>>>>> lsm=selinux,!apparmor,yama >>>>> >>>>> We're going to have lsm.order=, so I'd like to keep it with a dot >>>>> separator (this makes it more like module parameters, too). You want >>>>> to mix enable/disable in the same string? That implies you'd want >>>>> implicit enabling (i.e. it complements the builtin enabling), which is >>>>> opposite from what John wanted. >>>>> >>>> >>>> Why can't this be the order as well? >>> >>> That was covered extensively in the earlier threads. It boils down to >>> making sure we do not create a pattern of leaving LSMs disabled by >>> default when they are added to the kernel. The v1 series used >>> security= like this: >>> >>> + security= [SECURITY] An ordered comma-separated list of >>> + security modules to attempt to enable at boot. If >>> + this boot parameter is not specified, only the >>> + security modules asking for initialization will be >>> + enabled (see CONFIG_DEFAULT_SECURITY). Duplicate >>> + or invalid security modules will be ignored. The >>> + capability module is always loaded first, without >>> + regard to this parameter. >>> >>> This meant booting "security=apparmor" would disable all the other >>> LSMs, which wasn't friendly at all. So "security=" was left alone (to >>> leave it to only select the "major" LSM: all major LSMs not matching >>> "security=" would be disabled). So I proposed "lsm.order=" to specify >>> the order things were going to be initialized in, but to avoid kernels >>> booting with newly added LSMs forced-off due to not being listed in >>> "lsm.order=", it had to have implicit fall-back for unlisted LSMs. >>> (i.e. anything missing from lsm.order would then follow their order in >>> CONFIG_LSM_ORDER, and anything missing there would fall back to >>> link-time ordering.) However, then the objection was raised that this >>> didn't provide a way to explicitly disable an LSM. So I proposed >>> lsm.enable/disable, and John argued for CONFIG_LSM_ENABLE over >>> CONFIG_LSM_DISABLE. >> >> Ok, but it may end up being clearer, simpler, and thus more secure to just >> have a single way to configure LSM. >> >> For example: >> >> - All LSMs which are built are NOT enabled by default >> >> - You specify enablement and order via a Kconfig: >> >> CONFIG_LSM="selinux,yama" >> >> - This can be entirely overridden by a boot param: >> >> lsm="apparmor,landlock" > > This doesn't work with how SELinux and AppArmor do their bootparams, > unfortunately. (And Paul and Stephen have expressed that the > documented selinux on/off must continue to work.) For example, let's > say you've built an Ubuntu kernel with: > > CONFIG_SELINUX=y > ... > CONFIG_LSM="yama,apparmor" > > (i.e. you want SELinux available, but not enabled, so it's left out of > CONFIG_LSM) > > Then someone boots the system with: > > selinux=1 security=selinux > > In what order does selinux get initialized relative to yama? > (apparmor, flagged as a "legacy major", would have been disabled by > the "security=" not matching it.) > To me, "security=selinux" means SELinux and nothing else, so I think that all of these params are inviting a lot of confusion. Sorry, I don't have a good answer for this. > > The LSM order needs to be defined externally to enablement because > something may become enabled when not listed in the order. > > Now, maybe I misunderstood your earlier suggestion, and what you meant > was to do something like: > > CONFIG_LSM="yama,apparmor,!selinux" > > to mean "put selinux here in the order, but don't enable it". Then the > problem becomes what happens to an LSM that has been built in but not > listed in CONFIG_LSM? > > Related to that, this means that when new LSMs are added, they will > need to be added to any custom CONFIG_LSM= or lsm= parameters. If > that's really how we have to go, I'll accept it, but I think it's a > bit unfriendly. :P > > Another reason I don't like it is because it requires users to know > about all the LSMs to make changes. One LSM can't be added/removed > without specifying ALL of the LSMs. (i.e. there is no trivial way to > enable/disable a single LSM without it growing its own enable/disable > code as in SELinux/AppArmor. I'd hoped to make that easier for both > users and developers.) Again, I can live with it, but I think it's > unfriendly. > > I just want to have a direct I can go that meets all the requirements. > :) I'm fine to ignore my sense of aesthetics if everyone can agree on > the code.
On Wed, Oct 3, 2018 at 4:59 PM, Randy Dunlap <rdunlap@infradead.org> wrote: > To me, "security=selinux" means SELinux and nothing else, so I think that > all of these params are inviting a lot of confusion. > > Sorry, I don't have a good answer for this. This part, at least, has a pretty clear solution. :) The consensus is to limit "security=" to what have been considered the "major" LSMs" so it'll work in spirit the way it was designed. The goal of the new options, though, is to find something that'll fit all the ways LSMs are getting used: the majors, the minors, and the coming "medium" LSMs. The precedent is pretty good here, since "security=" already ignores the minor LSMs: Yama and LoadPin. So it'll just control the enable/disable of the "major" LSMs, who will carry an internal marking indicating that they're mediated by "security=" (and no new LSMs would get this marking). -Kees
On 10/03/2018 10:26 AM, Kees Cook wrote: > On Wed, Oct 3, 2018 at 6:39 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: >> On 10/02/2018 07:54 PM, Kees Cook wrote: >>> >>> On Tue, Oct 2, 2018 at 4:46 PM, John Johansen >>> <john.johansen@canonical.com> wrote: >>>> >>>> On 10/02/2018 04:06 PM, Kees Cook wrote: >>>>> >>>>> I think the current proposal (in the other thread) is likely the >>>>> sanest approach: >>>>> >>>>> - Drop CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE >>>>> - Drop CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE >>>>> - All enabled LSMs are listed at build-time in CONFIG_LSM_ENABLE >>>> >>>> >>>> Hrrmmm isn't this a Kconfig selectable list, with each built-in LSM >>>> available to be enabled by default at boot. >>> >>> >>> That's not how I have it currently. It's a comma-separated a string, >>> including the reserved name "all". The default would just be >>> "CONFIG_LSM_ENABLE=all". Casey and I wanted this to have a way to >>> capture new LSMs by default at build-time. >>> >>>>> - Boot time enabling for selinux= and apparmor= remain >>>>> - lsm.enable= is explicit: overrides above and omissions are disabled >>>> >>>> wfm >>> >>> >>> Okay, this is closer to v3 than v4. Paul or Stephen, how do you feel >>> about losing the SELinux bootparam CONFIG? (i.e. CONFIG_LSM_ENABLE >>> would be replacing its functionality.) >> >> >> I'd like to know how distro kernel maintainers feel about it. They would >> need to understand that if they were previously setting >> CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE to 0 and want to preserve that >> behavior, then they must set CONFIG_LSM_ENABLE explicitly to a list of >> security modules (that does not include selinux, of course). In practice, > > That's not how it would be done. See below... > >> this means that even the distros that choose to build all security modules >> into their kernels must explicitly set CONFIG_LSM_ENABLE to a specific list >> of security modules. So no one would use "all" in practice. > > This is why I had originally wanted to do CONFIG_LSM_DISABLE. Right > now, distro kernel maintainers have two ways to trigger enablement: > via the SELinux and AppArmor BOOTPARAM_VALUE _and_ DEFAULT_SECURITY > (which is an implicit "enable" for Smack or TOMOYO). All the minors > are on-if-built. So, really, the BOOTPARAM_VALUEs were only used for > disabling. Distros would build what they wanted, then use > DEFAULT_SECURITY for their desired major, and if their > DEFAULT_SECURITY wasn't SELinux or AppArmor, they'd _also_ have to set > those BOOTPARAM_VALUEs to 0. > > The goal of the series is to split this more cleanly between "enable" > and "order": the way to handle the LSMs is to enable _everything_ and > then set the desired init order: the first exclusive "wins". So I *do* > think the default would be CONFIG_LSM_ENALBE=all, since it's > CONFIG_LSM_ORDER= that effectively replaces CONFIG_DEFAULT_SECURITY. > but distinct of first exclusive (major) will likely be going away once full lsm stacking land. > Either a distro builds a very specific subset of LSMs, or they build > in all LSMs (for the user to choose from). In both cases, they set an > explicit order, which defines which exclusive LSM get selected. > and when lsm stacking lands, that exlusive LSM goes away. > AppArmor wants to drop BOOTPARAM_VALUE, which make sense, since it's > even now redundant to CONFIG_DEFAULT_SECURITY. I think it makes sense > to drop SELinux's BOOTPARAM_VALUE too. The current way to "enable" a > major LSM is via CONFIG_DEFAULT_SECURITY. No sane distro kernel is > going to set CONFIG_DEFAULT_SECURITY=selinux and > CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE=0. If you wanted no major LSM > (but still build them all in), you'd set CONFIG_DEFAULT_SECURITY="". >
On 10/03/2018 01:36 PM, Kees Cook wrote: > On Wed, Oct 3, 2018 at 1:10 PM, Kees Cook <keescook@chromium.org> wrote: >> On Wed, Oct 3, 2018 at 11:28 AM, James Morris <jmorris@namei.org> wrote: >>> On Wed, 3 Oct 2018, Kees Cook wrote: >>> >>>> On Wed, Oct 3, 2018 at 11:17 AM, James Morris <jmorris@namei.org> wrote: >>>>> On Tue, 2 Oct 2018, John Johansen wrote: >>>>>> To me a list like >>>>>> lsm.enable=X,Y,Z >>>>> >>>>> What about even simpler: >>>>> >>>>> lsm=selinux,!apparmor,yama >>>> >>>> We're going to have lsm.order=, so I'd like to keep it with a dot >>>> separator (this makes it more like module parameters, too). You want >>>> to mix enable/disable in the same string? That implies you'd want >>>> implicit enabling (i.e. it complements the builtin enabling), which is >>>> opposite from what John wanted. >>>> >>> >>> Why can't this be the order as well? >> >> That was covered extensively in the earlier threads. It boils down to >> making sure we do not create a pattern of leaving LSMs disabled by >> default when they are added to the kernel. The v1 series used >> security= like this: >> >> + security= [SECURITY] An ordered comma-separated list of >> + security modules to attempt to enable at boot. If >> + this boot parameter is not specified, only the >> + security modules asking for initialization will be >> + enabled (see CONFIG_DEFAULT_SECURITY). Duplicate >> + or invalid security modules will be ignored. The >> + capability module is always loaded first, without >> + regard to this parameter. >> >> This meant booting "security=apparmor" would disable all the other >> LSMs, which wasn't friendly at all. So "security=" was left alone (to >> leave it to only select the "major" LSM: all major LSMs not matching >> "security=" would be disabled). So I proposed "lsm.order=" to specify >> the order things were going to be initialized in, but to avoid kernels >> booting with newly added LSMs forced-off due to not being listed in >> "lsm.order=", it had to have implicit fall-back for unlisted LSMs. >> (i.e. anything missing from lsm.order would then follow their order in >> CONFIG_LSM_ORDER, and anything missing there would fall back to >> link-time ordering.) However, then the objection was raised that this >> didn't provide a way to explicitly disable an LSM. So I proposed >> lsm.enable/disable, and John argued for CONFIG_LSM_ENABLE over >> CONFIG_LSM_DISABLE. > > I still think we should have all built LSMs enabled by default, with > CONFIG_LSM_DISABLE available to turn stuff off. CONFIG_LSM_ORDER and this as a distro ubuntu does not want. Ubuntu wants to make yes available by building them in, but does NOT want all the LSM enabled by default, not even necessarily all minor LSMs. As a distro we want a supported set as default, and users can opt-in to new LSMs. If a new LSM comes along we don't want it enabled by default, which happens Using the lsm disable approach. > declares their order, "lsm.order=" works as mentioned, and > "lsm.enable/disable=" make changes to what's enabled. > > (This would be most like the v3 series, swapping CONFIG_LSM_ENABLE for > CONFIG_LSM_DISABLE.) > > It gives us centralized ordering and centralized disabling. Distros > wanting specific LSMs are already building them, so _also_ adding them > to CONFIG_LSM_ENABLE seems redundant to me. Distros wanting all the except as a disto we want to build in all LSMs by default but NOT have new LSMs enabled by default. The disable approach either mean we don't offer new LSMs in the supported kernels, OR me distro patch so we have the enabled by default list. > LSMs just want to declare the order of initialization, and maybe add > some to CONFIG_LSM_DISABLE some day, so they use CONFIG_LSM_ORDER. > individual LSMs may want that. > I should also note that I don't want to leave CONFIG_DEFAULT_SECURITY > in, since it's just a way to disable all the other majors. I don't > like this because it will force LSMs to be disabled that don't need to > be once blob-sharing lands. The whole point of this series is to get > us away from fixed ordering and thinking about "major" vs "minor" and > towards "exclusive" or not, where we can continue to slowly chip away > at exclusivity without breaking anything. > sure we definitely want to get away form "major" vs "minor" and in generally even exclusive, except where to LSMs just can't live with each other. But that doesn't mean dropping something like default security. The mistake with the current DEFAULT_SECURITY was that it only applied to major LSMs, not the minor ones.
On 10/03/2018 04:55 PM, Kees Cook wrote: > On Wed, Oct 3, 2018 at 2:34 PM, James Morris <jmorris@namei.org> wrote: >> On Wed, 3 Oct 2018, Kees Cook wrote: >> >>> On Wed, Oct 3, 2018 at 11:28 AM, James Morris <jmorris@namei.org> wrote: >>>> On Wed, 3 Oct 2018, Kees Cook wrote: >>>> >>>>> On Wed, Oct 3, 2018 at 11:17 AM, James Morris <jmorris@namei.org> wrote: >>>>>> On Tue, 2 Oct 2018, John Johansen wrote: >>>>>>> To me a list like >>>>>>> lsm.enable=X,Y,Z >>>>>> >>>>>> What about even simpler: >>>>>> >>>>>> lsm=selinux,!apparmor,yama >>>>> >>>>> We're going to have lsm.order=, so I'd like to keep it with a dot >>>>> separator (this makes it more like module parameters, too). You want >>>>> to mix enable/disable in the same string? That implies you'd want >>>>> implicit enabling (i.e. it complements the builtin enabling), which is >>>>> opposite from what John wanted. >>>>> >>>> >>>> Why can't this be the order as well? >>> >>> That was covered extensively in the earlier threads. It boils down to >>> making sure we do not create a pattern of leaving LSMs disabled by >>> default when they are added to the kernel. The v1 series used >>> security= like this: >>> >>> + security= [SECURITY] An ordered comma-separated list of >>> + security modules to attempt to enable at boot. If >>> + this boot parameter is not specified, only the >>> + security modules asking for initialization will be >>> + enabled (see CONFIG_DEFAULT_SECURITY). Duplicate >>> + or invalid security modules will be ignored. The >>> + capability module is always loaded first, without >>> + regard to this parameter. >>> >>> This meant booting "security=apparmor" would disable all the other >>> LSMs, which wasn't friendly at all. So "security=" was left alone (to >>> leave it to only select the "major" LSM: all major LSMs not matching >>> "security=" would be disabled). So I proposed "lsm.order=" to specify >>> the order things were going to be initialized in, but to avoid kernels >>> booting with newly added LSMs forced-off due to not being listed in >>> "lsm.order=", it had to have implicit fall-back for unlisted LSMs. >>> (i.e. anything missing from lsm.order would then follow their order in >>> CONFIG_LSM_ORDER, and anything missing there would fall back to >>> link-time ordering.) However, then the objection was raised that this >>> didn't provide a way to explicitly disable an LSM. So I proposed >>> lsm.enable/disable, and John argued for CONFIG_LSM_ENABLE over >>> CONFIG_LSM_DISABLE. >> >> Ok, but it may end up being clearer, simpler, and thus more secure to just >> have a single way to configure LSM. >> >> For example: >> >> - All LSMs which are built are NOT enabled by default >> >> - You specify enablement and order via a Kconfig: >> >> CONFIG_LSM="selinux,yama" >> >> - This can be entirely overridden by a boot param: >> >> lsm="apparmor,landlock" > > This doesn't work with how SELinux and AppArmor do their bootparams, > unfortunately. (And Paul and Stephen have expressed that the > documented selinux on/off must continue to work.) For example, let's > say you've built an Ubuntu kernel with: > > CONFIG_SELINUX=y > ... > CONFIG_LSM="yama,apparmor" > > (i.e. you want SELinux available, but not enabled, so it's left out of > CONFIG_LSM) > > Then someone boots the system with: > > selinux=1 security=selinux > > In what order does selinux get initialized relative to yama? > (apparmor, flagged as a "legacy major", would have been disabled by > the "security=" not matching it.) > > > The LSM order needs to be defined externally to enablement because > something may become enabled when not listed in the order. > it doesn't have to be that way. The only argument I can see for ordering being separate from enablement are - the order doesn't matter to some LSMs but other LSMs (like landlock) need to be in a specific order. Separating the ordering from enablement means the user doesn't have to take this into account when overriding a default enablement. > Now, maybe I misunderstood your earlier suggestion, and what you meant > was to do something like: > > CONFIG_LSM="yama,apparmor,!selinux" > > to mean "put selinux here in the order, but don't enable it". Then the > problem becomes what happens to an LSM that has been built in but not > listed in CONFIG_LSM? > it should be disabled. I know that is not what we have today but does current behavior of splitting how minor and major LSM enablement is done was a mistake > Related to that, this means that when new LSMs are added, they will > need to be added to any custom CONFIG_LSM= or lsm= parameters. If > that's really how we have to go, I'll accept it, but I think it's a > bit unfriendly. :P > That is exactly was will happen in at least some distros. Distros are committed to maintaining a certain configuration. If some new LSM comes along it shouldn't change the support config until the distro can evaluate any interaction will the current supported configuration. As a concrete example, the Ubuntu hwe kernels (newer kernels on LTS releases, are going to want to specify the hwe kernel config is the same as the release kernel except for few well vetted exceptions. > Another reason I don't like it is because it requires users to know > about all the LSMs to make changes. One LSM can't be added/removed The don't have to know about all LSMs, but they do need to know what the distro has as default set. > without specifying ALL of the LSMs. (i.e. there is no trivial way to > enable/disable a single LSM without it growing its own enable/disable > code as in SELinux/AppArmor. I'd hoped to make that easier for both > users and developers.) Again, I can live with it, but I think it's > unfriendly. > well unfriendly to who, and define users :) - its probably unfriendly to the developer wanting to work with a specific LSM. - a user messing with the different LSMs, yeah probably - a regular user, generally shouldn't have to know. - a distro trying to support a specific configuration, very much wants to define what is enabled/supported. Whether through individual LSM logic, or a centralized place > I just want to have a direct I can go that meets all the requirements. > :) I'm fine to ignore my sense of aesthetics if everyone can agree on > the code. > >> And that's it. >> >> Of course, capabilities is always enabled and not be visible to kconfig or >> boot params. > > Correct. I've made sure that's true in all the versions. > > BTW, there doesn't seem to be disagreement about the earlier part of > the series, though (patches 1-10). Could these go into -next just so I > don't have to keep sending them? :) > > LSM: Correctly announce start of LSM initialization > vmlinux.lds.h: Avoid copy/paste of security_init section > LSM: Rename .security_initcall section to .lsm_info > LSM: Remove initcall tracing > LSM: Convert from initcall to struct lsm_info > vmlinux.lds.h: Move LSM_TABLE into INIT_DATA > LSM: Convert security_initcall() into DEFINE_LSM() > LSM: Record LSM name in struct lsm_info > LSM: Provide init debugging infrastructure > LSM: Don't ignore initialization failures > > Thanks! > > -Kees >
On 10/03/2018 04:59 PM, Randy Dunlap wrote: > On 10/3/18 4:55 PM, Kees Cook wrote: >> On Wed, Oct 3, 2018 at 2:34 PM, James Morris <jmorris@namei.org> wrote: >>> On Wed, 3 Oct 2018, Kees Cook wrote: >>> >>>> On Wed, Oct 3, 2018 at 11:28 AM, James Morris <jmorris@namei.org> wrote: >>>>> On Wed, 3 Oct 2018, Kees Cook wrote: >>>>> >>>>>> On Wed, Oct 3, 2018 at 11:17 AM, James Morris <jmorris@namei.org> wrote: >>>>>>> On Tue, 2 Oct 2018, John Johansen wrote: >>>>>>>> To me a list like >>>>>>>> lsm.enable=X,Y,Z >>>>>>> >>>>>>> What about even simpler: >>>>>>> >>>>>>> lsm=selinux,!apparmor,yama >>>>>> >>>>>> We're going to have lsm.order=, so I'd like to keep it with a dot >>>>>> separator (this makes it more like module parameters, too). You want >>>>>> to mix enable/disable in the same string? That implies you'd want >>>>>> implicit enabling (i.e. it complements the builtin enabling), which is >>>>>> opposite from what John wanted. >>>>>> >>>>> >>>>> Why can't this be the order as well? >>>> >>>> That was covered extensively in the earlier threads. It boils down to >>>> making sure we do not create a pattern of leaving LSMs disabled by >>>> default when they are added to the kernel. The v1 series used >>>> security= like this: >>>> >>>> + security= [SECURITY] An ordered comma-separated list of >>>> + security modules to attempt to enable at boot. If >>>> + this boot parameter is not specified, only the >>>> + security modules asking for initialization will be >>>> + enabled (see CONFIG_DEFAULT_SECURITY). Duplicate >>>> + or invalid security modules will be ignored. The >>>> + capability module is always loaded first, without >>>> + regard to this parameter. >>>> >>>> This meant booting "security=apparmor" would disable all the other >>>> LSMs, which wasn't friendly at all. So "security=" was left alone (to >>>> leave it to only select the "major" LSM: all major LSMs not matching >>>> "security=" would be disabled). So I proposed "lsm.order=" to specify >>>> the order things were going to be initialized in, but to avoid kernels >>>> booting with newly added LSMs forced-off due to not being listed in >>>> "lsm.order=", it had to have implicit fall-back for unlisted LSMs. >>>> (i.e. anything missing from lsm.order would then follow their order in >>>> CONFIG_LSM_ORDER, and anything missing there would fall back to >>>> link-time ordering.) However, then the objection was raised that this >>>> didn't provide a way to explicitly disable an LSM. So I proposed >>>> lsm.enable/disable, and John argued for CONFIG_LSM_ENABLE over >>>> CONFIG_LSM_DISABLE. >>> >>> Ok, but it may end up being clearer, simpler, and thus more secure to just >>> have a single way to configure LSM. >>> >>> For example: >>> >>> - All LSMs which are built are NOT enabled by default >>> >>> - You specify enablement and order via a Kconfig: >>> >>> CONFIG_LSM="selinux,yama" >>> >>> - This can be entirely overridden by a boot param: >>> >>> lsm="apparmor,landlock" >> >> This doesn't work with how SELinux and AppArmor do their bootparams, >> unfortunately. (And Paul and Stephen have expressed that the >> documented selinux on/off must continue to work.) For example, let's >> say you've built an Ubuntu kernel with: >> >> CONFIG_SELINUX=y >> ... >> CONFIG_LSM="yama,apparmor" >> >> (i.e. you want SELinux available, but not enabled, so it's left out of >> CONFIG_LSM) >> >> Then someone boots the system with: >> >> selinux=1 security=selinux >> >> In what order does selinux get initialized relative to yama? >> (apparmor, flagged as a "legacy major", would have been disabled by >> the "security=" not matching it.) >> > > To me, "security=selinux" means SELinux and nothing else, so I think that > all of these params are inviting a lot of confusion. > > Sorry, I don't have a good answer for this. > Your not the only one. I have had users ask about why they are getting other security messures (yama in particular) when they specified a specific security= >> >> The LSM order needs to be defined externally to enablement because >> something may become enabled when not listed in the order. >> >> Now, maybe I misunderstood your earlier suggestion, and what you meant >> was to do something like: >> >> CONFIG_LSM="yama,apparmor,!selinux" >> >> to mean "put selinux here in the order, but don't enable it". Then the >> problem becomes what happens to an LSM that has been built in but not >> listed in CONFIG_LSM? >> >> Related to that, this means that when new LSMs are added, they will >> need to be added to any custom CONFIG_LSM= or lsm= parameters. If >> that's really how we have to go, I'll accept it, but I think it's a >> bit unfriendly. :P >> >> Another reason I don't like it is because it requires users to know >> about all the LSMs to make changes. One LSM can't be added/removed >> without specifying ALL of the LSMs. (i.e. there is no trivial way to >> enable/disable a single LSM without it growing its own enable/disable >> code as in SELinux/AppArmor. I'd hoped to make that easier for both >> users and developers.) Again, I can live with it, but I think it's >> unfriendly. >> >> I just want to have a direct I can go that meets all the requirements. >> :) I'm fine to ignore my sense of aesthetics if everyone can agree on >> the code. > >
On Wed, Oct 3, 2018 at 10:38 PM, John Johansen <john.johansen@canonical.com> wrote: > but distinct of first exclusive (major) will likely be going away > once full lsm stacking land. Right -- then policy loading because the "enabled" flag. The point is to get us to where we don't care about exclusivity at all. -Kees
On Wed, Oct 3, 2018 at 10:56 PM, John Johansen <john.johansen@canonical.com> wrote: > On 10/03/2018 01:36 PM, Kees Cook wrote: >> I still think we should have all built LSMs enabled by default, with >> CONFIG_LSM_DISABLE available to turn stuff off. CONFIG_LSM_ORDER > > and this as a distro ubuntu does not want. > Ubuntu wants to make yes available by building them in, but does NOT > want all the LSM enabled by default, not even necessarily all minor LSMs. > > As a distro we want a supported set as default, and users can opt-in > to new LSMs. If a new LSM comes along we don't want it enabled by > default, which happens Using the lsm disable approach. Okay, but order still matters. Where, in the order, should a disabled LSM go? It seems like the friendliest approach for an end-user would be to do something like lsm=+landlock and it all works correctly. That user doesn't need to know about ordering or the distro default LSMs. They just want to _add_ landlock. They want all the other LSMs to still be present, and they want the distro to have chosen where landlock is in the ordering. >> I should also note that I don't want to leave CONFIG_DEFAULT_SECURITY >> in, since it's just a way to disable all the other majors. I don't >> like this because it will force LSMs to be disabled that don't need to >> be once blob-sharing lands. The whole point of this series is to get >> us away from fixed ordering and thinking about "major" vs "minor" and >> towards "exclusive" or not, where we can continue to slowly chip away >> at exclusivity without breaking anything. >> > sure we definitely want to get away form "major" vs "minor" and in > generally even exclusive, except where to LSMs just can't live > with each other. > > But that doesn't mean dropping something like default security. The > mistake with the current DEFAULT_SECURITY was that it only applied > to major LSMs, not the minor ones. Right, we need to expand it to include a full description of ordering and enablement. How about this: CONFIG_LSM specifies order and enablement status. For example: CONFIG_LSM=yama,loadpin,apparmor,!selinux This means init order is yama, loadpin, apparmor, selinux, but selinux is disabled. Anything not listed in CONFIG_LSM but built in will be disabled and ordered in link-order. (i.e. an implicit trailing "!smack,!tomoyo".) Then we add "lsm=" which understands modifiers "-", and "+". "lsm=-apparmor,+selinux" wouldn't change ordering, but would disable apparmor and enable selinux. "lsm=smack,loadpin" would enable only smack and loadpin, in that order and disable everything else. I don't want to overload "security=", but we can if we want. It would be as above, but a trailing comma would be needed to trigger the "ordering" behavior. e.g. "security=selinux" would disable all other majors (retaining the current behavior), but "security=selinux," would disable all other LSMs. -Kees
Sent with ProtonMail Secure Email. ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Thursday, October 4, 2018 6:18 PM, Kees Cook <keescook@chromium.org> wrote: > > I don't want to overload "security=", but we can if we want. It would > be as above, but a trailing comma would be needed to trigger the > "ordering" behavior. e.g. "security=selinux" would disable all other > majors (retaining the current behavior), but "security=selinux," would > disable all other LSMs. > > -Kees > > I don't think giving such big impact to trailing comma is good idea :) Jordan
On Thu, Oct 4, 2018 at 10:40 AM, Jordan Glover <Golden_Miller83@protonmail.ch> wrote: > Sent with ProtonMail Secure Email. > > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ > On Thursday, October 4, 2018 6:18 PM, Kees Cook <keescook@chromium.org> wrote: > >> >> I don't want to overload "security=", but we can if we want. It would >> be as above, but a trailing comma would be needed to trigger the >> "ordering" behavior. e.g. "security=selinux" would disable all other >> majors (retaining the current behavior), but "security=selinux," would >> disable all other LSMs. >> >> -Kees >> >> > > I don't think giving such big impact to trailing comma is good idea :) That's why I prefer a new lsm= instead of confusing security=. :) -Kees
On Wed, 3 Oct 2018, Kees Cook wrote: > On Wed, Oct 3, 2018 at 2:34 PM, James Morris <jmorris@namei.org> wrote: > > On Wed, 3 Oct 2018, Kees Cook wrote: > > > > - All LSMs which are built are NOT enabled by default > > > > - You specify enablement and order via a Kconfig: > > > > CONFIG_LSM="selinux,yama" > > > > - This can be entirely overridden by a boot param: > > > > lsm="apparmor,landlock" > > This doesn't work with how SELinux and AppArmor do their bootparams, > unfortunately. (And Paul and Stephen have expressed that the > documented selinux on/off must continue to work.) For example, let's > say you've built an Ubuntu kernel with: > > CONFIG_SELINUX=y > ... > CONFIG_LSM="yama,apparmor" > > (i.e. you want SELinux available, but not enabled, so it's left out of > CONFIG_LSM) > > Then someone boots the system with: > > selinux=1 security=selinux > > In what order does selinux get initialized relative to yama? > (apparmor, flagged as a "legacy major", would have been disabled by > the "security=" not matching it.) It doesn't, it needs to be specified in one place. Distros will need to update boot parameter handling for this kernel onwards. Otherwise, we will need to carry this confusing mess forward forever. > The LSM order needs to be defined externally to enablement because > something may become enabled when not listed in the order. > > Now, maybe I misunderstood your earlier suggestion, and what you meant > was to do something like: > > CONFIG_LSM="yama,apparmor,!selinux" > > to mean "put selinux here in the order, but don't enable it". Then the > problem becomes what happens to an LSM that has been built in but not > listed in CONFIG_LSM? In my most recent suggestion, there is no '!' disablement, just enablement. If an LSM is not listed in CONFIG_LSM="", it's not enabled. > Related to that, this means that when new LSMs are added, they will > need to be added to any custom CONFIG_LSM= or lsm= parameters. If > that's really how we have to go, I'll accept it, but I think it's a > bit unfriendly. :P If you want to enable them, yes. How is this different to adding new mount options as new fs features become available? > Another reason I don't like it is because it requires users to know > about all the LSMs to make changes. One LSM can't be added/removed > without specifying ALL of the LSMs. (i.e. there is no trivial way to > enable/disable a single LSM without it growing its own enable/disable > code as in SELinux/AppArmor. I'd hoped to make that easier for both > users and developers.) Again, I can live with it, but I think it's > unfriendly. This is only done via boot params or KConfig.
On Thu, Oct 4, 2018 at 10:49 AM, James Morris <jmorris@namei.org> wrote: > On Wed, 3 Oct 2018, Kees Cook wrote: >> Then someone boots the system with: >> >> selinux=1 security=selinux >> >> In what order does selinux get initialized relative to yama? >> (apparmor, flagged as a "legacy major", would have been disabled by >> the "security=" not matching it.) > > It doesn't, it needs to be specified in one place. > > Distros will need to update boot parameter handling for this kernel > onwards. Otherwise, we will need to carry this confusing mess forward > forever. Are you saying that you want to overrule Paul and Stephen about keeping "selinux=1 secuiryt=selinux" working? >> CONFIG_LSM="yama,apparmor,!selinux" >> >> to mean "put selinux here in the order, but don't enable it". Then the >> problem becomes what happens to an LSM that has been built in but not >> listed in CONFIG_LSM? > > In my most recent suggestion, there is no '!' disablement, just > enablement. If an LSM is not listed in CONFIG_LSM="", it's not enabled. And a user would need to specify ALL lsms on the "lsm=" line? What do you think of my latest proposal? It could happily work all three ways: old boot params and security= work ("selinux=1 security=selinux" keeps working), individual LSM enable/disable works ("lsm=+loadpin"), and full LSM ordering works ("lsm=each,lsm,in,order,here"): https://lore.kernel.org/lkml/CAGXu5jJJit8bDNvgXaFkuvFPy7NWtJW2oRWFbG-6iWk0+A1qng@mail.gmail.com/ -Kees
On Thu, 4 Oct 2018, Kees Cook wrote: > On Thu, Oct 4, 2018 at 10:49 AM, James Morris <jmorris@namei.org> wrote: > > On Wed, 3 Oct 2018, Kees Cook wrote: > >> Then someone boots the system with: > >> > >> selinux=1 security=selinux > >> > >> In what order does selinux get initialized relative to yama? > >> (apparmor, flagged as a "legacy major", would have been disabled by > >> the "security=" not matching it.) > > > > It doesn't, it needs to be specified in one place. > > > > Distros will need to update boot parameter handling for this kernel > > onwards. Otherwise, we will need to carry this confusing mess forward > > forever. > > Are you saying that you want to overrule Paul and Stephen about > keeping "selinux=1 secuiryt=selinux" working? Not overrule, but convince. At least, deprecate selinux=1 and security=X, but not extend it any further. > > In my most recent suggestion, there is no '!' disablement, just > > enablement. If an LSM is not listed in CONFIG_LSM="", it's not enabled. > > And a user would need to specify ALL lsms on the "lsm=" line? > Yes, the ones they want enabled. > What do you think of my latest proposal? It could happily work all > three ways: old boot params and security= work ("selinux=1 > security=selinux" keeps working), individual LSM enable/disable works > ("lsm=+loadpin"), and full LSM ordering works > ("lsm=each,lsm,in,order,here"): > > https://lore.kernel.org/lkml/CAGXu5jJJit8bDNvgXaFkuvFPy7NWtJW2oRWFbG-6iWk0+A1qng@mail.gmail.com/ > I think having something like +yama will still lead to confusion. Explicitly stating each enabled LSM in order is totally unambiguous. If people are moving away from the distro defaults, and there is no high-level interface to manage this, it seems to me there's a deeper issue with the distro.
On Fri, 5 Oct 2018, James Morris wrote: > On Thu, 4 Oct 2018, Kees Cook wrote: > > And a user would need to specify ALL lsms on the "lsm=" line? > > > > Yes, the ones they want enabled. If they're overriding the kconfig value.
On Thu, Oct 4, 2018 at 9:58 PM, James Morris <jmorris@namei.org> wrote: > On Thu, 4 Oct 2018, Kees Cook wrote: > >> On Thu, Oct 4, 2018 at 10:49 AM, James Morris <jmorris@namei.org> wrote: >> > On Wed, 3 Oct 2018, Kees Cook wrote: >> >> Then someone boots the system with: >> >> >> >> selinux=1 security=selinux >> >> >> >> In what order does selinux get initialized relative to yama? >> >> (apparmor, flagged as a "legacy major", would have been disabled by >> >> the "security=" not matching it.) >> > >> > It doesn't, it needs to be specified in one place. >> > >> > Distros will need to update boot parameter handling for this kernel >> > onwards. Otherwise, we will need to carry this confusing mess forward >> > forever. >> >> Are you saying that you want to overrule Paul and Stephen about >> keeping "selinux=1 secuiryt=selinux" working? > > Not overrule, but convince. > > At least, deprecate selinux=1 and security=X, but not extend it any > further. Okay, this is the expectation from me as well. I think my series makes it work as-is with the new stuff just fine. >> > In my most recent suggestion, there is no '!' disablement, just >> > enablement. If an LSM is not listed in CONFIG_LSM="", it's not enabled. >> >> And a user would need to specify ALL lsms on the "lsm=" line? >> > > Yes, the ones they want enabled. > >> What do you think of my latest proposal? It could happily work all >> three ways: old boot params and security= work ("selinux=1 >> security=selinux" keeps working), individual LSM enable/disable works >> ("lsm=+loadpin"), and full LSM ordering works >> ("lsm=each,lsm,in,order,here"): >> >> https://lore.kernel.org/lkml/CAGXu5jJJit8bDNvgXaFkuvFPy7NWtJW2oRWFbG-6iWk0+A1qng@mail.gmail.com/ >> > > I think having something like +yama will still lead to confusion. > Explicitly stating each enabled LSM in order is totally unambiguous. > > If people are moving away from the distro defaults, and there is no > high-level interface to manage this, it seems to me there's a deeper > issue with the distro. Okay. I will adjust the series and send a v5. -Kees
On Thu, Oct 4, 2018 at 1:38 AM John Johansen <john.johansen@canonical.com> wrote: > On 10/03/2018 10:26 AM, Kees Cook wrote: ... > > Either a distro builds a very specific subset of LSMs, or they build > > in all LSMs (for the user to choose from). In both cases, they set an > > explicit order, which defines which exclusive LSM get selected. > > and when lsm stacking lands, that exlusive LSM goes away. FWIW, I still believe in my earlier statements supporting explicitly enabling LSM stacking via Kconfig.
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index cf963febebb0..0d10ab3d020e 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -4045,15 +4045,6 @@ loaded. An invalid security module name will be treated as if no module has been chosen. - selinux= [SELINUX] Disable or enable SELinux at boot time. - Format: { "0" | "1" } - See security/selinux/Kconfig help text. - 0 -- disable. - 1 -- enable. - Default value is set via kernel config option. - If enabled at boot time, /selinux/disable can be used - later to disable prior to initial policy load. - serialnumber [BUGS=X86-32] shapers= [NET] diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig index 8af7a690eb40..86936528a0bb 100644 --- a/security/selinux/Kconfig +++ b/security/selinux/Kconfig @@ -8,35 +8,6 @@ config SECURITY_SELINUX You will also need a policy configuration and a labeled filesystem. If you are unsure how to answer this question, answer N. -config SECURITY_SELINUX_BOOTPARAM - bool "NSA SELinux boot parameter" - depends on SECURITY_SELINUX - default n - help - This option adds a kernel parameter 'selinux', which allows SELinux - to be disabled at boot. If this option is selected, SELinux - functionality can be disabled with selinux=0 on the kernel - command line. The purpose of this option is to allow a single - kernel image to be distributed with SELinux built in, but not - necessarily enabled. - - If you are unsure how to answer this question, answer N. - -config SECURITY_SELINUX_BOOTPARAM_VALUE - int "NSA SELinux boot parameter default value" - depends on SECURITY_SELINUX_BOOTPARAM - range 0 1 - default 1 - help - This option sets the default value for the kernel parameter - 'selinux', which allows SELinux to be disabled at boot. If this - option is set to 0 (zero), the SELinux kernel parameter will - default to 0, disabling SELinux at bootup. If this option is - set to 1 (one), the SELinux kernel parameter will default to 1, - enabling SELinux at bootup. - - If you are unsure how to answer this question, answer 1. - config SECURITY_SELINUX_DISABLE bool "NSA SELinux runtime disable" depends on SECURITY_SELINUX diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 71a10fedecb3..8f5eea097612 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -120,20 +120,7 @@ __setup("enforcing=", enforcing_setup); #define selinux_enforcing_boot 1 #endif -#ifdef CONFIG_SECURITY_SELINUX_BOOTPARAM -int selinux_enabled = CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE; - -static int __init selinux_enabled_setup(char *str) -{ - unsigned long enabled; - if (!kstrtoul(str, 0, &enabled)) - selinux_enabled = enabled ? 1 : 0; - return 1; -} -__setup("selinux=", selinux_enabled_setup); -#else -int selinux_enabled = 1; -#endif +int selinux_enabled __lsm_ro_after_init; static unsigned int selinux_checkreqprot_boot = CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE;
Since LSM enabling is now centralized with CONFIG_LSM_ENABLE and "lsm.enable=...", this removes the LSM-specific enabling logic from SELinux. Signed-off-by: Kees Cook <keescook@chromium.org> --- .../admin-guide/kernel-parameters.txt | 9 ------ security/selinux/Kconfig | 29 ------------------- security/selinux/hooks.c | 15 +--------- 3 files changed, 1 insertion(+), 52 deletions(-)