Move the xattr IMA hooks into normal LSM layer. As with SELinux and Smack, handle calling cap_inode_setxattr() internally. Cc: Mimi Zohar <zohar@xxxxxxxxxxxxx> Cc: Dmitry Kasatkin <dmitry.kasatkin@xxxxxxxxx> Cc: Paul Moore <paul@xxxxxxxxxxxxxx> Cc: James Morris <jmorris@xxxxxxxxx> Cc: "Serge E. Hallyn" <serge@xxxxxxxxxx> Cc: Borislav Petkov <bp@xxxxxxx> Cc: Jonathan McDowell <noodles@xxxxxx> Cc: Takashi Iwai <tiwai@xxxxxxx> Cc: Petr Vorel <pvorel@xxxxxxx> Cc: linux-integrity@xxxxxxxxxxxxxxx Cc: linux-security-module@xxxxxxxxxxxxxxx Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> --- 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