[v2] fs: use type-safe uid representation for filesystem capabilities

[PATCH v2 11/25] security: add hooks for set/get/remove of fscaps

Posted by Seth Forshee (DigitalOcean) 1 year, 10 months ago

In preparation for moving fscaps out of the xattr code paths, add new
security hooks. These hooks are largely needed because common kernel
code will pass around struct vfs_caps pointers, which EVM will need to
convert to raw xattr data for verification and updates of its hashes.

Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org>
---
 include/linux/lsm_hook_defs.h |  7 +++++
 include/linux/security.h      | 33 +++++++++++++++++++++
 security/security.c           | 69 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 109 insertions(+)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 76458b6d53da..7b3c23f9e4a5 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -152,6 +152,13 @@ LSM_HOOK(int, 0, inode_get_acl, struct mnt_idmap *idmap,
 	 struct dentry *dentry, const char *acl_name)
 LSM_HOOK(int, 0, inode_remove_acl, struct mnt_idmap *idmap,
 	 struct dentry *dentry, const char *acl_name)
+LSM_HOOK(int, 0, inode_set_fscaps, struct mnt_idmap *idmap,
+	 struct dentry *dentry, const struct vfs_caps *caps, int flags);
+LSM_HOOK(void, LSM_RET_VOID, inode_post_set_fscaps, struct mnt_idmap *idmap,
+	 struct dentry *dentry, const struct vfs_caps *caps, int flags);
+LSM_HOOK(int, 0, inode_get_fscaps, struct mnt_idmap *idmap, struct dentry *dentry);
+LSM_HOOK(int, 0, inode_remove_fscaps, struct mnt_idmap *idmap,
+	 struct dentry *dentry);
 LSM_HOOK(int, 0, inode_need_killpriv, struct dentry *dentry)
 LSM_HOOK(int, 0, inode_killpriv, struct mnt_idmap *idmap,
 	 struct dentry *dentry)
diff --git a/include/linux/security.h b/include/linux/security.h
index d0eb20f90b26..40be548e5e12 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -378,6 +378,13 @@ int security_inode_getxattr(struct dentry *dentry, const char *name);
 int security_inode_listxattr(struct dentry *dentry);
 int security_inode_removexattr(struct mnt_idmap *idmap,
 			       struct dentry *dentry, const char *name);
