Subject: Re: [PATCH] Yama: remove needless CONFIG_SECURITY_YAMA_STACKED
To: Kees Cook <keescook@chromium.org>, linux-security-module@vger.kernel.org
References: <20150721190946.GA5127@www.outflux.net>
Cc: James Morris <james.l.morris@oracle.com>, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org
From: Casey Schaufler <casey@schaufler-ca.com>
Message-ID: <55AEA214.1010505@schaufler-ca.com>
Date: Tue, 21 Jul 2015 12:48:36 -0700
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101
 Thunderbird/38.1.0
MIME-Version: 1.0
In-Reply-To: <20150721190946.GA5127@www.outflux.net>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 6548
Lines: 175

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;
>  }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/