[PATCH 3/9] ima: Move xattr hooks into LSM

Kees Cook
2022-10-13T22:37:09Z

Move the xattr IMA hooks into normal LSM layer. As with SELinux and
Smack, handle calling cap_inode_setxattr() internally.

Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Jonathan McDowell <noodles@fb.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Petr Vorel <pvorel@suse.cz>
Cc: linux-integrity@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/ima.h | 16 ----------------
security/integrity/ima/ima.h | 10 ++++++++++
security/integrity/ima/ima_appraise.c | 19 ++++++++++++++++---
security/integrity/ima/ima_main.c | 4 ++++
security/security.c | 10 ++--------
5 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 3c641cc65270..6dc5143f89f2 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -135,9 +135,6 @@ static inline void ima_post_key_create_or_update(struct key *keyring,
extern bool is_ima_appraise_enabled(void);
extern void ima_inode_post_setattr(struct user_namespace *mnt_userns,
struct dentry *dentry);
-extern int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
- const void *xattr_value, size_t xattr_value_len);
-extern int ima_inode_removexattr(struct dentry *dentry, const char *xattr_name);
#else
static inline bool is_ima_appraise_enabled(void)
{
@@ -150,19 +147,6 @@ static inline void ima_inode_post_setattr(struct user_namespace *mnt_userns,
return;
}

-static inline int ima_inode_setxattr(struct dentry *dentry,
- const char *xattr_name,
- const void *xattr_value,
- size_t xattr_value_len)
-{
- return 0;
-}
-
-static inline int ima_inode_removexattr(struct dentry *dentry,
- const char *xattr_name)
-{
- return 0;
-}
#endif /* CONFIG_IMA_APPRAISE */

#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index be965a8715e4..15a369df4c00 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -168,6 +168,16 @@ int __init ima_init_digests(void);
int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
void *lsm_data);

+/* LSM hooks */
+#ifdef CONFIG_IMA_APPRAISE
+int ima_inode_setxattr(struct user_namespace *mnt_userns,
+ struct dentry *dentry, const char *xattr_name,
+ const void *xattr_value, size_t xattr_value_len,
+ int flags);
+int ima_inode_removexattr(struct user_namespace *mnt_userns,
+ struct dentry *dentry, const char *xattr_name);
+#endif
+
/*
* used to protect h_table and sha_table
*/
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index bde74fcecee3..ddd9df6b7dac 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -744,8 +744,10 @@ static int validate_hash_algo(struct dentry *dentry,
return -EACCES;
}

-int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
- const void *xattr_value, size_t xattr_value_len)
+int ima_inode_setxattr(struct user_namespace *mnt_userns,
+ struct dentry *dentry, const char *xattr_name,
+ const void *xattr_value, size_t xattr_value_len,
+ int flags)
{
const struct evm_ima_xattr_data *xvalue = xattr_value;
int digsig = 0;
@@ -754,6 +756,11 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
result = ima_protect_xattr(dentry, xattr_name, xattr_value,
xattr_value_len);
if (result == 1) {
+ result = cap_inode_setxattr(dentry, xattr_name, xattr_value,
+ xattr_value_len, flags);
+ if (result)
+ return result;
+
if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST))
return -EINVAL;
digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG);
@@ -770,11 +777,17 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
return result;
}

-int ima_inode_removexattr(struct dentry *dentry, const char *xattr_name)
+int ima_inode_removexattr(struct user_namespace *mnt_userns,
+ struct dentry *dentry, const char *xattr_name)
{
int result;

result = ima_protect_xattr(dentry, xattr_name, NULL, 0);
+ if (result == 1) {
+ result = cap_inode_removexattr(mnt_userns, dentry, xattr_name);
+ if (result)
+ return result;
+ }
if (result == 1 || evm_revalidate_status(xattr_name)) {
ima_reset_appraise_flags(d_backing_inode(dentry), 0);
if (result == 1)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2cff001b02e4..b3b79d030a67 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -1089,6 +1089,10 @@ static struct security_hook_list ima_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(kernel_post_read_file, ima_post_read_file),
LSM_HOOK_INIT(kernel_load_data, ima_load_data),
LSM_HOOK_INIT(kernel_post_load_data, ima_post_load_data),
+#ifdef CONFIG_IMA_APPRAISE
+ LSM_HOOK_INIT(inode_setxattr, ima_inode_setxattr),
+ LSM_HOOK_INIT(inode_removexattr, ima_inode_removexattr),
+#endif
};

void __init integrity_lsm_ima_init(void)
diff --git a/security/security.c b/security/security.c
index 8f7c1b5fa5fa..ca731132a0e9 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1349,7 +1349,7 @@ int security_inode_setxattr(struct user_namespace *mnt_userns,
if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
return 0;
/*
- * SELinux and Smack integrate the cap call,
+ * SELinux, Smack, and IMA integrate the cap call,
* so assume that all LSMs supplying this call do so.
*/
ret = call_int_hook(inode_setxattr, 1, mnt_userns, dentry, name, value,
@@ -1357,9 +1357,6 @@ int security_inode_setxattr(struct user_namespace *mnt_userns,

if (ret == 1)
ret = cap_inode_setxattr(dentry, name, value, size, flags);
- if (ret)
- return ret;
- ret = ima_inode_setxattr(dentry, name, value, size);
if (ret)
return ret;
return evm_inode_setxattr(mnt_userns, dentry, name, value, size);
@@ -1396,15 +1393,12 @@ int security_inode_removexattr(struct user_namespace *mnt_userns,
if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
return 0;
/*
- * SELinux and Smack integrate the cap call,
+ * SELinux, Smack, and IMA integrate the cap call,
* so assume that all LSMs supplying this call do so.
*/
ret = call_int_hook(inode_removexattr, 1, mnt_userns, dentry, name);
if (ret == 1)
ret = cap_inode_removexattr(mnt_userns, dentry, name);
- if (ret)
- return ret;
- ret = ima_inode_removexattr(dentry, name);
if (ret)
return ret;
return evm_inode_removexattr(mnt_userns, dentry, name);
--
2.34.1