+int security_inode_set_fscaps(struct mnt_idmap *idmap, struct dentry *dentry,
+			      const struct vfs_caps *caps, int flags);
+void security_inode_post_set_fscaps(struct mnt_idmap *idmap,
+				    struct dentry *dentry,
+				    const struct vfs_caps *caps, int flags);
+int security_inode_get_fscaps(struct mnt_idmap *idmap, struct dentry *dentry);
+int security_inode_remove_fscaps(struct mnt_idmap *idmap, struct dentry *dentry);
 int security_inode_need_killpriv(struct dentry *dentry);
 int security_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry);
 int security_inode_getsecurity(struct mnt_idmap *idmap,
@@ -935,6 +942,32 @@ static inline int security_inode_removexattr(struct mnt_idmap *idmap,
 	return cap_inode_removexattr(idmap, dentry, name);
 }
 
+static inline int security_inode_set_fscaps(struct mnt_idmap *idmap,
+					    struct dentry *dentry,
+					    const struct vfs_caps *caps,
+					    int flags)
+{
+	return 0;
+}
+static void security_inode_post_set_fscaps(struct mnt_idmap *idmap,
+					   struct dentry *dentry,
+					   const struct vfs_caps *caps,
+					   int flags)
+{
+}
+
+static int security_inode_get_fscaps(struct mnt_idmap *idmap,
+				     struct dentry *dentry)
+{
+	return 0;
+}
+
+static int security_inode_remove_fscaps(struct mnt_idmap *idmap,
+					struct dentry *dentry)
+{
+	return 0;
+}
+
 static inline int security_inode_need_killpriv(struct dentry *dentry)
 {
 	return cap_inode_need_killpriv(dentry);
diff --git a/security/security.c b/security/security.c
index 3aaad75c9ce8..0d210da9862c 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2351,6 +2351,75 @@ int security_inode_remove_acl(struct mnt_idmap *idmap,
 	return evm_inode_remove_acl(idmap, dentry, acl_name);
 }
 
+/**
+ * security_inode_set_fscaps() - Check if setting fscaps is allowed
+ * @idmap: idmap of the mount
+ * @dentry: file
+ * @caps: fscaps to be written
+ * @flags: flags for setxattr
+ *
+ * Check permission before setting the file capabilities given in @vfs_caps.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_inode_set_fscaps(struct mnt_idmap *idmap, struct dentry *dentry,
+			      const struct vfs_caps *caps, int flags)
+{
+	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
+		return 0;
+	return call_int_hook(inode_set_fscaps, 0, idmap, dentry, caps, flags);
+}
+
+/**
+ * security_inode_post_set_fscaps() - Update the inode after setting fscaps
+ * @idmap: idmap of the mount
+ * @dentry: file
+ * @caps: fscaps to be written
+ * @flags: flags for setxattr
+ *
+ * Update inode security field after successfully setting fscaps.
+ *
+ */
+void security_inode_post_set_fscaps(struct mnt_idmap *idmap,
+				    struct dentry *dentry,
+				    const struct vfs_caps *caps, int flags)
+{
+	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
+		return;
+	call_void_hook(inode_post_set_fscaps, idmap, dentry, caps, flags);
+}
+
+/**
+ * security_inode_get_fscaps() - Check if reading fscaps is allowed
+ * @dentry: file
+ *
+ * Check permission before getting fscaps.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_inode_get_fscaps(struct mnt_idmap *idmap, struct dentry *dentry)
+{
+	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
+		return 0;
+	return call_int_hook(inode_get_fscaps, 0, idmap, dentry);
+}
+
+/**
+ * security_inode_remove_fscaps() - Check if removing fscaps is allowed
+ * @idmap: idmap of the mount
+ * @dentry: file
+ *
+ * Check permission before removing fscaps.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_inode_remove_fscaps(struct mnt_idmap *idmap, struct dentry *dentry)
+{
+	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
+		return 0;
+	return call_int_hook(inode_remove_fscaps, 0, idmap, dentry);
+}
+
 /**
  * security_inode_post_setxattr() - Update the inode after a setxattr operation
  * @dentry: file

-- 
2.43.0

Re: [PATCH v2 11/25] security: add hooks for set/get/remove of fscaps

Posted by Roberto Sassu 1 year, 9 months ago

On Wed, 2024-02-21 at 15:24 -0600, Seth Forshee (DigitalOcean) wrote:
> In preparation for moving fscaps out of the xattr code paths, add new
> security hooks. These hooks are largely needed because common kernel
> code will pass around struct vfs_caps pointers, which EVM will need to
> convert to raw xattr data for verification and updates of its hashes.
> 
> Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org>
> ---
>  include/linux/lsm_hook_defs.h |  7 +++++
>  include/linux/security.h      | 33 +++++++++++++++++++++
>  security/security.c           | 69 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 109 insertions(+)
> 
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 76458b6d53da..7b3c23f9e4a5 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -152,6 +152,13 @@ LSM_HOOK(int, 0, inode_get_acl, struct mnt_idmap *idmap,
>  	 struct dentry *dentry, const char *acl_name)
>  LSM_HOOK(int, 0, inode_remove_acl, struct mnt_idmap *idmap,
>  	 struct dentry *dentry, const char *acl_name)
> +LSM_HOOK(int, 0, inode_set_fscaps, struct mnt_idmap *idmap,
> +	 struct dentry *dentry, const struct vfs_caps *caps, int flags);
> +LSM_HOOK(void, LSM_RET_VOID, inode_post_set_fscaps, struct mnt_idmap *idmap,
> +	 struct dentry *dentry, const struct vfs_caps *caps, int flags);
> +LSM_HOOK(int, 0, inode_get_fscaps, struct mnt_idmap *idmap, struct dentry *dentry);
> +LSM_HOOK(int, 0, inode_remove_fscaps, struct mnt_idmap *idmap,
> +	 struct dentry *dentry);

Uhm, there should not be semicolons here.

Roberto

>  LSM_HOOK(int, 0, inode_need_killpriv, struct dentry *dentry)
>  LSM_HOOK(int, 0, inode_killpriv, struct mnt_idmap *idmap,
>  	 struct dentry *dentry)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index d0eb20f90b26..40be548e5e12 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -378,6 +378,13 @@ int security_inode_getxattr(struct dentry *dentry, const char *name);
>  int security_inode_listxattr(struct dentry *dentry);
>  int security_inode_removexattr(struct mnt_idmap *idmap,
>  			       struct dentry *dentry, const char *name);
> +int security_inode_set_fscaps(struct mnt_idmap *idmap, struct dentry *dentry,
> +			      const struct vfs_caps *caps, int flags);
> +void security_inode_post_set_fscaps(struct mnt_idmap *idmap,
> +				    struct dentry *dentry,
> +				    const struct vfs_caps *caps, int flags);
> +int security_inode_get_fscaps(struct mnt_idmap *idmap, struct dentry *dentry);
> +int security_inode_remove_fscaps(struct mnt_idmap *idmap, struct dentry *dentry);
>  int security_inode_need_killpriv(struct dentry *dentry);
>  int security_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry);
>  int security_inode_getsecurity(struct mnt_idmap *idmap,
> @@ -935,6 +942,32 @@ static inline int security_inode_removexattr(struct mnt_idmap *idmap,
>  	return cap_inode_removexattr(idmap, dentry, name);
>  }
>  
> +static inline int security_inode_set_fscaps(struct mnt_idmap *idmap,
> +					    struct dentry *dentry,
> +					    const struct vfs_caps *caps,
> +					    int flags)
> +{
> +	return 0;
> +}
> +static void security_inode_post_set_fscaps(struct mnt_idmap *idmap,
> +					   struct dentry *dentry,
> +					   const struct vfs_caps *caps,
> +					   int flags)
> +{
> +}
> +
> +static int security_inode_get_fscaps(struct mnt_idmap *idmap,
> +				     struct dentry *dentry)
> +{
> +	return 0;
> +}
> +
> +static int security_inode_remove_fscaps(struct mnt_idmap *idmap,
> +					struct dentry *dentry)
> +{
> +	return 0;
> +}
> +
>  static inline int security_inode_need_killpriv(struct dentry *dentry)
>  {
>  	return cap_inode_need_killpriv(dentry);
> diff --git a/security/security.c b/security/security.c
> index 3aaad75c9ce8..0d210da9862c 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2351,6 +2351,75 @@ int security_inode_remove_acl(struct mnt_idmap *idmap,
>  	return evm_inode_remove_acl(idmap, dentry, acl_name);
>  }
>  
> +/**
> + * security_inode_set_fscaps() - Check if setting fscaps is allowed
> + * @idmap: idmap of the mount
> + * @dentry: file
> + * @caps: fscaps to be written
> + * @flags: flags for setxattr
> + *
> + * Check permission before setting the file capabilities given in @vfs_caps.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_inode_set_fscaps(struct mnt_idmap *idmap, struct dentry *dentry,
> +			      const struct vfs_caps *caps, int flags)
> +{
> +	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> +		return 0;
> +	return call_int_hook(inode_set_fscaps, 0, idmap, dentry, caps, flags);
> +}
> +
> +/**
> + * security_inode_post_set_fscaps() - Update the inode after setting fscaps
> + * @idmap: idmap of the mount
> + * @dentry: file
> + * @caps: fscaps to be written
> + * @flags: flags for setxattr
> + *
> + * Update inode security field after successfully setting fscaps.
> + *
> + */
> +void security_inode_post_set_fscaps(struct mnt_idmap *idmap,
> +				    struct dentry *dentry,
> +				    const struct vfs_caps *caps, int flags)
> +{
> +	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> +		return;
> +	call_void_hook(inode_post_set_fscaps, idmap, dentry, caps, flags);
> +}
> +
> +/**
> + * security_inode_get_fscaps() - Check if reading fscaps is allowed
> + * @dentry: file
> + *
> + * Check permission before getting fscaps.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_inode_get_fscaps(struct mnt_idmap *idmap, struct dentry *dentry)
> +{
> +	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> +		return 0;
> +	return call_int_hook(inode_get_fscaps, 0, idmap, dentry);
> +}
> +
> +/**
> + * security_inode_remove_fscaps() - Check if removing fscaps is allowed
> + * @idmap: idmap of the mount
> + * @dentry: file
> + *
> + * Check permission before removing fscaps.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_inode_remove_fscaps(struct mnt_idmap *idmap, struct dentry *dentry)
> +{
> +	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> +		return 0;
> +	return call_int_hook(inode_remove_fscaps, 0, idmap, dentry);
> +}
> +
>  /**
>   * security_inode_post_setxattr() - Update the inode after a setxattr operation
>   * @dentry: file
>

Re: [PATCH v2 11/25] security: add hooks for set/get/remove of fscaps

Posted by Seth Forshee (DigitalOcean) 1 year, 9 months ago

On Fri, Mar 01, 2024 at 04:59:16PM +0100, Roberto Sassu wrote:
> On Wed, 2024-02-21 at 15:24 -0600, Seth Forshee (DigitalOcean) wrote:
> > In preparation for moving fscaps out of the xattr code paths, add new
> > security hooks. These hooks are largely needed because common kernel
> > code will pass around struct vfs_caps pointers, which EVM will need to
> > convert to raw xattr data for verification and updates of its hashes.
> > 
> > Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org>
> > ---
> >  include/linux/lsm_hook_defs.h |  7 +++++
> >  include/linux/security.h      | 33 +++++++++++++++++++++
> >  security/security.c           | 69 +++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 109 insertions(+)
> > 
> > diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> > index 76458b6d53da..7b3c23f9e4a5 100644
> > --- a/include/linux/lsm_hook_defs.h
> > +++ b/include/linux/lsm_hook_defs.h
> > @@ -152,6 +152,13 @@ LSM_HOOK(int, 0, inode_get_acl, struct mnt_idmap *idmap,
> >  	 struct dentry *dentry, const char *acl_name)
> >  LSM_HOOK(int, 0, inode_remove_acl, struct mnt_idmap *idmap,
> >  	 struct dentry *dentry, const char *acl_name)
> > +LSM_HOOK(int, 0, inode_set_fscaps, struct mnt_idmap *idmap,
> > +	 struct dentry *dentry, const struct vfs_caps *caps, int flags);
> > +LSM_HOOK(void, LSM_RET_VOID, inode_post_set_fscaps, struct mnt_idmap *idmap,
> > +	 struct dentry *dentry, const struct vfs_caps *caps, int flags);
> > +LSM_HOOK(int, 0, inode_get_fscaps, struct mnt_idmap *idmap, struct dentry *dentry);
> > +LSM_HOOK(int, 0, inode_remove_fscaps, struct mnt_idmap *idmap,
> > +	 struct dentry *dentry);
> 
> Uhm, there should not be semicolons here.

Yes, I've fixed this already for the next version.

Thanks,
Seth

Re: [PATCH v2 11/25] security: add hooks for set/get/remove of fscaps

Posted by Christian Brauner 1 year, 9 months ago

On Wed, Feb 21, 2024 at 03:24:42PM -0600, Seth Forshee (DigitalOcean) wrote:
> In preparation for moving fscaps out of the xattr code paths, add new
> security hooks. These hooks are largely needed because common kernel
> code will pass around struct vfs_caps pointers, which EVM will need to
> convert to raw xattr data for verification and updates of its hashes.
> 
> Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org>
> ---

Looks good,
Reviewed-by: Christian Brauner <brauner@kernel.org>

Re: [PATCH v2 11/25] security: add hooks for set/get/remove of fscaps

Posted by Paul Moore 1 year, 10 months ago

On Wed, Feb 21, 2024 at 4:26 PM Seth Forshee (DigitalOcean)
<sforshee@kernel.org> wrote:
>
> In preparation for moving fscaps out of the xattr code paths, add new
> security hooks. These hooks are largely needed because common kernel
> code will pass around struct vfs_caps pointers, which EVM will need to
> convert to raw xattr data for verification and updates of its hashes.
>
> Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org>
> ---
>  include/linux/lsm_hook_defs.h |  7 +++++
>  include/linux/security.h      | 33 +++++++++++++++++++++
>  security/security.c           | 69 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 109 insertions(+)

One minor problem below, but assuming you fix that, this looks okay to me.

Acked-by: Paul Moore <paul@paul-moore.com>

> diff --git a/security/security.c b/security/security.c
> index 3aaad75c9ce8..0d210da9862c 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2351,6 +2351,75 @@ int security_inode_remove_acl(struct mnt_idmap *idmap,

...

> +/**
> + * security_inode_get_fscaps() - Check if reading fscaps is allowed
> + * @dentry: file

You are missing an entry for the @idmap parameter.

> + * Check permission before getting fscaps.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_inode_get_fscaps(struct mnt_idmap *idmap, struct dentry *dentry)
> +{
> +       if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> +               return 0;
> +       return call_int_hook(inode_get_fscaps, 0, idmap, dentry);
> +}

-- 
paul-moore.com

Re: [PATCH v2 11/25] security: add hooks for set/get/remove of fscaps

Posted by Seth Forshee (DigitalOcean) 1 year, 10 months ago

On Wed, Feb 21, 2024 at 06:31:42PM -0500, Paul Moore wrote:
> On Wed, Feb 21, 2024 at 4:26 PM Seth Forshee (DigitalOcean)
> <sforshee@kernel.org> wrote:
> >
> > In preparation for moving fscaps out of the xattr code paths, add new
> > security hooks. These hooks are largely needed because common kernel
> > code will pass around struct vfs_caps pointers, which EVM will need to
> > convert to raw xattr data for verification and updates of its hashes.
> >
> > Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org>
> > ---
> >  include/linux/lsm_hook_defs.h |  7 +++++
> >  include/linux/security.h      | 33 +++++++++++++++++++++
> >  security/security.c           | 69 +++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 109 insertions(+)
> 
> One minor problem below, but assuming you fix that, this looks okay to me.
> 
> Acked-by: Paul Moore <paul@paul-moore.com>
> 
> > diff --git a/security/security.c b/security/security.c
> > index 3aaad75c9ce8..0d210da9862c 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -2351,6 +2351,75 @@ int security_inode_remove_acl(struct mnt_idmap *idmap,
> 
> ...
> 
> > +/**
> > + * security_inode_get_fscaps() - Check if reading fscaps is allowed
> > + * @dentry: file
> 
> You are missing an entry for the @idmap parameter.

Fixed, thanks!