diff mbox series

[v3,18/25] security: Introduce inode_post_set_acl hook

Message ID 20230904133415.1799503-19-roberto.sassu@huaweicloud.com (mailing list archive)
State New, archived
Headers show
Series security: Move IMA and EVM to the LSM infrastructure | expand

Commit Message

Roberto Sassu Sept. 4, 2023, 1:34 p.m. UTC
From: Roberto Sassu <roberto.sassu@huawei.com>

In preparation for moving IMA and EVM to the LSM infrastructure, introduce
the inode_post_set_acl hook.

It is useful for EVM to recalculate the HMAC on the modified POSIX ACL and
other file metadata, after it verified the HMAC of current file metadata
with the inode_set_acl hook.

LSMs should use the new hook instead of inode_set_acl, when they need to
know that the operation was done successfully (not known in inode_set_acl).
The new hook cannot return an error and cannot cause the operation to be
reverted.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/posix_acl.c                |  1 +
 include/linux/lsm_hook_defs.h |  2 ++
 include/linux/security.h      |  7 +++++++
 security/security.c           | 17 +++++++++++++++++
 4 files changed, 27 insertions(+)

Comments

Stefan Berger Sept. 5, 2023, 7 p.m. UTC | #1
On 9/4/23 09:34, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> In preparation for moving IMA and EVM to the LSM infrastructure, introduce
> the inode_post_set_acl hook.
>
> It is useful for EVM to recalculate the HMAC on the modified POSIX ACL and
> other file metadata, after it verified the HMAC of current file metadata
> with the inode_set_acl hook.
>
> LSMs should use the new hook instead of inode_set_acl, when they need to
> know that the operation was done successfully (not known in inode_set_acl).
> The new hook cannot return an error and cannot cause the operation to be
> reverted.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>   fs/posix_acl.c                |  1 +
>   include/linux/lsm_hook_defs.h |  2 ++
>   include/linux/security.h      |  7 +++++++
>   security/security.c           | 17 +++++++++++++++++
>   4 files changed, 27 insertions(+)
>
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> index 7fa1b738bbab..3b7dbea5c3ff 100644
> --- a/fs/posix_acl.c
> +++ b/fs/posix_acl.c
> @@ -1137,6 +1137,7 @@ int vfs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
>   		error = -EIO;
>   	if (!error) {
>   		fsnotify_xattr(dentry);
> +		security_inode_post_set_acl(dentry, acl_name, kacl);
>   		evm_inode_post_set_acl(dentry, acl_name, kacl);
>   	}
>   
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 9ae573b83737..bba1fbd97207 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -157,6 +157,8 @@ LSM_HOOK(void, LSM_RET_VOID, inode_post_removexattr, struct dentry *dentry,
>   	 const char *name)
>   LSM_HOOK(int, 0, inode_set_acl, struct mnt_idmap *idmap,
>   	 struct dentry *dentry, const char *acl_name, struct posix_acl *kacl)
> +LSM_HOOK(void, LSM_RET_VOID, inode_post_set_acl, struct dentry *dentry,
> +	 const char *acl_name, struct posix_acl *kacl)
>   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,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 5f296761883f..556d019ebe5c 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -367,6 +367,8 @@ int security_inode_setxattr(struct mnt_idmap *idmap,
>   int security_inode_set_acl(struct mnt_idmap *idmap,
>   			   struct dentry *dentry, const char *acl_name,
>   			   struct posix_acl *kacl);
> +void security_inode_post_set_acl(struct dentry *dentry, const char *acl_name,
> +				 struct posix_acl *kacl);
>   int security_inode_get_acl(struct mnt_idmap *idmap,
>   			   struct dentry *dentry, const char *acl_name);
>   int security_inode_remove_acl(struct mnt_idmap *idmap,
> @@ -894,6 +896,11 @@ static inline int security_inode_set_acl(struct mnt_idmap *idmap,
>   	return 0;
>   }
>   
> +static inline void security_inode_post_set_acl(struct dentry *dentry,
> +					       const char *acl_name,
> +					       struct posix_acl *kacl)
> +{ }
> +
>   static inline int security_inode_get_acl(struct mnt_idmap *idmap,
>   					 struct dentry *dentry,
>   					 const char *acl_name)
> diff --git a/security/security.c b/security/security.c
> index aa6274c90147..aabace9e24d9 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2260,6 +2260,23 @@ int security_inode_set_acl(struct mnt_idmap *idmap,
>   	return evm_inode_set_acl(idmap, dentry, acl_name, kacl);
>   }
>   
> +/**
> + * security_inode_post_set_acl() - Update inode security after set_acl()
> + * @dentry: file
> + * @acl_name: acl name
> + * @kacl: acl struct
> + *
> + * Update inode security field after successful set_acl operation on @dentry.
> + * The posix acls in @kacl are identified by @acl_name.
> + */
> +void security_inode_post_set_acl(struct dentry *dentry, const char *acl_name,
> +				 struct posix_acl *kacl)
> +{
> +	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> +		return;
> +	call_void_hook(inode_post_set_acl, dentry, acl_name, kacl);
> +}
> +
>   /**
>    * security_inode_get_acl() - Check if reading posix acls is allowed
>    * @idmap: idmap of the mount

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Mimi Zohar Oct. 13, 2023, 1:27 p.m. UTC | #2
On Mon, 2023-09-04 at 15:34 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> In preparation for moving IMA and EVM to the LSM infrastructure, introduce
> the inode_post_set_acl hook.
> 
> It is useful for EVM to recalculate the HMAC on the modified POSIX ACL and
> other file metadata, after it verified the HMAC of current file metadata
> with the inode_set_acl hook.

Please reword this and the inode_post_remove_acl patch similar to my
comments on  "[PATCH v3 12/25] security: Introduce inode_post_setattr
hook]".
diff mbox series

Patch

diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 7fa1b738bbab..3b7dbea5c3ff 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -1137,6 +1137,7 @@  int vfs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
 		error = -EIO;
 	if (!error) {
 		fsnotify_xattr(dentry);
+		security_inode_post_set_acl(dentry, acl_name, kacl);
 		evm_inode_post_set_acl(dentry, acl_name, kacl);
 	}
 
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 9ae573b83737..bba1fbd97207 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -157,6 +157,8 @@  LSM_HOOK(void, LSM_RET_VOID, inode_post_removexattr, struct dentry *dentry,
 	 const char *name)
 LSM_HOOK(int, 0, inode_set_acl, struct mnt_idmap *idmap,
 	 struct dentry *dentry, const char *acl_name, struct posix_acl *kacl)
+LSM_HOOK(void, LSM_RET_VOID, inode_post_set_acl, struct dentry *dentry,
+	 const char *acl_name, struct posix_acl *kacl)
 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,
diff --git a/include/linux/security.h b/include/linux/security.h
index 5f296761883f..556d019ebe5c 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -367,6 +367,8 @@  int security_inode_setxattr(struct mnt_idmap *idmap,
 int security_inode_set_acl(struct mnt_idmap *idmap,
 			   struct dentry *dentry, const char *acl_name,
 			   struct posix_acl *kacl);
+void security_inode_post_set_acl(struct dentry *dentry, const char *acl_name,
+				 struct posix_acl *kacl);
 int security_inode_get_acl(struct mnt_idmap *idmap,
 			   struct dentry *dentry, const char *acl_name);
 int security_inode_remove_acl(struct mnt_idmap *idmap,
@@ -894,6 +896,11 @@  static inline int security_inode_set_acl(struct mnt_idmap *idmap,
 	return 0;
 }
 
+static inline void security_inode_post_set_acl(struct dentry *dentry,
+					       const char *acl_name,
+					       struct posix_acl *kacl)
+{ }
+
 static inline int security_inode_get_acl(struct mnt_idmap *idmap,
 					 struct dentry *dentry,
 					 const char *acl_name)
diff --git a/security/security.c b/security/security.c
index aa6274c90147..aabace9e24d9 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2260,6 +2260,23 @@  int security_inode_set_acl(struct mnt_idmap *idmap,
 	return evm_inode_set_acl(idmap, dentry, acl_name, kacl);
 }
 
+/**
+ * security_inode_post_set_acl() - Update inode security after set_acl()
+ * @dentry: file
+ * @acl_name: acl name
+ * @kacl: acl struct
+ *
+ * Update inode security field after successful set_acl operation on @dentry.
+ * The posix acls in @kacl are identified by @acl_name.
+ */
+void security_inode_post_set_acl(struct dentry *dentry, const char *acl_name,
+				 struct posix_acl *kacl)
+{
+	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
+		return;
+	call_void_hook(inode_post_set_acl, dentry, acl_name, kacl);
+}
+
 /**
  * security_inode_get_acl() - Check if reading posix acls is allowed
  * @idmap: idmap of the mount