v3:
- add CONFIG_LSM_ENABLE and refactor resulting logic
v2:
- add "lsm.order=" and CONFIG_LSM_ORDER instead of overloading "security="
- reorganize introduction of ordering logic code
Overview:
This refactors the LSM registration and initialization infrastructure to
more centrally support different LSM types for more cleanly supporting the
future expansion of LSM stacking via the "blob-sharing" patch series. What
was considered a "major" LSM is kept for legacy use of the "security="
boot parameter, and now overlaps with the new class of "exclusive" LSMs
for the future blob sharing. The "minor" LSMs become more well defined
as a result of the refactoring.
Approach:
To better show LSMs activation some debug reporting was added (enabled
with the "lsm.debug" boot commandline option).
I added a WARN() around LSM initialization failures, which appear to
have always been silently ignored. (Realistically any LSM init failures
would have only been due to catastrophic kernel issues that would render
a system unworkable anyway, but it'd be better to expose the problem as
early as possible.)
Instead of continuing to (somewhat improperly) overload the kernel's
initcall system, this changes the LSM infrastructure to store a
registration structure (struct lsm_info) table instead, where metadata
about each LSM can be recorded (name, flags, order, enable flag, init
function). This can be extended in the future to include things like
required blob size for the coming "blob sharing" LSMs.
The "major" LSMs had to individually negotiate which of them should be
enabled. This didn't provide a way to negotiate combinations of other
LSMs (as will be needed for "blob sharing" LSMs). This is solved by
providing the LSM infrastructure with all the details needed to make
the choice (exposing the per-LSM "enabled" flag, if used, the LSM
characteristics, and ordering expectations).
As a result of the refactoring, the "minor" LSMs are able to remove
the open-coded security_add_hooks() calls for "capability", "yama",
and "loadpin", and to redefine "integrity" properly as a general LSM.
(Note that "integrity" actually defined _no_ hooks, but needs the early
initialization).
With all LSMs being proessed centrally, it was possible to implement
a new boot parameter "lsm.order=" to provide explicit ordering, which
is helpful for the future "blob sharing" LSMs. Matching this is the
new CONFIG_LSM_ORDER, which replaces CONFIG_DEFAULT_SECURITY, as it
provides a higher granularity of control.
Breakdown of patches:
Infrastructure improvements (no logical changes):
LSM: Correctly announce start of LSM initialization
vmlinux.lds.h: Avoid copy/paste of security_init section
LSM: Rename .security_initcall section to .lsm_info
LSM: Remove initcall tracing
LSM: Convert from initcall to struct lsm_info
vmlinux.lds.h: Move LSM_TABLE into INIT_DATA
LSM: Convert security_initcall() into DEFINE_LSM()
LSM: Record LSM name in struct lsm_info
LSM: Provide init debugging infrastructure
LSM: Don't ignore initialization failures
Split "integrity" out into "ordered initialization" (no logical changes):
LSM: Introduce LSM_FLAG_LEGACY_MAJOR
LSM: Provide separate ordered initialization
Provide centralized LSM enable/disable infrastructure:
LoadPin: Rename "enable" to "enforce"
LSM: Plumb visibility into optional "enabled" state
LSM: Lift LSM selection out of individual LSMs
LSM: Prepare for arbitrary LSM enabling
LSM: Introduce CONFIG_LSM_ENABLE
LSM: Introduce lsm.enable= and lsm.disable=
LSM: Prepare for reorganizing "security=" logic
LSM: Refactor "security=" in terms of enable/disable
Provide centralized LSM ordering infrastructure:
LSM: Build ordered list of ordered LSMs for init
LSM: Introduce CONFIG_LSM_ORDER
LSM: Introduce "lsm.order=" for boottime ordering
Move minor LSMs into ordered LSM initialization:
LoadPin: Initialize as ordered LSM
Yama: Initialize as ordered LSM
LSM: Introduce enum lsm_order
capability: Initialize as LSM_ORDER_FIRST
Move major LSMs into ordered LSM initialization:
LSM: Separate idea of "major" LSM from "exclusive" LSM
LSM: Add all exclusive LSMs to ordered initialization
-Kees
.../admin-guide/kernel-parameters.txt | 20 +
arch/arc/kernel/vmlinux.lds.S | 1 -
arch/arm/kernel/vmlinux-xip.lds.S | 1 -
arch/arm64/kernel/vmlinux.lds.S | 1 -
arch/h8300/kernel/vmlinux.lds.S | 1 -
arch/microblaze/kernel/vmlinux.lds.S | 2 -
arch/powerpc/kernel/vmlinux.lds.S | 2 -
arch/um/include/asm/common.lds.S | 2 -
arch/xtensa/kernel/vmlinux.lds.S | 1 -
include/asm-generic/vmlinux.lds.h | 25 +-
include/linux/init.h | 2 -
include/linux/lsm_hooks.h | 43 ++-
include/linux/module.h | 1 -
security/Kconfig | 61 ++-
security/apparmor/lsm.c | 16 +-
security/commoncap.c | 8 +-
security/integrity/iint.c | 5 +-
security/loadpin/Kconfig | 4 +-
security/loadpin/loadpin.c | 28 +-
security/security.c | 351 +++++++++++++++---
security/selinux/hooks.c | 16 +-
security/smack/smack_lsm.c | 8 +-
security/tomoyo/tomoyo.c | 7 +-
security/yama/yama_lsm.c | 7 +-
24 files changed, 438 insertions(+), 175 deletions(-)
--
2.17.1
For a while now, the LSM core has said it was "initializED", rather than
"initializING". This adjust the report to be more accurate (i.e. before
this was reported before any LSMs had been initialized.)
Signed-off-by: Kees Cook <redacted>
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
---
security/security.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Since the struct lsm_info table is not an initcall, we can just move it
into INIT_DATA like all the other tables.
Cc: linux-arch at vger.kernel.org
Signed-off-by: Kees Cook <redacted>
---
arch/arc/kernel/vmlinux.lds.S | 1 -
arch/arm/kernel/vmlinux-xip.lds.S | 1 -
arch/arm64/kernel/vmlinux.lds.S | 1 -
arch/h8300/kernel/vmlinux.lds.S | 1 -
arch/microblaze/kernel/vmlinux.lds.S | 2 --
arch/powerpc/kernel/vmlinux.lds.S | 2 --
arch/um/include/asm/common.lds.S | 2 --
arch/xtensa/kernel/vmlinux.lds.S | 1 -
include/asm-generic/vmlinux.lds.h | 24 +++++++++++-------------
9 files changed, 11 insertions(+), 24 deletions(-)
In preparation for doing more interesting LSM init probing, this converts
the existing initcall system into an explicit call into a function pointer
from a section-collected struct lsm_info array.
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Ard Biesheuvel <redacted>
Cc: Paul Moore <paul@paul-moore.com>
Cc: linux-security-module at vger.kernel.org
Signed-off-by: Kees Cook <redacted>
---
include/linux/init.h | 2 --
include/linux/lsm_hooks.h | 12 ++++++++++++
include/linux/module.h | 1 -
security/integrity/iint.c | 1 +
security/security.c | 14 +++++---------
5 files changed, 18 insertions(+), 12 deletions(-)
In preparation for making LSM selections outside of the LSMs, include
the name of LSMs in struct lsm_info.
Cc: James Morris <redacted>
Signed-off-by: Kees Cook <redacted>
---
include/linux/lsm_hooks.h | 4 ++++
1 file changed, 4 insertions(+)
@@ -2274,6 +2274,8 @@ ltpc= [NET] Format: <io>,<irq>,<dma>+ lsm.debug [SECURITY] Enable LSM initialization debugging output.+ machvec= [IA-64] Force the use of a particular machine-vector (machvec) in a generic kernel. Example: machvec=hpzx1_swiotlb
LoadPin's "enable" setting is really about enforcement, not whether
or not the LSM is using LSM hooks. Instead, split this out so that LSM
enabling can be logically distinct from whether enforcement is happening
(for example, the pinning happens when the LSM is enabled, but the pin
is only checked when "enforce" is set). This allows LoadPin to continue
to operate sanely in test environments once LSM enable/disable is
centrally handled (i.e. we want LoadPin to be enabled separately from
its enforcement).
Signed-off-by: Kees Cook <redacted>
---
security/loadpin/Kconfig | 4 ++--
security/loadpin/loadpin.c | 21 +++++++++++----------
2 files changed, 13 insertions(+), 12 deletions(-)
@@ -97,7 +97,7 @@ static void check_pinning_enforcement(struct super_block *mnt_sb)loadpin_sysctl_table))pr_notice("sysctl registration failed!\n");else-pr_info("load pinning can be disabled.\n");+pr_info("enforcement can be disabled.\n");}elsepr_info("load pinning engaged.\n");}
@@ -128,7 +128,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)/* This handles the older init_module API that has a NULL file. */if(!file){-if(!enabled){+if(!enforcing){report_load(origin,NULL,"old-api-pinning-ignored");return0;}
@@ -186,10 +186,11 @@ static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {void__initloadpin_add_hooks(void){-pr_info("ready to pin (currently %sabled)",enabled?"en":"dis");+pr_info("ready to pin (currently %senforcing)\n",+enforcing?"":"not ");security_add_hooks(loadpin_hooks,ARRAY_SIZE(loadpin_hooks),"loadpin");}/* Should not be mutable after boot, so not listed in sysfs (perm == 0). */-module_param(enabled,int,0);-MODULE_PARM_DESC(enabled,"Pin module/firmware loading (default: true)");+module_param(enforcing,int,0);+MODULE_PARM_DESC(enforcing,"Enforce module/firmware pinning");
As a prerequisite to adjusting LSM selection logic in the future, this
moves the selection logic up out of the individual major LSMs, making
their init functions only run when actually enabled. This considers all
LSMs enabled by default unless they specified an external "enable"
variable.
Signed-off-by: Kees Cook <redacted>
---
include/linux/lsm_hooks.h | 1 -
security/apparmor/lsm.c | 6 ---
security/security.c | 84 ++++++++++++++++++++++++--------------
security/selinux/hooks.c | 10 -----
security/smack/smack_lsm.c | 3 --
security/tomoyo/tomoyo.c | 2 -
6 files changed, 53 insertions(+), 53 deletions(-)
@@ -1542,12 +1542,6 @@ static int __init apparmor_init(void){interror;-if(!apparmor_enabled||!security_module_enable("apparmor")){-aa_info_message("AppArmor disabled by boot time parameter");-apparmor_enabled=false;-return0;-}-aa_secids_init();error=aa_setup_dfa_engine();
@@ -52,33 +52,78 @@ static bool debug __initdata;pr_info(__VA_ARGS__);\}while(0)+staticbool__initis_enabled(structlsm_info*lsm)+{+if(!lsm->enabled||*lsm->enabled)+returntrue;++returnfalse;+}++/* Mark an LSM's enabled flag, if it exists. */+staticvoid__initset_enabled(structlsm_info*lsm,boolenabled)+{+if(lsm->enabled)+*lsm->enabled=enabled;+}++/* Is an LSM allowed to be initialized? */+staticbool__initlsm_allowed(structlsm_info*lsm)+{+/* Skip if the LSM is disabled. */+if(!is_enabled(lsm))+returnfalse;++/* Skip major-specific checks if not a major LSM. */+if((lsm->flags&LSM_FLAG_LEGACY_MAJOR)==0)+returntrue;++/* Disabled if this LSM isn't the chosen one. */+if(strcmp(lsm->name,chosen_lsm)!=0)+returnfalse;++returntrue;+}++/* Check if LSM should be enabled. Mark any that are disabled. */+staticvoid__initmaybe_initialize_lsm(structlsm_info*lsm)+{+intenabled=lsm_allowed(lsm);++/* Record enablement. */+set_enabled(lsm,enabled);++/* If selected, initialize the LSM. */+if(enabled){+intret;++init_debug("initializing %s\n",lsm->name);+ret=lsm->init();+WARN(ret,"%s failed to initialize: %d\n",lsm->name,ret);+}+}+staticvoid__initordered_lsm_init(void){structlsm_info*lsm;-intret;for(lsm=__start_lsm_info;lsm<__end_lsm_info;lsm++){if((lsm->flags&LSM_FLAG_LEGACY_MAJOR)!=0)continue;-init_debug("initializing %s\n",lsm->name);-ret=lsm->init();-WARN(ret,"%s failed to initialize: %d\n",lsm->name,ret);+maybe_initialize_lsm(lsm);}}staticvoid__initmajor_lsm_init(void){structlsm_info*lsm;-intret;for(lsm=__start_lsm_info;lsm<__end_lsm_info;lsm++){if((lsm->flags&LSM_FLAG_LEGACY_MAJOR)==0)continue;-init_debug("initializing %s\n",lsm->name);-ret=lsm->init();-WARN(ret,"%s failed to initialize: %d\n",lsm->name,ret);+maybe_initialize_lsm(lsm);}}
@@ -168,29 +213,6 @@ static int lsm_append(char *new, char **result)return0;}-/**-*security_module_enable-Loadgivensecuritymoduleonboot?-*@module:thenameofthemodule-*-*EachLSMmustpassthismethodbeforeregisteringitsownoperations-*toavoidsecurityregistrationraces.Thismethodmayalsobeused-*tocheckifyourLSMiscurrentlyloadedduringkernelinitialization.-*-*Returns:-*-*trueif:-*-*-ThepassedLSMistheonechosenbyuseratboottime,-*-orthepassedLSMisconfiguredasthedefaultandtheuserdidnot-*chooseanalternateLSMatboottime.-*-*Otherwise,returnfalse.-*/-int__initsecurity_module_enable(constchar*module)-{-return!strcmp(module,chosen_lsm);-}-/***security_add_hooks-Addamoduleshookstothehooklists.*@hooks:thehookstoadd
@@ -540,8 +540,6 @@ static int __init tomoyo_init(void){structcred*cred=(structcred*)current_cred();-if(!security_module_enable("tomoyo"))-return0;/* register ourselves with the security framework */security_add_hooks(tomoyo_hooks,ARRAY_SIZE(tomoyo_hooks),"tomoyo");printk(KERN_INFO"TOMOYO Linux initialized\n");
Before now, all the LSMs that did not specify an "enable" variable in their
struct lsm_info were considered enabled by default. This prepares to make
LSM enabling more explicit. For all LSMs without an explicit "enable"
variable, a hard-coded storage location is chosen, and all LSMs without
an external "enable" state have their state explicitly set to "enabled".
This code appears more complex than it needs to be (comma-separated
list parsing and "set" function parameter) because its use will be
expanded on in the following patches to provide more explicit enabling.
Signed-off-by: Kees Cook <redacted>
---
security/security.c | 69 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 65 insertions(+), 4 deletions(-)
@@ -54,17 +54,46 @@ static bool debug __initdata;staticbool__initis_enabled(structlsm_info*lsm){-if(!lsm->enabled||*lsm->enabled)-returntrue;+if(WARN_ON(!lsm->enabled))+returnfalse;-returnfalse;+return*lsm->enabled;}/* Mark an LSM's enabled flag, if it exists. */-staticvoid__initset_enabled(structlsm_info*lsm,boolenabled)+staticintlsm_enabled_true__initdata=1;+staticintlsm_enabled_false__initdata=0;++staticvoid__initdefault_enabled(structlsm_info*lsm,boolenabled){+/* If storage location already set, skip this one. */if(lsm->enabled)+return;++/*+*WhenanLSMhasn'tconfiguredanenablevariable,wecanuse+*ahard-codedlocationforstoringthedefaultenabledstate.+*/+if(enabled)+lsm->enabled=&lsm_enabled_true;+else+lsm->enabled=&lsm_enabled_false;+}++staticvoid__initset_enabled(structlsm_info*lsm,boolenabled)+{+if(WARN_ON(!lsm->enabled))+return;++if(lsm->enabled==&lsm_enabled_true){+if(!enabled)+lsm->enabled=&lsm_enabled_false;+}elseif(lsm->enabled==&lsm_enabled_false){+if(enabled)+lsm->enabled=&lsm_enabled_true;+}else{*lsm->enabled=enabled;+}}/* Is an LSM allowed to be initialized? */
@@ -143,6 +201,9 @@ int __init security_init(void)i++)INIT_HLIST_HEAD(&list[i]);+/* Figure out which LSMs are enabled and disabled. */+prepare_lsm_enable();+/**LoadminorLSMs,withthecapabilitymodulealwaysfirst.*/
In order to both support old "security=" Legacy Major LSM selection, and
handling real exclusivity, this creates LSM_FLAG_EXCLUSIVE and updates
the selection logic to handle them.
Signed-off-by: Kees Cook <redacted>
---
include/linux/lsm_hooks.h | 1 +
security/apparmor/lsm.c | 2 +-
security/security.c | 12 ++++++++++++
security/selinux/hooks.c | 2 +-
security/smack/smack_lsm.c | 2 +-
security/tomoyo/tomoyo.c | 2 +-
6 files changed, 17 insertions(+), 4 deletions(-)
@@ -2040,6 +2040,7 @@ extern void security_add_hooks(struct security_hook_list *hooks, int count,char*lsm);#define LSM_FLAG_LEGACY_MAJOR BIT(0)+#define LSM_FLAG_EXCLUSIVE BIT(1)enumlsm_order{LSM_ORDER_FIRST=-1,/* This is only for capabilities. */
@@ -7193,7 +7193,7 @@ void selinux_complete_init(void)/* SELinux requires early initialization in order to labelallprocessesandobjectswhentheyarecreated.*/DEFINE_LSM(selinux)-.flags=LSM_FLAG_LEGACY_MAJOR,+.flags=LSM_FLAG_LEGACY_MAJOR|LSM_FLAG_EXCLUSIVE,.enabled=&selinux_enabled,.init=selinux_init,END_LSM;
This constructs a list of ordered LSMs to initialize, using a hard-coded
list of only "integrity": minor LSMs continue to have direct hook calls,
and major LSMs continue to initialize separately.
Signed-off-by: Kees Cook <redacted>
---
security/security.c | 59 +++++++++++++++++++++++++++++++++++++++------
1 file changed, 52 insertions(+), 7 deletions(-)
@@ -34,6 +34,9 @@#define MAX_LSM_EVM_XATTR 2+/* How many LSMs were built into the kernel? */+#define LSM_COUNT (__end_lsm_info - __start_lsm_info)+structsecurity_hook_headssecurity_hook_heads__lsm_ro_after_init;staticATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
@@ -45,6 +48,9 @@ static __initdata const char *chosen_major_lsm;static__initconstconstchar*constbuiltin_lsm_enable=CONFIG_LSM_ENABLE;+/* Ordered list of LSMs to initialize. */+static__initdatastructlsm_info**ordered_lsms;+staticbooldebug__initdata;#define init_debug(...) \do{\
@@ -96,6 +102,45 @@ static void __init set_enabled(struct lsm_info *lsm, bool enabled)}}+/* Is an LSM already listed in the ordered LSMs list? */+staticbool__initexists_ordered_lsm(structlsm_info*lsm)+{+structlsm_info**check;++for(check=ordered_lsms;*check;check++)+if(*check==lsm)+returntrue;++returnfalse;+}++/* Append an LSM to the list of ordered LSMs to initialize. */+staticintlast_lsm__initdata;+staticvoid__initappend_ordered_lsm(structlsm_info*lsm,constchar*from)+{+/* Ignore duplicate selections. */+if(exists_ordered_lsm(lsm))+return;++if(WARN(last_lsm==LSM_COUNT,"%s: out of LSM slots!?\n",from))+return;++ordered_lsms[last_lsm++]=lsm;+init_debug("%s ordering: %s (%sabled)\n",from,lsm->name,+is_enabled(lsm)?"en":"dis");+}++/* Populate ordered LSMs list from hard-coded list of LSMs. */+staticvoid__initprepare_lsm_order(void)+{+structlsm_info*lsm;++for(lsm=__start_lsm_info;lsm<__end_lsm_info;lsm++){+if(strcmp(lsm->name,"integrity")==0)+append_ordered_lsm(lsm,"builtin");+}+}+/* Is an LSM allowed to be initialized? */staticbool__initlsm_allowed(structlsm_info*lsm){
@@ -215,6 +256,8 @@ int __init security_init(void)for(i=0;i<sizeof(security_hook_heads)/sizeof(structhlist_head);i++)INIT_HLIST_HEAD(&list[i]);+ordered_lsms=kcalloc(LSM_COUNT+1,sizeof(*ordered_lsms),+GFP_KERNEL);/* Figure out which LSMs are enabled and disabled. */prepare_lsm_enable();
@@ -227,6 +270,7 @@ int __init security_init(void)loadpin_add_hooks();/* Load LSMs in specified order. */+prepare_lsm_order();ordered_lsm_init();/*
@@ -234,6 +278,7 @@ int __init security_init(void)*/major_lsm_init();+kfree(ordered_lsms);return0;}
Provide a way to reorder LSM initialization using the new "lsm.order="
comma-separated list of LSMs. Any LSMs not listed will be added in builtin
order.
Signed-off-by: Kees Cook <redacted>
---
Documentation/admin-guide/kernel-parameters.txt | 6 ++++++
security/security.c | 14 +++++++++++++-
2 files changed, 19 insertions(+), 1 deletion(-)
@@ -2288,6 +2288,12 @@ CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and boot parameters.+ lsm.order=lsm1,...,lsmN+ [SECURITY] Choose order of enabled LSM+ initialization. Any builtin LSMs not listed here+ will be implicitly appended to the list in builtin+ order.+ machvec= [IA-64] Force the use of a particular machine-vector (machvec) in a generic kernel. Example: machvec=hpzx1_swiotlb
@@ -160,11 +161,14 @@ static void __init parse_lsm_order(const char *order, const char *origin)kfree(sep);}-/* Populate ordered LSMs list from builtin list of LSMs. */+/* Populate ordered LSMs list from commandline and builtin list of LSMs. */staticvoid__initprepare_lsm_order(void){structlsm_info*lsm;+/* Parse order from commandline, if present. */+parse_lsm_order(chosen_lsm_order,"cmdline");+/* Parse order from builtin list. */parse_lsm_order(builtin_lsm_order,"builtin");
@@ -324,6 +328,14 @@ static int __init choose_major_lsm(char *str)}__setup("security=",choose_major_lsm);+/* Explicitly choose LSM initialization order. */+staticint__initchoose_lsm_order(char*str)+{+chosen_lsm_order=str;+return1;+}+__setup("lsm.order=",choose_lsm_order);+/* Enable LSM order debugging. */staticint__initenable_debug(char*str){
@@ -314,11 +318,6 @@ int __init security_init(void)/* Figure out which LSMs are enabled and disabled. */prepare_lsm_enable();-/*-*LoadminorLSMs,withthecapabilitymodulealwaysfirst.-*/-capability_add_hooks();-/* Load LSMs in specified order. */prepare_lsm_order();ordered_lsm_init();
This removes CONFIG_DEFAULT_SECURITY in favor of the explicit build-time
ordering offered by CONFIG_LSM_ORDER, and adds all the exclusive LSMs to
the ordered LSM initialization. The old meaning of CONFIG_DEFAULT_SECURITY
is now captured by which exclusive LSM is listed first in the LSM order.
Signed-off-by: Kees Cook <redacted>
---
security/Kconfig | 43 ++++---------------------------------------
security/security.c | 23 +----------------------
2 files changed, 5 insertions(+), 61 deletions(-)
In preparation for lifting the "is this LSM enabled?" logic out of the
individual LSMs, pass in any special enabled state tracking (as needed
for SELinux, AppArmor, and LoadPin). This should be an "int" to include
handling any future cases where "enabled" is exposed via sysctl which
has no "bool" type.
Signed-off-by: Kees Cook <redacted>
---
include/linux/lsm_hooks.h | 1 +
security/apparmor/lsm.c | 5 +++--
security/selinux/hooks.c | 1 +
3 files changed, 5 insertions(+), 2 deletions(-)
LSM initialization failures have traditionally been ignored. We should
at least WARN when something goes wrong.
Signed-off-by: Kees Cook <redacted>
---
security/security.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
This adds a flag for the current "major" LSMs to distinguish them when
we have a universal method for ordering all LSMs. It's called "legacy"
since the distinction of "major" will go away in the blob-sharing world.
Signed-off-by: Kees Cook <redacted>
---
include/linux/lsm_hooks.h | 3 +++
security/apparmor/lsm.c | 1 +
security/selinux/hooks.c | 1 +
security/smack/smack_lsm.c | 1 +
security/tomoyo/tomoyo.c | 1 +
5 files changed, 7 insertions(+)
@@ -7203,6 +7203,7 @@ void selinux_complete_init(void)/* SELinux requires early initialization in order to labelallprocessesandobjectswhentheyarecreated.*/DEFINE_LSM(selinux)+.flags=LSM_FLAG_LEGACY_MAJOR,.init=selinux_init,END_LSM;
This provides a place for ordered LSMs to be initialized, separate from
the "major" LSMs. This is mainly a copy/paste from major_lsm_init() to
ordered_lsm_init(), but it will change drastically in later patches.
What is not obvious in the patch is that this change moves the integrity
LSM from major_lsm_init() into ordered_lsm_init(), since it is not marked
with the LSM_FLAG_LEGACY_MAJOR. As it is the only LSM in the "ordered"
list, there is no reordering yet created.
Signed-off-by: Kees Cook <redacted>
---
security/security.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
Instead of using argument-based initializers, switch to defining the
contents of struct lsm_info on a per-LSM basis. This also drops
the final use of the now inaccurate "initcall" naming.
Cc: John Johansen <john.johansen@canonical.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <redacted>
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Mimi Zohar <redacted>
Cc: linux-security-module at vger.kernel.org
Cc: selinux at tycho.nsa.gov
Signed-off-by: Kees Cook <redacted>
---
include/linux/lsm_hooks.h | 6 ++++--
security/apparmor/lsm.c | 4 +++-
security/integrity/iint.c | 4 +++-
security/selinux/hooks.c | 4 +++-
security/smack/smack_lsm.c | 4 +++-
security/tomoyo/tomoyo.c | 4 +++-
6 files changed, 19 insertions(+), 7 deletions(-)
@@ -7202,7 +7202,9 @@ void selinux_complete_init(void)/* SELinux requires early initialization in order to labelallprocessesandobjectswhentheyarecreated.*/-security_initcall(selinux_init);+DEFINE_LSM(selinux)+.init=selinux_init,+END_LSM;#if defined(CONFIG_NETFILTER)
In preparation for switching from initcall to just a regular set of
pointers in a section, rename the internal section name.
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Ard Biesheuvel <redacted>
Cc: linux-arch at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Signed-off-by: Kees Cook <redacted>
---
include/asm-generic/vmlinux.lds.h | 10 +++++-----
include/linux/init.h | 4 ++--
security/security.c | 4 ++--
3 files changed, 9 insertions(+), 9 deletions(-)
@@ -30,8 +30,6 @@#include<linux/string.h>#include<net/flow.h>-#include<trace/events/initcall.h>-#define MAX_LSM_EVM_XATTR 2/* Maximum number of letters for an LSM name string */
To provide a set of default-enabled LSMs at boot, this introduces the
new CONFIG_LSM_ENABLE. A value of "all" means all builtin LSMs are
enabled by default. Any unlisted LSMs will be implicitly disabled
(excepting those with LSM-specific CONFIGs for enabling/disabling).
The behavior of the LSM-specific CONFIGs for SELinux are AppArmor
unchanged: the default-enabled state for those LSMs remains controlled
through their LSM-specific "enable" CONFIGs.
Signed-off-by: Kees Cook <redacted>
---
include/linux/lsm_hooks.h | 2 +-
security/Kconfig | 8 ++++++++
security/security.c | 4 +++-
3 files changed, 12 insertions(+), 2 deletions(-)
@@ -184,13 +184,18 @@ static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {LSM_HOOK_INIT(kernel_load_data,loadpin_load_data),};-void__initloadpin_add_hooks(void)+staticint__initloadpin_init(void){pr_info("ready to pin (currently %senforcing)\n",enforcing?"":"not ");security_add_hooks(loadpin_hooks,ARRAY_SIZE(loadpin_hooks),"loadpin");+return0;}+DEFINE_LSM(loadpin)+.init=loadpin_init,+END_LSM;+/* Should not be mutable after boot, so not listed in sysfs (perm == 0). */module_param(enforcing,int,0);MODULE_PARM_DESC(enforcing,"Enforce module/firmware pinning");
For what are marked as the Legacy Major LSMs, make them effectively
exclusive when selected on the "security=" boot parameter, to handle
the future case of when a previously major LSMs become non-exclusive
(e.g. when TOMOYO starts blob-sharing).
Signed-off-by: Kees Cook <redacted>
---
security/security.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
@@ -103,14 +103,6 @@ static bool __init lsm_allowed(struct lsm_info *lsm)if(!is_enabled(lsm))returnfalse;-/* Skip major-specific checks if not a major LSM. */-if((lsm->flags&LSM_FLAG_LEGACY_MAJOR)==0)-returntrue;--/* Disabled if this LSM isn't the chosen one. */-if(strcmp(lsm->name,chosen_major_lsm)!=0)-returnfalse;-returntrue;}
@@ -188,8 +180,24 @@ static void __init prepare_lsm_enable(void)parse_lsm_enable(chosen_lsm_enable,set_enabled,true);parse_lsm_enable(chosen_lsm_disable,set_enabled,false);+/* Process "security=", if given. */if(!chosen_major_lsm)chosen_major_lsm=CONFIG_DEFAULT_SECURITY;+if(chosen_major_lsm){+structlsm_info*lsm;++/*+*Tomatchtheoriginal"security="behavior,this+*explicitlydoesNOTfallbacktoanotherLegacyMajor+*iftheselectedonewasseparatelydisabled:disable+*allnon-matchingLegacyMajorLSMs.+*/+for(lsm=__start_lsm_info;lsm<__end_lsm_info;lsm++){+if((lsm->flags&LSM_FLAG_LEGACY_MAJOR)&&+strcmp(lsm->name,chosen_major_lsm)!=0)+set_enabled(lsm,false);+}+}}/**
@@ -47,6 +47,7 @@ static __initdata const char *chosen_lsm_disable;static__initdataconstchar*chosen_major_lsm;static__initconstconstchar*constbuiltin_lsm_enable=CONFIG_LSM_ENABLE;+static__initconstconstchar*constbuiltin_lsm_order=CONFIG_LSM_ORDER;/* Ordered list of LSMs to initialize. */static__initdatastructlsm_info**ordered_lsms;
@@ -130,14 +131,47 @@ static void __init append_ordered_lsm(struct lsm_info *lsm, const char *from)is_enabled(lsm)?"en":"dis");}-/* Populate ordered LSMs list from hard-coded list of LSMs. */+/* Populate ordered LSMs list from given string. */+staticvoid__initparse_lsm_order(constchar*order,constchar*origin)+{+structlsm_info*lsm;+char*sep,*name,*next;++if(!order)+return;++sep=kstrdup(order,GFP_KERNEL);+next=sep;+/* Walk the list, looking for matching LSMs. */+while((name=strsep(&next,","))!=NULL){+boolfound=false;++for(lsm=__start_lsm_info;lsm<__end_lsm_info;lsm++){+if((lsm->flags&LSM_FLAG_LEGACY_MAJOR)==0&&+strcmp(lsm->name,name)==0){+append_ordered_lsm(lsm,origin);+found=true;+}+}++if(!found)+init_debug("%s ignored: %s\n",origin,name);+}+kfree(sep);+}++/* Populate ordered LSMs list from builtin list of LSMs. */staticvoid__initprepare_lsm_order(void){structlsm_info*lsm;+/* Parse order from builtin list. */+parse_lsm_order(builtin_lsm_order,"builtin");++/* Add any missing LSMs, in link order. */for(lsm=__start_lsm_info;lsm<__end_lsm_info;lsm++){-if(strcmp(lsm->name,"integrity")==0)-append_ordered_lsm(lsm,"builtin");+if((lsm->flags&LSM_FLAG_LEGACY_MAJOR)==0)+append_ordered_lsm(lsm,"link-time");}}
In preparation for distinguishing the "capability" LSM from other LSMs,
it must be ordered first. This introduces LSM_ORDER_MUTABLE for the
general LSMs, LSM_ORDER_FIRST for capabilities, and LSM_ORDER_LAST for
anything that must run last (e.g. Landlock may use this in the future).
Signed-off-by: Kees Cook <redacted>
---
include/linux/lsm_hooks.h | 7 +++++++
security/security.c | 18 ++++++++++++++++--
2 files changed, 23 insertions(+), 2 deletions(-)
@@ -166,6 +167,12 @@ static void __init prepare_lsm_order(void){structlsm_info*lsm;+/* LSM_ORDER_FIRST is always first. */+for(lsm=__start_lsm_info;lsm<__end_lsm_info;lsm++){+if(lsm->order==LSM_ORDER_FIRST)+append_ordered_lsm(lsm,"first");+}+/* Parse order from commandline, if present. */parse_lsm_order(chosen_lsm_order,"cmdline");
@@ -174,9 +181,16 @@ static void __init prepare_lsm_order(void)/* Add any missing LSMs, in link order. */for(lsm=__start_lsm_info;lsm<__end_lsm_info;lsm++){-if((lsm->flags&LSM_FLAG_LEGACY_MAJOR)==0)+if(lsm->order==LSM_ORDER_MUTABLE&&+(lsm->flags&LSM_FLAG_LEGACY_MAJOR)==0)append_ordered_lsm(lsm,"link-time");}++/* LSM_ORDER_LAST is always last. */+for(lsm=__start_lsm_info;lsm<__end_lsm_info;lsm++){+if(lsm->order==LSM_ORDER_LAST)+append_ordered_lsm(lsm,"last");+}}/* Is an LSM allowed to be initialized? */
This introduces the "lsm.enable=..." and "lsm.disable=..." boot parameters
which each can contain a comma-separated list of LSMs to enable or
disable, respectively. The string "all" matches all LSMs.
This has very similar functionality to the existing per-LSM enable
handling ("apparmor.enabled=...", etc), but provides a centralized
place to perform the changes. These parameters take precedent over any
LSM-specific boot parameters.
Disabling an LSM means it will not be considered when performing
initializations. Enabling an LSM means either undoing a previous
LSM-specific boot parameter disabling or a undoing a default-disabled
CONFIG setting.
For example: "lsm.disable=apparmor apparmor.enabled=1" will result in
AppArmor being disabled. "selinux.enabled=0 lsm.enable=selinux" will
result in SELinux being enabled.
Signed-off-by: Kees Cook <redacted>
---
.../admin-guide/kernel-parameters.txt | 12 ++++++++++
security/Kconfig | 4 +++-
security/security.c | 22 +++++++++++++++++++
3 files changed, 37 insertions(+), 1 deletion(-)
@@ -2276,6 +2276,18 @@ lsm.debug [SECURITY] Enable LSM initialization debugging output.+ lsm.disable=lsm1,...,lsmN+ [SECURITY] Comma-separated list of LSMs to disable+ at boot time. This overrides "lsm.enable=",+ CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and boot+ parameters.++ lsm.enable=lsm1,...,lsmN+ [SECURITY] Comma-separated list of LSMs to enable+ at boot time. This overrides any omissions from+ CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and+ boot parameters.+ machvec= [IA-64] Force the use of a particular machine-vector (machvec) in a generic kernel. Example: machvec=hpzx1_swiotlb
@@ -185,6 +187,10 @@ static void __init prepare_lsm_enable(void){/* Prepare defaults. */parse_lsm_enable(builtin_lsm_enable,default_enabled,true);++/* Process "lsm.enable=" and "lsm.disable=", if given. */+parse_lsm_enable(chosen_lsm_enable,set_enabled,true);+parse_lsm_enable(chosen_lsm_disable,set_enabled,false);}/**
@@ -240,6 +246,22 @@ static int __init enable_debug(char *str)}__setup("lsm.debug",enable_debug);+/* Explicitly enable a list of LSMs. */+staticint__initenable_lsm(char*str)+{+chosen_lsm_enable=str;+return1;+}+__setup("lsm.enable=",enable_lsm);++/* Explicitly disable a list of LSMs. */+staticint__initdisable_lsm(char*str)+{+chosen_lsm_disable=str;+return1;+}+__setup("lsm.disable=",disable_lsm);+staticboolmatch_last_lsm(constchar*list,constchar*lsm){constchar*last;
This moves the string handling for "security=" boot parameter into
a stored pointer instead of a string duplicate. This will allow
easier handling of the string when switching logic to use the coming
enable/disable infrastructure.
Signed-off-by: Kees Cook <redacted>
---
security/security.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
@@ -34,18 +34,14 @@#define MAX_LSM_EVM_XATTR 2-/* Maximum number of letters for an LSM name string */-#define SECURITY_NAME_MAX 10-structsecurity_hook_headssecurity_hook_heads__lsm_ro_after_init;staticATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);char*lsm_names;/* Boot-time LSM user choice */-static__initdatacharchosen_lsm[SECURITY_NAME_MAX+1]=-CONFIG_DEFAULT_SECURITY;static__initdataconstchar*chosen_lsm_enable;static__initdataconstchar*chosen_lsm_disable;+static__initdataconstchar*chosen_major_lsm;static__initconstconstchar*constbuiltin_lsm_enable=CONFIG_LSM_ENABLE;
@@ -112,7 +108,7 @@ static bool __init lsm_allowed(struct lsm_info *lsm)returntrue;/* Disabled if this LSM isn't the chosen one. */-if(strcmp(lsm->name,chosen_lsm)!=0)+if(strcmp(lsm->name,chosen_major_lsm)!=0)returnfalse;returntrue;
@@ -191,6 +187,9 @@ static void __init prepare_lsm_enable(void)/* Process "lsm.enable=" and "lsm.disable=", if given. */parse_lsm_enable(chosen_lsm_enable,set_enabled,true);parse_lsm_enable(chosen_lsm_disable,set_enabled,false);++if(!chosen_major_lsm)+chosen_major_lsm=CONFIG_DEFAULT_SECURITY;}/**
@@ -231,12 +230,12 @@ int __init security_init(void)}/* Save user chosen LSM */-staticint__initchoose_lsm(char*str)+staticint__initchoose_major_lsm(char*str){-strncpy(chosen_lsm,str,SECURITY_NAME_MAX);+chosen_major_lsm=str;return1;}-__setup("security=",choose_lsm);+__setup("security=",choose_major_lsm);/* Enable LSM order debugging. */staticint__initenable_debug(char*str)
This partially reverts commit 58eacfffc417 ("init, tracing: instrument
security and console initcall trace events") since security init calls
are about to no longer resemble regular init calls.
I'm not against the change, but how much are they going to "no longer
resemble regular init calls"?
-- Steve
quoted hunk
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Abderrahmane Benbachir <redacted>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: linux-security-module at vger.kernel.org
Signed-off-by: Kees Cook <redacted>
---
security/security.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
@@ -30,8 +30,6 @@#include<linux/string.h>#include<net/flow.h>-#include<trace/events/initcall.h>-#define MAX_LSM_EVM_XATTR 2/* Maximum number of letters for an LSM name string */
This partially reverts commit 58eacfffc417 ("init, tracing: instrument
security and console initcall trace events") since security init calls
are about to no longer resemble regular init calls.
I'm not against the change, but how much are they going to "no longer
resemble regular init calls"?
My take on "regular" init calls is that they're always run, link-time
ordered, etc. The changes proposed here will make it so not all
initialization are run depending on runtime configurations, ordering
will be flexible, etc.
-Kees
--
Kees Cook
Pixel Security
v3:
- add CONFIG_LSM_ENABLE and refactor resulting logic
Kees, you can add my
Reviewed-by:Casey Schaufler [off-list ref]
for this entire patch set. Thank you for taking this on, it's
a significant and important chunk of the LSM infrastructure
update.
...
Breakdown of patches:
Infrastructure improvements (no logical changes):
LSM: Correctly announce start of LSM initialization
vmlinux.lds.h: Avoid copy/paste of security_init section
LSM: Rename .security_initcall section to .lsm_info
LSM: Remove initcall tracing
LSM: Convert from initcall to struct lsm_info
vmlinux.lds.h: Move LSM_TABLE into INIT_DATA
LSM: Convert security_initcall() into DEFINE_LSM()
LSM: Record LSM name in struct lsm_info
LSM: Provide init debugging infrastructure
LSM: Don't ignore initialization failures
Split "integrity" out into "ordered initialization" (no logical changes):
LSM: Introduce LSM_FLAG_LEGACY_MAJOR
LSM: Provide separate ordered initialization
Provide centralized LSM enable/disable infrastructure:
LoadPin: Rename "enable" to "enforce"
LSM: Plumb visibility into optional "enabled" state
LSM: Lift LSM selection out of individual LSMs
LSM: Prepare for arbitrary LSM enabling
LSM: Introduce CONFIG_LSM_ENABLE
LSM: Introduce lsm.enable= and lsm.disable=
LSM: Prepare for reorganizing "security=" logic
LSM: Refactor "security=" in terms of enable/disable
Provide centralized LSM ordering infrastructure:
LSM: Build ordered list of ordered LSMs for init
LSM: Introduce CONFIG_LSM_ORDER
LSM: Introduce "lsm.order=" for boottime ordering
Move minor LSMs into ordered LSM initialization:
LoadPin: Initialize as ordered LSM
Yama: Initialize as ordered LSM
LSM: Introduce enum lsm_order
capability: Initialize as LSM_ORDER_FIRST
Move major LSMs into ordered LSM initialization:
LSM: Separate idea of "major" LSM from "exclusive" LSM
LSM: Add all exclusive LSMs to ordered initialization
-Kees
.../admin-guide/kernel-parameters.txt | 20 +
arch/arc/kernel/vmlinux.lds.S | 1 -
arch/arm/kernel/vmlinux-xip.lds.S | 1 -
arch/arm64/kernel/vmlinux.lds.S | 1 -
arch/h8300/kernel/vmlinux.lds.S | 1 -
arch/microblaze/kernel/vmlinux.lds.S | 2 -
arch/powerpc/kernel/vmlinux.lds.S | 2 -
arch/um/include/asm/common.lds.S | 2 -
arch/xtensa/kernel/vmlinux.lds.S | 1 -
include/asm-generic/vmlinux.lds.h | 25 +-
include/linux/init.h | 2 -
include/linux/lsm_hooks.h | 43 ++-
include/linux/module.h | 1 -
security/Kconfig | 61 ++-
security/apparmor/lsm.c | 16 +-
security/commoncap.c | 8 +-
security/integrity/iint.c | 5 +-
security/loadpin/Kconfig | 4 +-
security/loadpin/loadpin.c | 28 +-
security/security.c | 351 +++++++++++++++---
security/selinux/hooks.c | 16 +-
security/smack/smack_lsm.c | 8 +-
security/tomoyo/tomoyo.c | 7 +-
security/yama/yama_lsm.c | 7 +-
24 files changed, 438 insertions(+), 175 deletions(-)
On Fri, Sep 28, 2018 at 8:55 AM, Casey Schaufler [off-list ref] wrote:
On 9/24/2018 5:18 PM, Kees Cook wrote:
quoted
v3:
- add CONFIG_LSM_ENABLE and refactor resulting logic
Kees, you can add my
Reviewed-by:Casey Schaufler [off-list ref]
for this entire patch set. Thank you for taking this on, it's
a significant and important chunk of the LSM infrastructure
update.
Thanks!
John, you'd looked at this a bit too -- do the results line up with
your expectations?
Any thoughts from SELinux, TOMOYO, or IMA folks?
-Kees
quoted
...
Breakdown of patches:
Infrastructure improvements (no logical changes):
LSM: Correctly announce start of LSM initialization
vmlinux.lds.h: Avoid copy/paste of security_init section
LSM: Rename .security_initcall section to .lsm_info
LSM: Remove initcall tracing
LSM: Convert from initcall to struct lsm_info
vmlinux.lds.h: Move LSM_TABLE into INIT_DATA
LSM: Convert security_initcall() into DEFINE_LSM()
LSM: Record LSM name in struct lsm_info
LSM: Provide init debugging infrastructure
LSM: Don't ignore initialization failures
Split "integrity" out into "ordered initialization" (no logical changes):
LSM: Introduce LSM_FLAG_LEGACY_MAJOR
LSM: Provide separate ordered initialization
Provide centralized LSM enable/disable infrastructure:
LoadPin: Rename "enable" to "enforce"
LSM: Plumb visibility into optional "enabled" state
LSM: Lift LSM selection out of individual LSMs
LSM: Prepare for arbitrary LSM enabling
LSM: Introduce CONFIG_LSM_ENABLE
LSM: Introduce lsm.enable= and lsm.disable=
LSM: Prepare for reorganizing "security=" logic
LSM: Refactor "security=" in terms of enable/disable
Provide centralized LSM ordering infrastructure:
LSM: Build ordered list of ordered LSMs for init
LSM: Introduce CONFIG_LSM_ORDER
LSM: Introduce "lsm.order=" for boottime ordering
Move minor LSMs into ordered LSM initialization:
LoadPin: Initialize as ordered LSM
Yama: Initialize as ordered LSM
LSM: Introduce enum lsm_order
capability: Initialize as LSM_ORDER_FIRST
Move major LSMs into ordered LSM initialization:
LSM: Separate idea of "major" LSM from "exclusive" LSM
LSM: Add all exclusive LSMs to ordered initialization
-Kees
.../admin-guide/kernel-parameters.txt | 20 +
arch/arc/kernel/vmlinux.lds.S | 1 -
arch/arm/kernel/vmlinux-xip.lds.S | 1 -
arch/arm64/kernel/vmlinux.lds.S | 1 -
arch/h8300/kernel/vmlinux.lds.S | 1 -
arch/microblaze/kernel/vmlinux.lds.S | 2 -
arch/powerpc/kernel/vmlinux.lds.S | 2 -
arch/um/include/asm/common.lds.S | 2 -
arch/xtensa/kernel/vmlinux.lds.S | 1 -
include/asm-generic/vmlinux.lds.h | 25 +-
include/linux/init.h | 2 -
include/linux/lsm_hooks.h | 43 ++-
include/linux/module.h | 1 -
security/Kconfig | 61 ++-
security/apparmor/lsm.c | 16 +-
security/commoncap.c | 8 +-
security/integrity/iint.c | 5 +-
security/loadpin/Kconfig | 4 +-
security/loadpin/loadpin.c | 28 +-
security/security.c | 351 +++++++++++++++---
security/selinux/hooks.c | 16 +-
security/smack/smack_lsm.c | 8 +-
security/tomoyo/tomoyo.c | 7 +-
security/yama/yama_lsm.c | 7 +-
24 files changed, 438 insertions(+), 175 deletions(-)
From: Stephen Smalley <hidden> Date: 2018-09-28 20:23:51
On 09/28/2018 04:01 PM, Kees Cook wrote:
On Fri, Sep 28, 2018 at 8:55 AM, Casey Schaufler [off-list ref] wrote:
quoted
On 9/24/2018 5:18 PM, Kees Cook wrote:
quoted
v3:
- add CONFIG_LSM_ENABLE and refactor resulting logic
Kees, you can add my
Reviewed-by:Casey Schaufler [off-list ref]
for this entire patch set. Thank you for taking this on, it's
a significant and important chunk of the LSM infrastructure
update.
Thanks!
John, you'd looked at this a bit too -- do the results line up with
your expectations?
Any thoughts from SELinux, TOMOYO, or IMA folks?
What's it relative to? First patch fails for me on current security/next.
Is there a branch in your repo that has the v3 patches?
-Kees
quoted
quoted
...
Breakdown of patches:
Infrastructure improvements (no logical changes):
LSM: Correctly announce start of LSM initialization
vmlinux.lds.h: Avoid copy/paste of security_init section
LSM: Rename .security_initcall section to .lsm_info
LSM: Remove initcall tracing
LSM: Convert from initcall to struct lsm_info
vmlinux.lds.h: Move LSM_TABLE into INIT_DATA
LSM: Convert security_initcall() into DEFINE_LSM()
LSM: Record LSM name in struct lsm_info
LSM: Provide init debugging infrastructure
LSM: Don't ignore initialization failures
Split "integrity" out into "ordered initialization" (no logical changes):
LSM: Introduce LSM_FLAG_LEGACY_MAJOR
LSM: Provide separate ordered initialization
Provide centralized LSM enable/disable infrastructure:
LoadPin: Rename "enable" to "enforce"
LSM: Plumb visibility into optional "enabled" state
LSM: Lift LSM selection out of individual LSMs
LSM: Prepare for arbitrary LSM enabling
LSM: Introduce CONFIG_LSM_ENABLE
LSM: Introduce lsm.enable= and lsm.disable=
LSM: Prepare for reorganizing "security=" logic
LSM: Refactor "security=" in terms of enable/disable
Provide centralized LSM ordering infrastructure:
LSM: Build ordered list of ordered LSMs for init
LSM: Introduce CONFIG_LSM_ORDER
LSM: Introduce "lsm.order=" for boottime ordering
Move minor LSMs into ordered LSM initialization:
LoadPin: Initialize as ordered LSM
Yama: Initialize as ordered LSM
LSM: Introduce enum lsm_order
capability: Initialize as LSM_ORDER_FIRST
Move major LSMs into ordered LSM initialization:
LSM: Separate idea of "major" LSM from "exclusive" LSM
LSM: Add all exclusive LSMs to ordered initialization
-Kees
.../admin-guide/kernel-parameters.txt | 20 +
arch/arc/kernel/vmlinux.lds.S | 1 -
arch/arm/kernel/vmlinux-xip.lds.S | 1 -
arch/arm64/kernel/vmlinux.lds.S | 1 -
arch/h8300/kernel/vmlinux.lds.S | 1 -
arch/microblaze/kernel/vmlinux.lds.S | 2 -
arch/powerpc/kernel/vmlinux.lds.S | 2 -
arch/um/include/asm/common.lds.S | 2 -
arch/xtensa/kernel/vmlinux.lds.S | 1 -
include/asm-generic/vmlinux.lds.h | 25 +-
include/linux/init.h | 2 -
include/linux/lsm_hooks.h | 43 ++-
include/linux/module.h | 1 -
security/Kconfig | 61 ++-
security/apparmor/lsm.c | 16 +-
security/commoncap.c | 8 +-
security/integrity/iint.c | 5 +-
security/loadpin/Kconfig | 4 +-
security/loadpin/loadpin.c | 28 +-
security/security.c | 351 +++++++++++++++---
security/selinux/hooks.c | 16 +-
security/smack/smack_lsm.c | 8 +-
security/tomoyo/tomoyo.c | 7 +-
security/yama/yama_lsm.c | 7 +-
24 files changed, 438 insertions(+), 175 deletions(-)
From: Stephen Smalley <hidden> Date: 2018-09-28 20:31:43
On 09/28/2018 04:25 PM, Stephen Smalley wrote:
On 09/28/2018 04:01 PM, Kees Cook wrote:
quoted
On Fri, Sep 28, 2018 at 8:55 AM, Casey Schaufler
[off-list ref] wrote:
quoted
On 9/24/2018 5:18 PM, Kees Cook wrote:
quoted
v3:
- add CONFIG_LSM_ENABLE and refactor resulting logic
Kees, you can add my
???????? Reviewed-by:Casey Schaufler [off-list ref]
for this entire patch set. Thank you for taking this on, it's
a significant and important chunk of the LSM infrastructure
update.
Thanks!
John, you'd looked at this a bit too -- do the results line up with
your expectations?
Any thoughts from SELinux, TOMOYO, or IMA folks?
What's it relative to?? First patch fails for me on current security/next.
Never mind - user error ;)
Is there a branch in your repo that has the v3 patches?
But still wondered about this one.
quoted
-Kees
quoted
quoted
...
Breakdown of patches:
Infrastructure improvements (no logical changes):
?? LSM: Correctly announce start of LSM initialization
?? vmlinux.lds.h: Avoid copy/paste of security_init section
?? LSM: Rename .security_initcall section to .lsm_info
?? LSM: Remove initcall tracing
?? LSM: Convert from initcall to struct lsm_info
?? vmlinux.lds.h: Move LSM_TABLE into INIT_DATA
?? LSM: Convert security_initcall() into DEFINE_LSM()
?? LSM: Record LSM name in struct lsm_info
?? LSM: Provide init debugging infrastructure
?? LSM: Don't ignore initialization failures
Split "integrity" out into "ordered initialization" (no logical
changes):
?? LSM: Introduce LSM_FLAG_LEGACY_MAJOR
?? LSM: Provide separate ordered initialization
Provide centralized LSM enable/disable infrastructure:
?? LoadPin: Rename "enable" to "enforce"
?? LSM: Plumb visibility into optional "enabled" state
?? LSM: Lift LSM selection out of individual LSMs
?? LSM: Prepare for arbitrary LSM enabling
?? LSM: Introduce CONFIG_LSM_ENABLE
?? LSM: Introduce lsm.enable= and lsm.disable=
?? LSM: Prepare for reorganizing "security=" logic
?? LSM: Refactor "security=" in terms of enable/disable
Provide centralized LSM ordering infrastructure:
?? LSM: Build ordered list of ordered LSMs for init
?? LSM: Introduce CONFIG_LSM_ORDER
?? LSM: Introduce "lsm.order=" for boottime ordering
Move minor LSMs into ordered LSM initialization:
?? LoadPin: Initialize as ordered LSM
?? Yama: Initialize as ordered LSM
?? LSM: Introduce enum lsm_order
?? capability: Initialize as LSM_ORDER_FIRST
Move major LSMs into ordered LSM initialization:
?? LSM: Separate idea of "major" LSM from "exclusive" LSM
?? LSM: Add all exclusive LSMs to ordered initialization
-Kees
? .../admin-guide/kernel-parameters.txt???????? |? 20 +
? arch/arc/kernel/vmlinux.lds.S???????????????? |?? 1 -
? arch/arm/kernel/vmlinux-xip.lds.S???????????? |?? 1 -
? arch/arm64/kernel/vmlinux.lds.S?????????????? |?? 1 -
? arch/h8300/kernel/vmlinux.lds.S?????????????? |?? 1 -
? arch/microblaze/kernel/vmlinux.lds.S????????? |?? 2 -
? arch/powerpc/kernel/vmlinux.lds.S???????????? |?? 2 -
? arch/um/include/asm/common.lds.S????????????? |?? 2 -
? arch/xtensa/kernel/vmlinux.lds.S????????????? |?? 1 -
? include/asm-generic/vmlinux.lds.h???????????? |? 25 +-
? include/linux/init.h????????????????????????? |?? 2 -
? include/linux/lsm_hooks.h???????????????????? |? 43 ++-
? include/linux/module.h??????????????????????? |?? 1 -
? security/Kconfig????????????????????????????? |? 61 ++-
? security/apparmor/lsm.c?????????????????????? |? 16 +-
? security/commoncap.c????????????????????????? |?? 8 +-
? security/integrity/iint.c???????????????????? |?? 5 +-
? security/loadpin/Kconfig????????????????????? |?? 4 +-
? security/loadpin/loadpin.c??????????????????? |? 28 +-
? security/security.c?????????????????????????? | 351
+++++++++++++++---
? security/selinux/hooks.c????????????????????? |? 16 +-
? security/smack/smack_lsm.c??????????????????? |?? 8 +-
? security/tomoyo/tomoyo.c????????????????????? |?? 7 +-
? security/yama/yama_lsm.c????????????????????? |?? 7 +-
? 24 files changed, 438 insertions(+), 175 deletions(-)
On Fri, Sep 28, 2018 at 1:33 PM, Stephen Smalley [off-list ref] wrote:
On 09/28/2018 04:25 PM, Stephen Smalley wrote:
quoted
On 09/28/2018 04:01 PM, Kees Cook wrote:
quoted
On Fri, Sep 28, 2018 at 8:55 AM, Casey Schaufler [off-list ref]
wrote:
quoted
On 9/24/2018 5:18 PM, Kees Cook wrote:
quoted
v3:
- add CONFIG_LSM_ENABLE and refactor resulting logic
Kees, you can add my
Reviewed-by:Casey Schaufler [off-list ref]
for this entire patch set. Thank you for taking this on, it's
a significant and important chunk of the LSM infrastructure
update.
Thanks!
John, you'd looked at this a bit too -- do the results line up with
your expectations?
Any thoughts from SELinux, TOMOYO, or IMA folks?
What's it relative to? First patch fails for me on current security/next.
Never mind - user error ;)
FWIW, it's against v4.19-rc2.
quoted
Is there a branch in your repo that has the v3 patches?
On Fri, Sep 28, 2018 at 8:55 AM, Casey Schaufler [off-list ref] wrote:
quoted
On 9/24/2018 5:18 PM, Kees Cook wrote:
quoted
v3:
- add CONFIG_LSM_ENABLE and refactor resulting logic
Kees, you can add my
Reviewed-by:Casey Schaufler [off-list ref]
for this entire patch set. Thank you for taking this on, it's
a significant and important chunk of the LSM infrastructure
update.
Thanks!
John, you'd looked at this a bit too -- do the results line up with
your expectations?
Any thoughts from SELinux, TOMOYO, or IMA folks?
I'm OK with this approach. Thank you.
Just wondering what is "__lsm_name_##lsm" for...
+#define DEFINE_LSM(lsm) \
+ static const char __lsm_name_##lsm[] __initconst \
+ __aligned(1) = #lsm; \
+ static struct lsm_info __lsm_##lsm \
+ __used __section(.lsm_info.init) \
+ __aligned(sizeof(unsigned long)) \
+ = { \
+ .name = __lsm_name_##lsm, \
+
+#define END_LSM }
We could do something like below so that funny END_LSM is not required?
I felt } like a typo error at the first glance. What we need is to
gather into one section with appropriate alignment, isn't it?
#define LSM_INFO \
static struct lsm_info __lsm_ \
__used __section(.lsm_info.init) \
__aligned(sizeof(unsigned long)) \
LSM_INFO = {
.name = "tomoyo",
.flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
.init = tomoyo_init,
};
On Sat, Sep 29, 2018 at 3:48 AM, Tetsuo Handa
[off-list ref] wrote:
On 2018/09/29 5:01, Kees Cook wrote:
quoted
On Fri, Sep 28, 2018 at 8:55 AM, Casey Schaufler [off-list ref] wrote:
quoted
On 9/24/2018 5:18 PM, Kees Cook wrote:
quoted
v3:
- add CONFIG_LSM_ENABLE and refactor resulting logic
Kees, you can add my
Reviewed-by:Casey Schaufler [off-list ref]
for this entire patch set. Thank you for taking this on, it's
a significant and important chunk of the LSM infrastructure
update.
Thanks!
John, you'd looked at this a bit too -- do the results line up with
your expectations?
Any thoughts from SELinux, TOMOYO, or IMA folks?
I wasn't super happy with the END_LSM thing, but I wanted to be able
to declare the name as __initconst, otherwise it needlessly stays in
memory after init. That said, it's not a huge deal, and maybe
readability trumps a tiny meory savings?
We could do something like below so that funny END_LSM is not required?
I felt } like a typo error at the first glance. What we need is to
gather into one section with appropriate alignment, isn't it?
#define LSM_INFO \
static struct lsm_info __lsm_ \
__used __section(.lsm_info.init) \
__aligned(sizeof(unsigned long)) \
LSM_INFO = {
.name = "tomoyo",
.flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
.init = tomoyo_init,
};
I thought the structure instances would need a unique name, but it
seems the section naming removes that requirement. This seems only to
be needed if we had multiple LSMs defined in the same source file.
Though I wonder if this would be a problem for LTO in the future?
I'm happy to do whatever.
-Kees
--
Kees Cook
Pixel Security
From: John Johansen <john.johansen@canonical.com> Date: 2018-09-29 18:20:04
On 09/29/2018 03:48 AM, Tetsuo Handa wrote:
On 2018/09/29 5:01, Kees Cook wrote:
quoted
On Fri, Sep 28, 2018 at 8:55 AM, Casey Schaufler [off-list ref] wrote:
quoted
On 9/24/2018 5:18 PM, Kees Cook wrote:
quoted
v3:
- add CONFIG_LSM_ENABLE and refactor resulting logic
Kees, you can add my
Reviewed-by:Casey Schaufler [off-list ref]
for this entire patch set. Thank you for taking this on, it's
a significant and important chunk of the LSM infrastructure
update.
Thanks!
John, you'd looked at this a bit too -- do the results line up with
your expectations?
Any thoughts from SELinux, TOMOYO, or IMA folks?
I'm OK with this approach. Thank you.
Just wondering what is "__lsm_name_##lsm" for...
+#define DEFINE_LSM(lsm) \
+ static const char __lsm_name_##lsm[] __initconst \
+ __aligned(1) = #lsm; \
+ static struct lsm_info __lsm_##lsm \
+ __used __section(.lsm_info.init) \
+ __aligned(sizeof(unsigned long)) \
+ = { \
+ .name = __lsm_name_##lsm, \
+
+#define END_LSM }
We could do something like below so that funny END_LSM is not required?
I felt } like a typo error at the first glance. What we need is to
gather into one section with appropriate alignment, isn't it?
well and Kees was trying to automagically set the name. This threw
me off too at first and I am still trying to figure out if I would
prefer something simpler, and more standard like below.
I wasn't super happy with the END_LSM thing, but I wanted to be able
to declare the name as __initconst, otherwise it needlessly stays in
memory after init. That said, it's not a huge deal, and maybe
readability trumps a tiny meory savings?
The value of .name field is a few bytes string, and is not sensitive
information. Keeping such string in non-__initdata section unlikely
increases total memory pages required for that module.
Unless we need to generate unique address of such string for some reason,
I think that this saving is pointless.
I wasn't super happy with the END_LSM thing, but I wanted to be able
to declare the name as __initconst, otherwise it needlessly stays in
memory after init. That said, it's not a huge deal, and maybe
readability trumps a tiny meory savings?
The value of .name field is a few bytes string, and is not sensitive
information. Keeping such string in non-__initdata section unlikely
increases total memory pages required for that module.
Unless we need to generate unique address of such string for some reason,
I think that this saving is pointless.
Okay, sounds good. I will adjust the macro and respin with a v4.
Thanks!
-Kees
--
Kees Cook
Pixel Security
This partially reverts commit 58eacfffc417 ("init, tracing: instrument
security and console initcall trace events") since security init calls
are about to no longer resemble regular init calls.
I'm not against the change, but how much are they going to "no longer
resemble regular init calls"?
My take on "regular" init calls is that they're always run, link-time
ordered, etc. The changes proposed here will make it so not all
initialization are run depending on runtime configurations, ordering
will be flexible, etc.
Will it still be a good idea to have a tracepoint for those calls?
Perhaps not an initcall tracepoint but some other kind?
-- Steve
This partially reverts commit 58eacfffc417 ("init, tracing: instrument
security and console initcall trace events") since security init calls
are about to no longer resemble regular init calls.
I'm not against the change, but how much are they going to "no longer
resemble regular init calls"?
My take on "regular" init calls is that they're always run, link-time
ordered, etc. The changes proposed here will make it so not all
initialization are run depending on runtime configurations, ordering
will be flexible, etc.
Will it still be a good idea to have a tracepoint for those calls?
Perhaps not an initcall tracepoint but some other kind?
I'm not opposed. It could be a follow-up patch, I assume?
-Kees
--
Kees Cook
Pixel Security
From: James Morris <jmorris@namei.org> Date: 2018-10-01 19:53:58
On Mon, 24 Sep 2018, Kees Cook wrote:
For a while now, the LSM core has said it was "initializED", rather than
"initializING". This adjust the report to be more accurate (i.e. before
this was reported before any LSMs had been initialized.)
Signed-off-by: Kees Cook <redacted>
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: James Morris <redacted>
--
James Morris
[off-list ref]
From: James Morris <jmorris@namei.org> Date: 2018-10-01 19:57:34
On Mon, 24 Sep 2018, Kees Cook wrote:
In preparation for switching from initcall to just a regular set of
pointers in a section, rename the internal section name.
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Ard Biesheuvel <redacted>
Cc: linux-arch@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Kees Cook <redacted>
Reviewed-by: James Morris <redacted>
--
James Morris
[off-list ref]
From: James Morris <jmorris@namei.org> Date: 2018-10-01 19:59:46
On Mon, 24 Sep 2018, Kees Cook wrote:
In preparation for doing more interesting LSM init probing, this converts
the existing initcall system into an explicit call into a function pointer
from a section-collected struct lsm_info array.
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Ard Biesheuvel <redacted>
Cc: Paul Moore <paul@paul-moore.com>
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Kees Cook <redacted>
Reviewed-by: James Morris <redacted>
--
James Morris
[off-list ref]
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:05:45
On 09/24/2018 05:18 PM, Kees Cook wrote:
For a while now, the LSM core has said it was "initializED", rather than
"initializING". This adjust the report to be more accurate (i.e. before
this was reported before any LSMs had been initialized.)
Signed-off-by: Kees Cook <redacted>
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:06:27
On 09/24/2018 05:18 PM, Kees Cook wrote:
In preparation for switching from initcall to just a regular set of
pointers in a section, rename the internal section name.
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Ard Biesheuvel <redacted>
Cc: linux-arch@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Kees Cook <redacted>
Reviewed-by: John Johansen <john.johansen@canonical.com>
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:08:03
On 09/24/2018 05:18 PM, Kees Cook wrote:
This partially reverts commit 58eacfffc417 ("init, tracing: instrument
security and console initcall trace events") since security init calls
are about to no longer resemble regular init calls.
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Abderrahmane Benbachir <redacted>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Kees Cook <redacted>
Reviewed-by: John Johansen <john.johansen@canonical.com>
though I do think it would be a good idea to add a new set
of trace points, but that can come as a separate patch
@@ -30,8 +30,6 @@#include<linux/string.h>#include<net/flow.h>-#include<trace/events/initcall.h>-#define MAX_LSM_EVM_XATTR 2/* Maximum number of letters for an LSM name string */
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:08:36
On 09/24/2018 05:18 PM, Kees Cook wrote:
In preparation for doing more interesting LSM init probing, this converts
the existing initcall system into an explicit call into a function pointer
from a section-collected struct lsm_info array.
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Ard Biesheuvel <redacted>
Cc: Paul Moore <paul@paul-moore.com>
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Kees Cook <redacted>
Reviewed-by: John Johansen <john.johansen@canonical.com>
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:10:33
On 09/24/2018 05:18 PM, Kees Cook wrote:
Since the struct lsm_info table is not an initcall, we can just move it
into INIT_DATA like all the other tables.
Cc: linux-arch@vger.kernel.org
Signed-off-by: Kees Cook <redacted>
Reviewed-by: John Johansen <john.johansen@canonical.com>
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:12:25
On 09/24/2018 05:18 PM, Kees Cook wrote:
quoted hunk
Instead of using argument-based initializers, switch to defining the
contents of struct lsm_info on a per-LSM basis. This also drops
the final use of the now inaccurate "initcall" naming.
Cc: John Johansen <john.johansen@canonical.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <redacted>
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Mimi Zohar <redacted>
Cc: linux-security-module@vger.kernel.org
Cc: selinux@tycho.nsa.gov
Signed-off-by: Kees Cook <redacted>
---
include/linux/lsm_hooks.h | 6 ++++--
security/apparmor/lsm.c | 4 +++-
security/integrity/iint.c | 4 +++-
security/selinux/hooks.c | 4 +++-
security/smack/smack_lsm.c | 4 +++-
security/tomoyo/tomoyo.c | 4 +++-
6 files changed, 19 insertions(+), 7 deletions(-)
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:14:04
On 09/24/2018 05:18 PM, Kees Cook wrote:
In preparation for making LSM selections outside of the LSMs, include
the name of LSMs in struct lsm_info.
Cc: James Morris <redacted>
Signed-off-by: Kees Cook <redacted>
I'll leave this one until after the changes you have already discussed with Tetsuo around, END_LSM and .name
@@ -2274,6 +2274,8 @@ ltpc= [NET] Format: <io>,<irq>,<dma>+ lsm.debug [SECURITY] Enable LSM initialization debugging output.+ machvec= [IA-64] Force the use of a particular machine-vector (machvec) in a generic kernel. Example: machvec=hpzx1_swiotlb
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:15:32
On 09/24/2018 05:18 PM, Kees Cook wrote:
This adds a flag for the current "major" LSMs to distinguish them when
we have a universal method for ordering all LSMs. It's called "legacy"
since the distinction of "major" will go away in the blob-sharing world.
Signed-off-by: Kees Cook <redacted>
Reviewed-by: John Johansen <john.johansen@canonical.com>
@@ -7203,6 +7203,7 @@ void selinux_complete_init(void)/* SELinux requires early initialization in order to labelallprocessesandobjectswhentheyarecreated.*/DEFINE_LSM(selinux)+.flags=LSM_FLAG_LEGACY_MAJOR,.init=selinux_init,END_LSM;
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:17:20
On 09/24/2018 05:18 PM, Kees Cook wrote:
This provides a place for ordered LSMs to be initialized, separate from
the "major" LSMs. This is mainly a copy/paste from major_lsm_init() to
ordered_lsm_init(), but it will change drastically in later patches.
What is not obvious in the patch is that this change moves the integrity
LSM from major_lsm_init() into ordered_lsm_init(), since it is not marked
with the LSM_FLAG_LEGACY_MAJOR. As it is the only LSM in the "ordered"
list, there is no reordering yet created.
Signed-off-by: Kees Cook <redacted>
I know its already being done, but I don't like splitting the init
order
Reviewed-by: John Johansen <john.johansen@canonical.com>
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:17:44
On 09/24/2018 05:18 PM, Kees Cook wrote:
LoadPin's "enable" setting is really about enforcement, not whether
or not the LSM is using LSM hooks. Instead, split this out so that LSM
enabling can be logically distinct from whether enforcement is happening
(for example, the pinning happens when the LSM is enabled, but the pin
is only checked when "enforce" is set). This allows LoadPin to continue
to operate sanely in test environments once LSM enable/disable is
centrally handled (i.e. we want LoadPin to be enabled separately from
its enforcement).
Signed-off-by: Kees Cook <redacted>
Reviewed-by: John Johansen <john.johansen@canonical.com>
@@ -97,7 +97,7 @@ static void check_pinning_enforcement(struct super_block *mnt_sb)loadpin_sysctl_table))pr_notice("sysctl registration failed!\n");else-pr_info("load pinning can be disabled.\n");+pr_info("enforcement can be disabled.\n");}elsepr_info("load pinning engaged.\n");}
@@ -128,7 +128,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)/* This handles the older init_module API that has a NULL file. */if(!file){-if(!enabled){+if(!enforcing){report_load(origin,NULL,"old-api-pinning-ignored");return0;}
@@ -186,10 +186,11 @@ static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {void__initloadpin_add_hooks(void){-pr_info("ready to pin (currently %sabled)",enabled?"en":"dis");+pr_info("ready to pin (currently %senforcing)\n",+enforcing?"":"not ");security_add_hooks(loadpin_hooks,ARRAY_SIZE(loadpin_hooks),"loadpin");}/* Should not be mutable after boot, so not listed in sysfs (perm == 0). */-module_param(enabled,int,0);-MODULE_PARM_DESC(enabled,"Pin module/firmware loading (default: true)");+module_param(enforcing,int,0);+MODULE_PARM_DESC(enforcing,"Enforce module/firmware pinning");
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:18:15
On 09/24/2018 05:18 PM, Kees Cook wrote:
In preparation for lifting the "is this LSM enabled?" logic out of the
individual LSMs, pass in any special enabled state tracking (as needed
for SELinux, AppArmor, and LoadPin). This should be an "int" to include
handling any future cases where "enabled" is exposed via sysctl which
has no "bool" type.
Signed-off-by: Kees Cook <redacted>
Reviewed-by: John Johansen <john.johansen@canonical.com>
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:18:49
On 09/24/2018 05:18 PM, Kees Cook wrote:
As a prerequisite to adjusting LSM selection logic in the future, this
moves the selection logic up out of the individual major LSMs, making
their init functions only run when actually enabled. This considers all
LSMs enabled by default unless they specified an external "enable"
variable.
Signed-off-by: Kees Cook <redacted>
Reviewed-by: John Johansen <john.johansen@canonical.com>
@@ -1542,12 +1542,6 @@ static int __init apparmor_init(void){interror;-if(!apparmor_enabled||!security_module_enable("apparmor")){-aa_info_message("AppArmor disabled by boot time parameter");-apparmor_enabled=false;-return0;-}-aa_secids_init();error=aa_setup_dfa_engine();
@@ -52,33 +52,78 @@ static bool debug __initdata;pr_info(__VA_ARGS__);\}while(0)+staticbool__initis_enabled(structlsm_info*lsm)+{+if(!lsm->enabled||*lsm->enabled)+returntrue;++returnfalse;+}++/* Mark an LSM's enabled flag, if it exists. */+staticvoid__initset_enabled(structlsm_info*lsm,boolenabled)+{+if(lsm->enabled)+*lsm->enabled=enabled;+}++/* Is an LSM allowed to be initialized? */+staticbool__initlsm_allowed(structlsm_info*lsm)+{+/* Skip if the LSM is disabled. */+if(!is_enabled(lsm))+returnfalse;++/* Skip major-specific checks if not a major LSM. */+if((lsm->flags&LSM_FLAG_LEGACY_MAJOR)==0)+returntrue;++/* Disabled if this LSM isn't the chosen one. */+if(strcmp(lsm->name,chosen_lsm)!=0)+returnfalse;++returntrue;+}++/* Check if LSM should be enabled. Mark any that are disabled. */+staticvoid__initmaybe_initialize_lsm(structlsm_info*lsm)+{+intenabled=lsm_allowed(lsm);++/* Record enablement. */+set_enabled(lsm,enabled);++/* If selected, initialize the LSM. */+if(enabled){+intret;++init_debug("initializing %s\n",lsm->name);+ret=lsm->init();+WARN(ret,"%s failed to initialize: %d\n",lsm->name,ret);+}+}+staticvoid__initordered_lsm_init(void){structlsm_info*lsm;-intret;for(lsm=__start_lsm_info;lsm<__end_lsm_info;lsm++){if((lsm->flags&LSM_FLAG_LEGACY_MAJOR)!=0)continue;-init_debug("initializing %s\n",lsm->name);-ret=lsm->init();-WARN(ret,"%s failed to initialize: %d\n",lsm->name,ret);+maybe_initialize_lsm(lsm);}}staticvoid__initmajor_lsm_init(void){structlsm_info*lsm;-intret;for(lsm=__start_lsm_info;lsm<__end_lsm_info;lsm++){if((lsm->flags&LSM_FLAG_LEGACY_MAJOR)==0)continue;-init_debug("initializing %s\n",lsm->name);-ret=lsm->init();-WARN(ret,"%s failed to initialize: %d\n",lsm->name,ret);+maybe_initialize_lsm(lsm);}}
@@ -168,29 +213,6 @@ static int lsm_append(char *new, char **result)return0;}-/**-*security_module_enable-Loadgivensecuritymoduleonboot?-*@module:thenameofthemodule-*-*EachLSMmustpassthismethodbeforeregisteringitsownoperations-*toavoidsecurityregistrationraces.Thismethodmayalsobeused-*tocheckifyourLSMiscurrentlyloadedduringkernelinitialization.-*-*Returns:-*-*trueif:-*-*-ThepassedLSMistheonechosenbyuseratboottime,-*-orthepassedLSMisconfiguredasthedefaultandtheuserdidnot-*chooseanalternateLSMatboottime.-*-*Otherwise,returnfalse.-*/-int__initsecurity_module_enable(constchar*module)-{-return!strcmp(module,chosen_lsm);-}-/***security_add_hooks-Addamoduleshookstothehooklists.*@hooks:thehookstoadd
@@ -540,8 +540,6 @@ static int __init tomoyo_init(void){structcred*cred=(structcred*)current_cred();-if(!security_module_enable("tomoyo"))-return0;/* register ourselves with the security framework */security_add_hooks(tomoyo_hooks,ARRAY_SIZE(tomoyo_hooks),"tomoyo");printk(KERN_INFO"TOMOYO Linux initialized\n");
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:22:21
On 09/24/2018 05:18 PM, Kees Cook wrote:
Before now, all the LSMs that did not specify an "enable" variable in their
struct lsm_info were considered enabled by default. This prepares to make
LSM enabling more explicit. For all LSMs without an explicit "enable"
variable, a hard-coded storage location is chosen, and all LSMs without
an external "enable" state have their state explicitly set to "enabled".
This code appears more complex than it needs to be (comma-separated
list parsing and "set" function parameter) because its use will be
expanded on in the following patches to provide more explicit enabling.
Signed-off-by: Kees Cook <redacted>
Reviewed-by: John Johansen <john.johansen@canonical.com>
@@ -54,17 +54,46 @@ static bool debug __initdata;staticbool__initis_enabled(structlsm_info*lsm){-if(!lsm->enabled||*lsm->enabled)-returntrue;+if(WARN_ON(!lsm->enabled))+returnfalse;-returnfalse;+return*lsm->enabled;}/* Mark an LSM's enabled flag, if it exists. */-staticvoid__initset_enabled(structlsm_info*lsm,boolenabled)+staticintlsm_enabled_true__initdata=1;+staticintlsm_enabled_false__initdata=0;++staticvoid__initdefault_enabled(structlsm_info*lsm,boolenabled){+/* If storage location already set, skip this one. */if(lsm->enabled)+return;++/*+*WhenanLSMhasn'tconfiguredanenablevariable,wecanuse+*ahard-codedlocationforstoringthedefaultenabledstate.+*/+if(enabled)+lsm->enabled=&lsm_enabled_true;+else+lsm->enabled=&lsm_enabled_false;+}++staticvoid__initset_enabled(structlsm_info*lsm,boolenabled)+{+if(WARN_ON(!lsm->enabled))+return;++if(lsm->enabled==&lsm_enabled_true){+if(!enabled)+lsm->enabled=&lsm_enabled_false;+}elseif(lsm->enabled==&lsm_enabled_false){+if(enabled)+lsm->enabled=&lsm_enabled_true;+}else{*lsm->enabled=enabled;+}}/* Is an LSM allowed to be initialized? */
@@ -143,6 +201,9 @@ int __init security_init(void)i++)INIT_HLIST_HEAD(&list[i]);+/* Figure out which LSMs are enabled and disabled. */+prepare_lsm_enable();+/**LoadminorLSMs,withthecapabilitymodulealwaysfirst.*/
From: Steven Rostedt <rostedt@goodmis.org> Date: 2018-10-01 21:23:20
On Mon, 1 Oct 2018 14:07:55 -0700
John Johansen [off-list ref] wrote:
On 09/24/2018 05:18 PM, Kees Cook wrote:
quoted
This partially reverts commit 58eacfffc417 ("init, tracing: instrument
security and console initcall trace events") since security init calls
are about to no longer resemble regular init calls.
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Abderrahmane Benbachir <redacted>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Kees Cook <redacted>
Reviewed-by: John Johansen <john.johansen@canonical.com>
though I do think it would be a good idea to add a new set
of trace points, but that can come as a separate patch
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:35:00
On 09/24/2018 05:18 PM, Kees Cook wrote:
To provide a set of default-enabled LSMs at boot, this introduces the
new CONFIG_LSM_ENABLE. A value of "all" means all builtin LSMs are
enabled by default. Any unlisted LSMs will be implicitly disabled
(excepting those with LSM-specific CONFIGs for enabling/disabling).
The behavior of the LSM-specific CONFIGs for SELinux are AppArmor
unchanged: the default-enabled state for those LSMs remains controlled
through their LSM-specific "enable" CONFIGs.
Signed-off-by: Kees Cook <redacted>
The patch is fine but I am not sure I like the behavior. I much prefer
that its an explicit list and nothing is left to implicit. That is
if there is a conflict between the list and the LSM-specific config
the LSM is disabled.
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:47:03
On 09/24/2018 05:18 PM, Kees Cook wrote:
This introduces the "lsm.enable=..." and "lsm.disable=..." boot parameters
which each can contain a comma-separated list of LSMs to enable or
disable, respectively. The string "all" matches all LSMs.
This has very similar functionality to the existing per-LSM enable
handling ("apparmor.enabled=...", etc), but provides a centralized
place to perform the changes. These parameters take precedent over any
LSM-specific boot parameters.
Disabling an LSM means it will not be considered when performing
initializations. Enabling an LSM means either undoing a previous
LSM-specific boot parameter disabling or a undoing a default-disabled
CONFIG setting.
For example: "lsm.disable=apparmor apparmor.enabled=1" will result in
AppArmor being disabled. "selinux.enabled=0 lsm.enable=selinux" will
result in SELinux being enabled.
Signed-off-by: Kees Cook <redacted>
I don't like this. It brings about conflicting kernel params that are
bound to confuse users. Its pretty easy for a user to understand that
when they specify a parameter manually at boot, that it overrides the
build time default. But conflicting kernel parameters are a lot harder
to deal with.
I prefer a plain enabled= list being an override of the default build
time value. Where conflicts with LSM-specific configs always result in
the LSM being disabled with a complaint about the conflict.
Though I have yet to be convinced its worth the cost, I do recognize
it is sometimes convenient to disable a single LSM, instead of typing
in a whole list of what to enable. If we have to have conflicting
kernel parameters I would prefer that the conflict throw up a warning
and leaving the LSM with the conflicting config disabled.
@@ -2276,6 +2276,18 @@ lsm.debug [SECURITY] Enable LSM initialization debugging output.+ lsm.disable=lsm1,...,lsmN+ [SECURITY] Comma-separated list of LSMs to disable+ at boot time. This overrides "lsm.enable=",+ CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and boot+ parameters.++ lsm.enable=lsm1,...,lsmN+ [SECURITY] Comma-separated list of LSMs to enable+ at boot time. This overrides any omissions from+ CONFIG_LSM_ENABLE, and any per-LSM CONFIGs and+ boot parameters.+ machvec= [IA-64] Force the use of a particular machine-vector (machvec) in a generic kernel. Example: machvec=hpzx1_swiotlb
@@ -185,6 +187,10 @@ static void __init prepare_lsm_enable(void){/* Prepare defaults. */parse_lsm_enable(builtin_lsm_enable,default_enabled,true);++/* Process "lsm.enable=" and "lsm.disable=", if given. */+parse_lsm_enable(chosen_lsm_enable,set_enabled,true);+parse_lsm_enable(chosen_lsm_disable,set_enabled,false);}/**
@@ -240,6 +246,22 @@ static int __init enable_debug(char *str)}__setup("lsm.debug",enable_debug);+/* Explicitly enable a list of LSMs. */+staticint__initenable_lsm(char*str)+{+chosen_lsm_enable=str;+return1;+}+__setup("lsm.enable=",enable_lsm);++/* Explicitly disable a list of LSMs. */+staticint__initdisable_lsm(char*str)+{+chosen_lsm_disable=str;+return1;+}+__setup("lsm.disable=",disable_lsm);+staticboolmatch_last_lsm(constchar*list,constchar*lsm){constchar*last;
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 21:47:44
On 09/24/2018 05:18 PM, Kees Cook wrote:
This moves the string handling for "security=" boot parameter into
a stored pointer instead of a string duplicate. This will allow
easier handling of the string when switching logic to use the coming
enable/disable infrastructure.
Signed-off-by: Kees Cook <redacted>
Reviewed-by: John Johansen <john.johansen@canonical.com>
@@ -34,18 +34,14 @@#define MAX_LSM_EVM_XATTR 2-/* Maximum number of letters for an LSM name string */-#define SECURITY_NAME_MAX 10-structsecurity_hook_headssecurity_hook_heads__lsm_ro_after_init;staticATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);char*lsm_names;/* Boot-time LSM user choice */-static__initdatacharchosen_lsm[SECURITY_NAME_MAX+1]=-CONFIG_DEFAULT_SECURITY;static__initdataconstchar*chosen_lsm_enable;static__initdataconstchar*chosen_lsm_disable;+static__initdataconstchar*chosen_major_lsm;static__initconstconstchar*constbuiltin_lsm_enable=CONFIG_LSM_ENABLE;
@@ -112,7 +108,7 @@ static bool __init lsm_allowed(struct lsm_info *lsm)returntrue;/* Disabled if this LSM isn't the chosen one. */-if(strcmp(lsm->name,chosen_lsm)!=0)+if(strcmp(lsm->name,chosen_major_lsm)!=0)returnfalse;returntrue;
@@ -191,6 +187,9 @@ static void __init prepare_lsm_enable(void)/* Process "lsm.enable=" and "lsm.disable=", if given. */parse_lsm_enable(chosen_lsm_enable,set_enabled,true);parse_lsm_enable(chosen_lsm_disable,set_enabled,false);++if(!chosen_major_lsm)+chosen_major_lsm=CONFIG_DEFAULT_SECURITY;}/**
@@ -231,12 +230,12 @@ int __init security_init(void)}/* Save user chosen LSM */-staticint__initchoose_lsm(char*str)+staticint__initchoose_major_lsm(char*str){-strncpy(chosen_lsm,str,SECURITY_NAME_MAX);+chosen_major_lsm=str;return1;}-__setup("security=",choose_lsm);+__setup("security=",choose_major_lsm);/* Enable LSM order debugging. */staticint__initenable_debug(char*str)
From: James Morris <jmorris@namei.org> Date: 2018-10-01 21:48:00
On Mon, 24 Sep 2018, Kees Cook wrote:
quoted hunk
In preparation for lifting the "is this LSM enabled?" logic out of the
individual LSMs, pass in any special enabled state tracking (as needed
for SELinux, AppArmor, and LoadPin). This should be an "int" to include
handling any future cases where "enabled" is exposed via sysctl which
has no "bool" type.
Signed-off-by: Kees Cook <redacted>
---
include/linux/lsm_hooks.h | 1 +
security/apparmor/lsm.c | 5 +++--
security/selinux/hooks.c | 1 +
3 files changed, 5 insertions(+), 2 deletions(-)
On Mon, Oct 1, 2018 at 2:47 PM, James Morris [off-list ref] wrote:
On Mon, 24 Sep 2018, Kees Cook wrote:
quoted
In preparation for lifting the "is this LSM enabled?" logic out of the
individual LSMs, pass in any special enabled state tracking (as needed
for SELinux, AppArmor, and LoadPin). This should be an "int" to include
handling any future cases where "enabled" is exposed via sysctl which
has no "bool" type.
Signed-off-by: Kees Cook <redacted>
---
include/linux/lsm_hooks.h | 1 +
security/apparmor/lsm.c | 5 +++--
security/selinux/hooks.c | 1 +
3 files changed, 5 insertions(+), 2 deletions(-)
This seems potentially confusing.
Perhaps initialize 'enabled' to a default int pointer, like:
static int lsm_default_enabled = 1;
Then,
DEFINE_LSM(foobar)
flags = LSM_FLAG_LEGACY_MAJOR,
.enabled = &lsm_default_enabled,
.init = foobar_init,
END_LSM;
The reason I didn't do this is because there are only two LSMs that
expose this "enabled" variable, so I didn't like making the other LSMs
have to declare this. Internally, though, this is exactly what the
infrastructure does: if it finds a NULL, it aims it at
&lsm_default_enabled (in a later patch).
However, it seems more discussion is needed on the "enable" bit of
this, so I'll reply to John in a moment...
-Kees
--
Kees Cook
Pixel Security
On Mon, Oct 1, 2018 at 2:17 PM, John Johansen
[off-list ref] wrote:
On 09/24/2018 05:18 PM, Kees Cook wrote:
quoted
This provides a place for ordered LSMs to be initialized, separate from
the "major" LSMs. This is mainly a copy/paste from major_lsm_init() to
ordered_lsm_init(), but it will change drastically in later patches.
What is not obvious in the patch is that this change moves the integrity
LSM from major_lsm_init() into ordered_lsm_init(), since it is not marked
with the LSM_FLAG_LEGACY_MAJOR. As it is the only LSM in the "ordered"
list, there is no reordering yet created.
Signed-off-by: Kees Cook <redacted>
I know its already being done, but I don't like splitting the init
order
Can you describe what you mean here? Do you mean having two init
functions? This is only done temporarily while the other pieces are
reorganized. The later patches reintegrate this. (Before this series,
we effectively had three implicit init paths: minor, major, and
integrity, so even this patch "alone" is an improvement IMO.)
Thanks for the reviews!
-Kees
--
Kees Cook
Pixel Security
From: John Johansen <john.johansen@canonical.com> Date: 2018-10-01 22:20:22
On 10/01/2018 02:56 PM, Kees Cook wrote:
On Mon, Oct 1, 2018 at 2:47 PM, James Morris [off-list ref] wrote:
quoted
On Mon, 24 Sep 2018, Kees Cook wrote:
quoted
In preparation for lifting the "is this LSM enabled?" logic out of the
individual LSMs, pass in any special enabled state tracking (as needed
for SELinux, AppArmor, and LoadPin). This should be an "int" to include
handling any future cases where "enabled" is exposed via sysctl which
has no "bool" type.
Signed-off-by: Kees Cook <redacted>
---
include/linux/lsm_hooks.h | 1 +
security/apparmor/lsm.c | 5 +++--
security/selinux/hooks.c | 1 +
3 files changed, 5 insertions(+), 2 deletions(-)
This seems potentially confusing.
Perhaps initialize 'enabled' to a default int pointer, like:
static int lsm_default_enabled = 1;
Then,
DEFINE_LSM(foobar)
flags = LSM_FLAG_LEGACY_MAJOR,
.enabled = &lsm_default_enabled,
.init = foobar_init,
END_LSM;
The reason I didn't do this is because there are only two LSMs that
expose this "enabled" variable, so I didn't like making the other LSMs
have to declare this. Internally, though, this is exactly what the
infrastructure does: if it finds a NULL, it aims it at
&lsm_default_enabled (in a later patch).
However, it seems more discussion is needed on the "enable" bit of
this, so I'll reply to John in a moment...
fwiw the apparmor.enabled config is really only a meant to be used to
disable apparmor. I'd drop it entirely except its part of the userspace
api now and needs to show up in
/sys/module/apparmor/parameters/enabled
On Mon, Oct 1, 2018 at 2:46 PM, John Johansen
[off-list ref] wrote:
On 09/24/2018 05:18 PM, Kees Cook wrote:
quoted
This introduces the "lsm.enable=..." and "lsm.disable=..." boot parameters
which each can contain a comma-separated list of LSMs to enable or
disable, respectively. The string "all" matches all LSMs.
This has very similar functionality to the existing per-LSM enable
handling ("apparmor.enabled=...", etc), but provides a centralized
place to perform the changes. These parameters take precedent over any
LSM-specific boot parameters.
Disabling an LSM means it will not be considered when performing
initializations. Enabling an LSM means either undoing a previous
LSM-specific boot parameter disabling or a undoing a default-disabled
CONFIG setting.
For example: "lsm.disable=apparmor apparmor.enabled=1" will result in
AppArmor being disabled. "selinux.enabled=0 lsm.enable=selinux" will
result in SELinux being enabled.
Signed-off-by: Kees Cook <redacted>
I don't like this. It brings about conflicting kernel params that are
bound to confuse users. Its pretty easy for a user to understand that
when they specify a parameter manually at boot, that it overrides the
build time default. But conflicting kernel parameters are a lot harder
to deal with.
I prefer a plain enabled= list being an override of the default build
time value. Where conflicts with LSM-specific configs always result in
the LSM being disabled with a complaint about the conflict.
Though I have yet to be convinced its worth the cost, I do recognize
it is sometimes convenient to disable a single LSM, instead of typing
in a whole list of what to enable. If we have to have conflicting
kernel parameters I would prefer that the conflict throw up a warning
and leaving the LSM with the conflicting config disabled.
Alright, let's drill down a bit more. I thought I had all the
requirements sorted out here. :)
AppArmor and SELinux are "special" here in that they have both:
- CONFIG for enable-ness
- boot param for enable-ness
Now, the way this worked in the past was that combined with
CONFIG_DEFAULT_SECURITY and the link-time ordering, this resulted in a
way to get the LSM enabled, skipped, etc. But it was highly CONFIG
dependent.
SELinux does:
#ifdef CONFIG_SECURITY_SELINUX_BOOTPARAM
int selinux_enabled = CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE;
static int __init selinux_enabled_setup(char *str)
{
unsigned long enabled;
if (!kstrtoul(str, 0, &enabled))
selinux_enabled = enabled ? 1 : 0;
return 1;
}
__setup("selinux=", selinux_enabled_setup);
#else
int selinux_enabled = 1;
#endif
...
if (!security_module_enable("selinux")) {
selinux_enabled = 0;
return 0;
}
if (!selinux_enabled) {
pr_info("SELinux: Disabled at boot.\n");
return 0;
}
AppArmor does:
/* Boot time disable flag */
static bool apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
module_param_named(enabled, apparmor_enabled, bool, S_IRUGO);
static int __init apparmor_enabled_setup(char *str)
{
unsigned long enabled;
int error = kstrtoul(str, 0, &enabled);
if (!error)
apparmor_enabled = enabled ? 1 : 0;
return 1;
}
__setup("apparmor=", apparmor_enabled_setup);
...
if (!apparmor_enabled || !security_module_enable("apparmor")) {
aa_info_message("AppArmor disabled by boot time parameter");
apparmor_enabled = false;
return 0;
}
Smack and TOMOYO each do:
if (!security_module_enable("smack"))
return 0;
if (!security_module_enable("tomoyo"))
return 0;
Capability, Integrity, Yama, and LoadPin always run init. (This series
fixes LoadPin to separate enable vs enforce, so we can ignore its
"enable" setting, which isn't an "am I active?" boolean -- its init
was always run.) With the enable logic is lifted out of the LSMs, we
want to have "implicit enable" for 6 of 8 of the LSMs. (Which is why I
had originally suggested CONFIG_LSM_DISABLE, since the normal state is
enabled.) But given your feedback, I made this "implicit disable" and
added CONFIG_LSM_ENABLE instead. (For which "CONFIG_LSM_ENABLE=all"
gets the same results.)
I think, then, the first question (mainly for you and Paul) is:
Should we remove CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE and
CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE in favor of only
CONFIG_LSM_ENABLE?
The answer will affect the next question: what should be done with the
boot parameters? AppArmor has two ways to change enablement:
apparmor=0/1 and apparmor.enabled=0/1. SELinux just has selinux=0/1.
Should those be removed in favor of "lsm.enable=..."? (And if they're
not removed, how do people imagine they should interact?)
Thanks!
-Kees
--
Kees Cook
Pixel Security
On Mon, Oct 1, 2018 at 3:20 PM, John Johansen
[off-list ref] wrote:
On 10/01/2018 02:56 PM, Kees Cook wrote:
quoted
On Mon, Oct 1, 2018 at 2:47 PM, James Morris [off-list ref] wrote:
quoted
On Mon, 24 Sep 2018, Kees Cook wrote:
quoted
In preparation for lifting the "is this LSM enabled?" logic out of the
individual LSMs, pass in any special enabled state tracking (as needed
for SELinux, AppArmor, and LoadPin). This should be an "int" to include
handling any future cases where "enabled" is exposed via sysctl which
has no "bool" type.
Signed-off-by: Kees Cook <redacted>
---
include/linux/lsm_hooks.h | 1 +
security/apparmor/lsm.c | 5 +++--
security/selinux/hooks.c | 1 +
3 files changed, 5 insertions(+), 2 deletions(-)
This seems potentially confusing.
Perhaps initialize 'enabled' to a default int pointer, like:
static int lsm_default_enabled = 1;
Then,
DEFINE_LSM(foobar)
flags = LSM_FLAG_LEGACY_MAJOR,
.enabled = &lsm_default_enabled,
.init = foobar_init,
END_LSM;
The reason I didn't do this is because there are only two LSMs that
expose this "enabled" variable, so I didn't like making the other LSMs
have to declare this. Internally, though, this is exactly what the
infrastructure does: if it finds a NULL, it aims it at
&lsm_default_enabled (in a later patch).
However, it seems more discussion is needed on the "enable" bit of
this, so I'll reply to John in a moment...
fwiw the apparmor.enabled config is really only a meant to be used to
disable apparmor. I'd drop it entirely except its part of the userspace
api now and needs to show up in
/sys/module/apparmor/parameters/enabled
Showing the enabled-ness there can be wired up. What should happen if
someone sets apparmor.enabled=0/1 in new-series-world? (See other
thread...)
-Kees
--
Kees Cook
Pixel Security
On Mon, Oct 1, 2018 at 2:23 PM, Steven Rostedt [off-list ref] wrote:
On Mon, 1 Oct 2018 14:07:55 -0700
John Johansen [off-list ref] wrote:
quoted
On 09/24/2018 05:18 PM, Kees Cook wrote:
quoted
This partially reverts commit 58eacfffc417 ("init, tracing: instrument
security and console initcall trace events") since security init calls
are about to no longer resemble regular init calls.
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Abderrahmane Benbachir <redacted>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Kees Cook <redacted>
Reviewed-by: John Johansen <john.johansen@canonical.com>
though I do think it would be a good idea to add a new set
of trace points, but that can come as a separate patch
Agreed.
BTW, how would this look? I'm not familiar with adding new tracepoints...
-Kees
--
Kees Cook
Pixel Security