diff mbox series

[1/2,lsm] introduce a new hook to query LSM for functionality

Message ID 20201105173328.2539-1-olga.kornievskaia@gmail.com (mailing list archive)
State Superseded
Headers show
Series [1/2,lsm] introduce a new hook to query LSM for functionality | expand

Commit Message

Olga Kornievskaia Nov. 5, 2020, 5:33 p.m. UTC
From: Olga Kornievskaia <kolga@netapp.com>

Add a new hook func_query_vfs to query an LSM module (such as
SELinux) with the intention of finding whether or not it is enabled
or perhaps supports some functionality.

NFS stores security labels for file system objects and SElinux
or any other LSM module can query those objects. But it's
wasteful to do so when no security enforcement is taking place.
Using a new API call of security_func_query_vfs() and asking if

Suggested-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 include/linux/lsm_hook_defs.h | 1 +
 include/linux/security.h      | 4 ++++
 security/security.c           | 6 ++++++
 security/selinux/hooks.c      | 7 +++++++
 4 files changed, 18 insertions(+)

Comments

Casey Schaufler Nov. 5, 2020, 7:39 p.m. UTC | #1
On 11/5/2020 9:33 AM, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <kolga@netapp.com>
>
> Add a new hook func_query_vfs to query an LSM module (such as
> SELinux) with the intention of finding whether or not it is enabled
> or perhaps supports some functionality.
>
> NFS stores security labels for file system objects and SElinux
> or any other LSM module can query those objects. But it's
> wasteful to do so when no security enforcement is taking place.
> Using a new API call of security_func_query_vfs() and asking if

As I mentioned earlier, this is a sub-optimal implementation.
Instead of adding a func_query_vfs() hook add a field to
security_hook_list and have the LSM declare the "features"
it supports there. security_add_hooks() can gather the "features"
for the whole system into a single mask. security_func_query_vfs()
can then just return that mask.

> Suggested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  include/linux/lsm_hook_defs.h | 1 +
>  include/linux/security.h      | 4 ++++
>  security/security.c           | 6 ++++++
>  security/selinux/hooks.c      | 7 +++++++
>  4 files changed, 18 insertions(+)
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 32a940117e7a..df3454a1fcac 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -257,6 +257,7 @@ LSM_HOOK(int, 0, inode_notifysecctx, struct inode *inode, void *ctx, u32 ctxlen)
>  LSM_HOOK(int, 0, inode_setsecctx, struct dentry *dentry, void *ctx, u32 ctxlen)
>  LSM_HOOK(int, 0, inode_getsecctx, struct inode *inode, void **ctx,
>  	 u32 *ctxlen)
> +LSM_HOOK(int, 0, func_query_vfs, unsigned int)
>  
>  #if defined(CONFIG_SECURITY) && defined(CONFIG_WATCH_QUEUE)
>  LSM_HOOK(int, 0, post_notification, const struct cred *w_cred,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index bc2725491560..e15964059fa4 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -456,6 +456,10 @@ int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
>  int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
>  int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
>  int security_locked_down(enum lockdown_reason what);
> +#define LSM_FQUERY_VFS_NONE     0x00000000
> +#define LSM_FQUERY_VFS_XATTRS   0x00000001

This is the wrong granularity for the task at hand.
You don't care if an LSM supports vfs xattrs. You care if
the LSM provides an xattr that may be propagated by NFS
for the purpose of mandatory access control. An LSM that
provides time-of-day controls may use xattrs that are
meaningless to NFS.


> +int security_func_query_vfs(unsigned int flags);
> +
>  #else /* CONFIG_SECURITY */
>  
>  static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data)
> diff --git a/security/security.c b/security/security.c
> index a28045dc9e7f..502b33865238 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2067,6 +2067,12 @@ int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
>  }
>  EXPORT_SYMBOL(security_inode_getsecctx);
>  
> +int security_func_query_vfs(unsigned int flags)
> +{
> +	return call_int_hook(func_query_vfs, 0, flags);
> +}
> +EXPORT_SYMBOL(security_func_query_vfs);
> +
>  #ifdef CONFIG_WATCH_QUEUE
>  int security_post_notification(const struct cred *w_cred,
>  			       const struct cred *cred,
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 6b1826fc3658..38f47570e214 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -92,6 +92,7 @@
>  #include <uapi/linux/mount.h>
>  #include <linux/fsnotify.h>
>  #include <linux/fanotify.h>
> +#include <linux/security.h>
>  
>  #include "avc.h"
>  #include "objsec.h"
> @@ -6502,6 +6503,11 @@ static void selinux_inode_invalidate_secctx(struct inode *inode)
>  	spin_unlock(&isec->lock);
>  }
>  
> +static int selinux_func_query_vfs(unsigned int flags)
> +{
> +	return !!(flags & LSM_FQUERY_VFS_XATTRS);
> +}
> +
>  /*
>   *	called with inode->i_mutex locked
>   */
> @@ -7085,6 +7091,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>  	LSM_HOOK_INIT(inode_invalidate_secctx, selinux_inode_invalidate_secctx),
>  	LSM_HOOK_INIT(inode_notifysecctx, selinux_inode_notifysecctx),
>  	LSM_HOOK_INIT(inode_setsecctx, selinux_inode_setsecctx),
> +	LSM_HOOK_INIT(func_query_vfs, selinux_func_query_vfs),
>  
>  	LSM_HOOK_INIT(unix_stream_connect, selinux_socket_unix_stream_connect),
>  	LSM_HOOK_INIT(unix_may_send, selinux_socket_unix_may_send),
James Morris Nov. 7, 2020, 1:33 a.m. UTC | #2
On Thu, 5 Nov 2020, Olga Kornievskaia wrote:

> From: Olga Kornievskaia <kolga@netapp.com>
> 
> Add a new hook func_query_vfs to query an LSM module (such as
> SELinux) with the intention of finding whether or not it is enabled
> or perhaps supports some functionality.
> 
> NFS stores security labels for file system objects and SElinux
> or any other LSM module can query those objects. But it's
> wasteful to do so when no security enforcement is taking place.
> Using a new API call of security_func_query_vfs() and asking if

Seems we lost something here.
diff mbox series

Patch

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 32a940117e7a..df3454a1fcac 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -257,6 +257,7 @@  LSM_HOOK(int, 0, inode_notifysecctx, struct inode *inode, void *ctx, u32 ctxlen)
 LSM_HOOK(int, 0, inode_setsecctx, struct dentry *dentry, void *ctx, u32 ctxlen)
 LSM_HOOK(int, 0, inode_getsecctx, struct inode *inode, void **ctx,
 	 u32 *ctxlen)
+LSM_HOOK(int, 0, func_query_vfs, unsigned int)
 
 #if defined(CONFIG_SECURITY) && defined(CONFIG_WATCH_QUEUE)
 LSM_HOOK(int, 0, post_notification, const struct cred *w_cred,
diff --git a/include/linux/security.h b/include/linux/security.h
index bc2725491560..e15964059fa4 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -456,6 +456,10 @@  int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
 int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
 int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
 int security_locked_down(enum lockdown_reason what);
+#define LSM_FQUERY_VFS_NONE     0x00000000
+#define LSM_FQUERY_VFS_XATTRS   0x00000001
+int security_func_query_vfs(unsigned int flags);
+
 #else /* CONFIG_SECURITY */
 
 static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data)
diff --git a/security/security.c b/security/security.c
index a28045dc9e7f..502b33865238 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2067,6 +2067,12 @@  int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
 }
 EXPORT_SYMBOL(security_inode_getsecctx);
 
+int security_func_query_vfs(unsigned int flags)
+{
+	return call_int_hook(func_query_vfs, 0, flags);
+}
+EXPORT_SYMBOL(security_func_query_vfs);
+
 #ifdef CONFIG_WATCH_QUEUE
 int security_post_notification(const struct cred *w_cred,
 			       const struct cred *cred,
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 6b1826fc3658..38f47570e214 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -92,6 +92,7 @@ 
 #include <uapi/linux/mount.h>
 #include <linux/fsnotify.h>
 #include <linux/fanotify.h>
+#include <linux/security.h>
 
 #include "avc.h"
 #include "objsec.h"
@@ -6502,6 +6503,11 @@  static void selinux_inode_invalidate_secctx(struct inode *inode)
 	spin_unlock(&isec->lock);
 }
 
+static int selinux_func_query_vfs(unsigned int flags)
+{
+	return !!(flags & LSM_FQUERY_VFS_XATTRS);
+}
+
 /*
  *	called with inode->i_mutex locked
  */
@@ -7085,6 +7091,7 @@  static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(inode_invalidate_secctx, selinux_inode_invalidate_secctx),
 	LSM_HOOK_INIT(inode_notifysecctx, selinux_inode_notifysecctx),
 	LSM_HOOK_INIT(inode_setsecctx, selinux_inode_setsecctx),
+	LSM_HOOK_INIT(func_query_vfs, selinux_func_query_vfs),
 
 	LSM_HOOK_INIT(unix_stream_connect, selinux_socket_unix_stream_connect),
 	LSM_HOOK_INIT(unix_may_send, selinux_socket_unix_may_send),