| Message ID | 99cb1ae7-8881-eb9a-a8cb-a787abe454e1@schaufler-ca.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | LSM: Module stacking in support of S.A.R.A and Landlock | expand |
On Tue, Sep 11, 2018 at 9:42 AM, Casey Schaufler <casey@schaufler-ca.com> wrote: > Two proposed security modules require the ability to > share security blobs with existing "major" security modules. > These modules, S.A.R.A and LandLock, provide significantly > different services than SELinux, Smack or AppArmor. Using > either in conjunction with the existing modules is quite > reasonable. S.A.R.A requires access to the cred blob, while > LandLock uses the cred, file and inode blobs. > > The use of the cred, file and inode blobs has been > abstracted in preceding patches in the series. This > patch teaches the affected security modules how to access > the part of the blob set aside for their use in the case > where blobs are shared. The configuration option > CONFIG_SECURITY_STACKING identifies systems where the > blobs may be shared. > > The mechanism for selecting which security modules are > active has been changed to allow non-conflicting "major" > security modules to be used together. At this time the > TOMOYO module can safely be used with any of the others. > The two new modules would be non-conflicting as well. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > --- > Documentation/admin-guide/LSM/index.rst | 14 +++-- > include/linux/lsm_hooks.h | 2 +- > security/Kconfig | 81 +++++++++++++++++++++++++ > security/apparmor/include/cred.h | 8 +++ > security/apparmor/include/file.h | 9 ++- > security/apparmor/include/lib.h | 4 ++ > security/apparmor/lsm.c | 8 ++- > security/security.c | 30 ++++++++- > security/selinux/hooks.c | 3 +- > security/selinux/include/objsec.h | 18 +++++- > security/smack/smack.h | 19 +++++- > security/smack/smack_lsm.c | 17 +++--- > security/tomoyo/common.h | 12 +++- > security/tomoyo/tomoyo.c | 3 +- > 14 files changed, 200 insertions(+), 28 deletions(-) > > diff --git a/Documentation/admin-guide/LSM/index.rst b/Documentation/admin-guide/LSM/index.rst > index 9842e21afd4a..d3d8af174042 100644 > --- a/Documentation/admin-guide/LSM/index.rst > +++ b/Documentation/admin-guide/LSM/index.rst > @@ -17,10 +17,16 @@ MAC extensions, other extensions can be built using the LSM to provide > specific changes to system operation when these tweaks are not available > in the core functionality of Linux itself. > > -The Linux capabilities modules will always be included. This may be > -followed by any number of "minor" modules and at most one "major" module. > -For more details on capabilities, see ``capabilities(7)`` in the Linux > -man-pages project. > +The Linux capabilities modules will always be included. For more details > +on capabilities, see ``capabilities(7)`` in the Linux man-pages project. > + > +Security modules that do not use the security data blobs maintained > +by the LSM infrastructure are considered "minor" modules. These may be > +included at compile time and stacked explicitly. Security modules that > +use the LSM maintained security blobs are considered "major" modules. > +These may only be stacked if the CONFIG_LSM_STACKED configuration > +option is used. If this is chosen all of the security modules selected > +will be used. > > A list of the active security modules can be found by reading > ``/sys/kernel/security/lsm``. This is a comma separated list, and > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index 416b20c3795b..dddcced54fea 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -2079,7 +2079,7 @@ static inline void security_delete_hooks(struct security_hook_list *hooks, > #define __lsm_ro_after_init __ro_after_init > #endif /* CONFIG_SECURITY_WRITABLE_HOOKS */ > > -extern int __init security_module_enable(const char *module); > +extern bool __init security_module_enable(const char *lsm, const bool stacked); > extern void __init capability_add_hooks(void); > #ifdef CONFIG_SECURITY_YAMA > extern void __init yama_add_hooks(void); > diff --git a/security/Kconfig b/security/Kconfig > index 22f7664c4977..ed48025ae9e0 100644 > --- a/security/Kconfig > +++ b/security/Kconfig > @@ -36,6 +36,28 @@ config SECURITY_WRITABLE_HOOKS > bool > default n > > +config SECURITY_STACKING > + bool "Security module stacking" > + depends on SECURITY > + help > + Allows multiple major security modules to be stacked. > + Modules are invoked in the order registered with a > + "bail on fail" policy, in which the infrastructure > + will stop processing once a denial is detected. Not > + all modules can be stacked. SELinux, Smack and AppArmor are > + known to be incompatible. User space components may > + have trouble identifying the security module providing > + data in some cases. > + > + If you select this option you will have to select which > + of the stackable modules you wish to be active. The > + "Default security module" will be ignored. The boot line > + "security=" option can be used to specify that one of > + the modules identifed for stacking should be used instead > + of the entire stack. > + > + If you are unsure how to answer this question, answer N. I don't see a good reason to make this a config. Why shouldn't this always be enabled? > + > config SECURITY_LSM_DEBUG > bool "Enable debugging of the LSM infrastructure" > depends on SECURITY > @@ -250,6 +272,9 @@ source security/yama/Kconfig > > source security/integrity/Kconfig > > +menu "Security Module Selection" > + visible if !SECURITY_STACKING > + > choice > prompt "Default security module" > default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX > @@ -289,3 +314,59 @@ config DEFAULT_SECURITY > > endmenu > > +menu "Security Module Stack" > + visible if SECURITY_STACKING > + > +choice > + prompt "Stacked 'extreme' security module" > + default SECURITY_SELINUX_STACKED if SECURITY_SELINUX > + default SECURITY_SMACK_STACKED if SECURITY_SMACK > + default SECURITY_APPARMOR_STACKED if SECURITY_APPARMOR I don't think any of this is needed either: we already have the CONFIG_DEFAULT_SECURITY_* choice. > [...] > diff --git a/security/apparmor/include/cred.h b/security/apparmor/include/cred.h > index a90eae76d7c1..be7575adf6f0 100644 > --- a/security/apparmor/include/cred.h > +++ b/security/apparmor/include/cred.h > @@ -25,7 +25,11 @@ > > static inline struct aa_label *cred_label(const struct cred *cred) > { > +#ifdef CONFIG_SECURITY_STACKING > + struct aa_label **blob = cred->security + apparmor_blob_sizes.lbs_cred; > +#else > struct aa_label **blob = cred->security; > +#endif Without the config, then there's no need for all these #ifdefs. > -#define file_ctx(X) ((struct aa_file_ctx *)(X)->f_security) > +static inline struct aa_file_ctx *file_ctx(struct file *file) > +{ > +#ifdef CONFIG_SECURITY_STACKING > + return file->f_security + apparmor_blob_sizes.lbs_file; > +#else > + return file->f_security; > +#endif > +} This define->inline should have been part of an earlier patch. > /* struct aa_file_ctx - the AppArmor context the file was opened in > * @lock: lock to update the ctx > diff --git a/security/apparmor/include/lib.h b/security/apparmor/include/lib.h > index 6505e1ad9e23..bbe9b384d71d 100644 > --- a/security/apparmor/include/lib.h > +++ b/security/apparmor/include/lib.h > @@ -16,6 +16,7 @@ > > #include <linux/slab.h> > #include <linux/fs.h> > +#include <linux/lsm_hooks.h> > > #include "match.h" > > @@ -55,6 +56,9 @@ const char *aa_splitn_fqname(const char *fqname, size_t n, const char **ns_name, > size_t *ns_len); > void aa_info_message(const char *str); > > +/* Security blob offsets */ > +extern struct lsm_blob_sizes apparmor_blob_sizes; > + > /** > * aa_strneq - compare null terminated @str to a non null terminated substring > * @str: a null terminated string > diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c > index 15716b6ff860..36d8386170e8 100644 > --- a/security/apparmor/lsm.c > +++ b/security/apparmor/lsm.c > @@ -1553,7 +1553,9 @@ static int __init apparmor_init(void) > int error; > > if (!finish) { > - if (apparmor_enabled && security_module_enable("apparmor")) > + if (apparmor_enabled && > + security_module_enable("apparmor", > + IS_ENABLED(CONFIG_SECURITY_APPARMOR_STACKED))) > security_add_blobs(&apparmor_blob_sizes); > else > apparmor_enabled = false; > @@ -1561,7 +1563,9 @@ static int __init apparmor_init(void) > return 0; > } > > - if (!apparmor_enabled || !security_module_enable("apparmor")) { > + if (!apparmor_enabled || > + !security_module_enable("apparmor", > + IS_ENABLED(CONFIG_SECURITY_APPARMOR_STACKED))) { > aa_info_message("AppArmor disabled by boot time parameter"); > apparmor_enabled = false; > return 0; I don't think any of these changes are needed either with the loss of the config. > diff --git a/security/security.c b/security/security.c > index 2501cdcbebff..06bed74d1ed0 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -36,6 +36,7 @@ > > /* Maximum number of letters for an LSM name string */ > #define SECURITY_NAME_MAX 10 > +#define MODULE_STACK "(stacking)" > > struct security_hook_heads security_hook_heads __lsm_ro_after_init; > static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain); > @@ -48,7 +49,11 @@ static struct lsm_blob_sizes blob_sizes; > > /* Boot-time LSM user choice */ > static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] = > +#ifdef CONFIG_SECURITY_STACKING > + MODULE_STACK; > +#else > CONFIG_DEFAULT_SECURITY; > +#endif > > static void __init do_security_initcalls(void) > { > @@ -169,6 +174,7 @@ static int lsm_append(char *new, char **result) > /** > * security_module_enable - Load given security module on boot ? > * @module: the name of the module > + * @stacked: indicates that the module wants to be stacked > * > * Each LSM must pass this method before registering its own operations > * to avoid security registration races. This method may also be used > @@ -184,9 +190,29 @@ static int lsm_append(char *new, char **result) > * > * Otherwise, return false. > */ > -int __init security_module_enable(const char *module) > +bool __init security_module_enable(const char *lsm, const bool stacked) > { > - return !strcmp(module, chosen_lsm); > +#ifdef CONFIG_SECURITY_STACKING > + /* > + * Module defined on the command line security=XXXX > + */ > + if (strcmp(chosen_lsm, MODULE_STACK)) { > + if (!strcmp(lsm, chosen_lsm)) { > + pr_info("Command line sets the %s security module.\n", > + lsm); > + return true; > + } > + return false; > + } > + /* > + * Module configured as stacked. > + */ > + return stacked; > +#else > + if (strcmp(lsm, chosen_lsm) == 0) > + return true; > + return false; > +#endif > } I don't see the need for this? security_module_enable() will already specify if it's been selected as the "primary" LSM. The only change I think is needed here is to treat tomoyo differently. It can now operate independently, like the "minor" LSMs. So we have three types of LSMs now: "conflicting", "non-conflicting", and "minor". The only differences are how they get initialized. Major use security_initcall() and minor use explicit calls to $lsm_add_hooks(). Yama is always enabled if built in. Loadpin uses a module parameter ("loadpin.enabled") and checks it when loadpin_add_hooks() is called. To split tomoyo away from the other security modules, we need to track its enablement _separately_ from the conflicting LSMs. i.e. choose_lsm() needs to change, or tomoyo needs to use a module parameter ("tomoyo.enabled"), or a __setup() call like apparmor and selinux do for enablement. (Note that apparmor and selinux check _both_ their __setup() and security_module_enable() values...) For example (untested, likely whitespace damaged): diff --git a/security/tomoyo/Kconfig b/security/tomoyo/Kconfig index 404dce66952a..4edc8e733245 100644 --- a/security/tomoyo/Kconfig +++ b/security/tomoyo/Kconfig @@ -14,6 +14,14 @@ config SECURITY_TOMOYO found at <http://tomoyo.sourceforge.jp/>. If you are unsure how to answer this question, answer N. +config SECURITY_TOMOYO_ENABLED + bool "Enable TOMOYO at boot" + depends on SECURITY_TOMOYO + default SECURITY_TOMOYO + help + If selected, TOMOYO will be enabled at boot. If not selected, it + can be enabled at boot with the kernel parameter "tomoyo.enabled=1". + config SECURITY_TOMOYO_MAX_ACCEPT_ENTRY int "Default maximal count for learning mode" default 2048 diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c index 9f932e2d6852..8dc9ef2096ab 100644 --- a/security/tomoyo/tomoyo.c +++ b/security/tomoyo/tomoyo.c @@ -531,6 +531,8 @@ static struct security_hook_list tomoyo_hooks[] __lsm_ro_after_init = { /* Lock for GC. */ DEFINE_SRCU(tomoyo_ss); +static int enabled = IS_ENABLED(CONFIG_SECURITY_TOMOYO_ENABLED); + /** * tomoyo_init - Register TOMOYO Linux as a LSM module. * @@ -540,7 +542,7 @@ static int __init tomoyo_init(void) { struct cred *cred = (struct cred *) current_cred(); - if (!security_module_enable("tomoyo")) + if (!enabled) return 0; /* register ourselves with the security framework */ security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks), "tomoyo"); @@ -550,4 +552,8 @@ static int __init tomoyo_init(void) return 0; } +/* Should not be mutable after boot, so not listed in sysfs (perm == 0). */ +module_param(enabled, int, 0); +MODULE_PARM_DESC(enabled, "Tomoyo enabled at boot"); + security_initcall(tomoyo_init); (I prefer LSMs do enablement with module params so that they leave their namespace open for other runtime configuration. I think "apparmor=" and "selinux=" for enable/disable isn't good to replicate.) We will quickly encounter "LSM ordering" as an issue, but not in this case yet: there's no new LSM. Ordering is preserved even with my suggestion: major order is controlled by Makefile link ordering, and minor is controlled by explicit ordering in security/security.c's add_hooks() calls. > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index 6617abb51732..be14540ce09c 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -3493,18 +3493,16 @@ static int smack_getprocattr(struct task_struct *p, char *name, char **value) > { > struct smack_known *skp = smk_of_task_struct(p); > char *cp; > - int slen; > > - if (strcmp(name, "current") != 0) > + if (strcmp(name, "current") == 0) { > + cp = kstrdup(skp->smk_known, GFP_KERNEL); > + if (cp == NULL) > + return -ENOMEM; > + } else > return -EINVAL; > > - cp = kstrdup(skp->smk_known, GFP_KERNEL); > - if (cp == NULL) > - return -ENOMEM; > - > - slen = strlen(cp); > *value = cp; > - return slen; > + return strlen(cp); > } This refactoring seems out of place? -Kees
On Thu, Sep 13, 2018 at 12:19 AM Kees Cook <keescook@chromium.org> wrote: > On Tue, Sep 11, 2018 at 9:42 AM, Casey Schaufler <casey@schaufler-ca.com> wrote: > > Two proposed security modules require the ability to > > share security blobs with existing "major" security modules. > > These modules, S.A.R.A and LandLock, provide significantly > > different services than SELinux, Smack or AppArmor. Using > > either in conjunction with the existing modules is quite > > reasonable. S.A.R.A requires access to the cred blob, while > > LandLock uses the cred, file and inode blobs. > > > > The use of the cred, file and inode blobs has been > > abstracted in preceding patches in the series. This > > patch teaches the affected security modules how to access > > the part of the blob set aside for their use in the case > > where blobs are shared. The configuration option > > CONFIG_SECURITY_STACKING identifies systems where the > > blobs may be shared. > > > > The mechanism for selecting which security modules are > > active has been changed to allow non-conflicting "major" > > security modules to be used together. At this time the > > TOMOYO module can safely be used with any of the others. > > The two new modules would be non-conflicting as well. > > > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > > --- > > Documentation/admin-guide/LSM/index.rst | 14 +++-- > > include/linux/lsm_hooks.h | 2 +- > > security/Kconfig | 81 +++++++++++++++++++++++++ > > security/apparmor/include/cred.h | 8 +++ > > security/apparmor/include/file.h | 9 ++- > > security/apparmor/include/lib.h | 4 ++ > > security/apparmor/lsm.c | 8 ++- > > security/security.c | 30 ++++++++- > > security/selinux/hooks.c | 3 +- > > security/selinux/include/objsec.h | 18 +++++- > > security/smack/smack.h | 19 +++++- > > security/smack/smack_lsm.c | 17 +++--- > > security/tomoyo/common.h | 12 +++- > > security/tomoyo/tomoyo.c | 3 +- > > 14 files changed, 200 insertions(+), 28 deletions(-) ... > > diff --git a/security/Kconfig b/security/Kconfig > > index 22f7664c4977..ed48025ae9e0 100644 > > --- a/security/Kconfig > > +++ b/security/Kconfig > > @@ -36,6 +36,28 @@ config SECURITY_WRITABLE_HOOKS > > bool > > default n > > > > +config SECURITY_STACKING > > + bool "Security module stacking" > > + depends on SECURITY > > + help > > + Allows multiple major security modules to be stacked. > > + Modules are invoked in the order registered with a > > + "bail on fail" policy, in which the infrastructure > > + will stop processing once a denial is detected. Not > > + all modules can be stacked. SELinux, Smack and AppArmor are > > + known to be incompatible. User space components may > > + have trouble identifying the security module providing > > + data in some cases. > > + > > + If you select this option you will have to select which > > + of the stackable modules you wish to be active. The > > + "Default security module" will be ignored. The boot line > > + "security=" option can be used to specify that one of > > + the modules identifed for stacking should be used instead > > + of the entire stack. > > + > > + If you are unsure how to answer this question, answer N. > > I don't see a good reason to make this a config. Why shouldn't this > always be enabled? I do. From a user perspective it is sometimes difficult to determine the reason behind a failed operation; its is a DAC based denial, the LSM, or some other failure? Stacking additional LSMs has the potential to make this worse. The boot time configuration adds to the complexity. I think we should leave this decision to the individual distros so that they can make their own decision on LSM stacking based on the savviness of their user base and the quality of their support infrastructure.
On Thu, Sep 13, 2018 at 6:16 AM, Paul Moore <paul@paul-moore.com> wrote: > On Thu, Sep 13, 2018 at 12:19 AM Kees Cook <keescook@chromium.org> wrote: >> On Tue, Sep 11, 2018 at 9:42 AM, Casey Schaufler <casey@schaufler-ca.com> wrote: >> > Two proposed security modules require the ability to >> > share security blobs with existing "major" security modules. >> > These modules, S.A.R.A and LandLock, provide significantly >> > different services than SELinux, Smack or AppArmor. Using >> > either in conjunction with the existing modules is quite >> > reasonable. S.A.R.A requires access to the cred blob, while >> > LandLock uses the cred, file and inode blobs. >> > >> > The use of the cred, file and inode blobs has been >> > abstracted in preceding patches in the series. This >> > patch teaches the affected security modules how to access >> > the part of the blob set aside for their use in the case >> > where blobs are shared. The configuration option >> > CONFIG_SECURITY_STACKING identifies systems where the >> > blobs may be shared. >> > >> > The mechanism for selecting which security modules are >> > active has been changed to allow non-conflicting "major" >> > security modules to be used together. At this time the >> > TOMOYO module can safely be used with any of the others. >> > The two new modules would be non-conflicting as well. >> > >> > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >> > --- >> > Documentation/admin-guide/LSM/index.rst | 14 +++-- >> > include/linux/lsm_hooks.h | 2 +- >> > security/Kconfig | 81 +++++++++++++++++++++++++ >> > security/apparmor/include/cred.h | 8 +++ >> > security/apparmor/include/file.h | 9 ++- >> > security/apparmor/include/lib.h | 4 ++ >> > security/apparmor/lsm.c | 8 ++- >> > security/security.c | 30 ++++++++- >> > security/selinux/hooks.c | 3 +- >> > security/selinux/include/objsec.h | 18 +++++- >> > security/smack/smack.h | 19 +++++- >> > security/smack/smack_lsm.c | 17 +++--- >> > security/tomoyo/common.h | 12 +++- >> > security/tomoyo/tomoyo.c | 3 +- >> > 14 files changed, 200 insertions(+), 28 deletions(-) > > ... > >> > diff --git a/security/Kconfig b/security/Kconfig >> > index 22f7664c4977..ed48025ae9e0 100644 >> > --- a/security/Kconfig >> > +++ b/security/Kconfig >> > @@ -36,6 +36,28 @@ config SECURITY_WRITABLE_HOOKS >> > bool >> > default n >> > >> > +config SECURITY_STACKING >> > + bool "Security module stacking" >> > + depends on SECURITY >> > + help >> > + Allows multiple major security modules to be stacked. >> > + Modules are invoked in the order registered with a >> > + "bail on fail" policy, in which the infrastructure >> > + will stop processing once a denial is detected. Not >> > + all modules can be stacked. SELinux, Smack and AppArmor are >> > + known to be incompatible. User space components may >> > + have trouble identifying the security module providing >> > + data in some cases. >> > + >> > + If you select this option you will have to select which >> > + of the stackable modules you wish to be active. The >> > + "Default security module" will be ignored. The boot line >> > + "security=" option can be used to specify that one of >> > + the modules identifed for stacking should be used instead >> > + of the entire stack. >> > + >> > + If you are unsure how to answer this question, answer N. >> >> I don't see a good reason to make this a config. Why shouldn't this >> always be enabled? > > I do. From a user perspective it is sometimes difficult to determine > the reason behind a failed operation; its is a DAC based denial, the > LSM, or some other failure? Stacking additional LSMs has the > potential to make this worse. The boot time configuration adds to the > complexity. Let me try to convince you otherwise. :) The reason I think there's no need for this is because the only functional change here is how _TOMOYO_ gets stacked. And in my proposal, we can convert TOMOYO to be enabled/disabled like LoadPin. Given the configs I showed, stacking TOMOYO with the other major LSMs becomes a config (and/or boottime) option. The changes for TOMOYO are still needed even _with_ SECURITY_STACKING, and I argue that the other major LSMs remain the same. It's only infrastructure that has changed. So, I think having SECURITY_STACKING actually makes things more complex internally (all the ifdefs, weird enable logic) and for distros ("what's this stacking option", etc?) > I think we should leave this decision to the individual distros so > that they can make their own decision on LSM stacking based on the > savviness of their user base and the quality of their support > infrastructure. If we reach the "extreme" stacking case, then yes, I want to make sure we've got something that makes sense. But this first step doesn't get us there, so I'd prefer we avoid making it overly complex. I think everything else in this series _reduces_ complexity, except for this new config. -Kees
On Thu, Sep 13, 2018 at 11:19 AM Kees Cook <keescook@chromium.org> wrote: > On Thu, Sep 13, 2018 at 6:16 AM, Paul Moore <paul@paul-moore.com> wrote: > > On Thu, Sep 13, 2018 at 12:19 AM Kees Cook <keescook@chromium.org> wrote: > >> On Tue, Sep 11, 2018 at 9:42 AM, Casey Schaufler <casey@schaufler-ca.com> wrote: > >> > Two proposed security modules require the ability to > >> > share security blobs with existing "major" security modules. > >> > These modules, S.A.R.A and LandLock, provide significantly > >> > different services than SELinux, Smack or AppArmor. Using > >> > either in conjunction with the existing modules is quite > >> > reasonable. S.A.R.A requires access to the cred blob, while > >> > LandLock uses the cred, file and inode blobs. > >> > > >> > The use of the cred, file and inode blobs has been > >> > abstracted in preceding patches in the series. This > >> > patch teaches the affected security modules how to access > >> > the part of the blob set aside for their use in the case > >> > where blobs are shared. The configuration option > >> > CONFIG_SECURITY_STACKING identifies systems where the > >> > blobs may be shared. > >> > > >> > The mechanism for selecting which security modules are > >> > active has been changed to allow non-conflicting "major" > >> > security modules to be used together. At this time the > >> > TOMOYO module can safely be used with any of the others. > >> > The two new modules would be non-conflicting as well. > >> > > >> > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > >> > --- > >> > Documentation/admin-guide/LSM/index.rst | 14 +++-- > >> > include/linux/lsm_hooks.h | 2 +- > >> > security/Kconfig | 81 +++++++++++++++++++++++++ > >> > security/apparmor/include/cred.h | 8 +++ > >> > security/apparmor/include/file.h | 9 ++- > >> > security/apparmor/include/lib.h | 4 ++ > >> > security/apparmor/lsm.c | 8 ++- > >> > security/security.c | 30 ++++++++- > >> > security/selinux/hooks.c | 3 +- > >> > security/selinux/include/objsec.h | 18 +++++- > >> > security/smack/smack.h | 19 +++++- > >> > security/smack/smack_lsm.c | 17 +++--- > >> > security/tomoyo/common.h | 12 +++- > >> > security/tomoyo/tomoyo.c | 3 +- > >> > 14 files changed, 200 insertions(+), 28 deletions(-) > > > > ... > > > >> > diff --git a/security/Kconfig b/security/Kconfig > >> > index 22f7664c4977..ed48025ae9e0 100644 > >> > --- a/security/Kconfig > >> > +++ b/security/Kconfig > >> > @@ -36,6 +36,28 @@ config SECURITY_WRITABLE_HOOKS > >> > bool > >> > default n > >> > > >> > +config SECURITY_STACKING > >> > + bool "Security module stacking" > >> > + depends on SECURITY > >> > + help > >> > + Allows multiple major security modules to be stacked. > >> > + Modules are invoked in the order registered with a > >> > + "bail on fail" policy, in which the infrastructure > >> > + will stop processing once a denial is detected. Not > >> > + all modules can be stacked. SELinux, Smack and AppArmor are > >> > + known to be incompatible. User space components may > >> > + have trouble identifying the security module providing > >> > + data in some cases. > >> > + > >> > + If you select this option you will have to select which > >> > + of the stackable modules you wish to be active. The > >> > + "Default security module" will be ignored. The boot line > >> > + "security=" option can be used to specify that one of > >> > + the modules identifed for stacking should be used instead > >> > + of the entire stack. > >> > + > >> > + If you are unsure how to answer this question, answer N. > >> > >> I don't see a good reason to make this a config. Why shouldn't this > >> always be enabled? > > > > I do. From a user perspective it is sometimes difficult to determine > > the reason behind a failed operation; its is a DAC based denial, the > > LSM, or some other failure? Stacking additional LSMs has the > > potential to make this worse. The boot time configuration adds to the > > complexity. > > Let me try to convince you otherwise. :) The reason I think there's no > need for this is because the only functional change here is how > _TOMOYO_ gets stacked. And in my proposal, we can convert TOMOYO to be > enabled/disabled like LoadPin. Given the configs I showed, stacking > TOMOYO with the other major LSMs becomes a config (and/or boottime) > option. > > The changes for TOMOYO are still needed even _with_ SECURITY_STACKING, > and I argue that the other major LSMs remain the same. It's only > infrastructure that has changed. So, I think having SECURITY_STACKING > actually makes things more complex internally (all the ifdefs, weird > enable logic) and for distros ("what's this stacking option", etc?) None of the above deals with the user experience or support burden a distro would have by forcing stacking on. If we make it an option the distros can choose for themselves; picking a kernel build config is not something new to distros, and I think Casey's text adequately explains CONFIG_SECURITY_STACKING in terms that would be sufficient. I currently have a neutral stance on stacking, making it mandatory pushes me more towards a "no". As far as the cpp ifdef's, and other conditionals are concerned, I remain unconvinced this is any worse than any other significant feature that is a build time option.
On Thursday, September 13, 2018 9:12 PM, Paul Moore <paul@paul-moore.com> wrote: > On Thu, Sep 13, 2018 at 11:19 AM Kees Cook keescook@chromium.org wrote: > > > On Thu, Sep 13, 2018 at 6:16 AM, Paul Moore paul@paul-moore.com wrote: > > > > > On Thu, Sep 13, 2018 at 12:19 AM Kees Cook keescook@chromium.org wrote: > > > > > > > On Tue, Sep 11, 2018 at 9:42 AM, Casey Schaufler casey@schaufler-ca.com wrote: > > > > > > > > > Two proposed security modules require the ability to > > > > > share security blobs with existing "major" security modules. > > > > > These modules, S.A.R.A and LandLock, provide significantly > > > > > different services than SELinux, Smack or AppArmor. Using > > > > > either in conjunction with the existing modules is quite > > > > > reasonable. S.A.R.A requires access to the cred blob, while > > > > > LandLock uses the cred, file and inode blobs. > > > > > The use of the cred, file and inode blobs has been > > > > > abstracted in preceding patches in the series. This > > > > > patch teaches the affected security modules how to access > > > > > the part of the blob set aside for their use in the case > > > > > where blobs are shared. The configuration option > > > > > CONFIG_SECURITY_STACKING identifies systems where the > > > > > blobs may be shared. > > > > > The mechanism for selecting which security modules are > > > > > active has been changed to allow non-conflicting "major" > > > > > security modules to be used together. At this time the > > > > > TOMOYO module can safely be used with any of the others. > > > > > The two new modules would be non-conflicting as well. > > > > > > > > > > Signed-off-by: Casey Schaufler casey@schaufler-ca.com > > > > > > > > > > ------------------------------------------------------ > > > > > > > > > > Documentation/admin-guide/LSM/index.rst | 14 +++-- > > > > > include/linux/lsm_hooks.h | 2 +- > > > > > security/Kconfig | 81 +++++++++++++++++++++++++ > > > > > security/apparmor/include/cred.h | 8 +++ > > > > > security/apparmor/include/file.h | 9 ++- > > > > > security/apparmor/include/lib.h | 4 ++ > > > > > security/apparmor/lsm.c | 8 ++- > > > > > security/security.c | 30 ++++++++- > > > > > security/selinux/hooks.c | 3 +- > > > > > security/selinux/include/objsec.h | 18 +++++- > > > > > security/smack/smack.h | 19 +++++- > > > > > security/smack/smack_lsm.c | 17 +++--- > > > > > security/tomoyo/common.h | 12 +++- > > > > > security/tomoyo/tomoyo.c | 3 +- > > > > > 14 files changed, 200 insertions(+), 28 deletions(-) > > > > > > ... > > > > > > > > diff --git a/security/Kconfig b/security/Kconfig > > > > > index 22f7664c4977..ed48025ae9e0 100644 > > > > > --- a/security/Kconfig > > > > > +++ b/security/Kconfig > > > > > @@ -36,6 +36,28 @@ config SECURITY_WRITABLE_HOOKS > > > > > bool > > > > > default n > > > > > +config SECURITY_STACKING > > > > > > > > > > - bool "Security module stacking" > > > > > > > > > > > > > > > - depends on SECURITY > > > > > > > > > > > > > > > - help > > > > > > > > > > > > > > > - Allows multiple major security modules to be stacked. > > > > > > > > > > > > > > > - Modules are invoked in the order registered with a > > > > > > > > > > > > > > > - "bail on fail" policy, in which the infrastructure > > > > > > > > > > > > > > > - will stop processing once a denial is detected. Not > > > > > > > > > > > > > > > - all modules can be stacked. SELinux, Smack and AppArmor are > > > > > > > > > > > > > > > - known to be incompatible. User space components may > > > > > > > > > > > > > > > - have trouble identifying the security module providing > > > > > > > > > > > > > > > - data in some cases. > > > > > > > > > > > > > > > - > > > > > - If you select this option you will have to select which > > > > > > > > > > > > > > > - of the stackable modules you wish to be active. The > > > > > > > > > > > > > > > - "Default security module" will be ignored. The boot line > > > > > > > > > > > > > > > - "security=" option can be used to specify that one of > > > > > > > > > > > > > > > - the modules identifed for stacking should be used instead > > > > > > > > > > > > > > > - of the entire stack. > > > > > > > > > > > > > > > - > > > > > - If you are unsure how to answer this question, answer N. > > > > > > > > > > > > > > > > > > I don't see a good reason to make this a config. Why shouldn't this > > > > always be enabled? > > > > > > I do. From a user perspective it is sometimes difficult to determine > > > the reason behind a failed operation; its is a DAC based denial, the > > > LSM, or some other failure? Stacking additional LSMs has the > > > potential to make this worse. The boot time configuration adds to the > > > complexity. > > > > Let me try to convince you otherwise. :) The reason I think there's no > > need for this is because the only functional change here is how > > TOMOYO gets stacked. And in my proposal, we can convert TOMOYO to be > > enabled/disabled like LoadPin. Given the configs I showed, stacking > > TOMOYO with the other major LSMs becomes a config (and/or boottime) > > option. > > The changes for TOMOYO are still needed even with SECURITY_STACKING, > > and I argue that the other major LSMs remain the same. It's only > > infrastructure that has changed. So, I think having SECURITY_STACKING > > actually makes things more complex internally (all the ifdefs, weird > > enable logic) and for distros ("what's this stacking option", etc?) > > None of the above deals with the user experience or support burden a > distro would have by forcing stacking on. If we make it an option the > distros can choose for themselves; picking a kernel build config is > not something new to distros, and I think Casey's text adequately > explains CONFIG_SECURITY_STACKING in terms that would be sufficient. CONFIG_SECURITY_STACKING doesn't make any user visible changes on itself as it doesn't automatically enable any new LSM. The LSM specific configs are place where users/distros make decisions. If there is only one LSM enabled to run then there's nothing to stack. If someone choose to run two or more LSM in config/boot cmdline then we can assume having it stacked is what they wanted. As Kees pointed there is already CONFIG_SECURITY_DEFAULT_XXX. In both cases CONFIG_SECURITY_STACKING is redundant and only adds burden instead of removing it. > I currently have a neutral stance on stacking, making it mandatory > pushes me more towards a "no". > This implies that your real concern is something else than CONFIG_SECURITY_STACKING which only allows you to ignore the whole thing. Please reveal it. There are a lot of people waiting for LSM stacking which is several years late and it would be great to resolve potential issues earlier rather later. > As far as the cpp ifdef's, and other conditionals are concerned, I > remain unconvinced this is any worse than any other significant > feature that is a build time option. > > paul moore Jordan
On Thu, Sep 13, 2018 at 12:12 PM, Paul Moore <paul@paul-moore.com> wrote: > None of the above deals with the user experience or support burden a > distro would have by forcing stacking on. If we make it an option the Just to make sure we're clear here: this series does not provide "extreme" stacking: SELinux, AppArmor, and SMACK remain boot-exclusive no matter what the CONFIGs. > distros can choose for themselves; picking a kernel build config is > not something new to distros, and I think Casey's text adequately > explains CONFIG_SECURITY_STACKING in terms that would be sufficient. I absolutely want stacking to be configurable, but I want to point out that there is no operational difference between CONFIG_SECURITY_STACKING=n and CONFIG_SECURITY_STACKING=y in the code here: - all the new accessor and allocation code is exercised in both cases - with stacking enabled: selinux, apparmor, and smack have an offset of 0 into blobs (and only one can be enabled at a time) - with stacking disabled: selinux, apparmor, and smack have an offset of 0 into blobs (and only one can be enabled at a time) The only behavioral difference is TOMOYO: 1- with stacking disabled and TOMOYO as the only major LSM, it will have a 0 offset into blobs (like above) 2- with stacking enabled and TOMOYO as the only major LSM, it will have a 0 offset into blobs (like above) 3- with stacking disabled and another major LSM is enabled, TOMOYO will be disabled (like always) 4- with stacking enabled and another major LSM is enabled, TOMOYO will have a non-0 offset into blobs and will run after selinux or smack or run before apparmor (based on link ordering defined by the Makefile). Note that cases 1, 2, and 3 are identical in behavior to before this series. Only case 4 is different, which is why I'm saying that instead of creating a redundant and needlessly complex config, or reinventing the "enable" wheel, we should simply drop the no-op CONFIG_SECURITY_STACKING config and provide TOMOYO with an "enable" parameter (and CONFIG). And it should be _separate_ from the "security=" line. This will be the SAME outcome for distros: if they want stacking, they choose the "enable TOMOYO by default" CONFIG. If they don't want stacking, they don't. > I currently have a neutral stance on stacking, making it mandatory > pushes me more towards a "no". This is why I'm trying to explain myself: the infrastructure proposed here is always exercised, no matter the CONFIG. From that sense it is "mandatory" no matter what the config is. There isn't a reality where you could "turn off stacking", because it's not stacking until you actually stack something, and that will be disabled by default as I've proposed it. Let me put this another way: if we simply leave off patch 10, we can take the other 9 patches (modulo feedback), and we only have to decide how to expose "stacking"; all the infrastructure work for supporting it is done. I'm arguing that "security=" is likely insufficient to describe what we want, and instead we should focus on individual LSM enablement via parameters ("tomoyo.enabled=1"). If _ordering_ becomes an issue, we could either use parameter order, or use "security=" again maybe, but for now, ordering is already defined by the Makefile (and security/security.c). "Stacking" only exists if you try to enable one of [selinux, apparmor, or smack] AND tomoyo. CONFIG_SECURITY_STACKING is redundant: if you want to disable stacking, you just disable tomoyo if you have another LSM (which we can already enforce in the Kconfig). If you want something more explicit than per-LSM config, then a simple "security.lsm_stack=1/0" with a CONFIG for the default would be fine. I'm trying to argue against what appears to be needless complexity around CONFIG_SECURITY_STACK as it was proposed, since it doesn't provide a meaningful operational change, since exclusivity of major LSMs is already handled. > As far as the cpp ifdef's, and other conditionals are concerned, I > remain unconvinced this is any worse than any other significant > feature that is a build time option. That's fine. That's just a code style issue. What I'm trying to show is that by lifting the allocation logic up out of the LSMs, we've actually simplified the logic. The "stacking" part will only become a distro-choice once they knowingly enable TOMOYO or build in SARA and/or Landlock in the future. -Kees
On Thu, Sep 13, 2018 at 5:01 PM Kees Cook <keescook@chromium.org> wrote: > On Thu, Sep 13, 2018 at 12:12 PM, Paul Moore <paul@paul-moore.com> wrote: > > None of the above deals with the user experience or support burden a > > distro would have by forcing stacking on. If we make it an option the > > Just to make sure we're clear here: this series does not provide > "extreme" stacking: SELinux, AppArmor, and SMACK remain boot-exclusive > no matter what the CONFIGs. > > > distros can choose for themselves; picking a kernel build config is > > not something new to distros, and I think Casey's text adequately > > explains CONFIG_SECURITY_STACKING in terms that would be sufficient. > > I absolutely want stacking to be configurable, but I want to point out > that there is no operational difference between > CONFIG_SECURITY_STACKING=n and CONFIG_SECURITY_STACKING=y in the code > here: > > - all the new accessor and allocation code is exercised in both cases > > - with stacking enabled: selinux, apparmor, and smack have an offset > of 0 into blobs (and only one can be enabled at a time) > > - with stacking disabled: selinux, apparmor, and smack have an offset > of 0 into blobs (and only one can be enabled at a time) > > The only behavioral difference is TOMOYO: > > 1- with stacking disabled and TOMOYO as the only major LSM, it will > have a 0 offset into blobs (like above) > > 2- with stacking enabled and TOMOYO as the only major LSM, it will > have a 0 offset into blobs (like above) > > 3- with stacking disabled and another major LSM is enabled, TOMOYO > will be disabled (like always) > > 4- with stacking enabled and another major LSM is enabled, TOMOYO will > have a non-0 offset into blobs and will run after selinux or smack or > run before apparmor (based on link ordering defined by the Makefile). Case #3/#4 is what I'm getting at, and I would argue demonstrates an operational difference that is user visible/configurable. Unless something has changed and I missed it, you can currently build all of the LSMs into a single kernel image, and the admin/user can choose one at boot time. CONFIG_SECURITY_STACKING=y enables the admin/user to stack LSMs (albeit with restrictions in the current iteration) and CONFIG_SECURITY_STACKING=n limits the admin/user to a single LSM (what we have now). I understand that as of this moment we are talking only about TOMOYO and AppArmor/Smack/SELinux, but everyone knows that S.A.R.A/SARA and LandLock are going to follow shortly - that's the whole point of this latest spin, isn't it? > > I currently have a neutral stance on stacking, making it mandatory > > pushes me more towards a "no". > > This is why I'm trying to explain myself: the infrastructure proposed > here is always exercised, no matter the CONFIG. From that sense it is > "mandatory" no matter what the config is. There isn't a reality where > you could "turn off stacking", because it's not stacking until you > actually stack something, and that will be disabled by default as I've > proposed it. > > Let me put this another way: if we simply leave off patch 10, we can > take the other 9 patches (modulo feedback), and we only have to decide > how to expose "stacking"; all the infrastructure work for supporting > it is done. > > I'm arguing that "security=" is likely insufficient to describe what > we want, and instead we should focus on individual LSM enablement via > parameters ("tomoyo.enabled=1"). If _ordering_ becomes an issue, we > could either use parameter order, or use "security=" again maybe, but > for now, ordering is already defined by the Makefile (and > security/security.c). The infrastructure bits aren't really my concern; in fact I *like* that the infrastructure is always exercised, it makes testing/debugging easier. I also like the ability to limit the user/admin to one LSM at boot time to make support easier; my goal is to allow a distro to build support for multiple LSMs without also requiring that distro to support *stacked* LSMs (see my earlier comments about the difficulty in determining the source of a failed operation).
On Thu, Sep 13, 2018 at 4:58 PM Jordan Glover <Golden_Miller83@protonmail.ch> wrote: > > On Thursday, September 13, 2018 9:12 PM, Paul Moore <paul@paul-moore.com> wrote: > > > On Thu, Sep 13, 2018 at 11:19 AM Kees Cook keescook@chromium.org wrote: > > > > > On Thu, Sep 13, 2018 at 6:16 AM, Paul Moore paul@paul-moore.com wrote: > > > > > > > On Thu, Sep 13, 2018 at 12:19 AM Kees Cook keescook@chromium.org wrote: ... > > > > > I don't see a good reason to make this a config. Why shouldn't this > > > > > always be enabled? > > > > > > > > I do. From a user perspective it is sometimes difficult to determine > > > > the reason behind a failed operation; its is a DAC based denial, the > > > > LSM, or some other failure? Stacking additional LSMs has the > > > > potential to make this worse. The boot time configuration adds to the > > > > complexity. > > > > > > Let me try to convince you otherwise. :) The reason I think there's no > > > need for this is because the only functional change here is how > > > TOMOYO gets stacked. And in my proposal, we can convert TOMOYO to be > > > enabled/disabled like LoadPin. Given the configs I showed, stacking > > > TOMOYO with the other major LSMs becomes a config (and/or boottime) > > > option. > > > The changes for TOMOYO are still needed even with SECURITY_STACKING, > > > and I argue that the other major LSMs remain the same. It's only > > > infrastructure that has changed. So, I think having SECURITY_STACKING > > > actually makes things more complex internally (all the ifdefs, weird > > > enable logic) and for distros ("what's this stacking option", etc?) > > > > None of the above deals with the user experience or support burden a > > distro would have by forcing stacking on. If we make it an option the > > distros can choose for themselves; picking a kernel build config is > > not something new to distros, and I think Casey's text adequately > > explains CONFIG_SECURITY_STACKING in terms that would be sufficient. > > CONFIG_SECURITY_STACKING doesn't make any user visible changes on > itself as it doesn't automatically enable any new LSM. The LSM > specific configs are place where users/distros make decisions. If > there is only one LSM enabled to run then there's nothing to stack. > If someone choose to run two or more LSM in config/boot cmdline > then we can assume having it stacked is what they wanted. As Kees > pointed there is already CONFIG_SECURITY_DEFAULT_XXX. In both cases > CONFIG_SECURITY_STACKING is redundant and only adds burden instead > of removing it. See my last response to Kees. > > I currently have a neutral stance on stacking, making it mandatory > > pushes me more towards a "no". > > This implies that your real concern is something else than > CONFIG_SECURITY_STACKING which only allows you to ignore the whole > thing. Please reveal it. There are a lot of people waiting for LSM > stacking which is several years late and it would be great to > resolve potential issues earlier rather later. What? I resent the implication that I'm hiding anything; there are a lot of fair criticisms you could level at me, but I take offense at the idea that I'm not being honest here. I've been speaking with Casey, John, and others about stacking for years, both on-list and in-person at conferences, and my neutral-opinion-just-make-it-work-for-everything-and-make-it-optional stance has been pretty consistent and isn't new. Also, let's be really clear here: I'm only asking that stacking be made a build time option (as it is in Casey's patchset). That seems like a pretty modest ask for something so significant and "several years late" as you put it.
On Thu, Sep 13, 2018 at 2:38 PM, Paul Moore <paul@paul-moore.com> wrote: > The infrastructure bits aren't really my concern; in fact I *like* > that the infrastructure is always exercised, it makes > testing/debugging easier. I also like the ability to limit the > user/admin to one LSM at boot time to make support easier; my goal is > to allow a distro to build support for multiple LSMs without also > requiring that distro to support *stacked* LSMs I see your point, but as soon as SARA and Landlock appear, they'll have: depends on SECURITY_STACKING and then all distros will enable it and there will be no sensible runtime way to manage it. If, instead, we make it entirely runtime now, then a CONFIG can control the default state and we can provide guidance to how SARA and Landlock should expose their "enable"ness. At the very least, to avoid stacking now (i.e. TOMOYO being enabled with another major LSM), we just do nothing. The existing code already makes the existing major LSMs exclusive. Adding a stackable LSM would need to just have its own "enable" flag (i.e. ignore security_module_enable()), and then either check a runtime "is stacking allowed?" flag or have new "depends on SECURITY_STACKING". (I think the CONFIG will force distros into enabling it without any runtime opt-out.) > (see my earlier > comments about the difficulty in determining the source of a failed > operation). Agreed. I would hope that audit could help for that case. *stare at blue sky* -Kees
On Thursday, September 13, 2018 11:50 PM, Paul Moore <paul@paul-moore.com> wrote: > On Thu, Sep 13, 2018 at 4:58 PM Jordan Glover > Golden_Miller83@protonmail.ch wrote: > > > On Thursday, September 13, 2018 9:12 PM, Paul Moore paul@paul-moore.com wrote: > > > > > On Thu, Sep 13, 2018 at 11:19 AM Kees Cook keescook@chromium.org wrote: > > > > > > > On Thu, Sep 13, 2018 at 6:16 AM, Paul Moore paul@paul-moore.com wrote: > > > > > > > > > On Thu, Sep 13, 2018 at 12:19 AM Kees Cook keescook@chromium.org wrote: > > ... > > > > > > > I don't see a good reason to make this a config. Why shouldn't this > > > > > > always be enabled? > > > > > > > > > > I do. From a user perspective it is sometimes difficult to determine > > > > > the reason behind a failed operation; its is a DAC based denial, the > > > > > LSM, or some other failure? Stacking additional LSMs has the > > > > > potential to make this worse. The boot time configuration adds to the > > > > > complexity. > > > > > > > > Let me try to convince you otherwise. :) The reason I think there's no > > > > need for this is because the only functional change here is how > > > > TOMOYO gets stacked. And in my proposal, we can convert TOMOYO to be > > > > enabled/disabled like LoadPin. Given the configs I showed, stacking > > > > TOMOYO with the other major LSMs becomes a config (and/or boottime) > > > > option. > > > > The changes for TOMOYO are still needed even with SECURITY_STACKING, > > > > and I argue that the other major LSMs remain the same. It's only > > > > infrastructure that has changed. So, I think having SECURITY_STACKING > > > > actually makes things more complex internally (all the ifdefs, weird > > > > enable logic) and for distros ("what's this stacking option", etc?) > > > > > > None of the above deals with the user experience or support burden a > > > distro would have by forcing stacking on. If we make it an option the > > > distros can choose for themselves; picking a kernel build config is > > > not something new to distros, and I think Casey's text adequately > > > explains CONFIG_SECURITY_STACKING in terms that would be sufficient. > > > > CONFIG_SECURITY_STACKING doesn't make any user visible changes on > > itself as it doesn't automatically enable any new LSM. The LSM > > specific configs are place where users/distros make decisions. If > > there is only one LSM enabled to run then there's nothing to stack. > > If someone choose to run two or more LSM in config/boot cmdline > > then we can assume having it stacked is what they wanted. As Kees > > pointed there is already CONFIG_SECURITY_DEFAULT_XXX. In both cases > > CONFIG_SECURITY_STACKING is redundant and only adds burden instead > > of removing it. > > See my last response to Kees. > > > > I currently have a neutral stance on stacking, making it mandatory > > > pushes me more towards a "no". > > > > This implies that your real concern is something else than > > CONFIG_SECURITY_STACKING which only allows you to ignore the whole > > thing. Please reveal it. There are a lot of people waiting for LSM > > stacking which is several years late and it would be great to > > resolve potential issues earlier rather later. > > What? I resent the implication that I'm hiding anything; there are a > lot of fair criticisms you could level at me, but I take offense at > the idea that I'm not being honest here. I've been speaking with > Casey, John, and others about stacking for years, both on-list and > in-person at conferences, and my > neutral-opinion-just-make-it-work-for-everything-and-make-it-optional > stance has been pretty consistent and isn't new. > > Also, let's be really clear here: I'm only asking that stacking be > made a build time option (as it is in Casey's patchset). That seems > like a pretty modest ask for something so significant and "several > years late" as you put it. > > paul moore Fair enough. I apologize then. Jordan
On 9/13/2018 2:50 PM, Paul Moore wrote: > On Thu, Sep 13, 2018 at 4:58 PM Jordan Glover > <Golden_Miller83@protonmail.ch> wrote: > >> This implies that your real concern is something else than >> CONFIG_SECURITY_STACKING which only allows you to ignore the whole >> thing. Please reveal it. There are a lot of people waiting for LSM >> stacking which is several years late and it would be great to >> resolve potential issues earlier rather later. It would be really handy if the "lot of people" were a lot more vocal about their impatience. Keeping the stacking work on the stove, much less on a front burner, hasn't always been easy. > What? I resent the implication that I'm hiding anything; there are a > lot of fair criticisms you could level at me, but I take offense at > the idea that I'm not being honest here. I've been speaking with > Casey, John, and others about stacking for years, both on-list and > in-person at conferences, and my > neutral-opinion-just-make-it-work-for-everything-and-make-it-optional > stance has been pretty consistent and isn't new. Paul has always been quite upfront about this, and responsive as well. I won't say that we always agree because we don't, but I don't have a good argument against either point. > Also, let's be really clear here: I'm only asking that stacking be > made a build time option (as it is in Casey's patchset). That seems > like a pretty modest ask for something so significant and "several > years late" as you put it. There's a significant difference between something taking a long time and something being late. I hope that I haven't given anyone the impression that I'd have this finished years ago. If so, I owe whoever that was a beer. This patch set may look deceptively straight forward, but there have been many heavy branches pruned from the tree. This subset of the total change for "extreme" stacking represents the easy part. Without a road map for completing the task (i.e. any/all modules together) Paul's hesitation to take anything is defensible, and the desire that it be configurable reasonable.
On Thu, Sep 13, 2018 at 2:51 PM, Kees Cook <keescook@chromium.org> wrote: > At the very least, to avoid stacking now (i.e. TOMOYO being enabled > with another major LSM), we just do nothing. The existing code already > makes the existing major LSMs exclusive. Adding a stackable LSM would > need to just have its own "enable" flag (i.e. ignore > security_module_enable()), and then either check a runtime "is > stacking allowed?" flag or have new "depends on SECURITY_STACKING". (I > think the CONFIG will force distros into enabling it without any > runtime opt-out.) Before stacking, we have: - major LSM, pick one - all CONFIG minor LSMs, in security.c order There are two minor LSMs: Yama and LoadPin. If built, Yama is always on (though it has sysctl knobs). If built, LoadPin is controlled by a boot param. Picking the major LSM happens via "security=$LSM" and a per-LSM check of security_module_enable("$LSM"). Ordering is major, then per security.c for minors. Under stacking, we have: The minor LSMs remain unchanged. We don't have SARA and Landlock yet, but we do have TOMOYO, which we can use as an example to future "compatible blob-using LSMs". I see two issues: - how to determine which set of LSMs are enabled at boot - how to determine the ORDER of the LSMs Casey's implementation does this (correct me if I'm wrong): The minor LSMs remain unchanged. SECURITY_$lsm_STACKED determines which major is enabled, with SECURITY_TOMOYO_STACKED allowed in addition. If security= is specified, all other major LSMs are disabled (i.e. it is not possible to switch between SELinux/AppArmor/SMACK without also disabling TOMOYO). Ordering is per security/Makefile modulo enabled-ness for majors (i.e. TOMOYO is always _before_ AppArmor if stacked together, otherwise after SELinux and SMACK), and per security.c for minors. I don't think this is how we want it to work. For example, Ubuntu builds in all LSMs, and default-enables AppArmor. If an Ubuntu user wants TOMOYO, the boot with "security=tomoyo". If Ubuntu wants to make stacking available to users but off by default, what CONFIGs do they pick? They could try SECURITY_APPARMOR_STACKED=y and SECURITY_TOMOYO_STACKED=n, but then how does an end user choose "apparmor and tomoyo" (or more meaningfully, for the future: "apparmor, sara, and landlock")? They can still pick "security=tomoyo", but there isn't a runtime way to opt into stacking. What about leaving SECURITY_$lsm_DEFAULT as-is, and then... In the past I'd suggested using "security=" to determine both enabled and order: "security=tomoyo,smack" would mean stacked LSMs, with tomoyo going first. Currently I'm leaning towards "security=" to select ONLY the incompatible LSM, and per-LSM "enable" flags to determine stacking: tomoyo.enabled=1 security=smack This doesn't explicitly address ordering, though. If we made param _position_ meaningful, then we could get ordering (i.e. above would mean "tomoyo first"). Note, ordering matters because call_int_hook() will _stop_ on a non-zero return value: potentially hiding events from later LSMs. Do we need to revisit this? I seem to remember if being a very dead horse, and we needed to quick-abort otherwise we ended up in nonsensical states. The reason for the new approach is because I can't find a meaningful way to provide CONFIGs that make sense. We want to provide a few things: - is an LSM built into the kernel at all? (CONFIG_SECURITY_$lsm) - is an LSM enabled by default? (CONFIG_SECURITY_$lsm_ENABLED?) - has an LSM been enable for this boot? $lsm.enabled=1 or security=$lsm,$lsm ? - what order should any stacking happen? Makefile? security=? And for the incompatible-major, do we stick with CONFIG_SECURITY_$lsm_DEFAULT ? Anyway, if the concern is with exposed behavior for distros, what do we want? i.e. what should be done for patch 10. Everything else looks good. -Kees
On 09/13/2018 04:06 PM, Kees Cook wrote: > On Thu, Sep 13, 2018 at 2:51 PM, Kees Cook <keescook@chromium.org> wrote: >> At the very least, to avoid stacking now (i.e. TOMOYO being enabled >> with another major LSM), we just do nothing. The existing code already >> makes the existing major LSMs exclusive. Adding a stackable LSM would >> need to just have its own "enable" flag (i.e. ignore >> security_module_enable()), and then either check a runtime "is >> stacking allowed?" flag or have new "depends on SECURITY_STACKING". (I >> think the CONFIG will force distros into enabling it without any >> runtime opt-out.) > > Before stacking, we have: > > - major LSM, pick one > - all CONFIG minor LSMs, in security.c order > > There are two minor LSMs: Yama and LoadPin. If built, Yama is always > on (though it has sysctl knobs). If built, LoadPin is controlled by a > boot param. > > Picking the major LSM happens via "security=$LSM" and a per-LSM check > of security_module_enable("$LSM"). > > Ordering is major, then per security.c for minors. > > > Under stacking, we have: > > The minor LSMs remain unchanged. > > We don't have SARA and Landlock yet, but we do have TOMOYO, which we > can use as an example to future "compatible blob-using LSMs". > > I see two issues: > > - how to determine which set of LSMs are enabled at boot > - how to determine the ORDER of the LSMs > > > Casey's implementation does this (correct me if I'm wrong): > > The minor LSMs remain unchanged. > > SECURITY_$lsm_STACKED determines which major is enabled, with > SECURITY_TOMOYO_STACKED allowed in addition. If security= is > specified, all other major LSMs are disabled (i.e. it is not possible > to switch between SELinux/AppArmor/SMACK without also disabling > TOMOYO). > > Ordering is per security/Makefile modulo enabled-ness for majors (i.e. > TOMOYO is always _before_ AppArmor if stacked together, otherwise > after SELinux and SMACK), and per security.c for minors. > > > I don't think this is how we want it to work. For example, Ubuntu > builds in all LSMs, and default-enables AppArmor. If an Ubuntu user > wants TOMOYO, the boot with "security=tomoyo". If Ubuntu wants to make > stacking available to users but off by default, what CONFIGs do they > pick? They could try SECURITY_APPARMOR_STACKED=y and > SECURITY_TOMOYO_STACKED=n, but then how does an end user choose > "apparmor and tomoyo" (or more meaningfully, for the future: > "apparmor, sara, and landlock")? They can still pick > "security=tomoyo", but there isn't a runtime way to opt into stacking. > Ubuntu is carrying an early set of the stacking patches and about a dozen patches on top of that. It allows choosing to build the LSMs separate from the what the default LSM(s) are. There is some config patching here that I need to update and drop onto this thread. There is also a patch that allows selecting the set of LSMs at boot. security=selinux,apparmor but yama and loadpin are treated differently and I never liked that. But again another patch to rebase and drop on this thread for discussion. > > What about leaving SECURITY_$lsm_DEFAULT as-is, and then... > > In the past I'd suggested using "security=" to determine both enabled > and order: "security=tomoyo,smack" would mean stacked LSMs, with > tomoyo going first. > > Currently I'm leaning towards "security=" to select ONLY the > incompatible LSM, and per-LSM "enable" flags to determine stacking: > > tomoyo.enabled=1 security=smack > I don't like this, it makes it hard for the end user specifying option at boot time. They have to set security= for the modules they want and then also individual modules $lsm.enabled= which is very inconvenient for users. apparmor.enabled really should only default to enabled and provides a legacy way to disable apparmor during boot. This option was around before security= was added. I need to leave it available as its become part of the api (there are applications checking /sys/module/apparmor/parameters/enabled but we can remove it from the Kconfig. > This doesn't explicitly address ordering, though. If we made param > _position_ meaningful, then we could get ordering (i.e. above would > mean "tomoyo first"). > yes, I think that makes sense > Note, ordering matters because call_int_hook() will _stop_ on a > non-zero return value: potentially hiding events from later LSMs. Do > we need to revisit this? I seem to remember if being a very dead > horse, and we needed to quick-abort otherwise we ended up in > nonsensical states. > we shouldn't be in nonsensical states but we are doing work that is thrown away. Another potential solution is allowing an LSM to register a set of post hooks that will get called after the regular LSM hooks are called. This would allow an LSM that cared about whether a hook succeeded or not to do some processing. > The reason for the new approach is because I can't find a meaningful > way to provide CONFIGs that make sense. We want to provide a few > things: > > - is an LSM built into the kernel at all? (CONFIG_SECURITY_$lsm) > - is an LSM enabled by default? (CONFIG_SECURITY_$lsm_ENABLED?) > - has an LSM been enable for this boot? $lsm.enabled=1 or security=$lsm,$lsm ? at least for apparmor it is apparmor.enabled=1 AND security=apparmor or security=$lsm,apparmor ... > - what order should any stacking happen? Makefile? security=? > Preferably not. For the single LSM we have the ability to choose the default LSM, ideally we let the distro decide in the Kconfig and the user with security=... > And for the incompatible-major, do we stick with CONFIG_SECURITY_$lsm_DEFAULT ? Ideally Kconfig wouldn't let you choose incompatible LSMs when selecting what is going to be enabled by default. > > > > Anyway, if the concern is with exposed behavior for distros, what do > we want? i.e. what should be done for patch 10. Everything else looks > good. > well speaking for Ubuntu we want to be able to specify the default LSMs and we want to enable our users to change this. The current patchset we have is less than ideal partly because of the whole $lsm.enabled vs security= mess that needs to be resolved.
On 9/13/2018 4:06 PM, Kees Cook wrote: > On Thu, Sep 13, 2018 at 2:51 PM, Kees Cook <keescook@chromium.org> wrote: >> At the very least, to avoid stacking now (i.e. TOMOYO being enabled >> with another major LSM), we just do nothing. The existing code already >> makes the existing major LSMs exclusive. Adding a stackable LSM would >> need to just have its own "enable" flag (i.e. ignore >> security_module_enable()), and then either check a runtime "is >> stacking allowed?" flag or have new "depends on SECURITY_STACKING". (I >> think the CONFIG will force distros into enabling it without any >> runtime opt-out.) > Before stacking, we have: > > - major LSM, pick one - major LSM, pick at most one > - all CONFIG minor LSMs, in security.c order > > There are two minor LSMs: Yama and LoadPin. If built, Yama is always > on (though it has sysctl knobs). If built, LoadPin is controlled by a > boot param. > > Picking the major LSM happens via "security=$LSM" and a per-LSM check > of security_module_enable("$LSM"). > > Ordering is major, then per security.c for minors. Modules are invoked in the order they are registered. Minor modules are registered first, so they are invoked before the major module. This is important because capability comes first. > Under stacking, we have: > > The minor LSMs remain unchanged. It wouldn't hurt to use the same registration process for minor modules as major modules now that you can register more than one. I haven't changed that because it's unnecessary and just adds pointless overhead. If we adopt some of the suggestions below, it may make sense to use the full-up process. > We don't have SARA and Landlock yet, but we do have TOMOYO, which we > can use as an example to future "compatible blob-using LSMs". > > I see two issues: > > - how to determine which set of LSMs are enabled at boot > - how to determine the ORDER of the LSMs > > > Casey's implementation does this (correct me if I'm wrong): > > The minor LSMs remain unchanged. > > SECURITY_$lsm_STACKED determines which major is enabled, with > SECURITY_TOMOYO_STACKED allowed in addition. Mostly correct. SECURITY_$lsm_STACKED instructs the security module to register its hooks. Kconfig enforces that you can't specify more than one of SECURITY_SELINUX_STACKED, SECURITY_SMACK_STACKED, and SECURITY_APPARMOR_STACKED. > If security= is > specified, all other major LSMs are disabled (i.e. it is not possible > to switch between SELinux/AppArmor/SMACK without also disabling > TOMOYO). Correct. > Ordering is per security/Makefile modulo enabled-ness for majors (i.e. > TOMOYO is always _before_ AppArmor if stacked together, otherwise > after SELinux and SMACK), and per security.c for minors. Correct. > I don't think this is how we want it to work. For example, Ubuntu > builds in all LSMs, and default-enables AppArmor. If an Ubuntu user > wants TOMOYO, the boot with "security=tomoyo". If Ubuntu wants to make > stacking available to users but off by default, what CONFIGs do they > pick? They could try SECURITY_APPARMOR_STACKED=y and > SECURITY_TOMOYO_STACKED=n, but then how does an end user choose > "apparmor and tomoyo" (or more meaningfully, for the future: > "apparmor, sara, and landlock")? They can still pick > "security=tomoyo", but there isn't a runtime way to opt into stacking. Correct. Earlier versions of the stacking patchset allowed security=lsm1,lsm2,...,lsmN. I haven't retrofit that to the current module registration mechanism. I believe that John Johansen has a patch in Ubuntu for doing this. I don't have it handy. It's the obvious and backward compatible way to do it. > What about leaving SECURITY_$lsm_DEFAULT as-is, and then... That works when you're not stacking or only want one, but isn't sensible for the general case. > In the past I'd suggested using "security=" to determine both enabled > and order: "security=tomoyo,smack" would mean stacked LSMs, with > tomoyo going first. > > Currently I'm leaning towards "security=" to select ONLY the > incompatible LSM, and per-LSM "enable" flags to determine stacking: > > tomoyo.enabled=1 security=smack I know that I've seen a TOMOYO installation that explicitly sets security=tomoyo, so I don't see that working. > This doesn't explicitly address ordering, though. If we made param > _position_ meaningful, then we could get ordering (i.e. above would > mean "tomoyo first"). security.tomoyo=1 security.apparmor=2 security.landlock=5 could tell the boot that tomoyo comes 1st, apparmor 2nd, landlock 5th. The registration code would be smart enough to deal with the fact that there is no module 3rd or 4th. > Note, ordering matters because call_int_hook() will _stop_ on a > non-zero return value: potentially hiding events from later LSMs. Do > we need to revisit this? I seem to remember if being a very dead > horse, and we needed to quick-abort otherwise we ended up in > nonsensical states. I have done it both ways, and "bail on fail" is far simpler. If SELinux returns -EACCES and SARA returns -EPERM, which do you tell the user? > The reason for the new approach is because I can't find a meaningful > way to provide CONFIGs that make sense. We want to provide a few > things: > > - is an LSM built into the kernel at all? (CONFIG_SECURITY_$lsm) That's how it works today. > - is an LSM enabled by default? (CONFIG_SECURITY_$lsm_ENABLED?) That's how it's done in this patchset. > - has an LSM been enable for this boot? $lsm.enabled=1 or security=$lsm,$lsm ? Currently security=$lsm for one LSM. > - what order should any stacking happen? Makefile? security=? Makefile by default. > > And for the incompatible-major, do we stick with CONFIG_SECURITY_$lsm_DEFAULT ? > > > > Anyway, if the concern is with exposed behavior for distros, what do > we want? i.e. what should be done for patch 10. Everything else looks > good. > > -Kees >
On Thu, Sep 13, 2018 at 4:32 PM, John Johansen <john.johansen@canonical.com> wrote: > On 09/13/2018 04:06 PM, Kees Cook wrote: >> - what order should any stacking happen? Makefile? security=? >> > Preferably not. For the single LSM we have the ability to choose the default LSM, ideally we let the distro decide in the Kconfig and the user with security=... I can't find a non-crazy way to do this in Kconfig. Right now, if I threw out all the _DEFAULT stuff, I could do: config SECURITY_SELINUX_ENABLED bool "SELinux LSM enabled at boot time" depends on SECURITY_SELINUX depends on !SECURITY_APPARMOR_ENABLED && !SECURITY_SMACK_ENABLED default SECURITY_SELINUX config SECURITY_SMACK_ENABLED bool "SMACK LSM enabled at boot time" depends on SECURITY_SMACK depends on !SECURITY_APPARMOR_ENABLED && !SECURITY_SELINUX_ENABLED default SECURITY_SMACK config SECURITY_APPARMOR_ENABLED bool "AppArmor LSM enabled at boot time" depends on SECURITY_APPARMOR depends on !SECURITY_SMACK_ENABLED && !SECURITY_SELINUX_ENABLED default SECURITY_APPARMOR config SECURITY_TOMOYO_ENABLED bool "TOMOYO LSM enabled at boot time" depends on SECURITY_TOMOYO default y if !SECURITY_SELINUX_ENABLED && !SECURITY_SMACK_ENABLED && !SECURITY_APPARMOR_ENABLED config DEFAULT_SECURITY string default "selinux" if SECURITY_SELINUX_ENABLED default "smack" if SECURITY_SMACK_ENABLED default "apparmor" if SECURITY_APPARMOR_ENABLED default "tomoyo" if SECURITY_TOMOYO_ENABLED (As before CONFIG_DEFAULT_SECURITY basically means the effective "security=" contents. Reminder than Kconfig default are "first match", so tomoyo would only happen if all others are not enabled by default.) But this doesn't provide a way for Kconfig to declare the ordering of TOMOYO followed by SELinux. If we just declare ordering is a function of the Makefile, then the above would work as expected. The "conflicting major LSM" could be specified on "security=" and stacked could be enabled with $lsm.enable=1 (or disabled). So, before we can really make a decision, I think we have to decide: should ordering be arbitrary for even this level of stacking? -Kees
On Thu, Sep 13, 2018 at 4:51 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: > On 9/13/2018 4:06 PM, Kees Cook wrote: >> - what order should any stacking happen? Makefile? security=? > Makefile by default. Okay, if ordering is by Makefile and everyone dislikes my $lsm.enabled=0/1 thing, then these mean the same thing: security=selinux,tomoyo security=tomoyo,selinux i.e. order of security= is _ignored_ in favor of the Makefile ordering. That seems dangerous to accept input that is "out of order", though, if we ever DO want to make order definable. -Kees
On Thu, Sep 13, 2018 at 4:51 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: > On 9/13/2018 4:06 PM, Kees Cook wrote: >> If security= is >> specified, all other major LSMs are disabled (i.e. it is not possible >> to switch between SELinux/AppArmor/SMACK without also disabling >> TOMOYO). > > Correct. If we assume patch 10 is the way forward, how could we go about fixing this specific problem? -Kees
On 9/13/2018 4:51 PM, Kees Cook wrote: > On Thu, Sep 13, 2018 at 4:32 PM, John Johansen > <john.johansen@canonical.com> wrote: >> On 09/13/2018 04:06 PM, Kees Cook wrote: >>> - what order should any stacking happen? Makefile? security=? >>> >> Preferably not. For the single LSM we have the ability to choose the default LSM, ideally we let the distro decide in the Kconfig and the user with security=... > I can't find a non-crazy way to do this in Kconfig. Right now, if I > threw out all the _DEFAULT stuff, I could do: > > config SECURITY_SELINUX_ENABLED > bool "SELinux LSM enabled at boot time" > depends on SECURITY_SELINUX > depends on !SECURITY_APPARMOR_ENABLED && !SECURITY_SMACK_ENABLED > default SECURITY_SELINUX > > config SECURITY_SMACK_ENABLED > bool "SMACK LSM enabled at boot time" > depends on SECURITY_SMACK > depends on !SECURITY_APPARMOR_ENABLED && !SECURITY_SELINUX_ENABLED > default SECURITY_SMACK > > config SECURITY_APPARMOR_ENABLED > bool "AppArmor LSM enabled at boot time" > depends on SECURITY_APPARMOR > depends on !SECURITY_SMACK_ENABLED && !SECURITY_SELINUX_ENABLED > default SECURITY_APPARMOR > > config SECURITY_TOMOYO_ENABLED > bool "TOMOYO LSM enabled at boot time" > depends on SECURITY_TOMOYO > default y if !SECURITY_SELINUX_ENABLED && > !SECURITY_SMACK_ENABLED && !SECURITY_APPARMOR_ENABLED > > config DEFAULT_SECURITY > string > default "selinux" if SECURITY_SELINUX_ENABLED > default "smack" if SECURITY_SMACK_ENABLED > default "apparmor" if SECURITY_APPARMOR_ENABLED > default "tomoyo" if SECURITY_TOMOYO_ENABLED > > (As before CONFIG_DEFAULT_SECURITY basically means the effective > "security=" contents. Reminder than Kconfig default are "first match", > so tomoyo would only happen if all others are not enabled by default.) > > But this doesn't provide a way for Kconfig to declare the ordering of > TOMOYO followed by SELinux. If we just declare ordering is a function > of the Makefile, then the above would work as expected. The > "conflicting major LSM" could be specified on "security=" and stacked > could be enabled with $lsm.enable=1 (or disabled). > > So, before we can really make a decision, I think we have to decide: > should ordering be arbitrary for even this level of stacking? Do we have a case where it matters? I know that I could write a module that would have issues if one hook got called and another didn't because because a precursor module hook failed. I don't think that any of the existing modules have this problem.
On Thu, Sep 13, 2018 at 5:03 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: > On 9/13/2018 4:51 PM, Kees Cook wrote: >> So, before we can really make a decision, I think we have to decide: >> should ordering be arbitrary for even this level of stacking? > > Do we have a case where it matters? I know that I could write a > module that would have issues if one hook got called and another > didn't because because a precursor module hook failed. I don't > think that any of the existing modules have this problem. FWIW, I prefer having explicit ordering that cannot be changed at runtime. I'm just concerned about painting ourselves (further) into a corner with security= suddenly gaining ordering semantics, but maybe I can just ignore this and we can point and laugh at anyone who gets burned by some future change to making it order-sensitive. :) -Kees
On 9/13/2018 4:57 PM, Kees Cook wrote: > On Thu, Sep 13, 2018 at 4:51 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: >> On 9/13/2018 4:06 PM, Kees Cook wrote: >>> - what order should any stacking happen? Makefile? security=? >> Makefile by default. > Okay, if ordering is by Makefile and everyone dislikes my > $lsm.enabled=0/1 thing, then these mean the same thing: > > security=selinux,tomoyo > security=tomoyo,selinux > > i.e. order of security= is _ignored_ in favor of the Makefile ordering. No, I think that the two lines above should have a different execution order. If we really need to specify multiple modules at boot time that is what makes the most sense. It's a matter of mechanics and probably another pass during the init process, but it's doable. If we determine it's necessary for this stage it is just work.
On Thu, Sep 13, 2018 at 5:08 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: > On 9/13/2018 4:57 PM, Kees Cook wrote: >> On Thu, Sep 13, 2018 at 4:51 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: >>> On 9/13/2018 4:06 PM, Kees Cook wrote: >>>> - what order should any stacking happen? Makefile? security=? >>> Makefile by default. >> Okay, if ordering is by Makefile and everyone dislikes my >> $lsm.enabled=0/1 thing, then these mean the same thing: >> >> security=selinux,tomoyo >> security=tomoyo,selinux >> >> i.e. order of security= is _ignored_ in favor of the Makefile ordering. > > No, I think that the two lines above should have a different > execution order. If we really need to specify multiple modules > at boot time that is what makes the most sense. > > It's a matter of mechanics and probably another pass during the > init process, but it's doable. If we determine it's necessary for > this stage it is just work. We already have the minor LSMs that cannot change order. They aren't part of security= parsing either. To enable/disable LoadPin, you do "loadpin.enabled=1/0" separate from "security=". Should "blob-sharing" LSMs be like major LSMs or minor LSMs? If someone is booting with "security=selinux,tomoyo" and then SARA lands upstream, does that person have to explicitly add "sara" to their boot args, since they're doing a non-default list of LSMs? (I actually prefer the answer being "yes" here, FWIW, I just want to nail down the expectations.) -Kees
On Thu, Sep 13, 2018 at 5:52 PM Kees Cook <keescook@chromium.org> wrote: > On Thu, Sep 13, 2018 at 2:38 PM, Paul Moore <paul@paul-moore.com> wrote: > > The infrastructure bits aren't really my concern; in fact I *like* > > that the infrastructure is always exercised, it makes > > testing/debugging easier. I also like the ability to limit the > > user/admin to one LSM at boot time to make support easier; my goal is > > to allow a distro to build support for multiple LSMs without also > > requiring that distro to support *stacked* LSMs > > I see your point, but as soon as SARA and Landlock appear, they'll have: > > depends on SECURITY_STACKING > > and then all distros will enable it and there will be no sensible > runtime way to manage it. If, instead, we make it entirely runtime > now, then a CONFIG can control the default state and we can provide > guidance to how SARA and Landlock should expose their "enable"ness. I question why SARA and LandLock require stacking. While some LSMs may benefit from stacking, e.g. Yama, traditionally each LSM has been able to stand on its own. I think this is a quality that should be preserved. > > (see my earlier > > comments about the difficulty in determining the source of a failed > > operation). > > Agreed. I would hope that audit could help for that case. *stare at blue sky* *also staring at blue sky* Audit can help, but it is independent of the LSMs and not a hard requirement for all, and even when it is enabled the config might not be suitable to provide enough information to be helpful in this case.
On 9/13/2018 5:19 PM, Kees Cook wrote: > On Thu, Sep 13, 2018 at 5:08 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: >> On 9/13/2018 4:57 PM, Kees Cook wrote: >>> On Thu, Sep 13, 2018 at 4:51 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: >>>> On 9/13/2018 4:06 PM, Kees Cook wrote: >>>>> - what order should any stacking happen? Makefile? security=? >>>> Makefile by default. >>> Okay, if ordering is by Makefile and everyone dislikes my >>> $lsm.enabled=0/1 thing, then these mean the same thing: >>> >>> security=selinux,tomoyo >>> security=tomoyo,selinux >>> >>> i.e. order of security= is _ignored_ in favor of the Makefile ordering. >> No, I think that the two lines above should have a different >> execution order. If we really need to specify multiple modules >> at boot time that is what makes the most sense. >> >> It's a matter of mechanics and probably another pass during the >> init process, but it's doable. If we determine it's necessary for >> this stage it is just work. > We already have the minor LSMs that cannot change order. Are you saying that we don't have a mechanism to change the order, or that they wouldn't work right in a different order? Well, there's the capability module that has to be first. > They aren't > part of security= parsing either. True, but there's no reason now that we couldn't change that. Except for capability. Hmm. > To enable/disable LoadPin, you do > "loadpin.enabled=1/0" separate from "security=". SELinux and AppArmor have the same. > Should "blob-sharing" LSMs be like major LSMs or minor LSMs? I like the idea of changing the minor modules to do the full registration process. That would make them all the same. Except for capability. In any case, the "blob-sharing" LSMs need to do the full registration process to account for their blobs sizes, and that brings the "major" behavior along with it. > If someone is booting with "security=selinux,tomoyo" and then SARA > lands upstream, does that person have to explicitly add "sara" to > their boot args, since they're doing a non-default list of LSMs? Yes. security= is explicit. > (I actually prefer the answer being "yes" here, FWIW, I just want to > nail down the expectations.) For now let's leave the minor (capability, yama, loadpin) as they are, and require all new modules of any flavor to use full registration. We could consider something like security=$lsm # Stack with $lsm at priority 2 - Existing behavior $lsm.stacked=N # Add $lsm to the stack at priority N. Delete if N == 0 It's OK to specify "selinux.stacked=2" and "sara.stacked=2". Which gets called first is left up to the system to decide. Whatever the behavior is gets documented. Capability will always be first and have priority 1. It's OK to specify "smack.stacked=1". The default stack is determined by CONFIG_SECURITY_$lsm_STACKED at build time. CONFIG_SECURITY_$lsm_STACKED changes from a boolean to an integer value to establish the default hook order. /sys/kernel/security/lsm reports the modules in hook call order. /sys/kernel/security/lsm-stack reports the list with the hook call priority capability:1,yama:1,selinux:1,sara:5,landlack:17 If stacking is not configured $lsm.stacked=0 is treated as security=none. For other values of N $lsm.stacked=N is treated as security=$lsm.
On Thu, 13 Sep 2018, Casey Schaufler wrote: > On 9/13/2018 4:57 PM, Kees Cook wrote: > > On Thu, Sep 13, 2018 at 4:51 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: > >> On 9/13/2018 4:06 PM, Kees Cook wrote: > >>> - what order should any stacking happen? Makefile? security=? > >> Makefile by default. > > Okay, if ordering is by Makefile and everyone dislikes my > > $lsm.enabled=0/1 thing, then these mean the same thing: > > > > security=selinux,tomoyo > > security=tomoyo,selinux > > > > i.e. order of security= is _ignored_ in favor of the Makefile ordering. > > No, I think that the two lines above should have a different > execution order. If we really need to specify multiple modules > at boot time that is what makes the most sense. Agreed.
On 09/14/2018 11:18 AM, James Morris wrote: > On Thu, 13 Sep 2018, Casey Schaufler wrote: > >> On 9/13/2018 4:57 PM, Kees Cook wrote: >>> On Thu, Sep 13, 2018 at 4:51 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: >>>> On 9/13/2018 4:06 PM, Kees Cook wrote: >>>>> - what order should any stacking happen? Makefile? security=? >>>> Makefile by default. >>> Okay, if ordering is by Makefile and everyone dislikes my >>> $lsm.enabled=0/1 thing, then these mean the same thing: >>> >>> security=selinux,tomoyo >>> security=tomoyo,selinux >>> >>> i.e. order of security= is _ignored_ in favor of the Makefile ordering. >> >> No, I think that the two lines above should have a different >> execution order. If we really need to specify multiple modules >> at boot time that is what makes the most sense. > > Agreed. > > +1 partly because if order is ever going to be important, it needs to be done now. It easy to loosen restrictions (ordering) in the future but will be problematic to add it in.
On Fri, Sep 14, 2018 at 8:57 AM, Casey Schaufler <casey@schaufler-ca.com> wrote: > On 9/13/2018 5:19 PM, Kees Cook wrote: >> We already have the minor LSMs that cannot change order. > > Are you saying that we don't have a mechanism to change > the order, or that they wouldn't work right in a different > order? Well, there's the capability module that has to be > first. I just meant their order is explicit in security.c. >> They aren't >> part of security= parsing either. > > True, but there's no reason now that we couldn't change that. > Except for capability. Hmm. Right, we have at least one that MUST be first (and must not be disabled). >> Should "blob-sharing" LSMs be like major LSMs or minor LSMs? > > I like the idea of changing the minor modules to do the full > registration process. That would make them all the same. > Except for capability. In any case, the "blob-sharing" LSMs > need to do the full registration process to account for their > blobs sizes, and that brings the "major" behavior along with it. I agree. I'm working on some clean-ups that I'll send out soon, though I'm worried about some of the various boot-time options... >> If someone is booting with "security=selinux,tomoyo" and then SARA >> lands upstream, does that person have to explicitly add "sara" to >> their boot args, since they're doing a non-default list of LSMs? > > Yes. security= is explicit. > >> (I actually prefer the answer being "yes" here, FWIW, I just want to >> nail down the expectations.) > > For now let's leave the minor (capability, yama, loadpin) as they are, > and require all new modules of any flavor to use full registration. I would even be fine to convert yama and loadpin. > We could consider something like > > security=$lsm # Stack with $lsm at priority 2 - Existing behavior > $lsm.stacked=N # Add $lsm to the stack at priority N. Delete if N == 0 > > It's OK to specify "selinux.stacked=2" and "sara.stacked=2". Which gets > called first is left up to the system to decide. Whatever the behavior is > gets documented. Capability will always be first and have priority 1. > It's OK to specify "smack.stacked=1". I'm less excited about this kind of stacking priority, but, whatever the case, I think my cleanups may help with whatever we decide. > The default stack is determined by CONFIG_SECURITY_$lsm_STACKED at > build time. CONFIG_SECURITY_$lsm_STACKED changes from a boolean to > an integer value to establish the default hook order. > > /sys/kernel/security/lsm reports the modules in hook call order. Didn't I send a patch to new-line terminate this list? I always get annoyed when I "cat" it. ;) > /sys/kernel/security/lsm-stack reports the list with the hook call priority > > capability:1,yama:1,selinux:1,sara:5,landlack:17 > > If stacking is not configured $lsm.stacked=0 is treated as security=none. > For other values of N $lsm.stacked=N is treated as security=$lsm. I feel like "order" is bad enough. Can we avoid adding "priority"? -Kees
On 9/14/2018 1:05 PM, Kees Cook wrote: > On Fri, Sep 14, 2018 at 8:57 AM, Casey Schaufler <casey@schaufler-ca.com> wrote: >> On 9/13/2018 5:19 PM, Kees Cook wrote: >>> We already have the minor LSMs that cannot change order. >> Are you saying that we don't have a mechanism to change >> the order, or that they wouldn't work right in a different >> order? Well, there's the capability module that has to be >> first. > I just meant their order is explicit in security.c. > >>> They aren't >>> part of security= parsing either. >> True, but there's no reason now that we couldn't change that. >> Except for capability. Hmm. > Right, we have at least one that MUST be first (and must not be disabled). > >>> Should "blob-sharing" LSMs be like major LSMs or minor LSMs? >> I like the idea of changing the minor modules to do the full >> registration process. That would make them all the same. >> Except for capability. In any case, the "blob-sharing" LSMs >> need to do the full registration process to account for their >> blobs sizes, and that brings the "major" behavior along with it. > I agree. I'm working on some clean-ups that I'll send out soon, though > I'm worried about some of the various boot-time options... Looking forward to seeing them. >>> If someone is booting with "security=selinux,tomoyo" and then SARA >>> lands upstream, does that person have to explicitly add "sara" to >>> their boot args, since they're doing a non-default list of LSMs? >> Yes. security= is explicit. >> >>> (I actually prefer the answer being "yes" here, FWIW, I just want to >>> nail down the expectations.) >> For now let's leave the minor (capability, yama, loadpin) as they are, >> and require all new modules of any flavor to use full registration. > I would even be fine to convert yama and loadpin. That shouldn't be difficult. >> We could consider something like >> >> security=$lsm # Stack with $lsm at priority 2 - Existing behavior >> $lsm.stacked=N # Add $lsm to the stack at priority N. Delete if N == 0 >> >> It's OK to specify "selinux.stacked=2" and "sara.stacked=2". Which gets >> called first is left up to the system to decide. Whatever the behavior is >> gets documented. Capability will always be first and have priority 1. >> It's OK to specify "smack.stacked=1". > I'm less excited about this kind of stacking priority, but, whatever > the case, I think my cleanups may help with whatever we decide. OK >> The default stack is determined by CONFIG_SECURITY_$lsm_STACKED at >> build time. CONFIG_SECURITY_$lsm_STACKED changes from a boolean to >> an integer value to establish the default hook order. >> >> /sys/kernel/security/lsm reports the modules in hook call order. > Didn't I send a patch to new-line terminate this list? I always get > annoyed when I "cat" it. ;) SELinux set the precedence on that one. Not my fault! >> /sys/kernel/security/lsm-stack reports the list with the hook call priority >> >> capability:1,yama:1,selinux:1,sara:5,landlack:17 >> >> If stacking is not configured $lsm.stacked=0 is treated as security=none. >> For other values of N $lsm.stacked=N is treated as security=$lsm. > I feel like "order" is bad enough. Can we avoid adding "priority"? Sorry. I changed terminology (order and priority) halfway through the message. Yes, I like order better. We should stick with that.
diff --git a/Documentation/admin-guide/LSM/index.rst b/Documentation/admin-guide/LSM/index.rst index 9842e21afd4a..d3d8af174042 100644 --- a/Documentation/admin-guide/LSM/index.rst +++ b/Documentation/admin-guide/LSM/index.rst @@ -17,10 +17,16 @@ MAC extensions, other extensions can be built using the LSM to provide specific changes to system operation when these tweaks are not available in the core functionality of Linux itself. -The Linux capabilities modules will always be included. This may be -followed by any number of "minor" modules and at most one "major" module. -For more details on capabilities, see ``capabilities(7)`` in the Linux -man-pages project. +The Linux capabilities modules will always be included. For more details +on capabilities, see ``capabilities(7)`` in the Linux man-pages project. + +Security modules that do not use the security data blobs maintained +by the LSM infrastructure are considered "minor" modules. These may be +included at compile time and stacked explicitly. Security modules that +use the LSM maintained security blobs are considered "major" modules. +These may only be stacked if the CONFIG_LSM_STACKED configuration +option is used. If this is chosen all of the security modules selected +will be used. A list of the active security modules can be found by reading ``/sys/kernel/security/lsm``. This is a comma separated list, and diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 416b20c3795b..dddcced54fea 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -2079,7 +2079,7 @@ static inline void security_delete_hooks(struct security_hook_list *hooks, #define __lsm_ro_after_init __ro_after_init #endif /* CONFIG_SECURITY_WRITABLE_HOOKS */ -extern int __init security_module_enable(const char *module); +extern bool __init security_module_enable(const char *lsm, const bool stacked); extern void __init capability_add_hooks(void); #ifdef CONFIG_SECURITY_YAMA extern void __init yama_add_hooks(void); diff --git a/security/Kconfig b/security/Kconfig index 22f7664c4977..ed48025ae9e0 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -36,6 +36,28 @@ config SECURITY_WRITABLE_HOOKS bool default n +config SECURITY_STACKING + bool "Security module stacking" + depends on SECURITY + help + Allows multiple major security modules to be stacked. + Modules are invoked in the order registered with a + "bail on fail" policy, in which the infrastructure + will stop processing once a denial is detected. Not + all modules can be stacked. SELinux, Smack and AppArmor are + known to be incompatible. User space components may + have trouble identifying the security module providing + data in some cases. + + If you select this option you will have to select which + of the stackable modules you wish to be active. The + "Default security module" will be ignored. The boot line + "security=" option can be used to specify that one of + the modules identifed for stacking should be used instead + of the entire stack. + + If you are unsure how to answer this question, answer N. + config SECURITY_LSM_DEBUG bool "Enable debugging of the LSM infrastructure" depends on SECURITY @@ -250,6 +272,9 @@ source security/yama/Kconfig source security/integrity/Kconfig +menu "Security Module Selection" + visible if !SECURITY_STACKING + choice prompt "Default security module" default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX @@ -289,3 +314,59 @@ config DEFAULT_SECURITY endmenu +menu "Security Module Stack" + visible if SECURITY_STACKING + +choice + prompt "Stacked 'extreme' security module" + default SECURITY_SELINUX_STACKED if SECURITY_SELINUX + default SECURITY_SMACK_STACKED if SECURITY_SMACK + default SECURITY_APPARMOR_STACKED if SECURITY_APPARMOR + + help + Enable an extreme security module. These modules cannot + be used at the same time. + + config SECURITY_SELINUX_STACKED + bool "SELinux" if SECURITY_SELINUX=y + help + This option instructs the system to use the SELinux checks. + At this time the Smack security module is incompatible with this + module. + At this time the AppArmor security module is incompatible with this + module. + + config SECURITY_SMACK_STACKED + bool "Simplified Mandatory Access Control" if SECURITY_SMACK=y + help + This option instructs the system to use the Smack checks. + At this time the SELinux security module is incompatible with this + module. + At this time the AppArmor security module is incompatible with this + module. + + config SECURITY_APPARMOR_STACKED + bool "AppArmor" if SECURITY_APPARMOR=y + help + This option instructs the system to use the AppArmor checks. + At this time the SELinux security module is incompatible with this + module. + At this time the Smack security module is incompatible with this + module. + +endchoice + +config SECURITY_TOMOYO_STACKED + bool "TOMOYO support is enabled by default" + depends on SECURITY_TOMOYO && SECURITY_STACKING + default n + help + This option instructs the system to use the TOMOYO checks. + If not selected the module will not be invoked. + Stacked security modules may interact in unexpected ways. + + If you are unsure how to answer this question, answer N. + +endmenu + +endmenu diff --git a/security/apparmor/include/cred.h b/security/apparmor/include/cred.h index a90eae76d7c1..be7575adf6f0 100644 --- a/security/apparmor/include/cred.h +++ b/security/apparmor/include/cred.h @@ -25,7 +25,11 @@ static inline struct aa_label *cred_label(const struct cred *cred) { +#ifdef CONFIG_SECURITY_STACKING + struct aa_label **blob = cred->security + apparmor_blob_sizes.lbs_cred; +#else struct aa_label **blob = cred->security; +#endif AA_BUG(!blob); return *blob; @@ -34,7 +38,11 @@ static inline struct aa_label *cred_label(const struct cred *cred) static inline void set_cred_label(const struct cred *cred, struct aa_label *label) { +#ifdef CONFIG_SECURITY_STACKING + struct aa_label **blob = cred->security + apparmor_blob_sizes.lbs_cred; +#else struct aa_label **blob = cred->security; +#endif AA_BUG(!blob); *blob = label; diff --git a/security/apparmor/include/file.h b/security/apparmor/include/file.h index 4c2c8ac8842f..aeb757471cc0 100644 --- a/security/apparmor/include/file.h +++ b/security/apparmor/include/file.h @@ -32,7 +32,14 @@ struct path; AA_MAY_CHMOD | AA_MAY_CHOWN | AA_MAY_LOCK | \ AA_EXEC_MMAP | AA_MAY_LINK) -#define file_ctx(X) ((struct aa_file_ctx *)(X)->f_security) +static inline struct aa_file_ctx *file_ctx(struct file *file) +{ +#ifdef CONFIG_SECURITY_STACKING + return file->f_security + apparmor_blob_sizes.lbs_file; +#else + return file->f_security; +#endif +} /* struct aa_file_ctx - the AppArmor context the file was opened in * @lock: lock to update the ctx diff --git a/security/apparmor/include/lib.h b/security/apparmor/include/lib.h index 6505e1ad9e23..bbe9b384d71d 100644 --- a/security/apparmor/include/lib.h +++ b/security/apparmor/include/lib.h @@ -16,6 +16,7 @@ #include <linux/slab.h> #include <linux/fs.h> +#include <linux/lsm_hooks.h> #include "match.h" @@ -55,6 +56,9 @@ const char *aa_splitn_fqname(const char *fqname, size_t n, const char **ns_name, size_t *ns_len); void aa_info_message(const char *str); +/* Security blob offsets */ +extern struct lsm_blob_sizes apparmor_blob_sizes; + /** * aa_strneq - compare null terminated @str to a non null terminated substring * @str: a null terminated string diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 15716b6ff860..36d8386170e8 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -1553,7 +1553,9 @@ static int __init apparmor_init(void) int error; if (!finish) { - if (apparmor_enabled && security_module_enable("apparmor")) + if (apparmor_enabled && + security_module_enable("apparmor", + IS_ENABLED(CONFIG_SECURITY_APPARMOR_STACKED))) security_add_blobs(&apparmor_blob_sizes); else apparmor_enabled = false; @@ -1561,7 +1563,9 @@ static int __init apparmor_init(void) return 0; } - if (!apparmor_enabled || !security_module_enable("apparmor")) { + if (!apparmor_enabled || + !security_module_enable("apparmor", + IS_ENABLED(CONFIG_SECURITY_APPARMOR_STACKED))) { aa_info_message("AppArmor disabled by boot time parameter"); apparmor_enabled = false; return 0; diff --git a/security/security.c b/security/security.c index 2501cdcbebff..06bed74d1ed0 100644 --- a/security/security.c +++ b/security/security.c @@ -36,6 +36,7 @@ /* Maximum number of letters for an LSM name string */ #define SECURITY_NAME_MAX 10 +#define MODULE_STACK "(stacking)" struct security_hook_heads security_hook_heads __lsm_ro_after_init; static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain); @@ -48,7 +49,11 @@ static struct lsm_blob_sizes blob_sizes; /* Boot-time LSM user choice */ static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] = +#ifdef CONFIG_SECURITY_STACKING + MODULE_STACK; +#else CONFIG_DEFAULT_SECURITY; +#endif static void __init do_security_initcalls(void) { @@ -169,6 +174,7 @@ static int lsm_append(char *new, char **result) /** * security_module_enable - Load given security module on boot ? * @module: the name of the module + * @stacked: indicates that the module wants to be stacked * * Each LSM must pass this method before registering its own operations * to avoid security registration races. This method may also be used @@ -184,9 +190,29 @@ static int lsm_append(char *new, char **result) * * Otherwise, return false. */ -int __init security_module_enable(const char *module) +bool __init security_module_enable(const char *lsm, const bool stacked) { - return !strcmp(module, chosen_lsm); +#ifdef CONFIG_SECURITY_STACKING + /* + * Module defined on the command line security=XXXX + */ + if (strcmp(chosen_lsm, MODULE_STACK)) { + if (!strcmp(lsm, chosen_lsm)) { + pr_info("Command line sets the %s security module.\n", + lsm); + return true; + } + return false; + } + /* + * Module configured as stacked. + */ + return stacked; +#else + if (strcmp(lsm, chosen_lsm) == 0) + return true; + return false; +#endif } /** diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 0b593030d9f2..1de307286bcc 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -7055,7 +7055,8 @@ static __init int selinux_init(void) { static int finish; - if (!security_module_enable("selinux")) { + if (!security_module_enable("selinux", + IS_ENABLED(CONFIG_SECURITY_SELINUX_STACKED))) { selinux_enabled = 0; return 0; } diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h index 7a3d18fa9b13..24e14560a765 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -161,18 +161,30 @@ struct bpf_security_struct { extern struct lsm_blob_sizes selinux_blob_sizes; static inline struct task_security_struct *selinux_cred(const struct cred *cred) { - return cred->security; +#ifdef CONFIG_SECURITY_STACKING + return cred->security + selinux_blob_sizes.lbs_cred; +#else + return cred->security; +#endif } static inline struct file_security_struct *selinux_file(const struct file *file) { - return file->f_security; +#ifdef CONFIG_SECURITY_STACKING + return file->f_security + selinux_blob_sizes.lbs_file; +#else + return file->f_security; +#endif } static inline struct inode_security_struct *selinux_inode( const struct inode *inode) { - return inode->i_security; +#ifdef CONFIG_SECURITY_STACKING + return inode->i_security + selinux_blob_sizes.lbs_inode; +#else + return inode->i_security; +#endif } #endif /* _SELINUX_OBJSEC_H_ */ diff --git a/security/smack/smack.h b/security/smack/smack.h index 5da5bd1b9b47..6513a71c5ca1 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -337,6 +337,7 @@ extern struct smack_known *smack_syslog_label; extern struct smack_known *smack_unconfined; #endif extern int smack_ptrace_rule; +extern struct lsm_blob_sizes smack_blob_sizes; extern struct smack_known smack_known_floor; extern struct smack_known smack_known_hat; @@ -359,17 +360,29 @@ extern struct hlist_head smack_known_hash[SMACK_HASH_SLOTS]; static inline struct task_smack *smack_cred(const struct cred *cred) { - return cred->security; +#ifdef CONFIG_SECURITY_STACKING + return cred->security + smack_blob_sizes.lbs_cred; +#else + return cred->security; +#endif } static inline struct smack_known **smack_file(const struct file *file) { - return file->f_security; +#ifdef CONFIG_SECURITY_STACKING + return file->f_security + smack_blob_sizes.lbs_file; +#else + return file->f_security; +#endif } static inline struct inode_smack *smack_inode(const struct inode *inode) { - return inode->i_security; +#ifdef CONFIG_SECURITY_STACKING + return inode->i_security + smack_blob_sizes.lbs_inode; +#else + return inode->i_security; +#endif } /* diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 6617abb51732..be14540ce09c 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -3493,18 +3493,16 @@ static int smack_getprocattr(struct task_struct *p, char *name, char **value) { struct smack_known *skp = smk_of_task_struct(p); char *cp; - int slen; - if (strcmp(name, "current") != 0) + if (strcmp(name, "current") == 0) { + cp = kstrdup(skp->smk_known, GFP_KERNEL); + if (cp == NULL) + return -ENOMEM; + } else return -EINVAL; - cp = kstrdup(skp->smk_known, GFP_KERNEL); - if (cp == NULL) - return -ENOMEM; - - slen = strlen(cp); *value = cp; - return slen; + return strlen(cp); } /** @@ -4754,7 +4752,8 @@ static __init int smack_init(void) struct cred *cred = (struct cred *) current->cred; struct task_smack *tsp; - if (!security_module_enable("smack")) + if (!security_module_enable("smack", + IS_ENABLED(CONFIG_SECURITY_SMACK_STACKED))) return 0; if (!finish) { diff --git a/security/tomoyo/common.h b/security/tomoyo/common.h index 0110bebe86e2..f386f92c57c5 100644 --- a/security/tomoyo/common.h +++ b/security/tomoyo/common.h @@ -1087,6 +1087,7 @@ extern struct tomoyo_domain_info tomoyo_kernel_domain; extern struct tomoyo_policy_namespace tomoyo_kernel_namespace; extern unsigned int tomoyo_memory_quota[TOMOYO_MAX_MEMORY_STAT]; extern unsigned int tomoyo_memory_used[TOMOYO_MAX_MEMORY_STAT]; +extern struct lsm_blob_sizes tomoyo_blob_sizes; /********** Inlined functions. **********/ @@ -1206,7 +1207,11 @@ static inline void tomoyo_put_group(struct tomoyo_group *group) */ static inline struct tomoyo_domain_info **tomoyo_cred(const struct cred *cred) { +#ifdef CONFIG_SECURITY_STACKING + return cred->security + tomoyo_blob_sizes.lbs_cred; +#else return cred->security; +#endif } /** @@ -1216,8 +1221,13 @@ static inline struct tomoyo_domain_info **tomoyo_cred(const struct cred *cred) */ static inline struct tomoyo_domain_info *tomoyo_domain(void) { - struct tomoyo_domain_info **blob = tomoyo_cred(current_cred()); + const struct cred *cred = current_cred(); + struct tomoyo_domain_info **blob; + + if (cred->security == NULL) + return NULL; + blob = tomoyo_cred(cred); return *blob; } diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c index bb84e6ec3886..fa121ad8534a 100644 --- a/security/tomoyo/tomoyo.c +++ b/security/tomoyo/tomoyo.c @@ -564,7 +564,8 @@ static int __init tomoyo_init(void) struct cred *cred = (struct cred *) current_cred(); struct tomoyo_domain_info **blob; - if (!security_module_enable("tomoyo")) { + if (!security_module_enable("tomoyo", + IS_ENABLED(CONFIG_SECURITY_TOMOYO_STACKED))) { tomoyo_enabled = false; return 0; }
Two proposed security modules require the ability to share security blobs with existing "major" security modules. These modules, S.A.R.A and LandLock, provide significantly different services than SELinux, Smack or AppArmor. Using either in conjunction with the existing modules is quite reasonable. S.A.R.A requires access to the cred blob, while LandLock uses the cred, file and inode blobs. The use of the cred, file and inode blobs has been abstracted in preceding patches in the series. This patch teaches the affected security modules how to access the part of the blob set aside for their use in the case where blobs are shared. The configuration option CONFIG_SECURITY_STACKING identifies systems where the blobs may be shared. The mechanism for selecting which security modules are active has been changed to allow non-conflicting "major" security modules to be used together. At this time the TOMOYO module can safely be used with any of the others. The two new modules would be non-conflicting as well. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> --- Documentation/admin-guide/LSM/index.rst | 14 +++-- include/linux/lsm_hooks.h | 2 +- security/Kconfig | 81 +++++++++++++++++++++++++ security/apparmor/include/cred.h | 8 +++ security/apparmor/include/file.h | 9 ++- security/apparmor/include/lib.h | 4 ++ security/apparmor/lsm.c | 8 ++- security/security.c | 30 ++++++++- security/selinux/hooks.c | 3 +- security/selinux/include/objsec.h | 18 +++++- security/smack/smack.h | 19 +++++- security/smack/smack_lsm.c | 17 +++--- security/tomoyo/common.h | 12 +++- security/tomoyo/tomoyo.c | 3 +- 14 files changed, 200 insertions(+), 28 deletions(-)