Re: [PATCH] Yama: remove needless CONFIG_SECURITY_YAMA_STACKED

[Casey Schaufler <casey@schaufler-ca.com>]

On 7/21/2015 12:09 PM, Kees Cook wrote:
> Now that minor LSMs can cleanly stack with major LSMs, remove the unneeded
> config for Yama to be made to explicitly stack. Just selecting the main
> Yama CONFIG will allow it to work, regardless of the major LSM. Since
> distros using Yama are already forcing it to stack, this is effectively
> a no-op change.

Today I can compile in all LSMs including Yama and pick the one I want.
If we made your change it would be impossible to build in Yama and not
use it. I suggest we hold off until after the security summit discussion
on the next steps for module stacking. It's my hope we'll agree to a
convention for using kconfig and the security= boot parameter to specify
the variety of possible desired behaviors. I'm hoping for:

	CONFIG_DEFAULT_SECURITY=yama,smack
	security=yama,selinux

with checks in kconfig to prevent illegal combinations and a rational
behavior in the kernel for security=apparmor,selinux (which won't work
today).

>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  Documentation/security/Yama.txt       | 10 ++++------
>  arch/mips/configs/pistachio_defconfig |  1 -
>  include/linux/lsm_hooks.h             |  3 ---
>  security/security.c                   | 11 ++---------
>  security/yama/Kconfig                 |  9 +--------
>  security/yama/yama_lsm.c              | 26 +++++++++-----------------
>  6 files changed, 16 insertions(+), 44 deletions(-)
>
> diff --git a/Documentation/security/Yama.txt b/Documentation/security/Yama.txt
> index 227a63f018a2..d9ee7d7a6c7f 100644
> --- a/Documentation/security/Yama.txt
> +++ b/Documentation/security/Yama.txt
> @@ -1,9 +1,7 @@
> -Yama is a Linux Security Module that collects a number of system-wide DAC
> -security protections that are not handled by the core kernel itself. To
> -select it at boot time, specify "security=yama" (though this will disable
> -any other LSM).
> -
> -Yama is controlled through sysctl in /proc/sys/kernel/yama:
> +Yama is a Linux Security Module that collects system-wide DAC security
> +protections that are not handled by the core kernel itself. This is
> +selectable at build-time with CONFIG_SECURITY_YAMA, and can be controlled
> +at run-time through sysctls in /proc/sys/kernel/yama:
>  
>  - ptrace_scope
>  
> diff --git a/arch/mips/configs/pistachio_defconfig b/arch/mips/configs/pistachio_defconfig
> index 1646cce032c3..642b50946943 100644
> --- a/arch/mips/configs/pistachio_defconfig
> +++ b/arch/mips/configs/pistachio_defconfig
> @@ -320,7 +320,6 @@ CONFIG_KEYS=y
>  CONFIG_SECURITY=y
>  CONFIG_SECURITY_NETWORK=y
>  CONFIG_SECURITY_YAMA=y
> -CONFIG_SECURITY_YAMA_STACKED=y
>  CONFIG_DEFAULT_SECURITY_DAC=y
>  CONFIG_CRYPTO_AUTHENC=y
>  CONFIG_CRYPTO_HMAC=y
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 9429f054c323..4ea92e8968c8 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1881,8 +1881,5 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
>  
>  extern int __init security_module_enable(const char *module);
>  extern void __init capability_add_hooks(void);
> -#ifdef CONFIG_SECURITY_YAMA_STACKED
> -void __init yama_add_hooks(void);
> -#endif
>  
>  #endif /* ! __LINUX_LSM_HOOKS_H */
> diff --git a/security/security.c b/security/security.c
> index 595fffab48b0..aada79d281e5 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -56,18 +56,11 @@ int __init security_init(void)
>  	pr_info("Security Framework initialized\n");
>  
>  	/*
> -	 * Always load the capability module.
> +	 * Always load the capability module first.
>  	 */
>  	capability_add_hooks();
> -#ifdef CONFIG_SECURITY_YAMA_STACKED
>  	/*
> -	 * If Yama is configured for stacking load it next.
> -	 */
> -	yama_add_hooks();
> -#endif
> -	/*
> -	 * Load the chosen module if there is one.
> -	 * This will also find yama if it is stacking
> +	 * Load all the remaining security modules.
>  	 */
>  	do_security_initcalls();
>  
> diff --git a/security/yama/Kconfig b/security/yama/Kconfig
> index 3123e1da2fed..90c605eea892 100644
> --- a/security/yama/Kconfig
> +++ b/security/yama/Kconfig
> @@ -6,14 +6,7 @@ config SECURITY_YAMA
>  	  This selects Yama, which extends DAC support with additional
>  	  system-wide security settings beyond regular Linux discretionary
>  	  access controls. Currently available is ptrace scope restriction.
> +	  Like capabilities, this security module stacks with other LSMs.
>  	  Further information can be found in Documentation/security/Yama.txt.
>  
>  	  If you are unsure how to answer this question, answer N.
> -
> -config SECURITY_YAMA_STACKED
> -	bool "Yama stacked with other LSMs"
> -	depends on SECURITY_YAMA
> -	default n
> -	help
> -	  When Yama is built into the kernel, force it to stack with the
> -	  selected primary LSM.
> diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
> index 9ed32502470e..15ce2bac75e3 100644
> --- a/security/yama/yama_lsm.c
> +++ b/security/yama/yama_lsm.c
> @@ -353,11 +353,6 @@ static struct security_hook_list yama_hooks[] = {
>  	LSM_HOOK_INIT(task_free, yama_task_free),
>  };
>  
> -void __init yama_add_hooks(void)
> -{
> -	security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks));
> -}
> -
>  #ifdef CONFIG_SYSCTL
>  static int yama_dointvec_minmax(struct ctl_table *table, int write,
>  				void __user *buffer, size_t *lenp, loff_t *ppos)
> @@ -396,23 +391,20 @@ static struct ctl_table yama_sysctl_table[] = {
>  	},
>  	{ }
>  };
> +static __init void yama_init_sysctl(void)
> +{
> +	if (!register_sysctl_paths(yama_sysctl_path, yama_sysctl_table))
> +		panic("Yama: sysctl registration failed.\n");
> +}
> +#else
> +static __init void yama_init_sysctl(void) { };
>  #endif /* CONFIG_SYSCTL */
>  
>  static __init int yama_init(void)
>  {
> -#ifndef CONFIG_SECURITY_YAMA_STACKED
> -	/*
> -	 * If yama is being stacked this is already taken care of.
> -	 */
> -	if (!security_module_enable("yama"))
> -		return 0;
> -#endif
>  	pr_info("Yama: becoming mindful.\n");
> -
> -#ifdef CONFIG_SYSCTL
> -	if (!register_sysctl_paths(yama_sysctl_path, yama_sysctl_table))
> -		panic("Yama: sysctl registration failed.\n");
> -#endif
> +	security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks));
> +	yama_init_sysctl();
>  
>  	return 0;
>  }