diff mbox

[RFC,v2,1/2] security: introduce CONFIG_SECURITY_WRITABLE_HOOKS

Message ID alpine.LRH.2.20.1702150016220.32759@namei.org (mailing list archive)
State New, archived
Headers show

Commit Message

James Morris Feb. 14, 2017, 1:17 p.m. UTC
Subsequent patches will add RO hardening to LSM hooks, however, SELinux
still needs to be able to perform runtime disablement after init to handle
architectures where init-time disablement via boot parameters is not feasible.

Introduce a new kernel configuration parameter CONFIG_SECURITY_WRITABLE_HOOKS,
and a helper macro __lsm_ro_after_init, to handle this case.

Signed-off-by: James Morris <james.l.morris@oracle.com>
---
 include/linux/lsm_hooks.h |    7 +++++++
 security/Kconfig          |    5 +++++
 security/selinux/Kconfig  |    6 ++++++
 3 files changed, 18 insertions(+), 0 deletions(-)

Comments

Tetsuo Handa Feb. 14, 2017, 2:24 p.m. UTC | #1
James Morris wrote:
> > Loadable kernel modules used by antivirus software temporarily modify syscall tables
> > ( http://stackoverflow.com/questions/13876369/system-call-interception-in-linux-kernel-module-kernel-3-5 )
> > in order to register hooks for execve()/open()/close(). It is very frustrating for
> > many users if you enforce CONFIG_MODULES=n or forbid post-__init registration of hooks.
> 
> We don't cater to out of tree code.
> 
> Additionally, I'd also seriously question whether the security benefits of 
> kernel AV outweigh its security risks.

Are you aware that loadable kernel modules used by antivirus software are used for
relaying events to userspace daemons? Such modules don't do scanning inside kernel.
Why such modules are considered as security risks?

Are you aware that SELinux (or AppArmor or whatever so-called MAC) is not always
enabled in all enterprise systems? Are you aware that there are systems where
administrators cannot afford using rule based access restriction mechanisms?
In such systems, the security benefits of loadable security modules (provided by
AV or alike) can outweigh read only LSM hooks.

> diff --git a/security/Kconfig b/security/Kconfig
> index 118f454..f6f90c4 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -31,6 +31,11 @@ config SECURITY
>  
>  	  If you are unsure how to answer this question, answer N.
>  
> +config SECURITY_WRITABLE_HOOKS
> +	depends on SECURITY
> +	bool
> +	default n
> +

This configuration option must not be set to N without big fat explanation
about implications of setting this option to N.

Honestly, I still don't like this option, regardless of whether SELinux
needs this option or not.
James Morris Feb. 14, 2017, 10:55 p.m. UTC | #2
On Tue, 14 Feb 2017, Tetsuo Handa wrote:

> > diff --git a/security/Kconfig b/security/Kconfig
> > index 118f454..f6f90c4 100644
> > --- a/security/Kconfig
> > +++ b/security/Kconfig
> > @@ -31,6 +31,11 @@ config SECURITY
> >  
> >  	  If you are unsure how to answer this question, answer N.
> >  
> > +config SECURITY_WRITABLE_HOOKS
> > +	depends on SECURITY
> > +	bool
> > +	default n
> > +
> 
> This configuration option must not be set to N without big fat explanation
> about implications of setting this option to N.

It's not visible in the config menu, it's only there to support SELinux 
runtime disablement, otherwise it wouldn't even be an option.

> 
> Honestly, I still don't like this option, regardless of whether SELinux
> needs this option or not.
> 

I agree, it would be better to just enable RO hardening without an option 
to disable it.
Stephen Smalley Feb. 17, 2017, 3:30 p.m. UTC | #3
On Wed, 2017-02-15 at 00:17 +1100, James Morris wrote:
> Subsequent patches will add RO hardening to LSM hooks, however,
> SELinux
> still needs to be able to perform runtime disablement after init to
> handle
> architectures where init-time disablement via boot parameters is not
> feasible.
> 
> Introduce a new kernel configuration parameter
> CONFIG_SECURITY_WRITABLE_HOOKS,
> and a helper macro __lsm_ro_after_init, to handle this case.
> 
> Signed-off-by: James Morris <james.l.morris@oracle.com>

Acked-by:  Stephen Smalley <sds@tycho.nsa.gov>

> ---
>  include/linux/lsm_hooks.h |    7 +++++++
>  security/Kconfig          |    5 +++++
>  security/selinux/Kconfig  |    6 ++++++
>  3 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index e29d4c6..c4b149f 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1908,6 +1908,13 @@ static inline void
> security_delete_hooks(struct security_hook_list *hooks,
>  }
>  #endif /* CONFIG_SECURITY_SELINUX_DISABLE */
>  
> +/* Currently required to handle SELinux runtime hook disable. */
> +#ifdef CONFIG_SECURITY_WRITABLE_HOOKS
> +#define __lsm_ro_after_init
> +#else
> +#define __lsm_ro_after_init	__ro_after_init
> +#endif /* CONFIG_SECURITY_WRITABLE_HOOKS */
> +
>  extern int __init security_module_enable(const char *module);
>  extern void __init capability_add_hooks(void);
>  #ifdef CONFIG_SECURITY_YAMA
> diff --git a/security/Kconfig b/security/Kconfig
> index 118f454..f6f90c4 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -31,6 +31,11 @@ config SECURITY
>  
>  	  If you are unsure how to answer this question, answer N.
>  
> +config SECURITY_WRITABLE_HOOKS
> +	depends on SECURITY
> +	bool
> +	default n
> +
>  config SECURITYFS
>  	bool "Enable the securityfs filesystem"
>  	help
> diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig
> index ea7e3ef..8af7a69 100644
> --- a/security/selinux/Kconfig
> +++ b/security/selinux/Kconfig
> @@ -40,6 +40,7 @@ config SECURITY_SELINUX_BOOTPARAM_VALUE
>  config SECURITY_SELINUX_DISABLE
>  	bool "NSA SELinux runtime disable"
>  	depends on SECURITY_SELINUX
> +	select SECURITY_WRITABLE_HOOKS
>  	default n
>  	help
>  	  This option enables writing to a selinuxfs node 'disable',
> which
> @@ -50,6 +51,11 @@ config SECURITY_SELINUX_DISABLE
>  	  portability across platforms where boot parameters are
> difficult
>  	  to employ.
>  
> +	  NOTE: selecting this option will disable the
> '__ro_after_init'
> +	  kernel hardening feature for security hooks.   Please
> consider
> +	  using the selinux=0 boot parameter instead of enabling
> this
> +	  option.
> +
>  	  If you are unsure how to answer this question, answer N.
>  
>  config SECURITY_SELINUX_DEVELOP
diff mbox

Patch

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index e29d4c6..c4b149f 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1908,6 +1908,13 @@  static inline void security_delete_hooks(struct security_hook_list *hooks,
 }
 #endif /* CONFIG_SECURITY_SELINUX_DISABLE */
 
+/* Currently required to handle SELinux runtime hook disable. */
+#ifdef CONFIG_SECURITY_WRITABLE_HOOKS
+#define __lsm_ro_after_init
+#else
+#define __lsm_ro_after_init	__ro_after_init
+#endif /* CONFIG_SECURITY_WRITABLE_HOOKS */
+
 extern int __init security_module_enable(const char *module);
 extern void __init capability_add_hooks(void);
 #ifdef CONFIG_SECURITY_YAMA
diff --git a/security/Kconfig b/security/Kconfig
index 118f454..f6f90c4 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -31,6 +31,11 @@  config SECURITY
 
 	  If you are unsure how to answer this question, answer N.
 
+config SECURITY_WRITABLE_HOOKS
+	depends on SECURITY
+	bool
+	default n
+
 config SECURITYFS
 	bool "Enable the securityfs filesystem"
 	help
diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig
index ea7e3ef..8af7a69 100644
--- a/security/selinux/Kconfig
+++ b/security/selinux/Kconfig
@@ -40,6 +40,7 @@  config SECURITY_SELINUX_BOOTPARAM_VALUE
 config SECURITY_SELINUX_DISABLE
 	bool "NSA SELinux runtime disable"
 	depends on SECURITY_SELINUX
+	select SECURITY_WRITABLE_HOOKS
 	default n
 	help
 	  This option enables writing to a selinuxfs node 'disable', which
@@ -50,6 +51,11 @@  config SECURITY_SELINUX_DISABLE
 	  portability across platforms where boot parameters are difficult
 	  to employ.
 
+	  NOTE: selecting this option will disable the '__ro_after_init'
+	  kernel hardening feature for security hooks.   Please consider
+	  using the selinux=0 boot parameter instead of enabling this
+	  option.
+
 	  If you are unsure how to answer this question, answer N.
 
 config SECURITY_SELINUX_DEVELOP