| Message ID | 20230904134049.1802006-3-roberto.sassu@huaweicloud.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | security: Move IMA and EVM to the LSM infrastructure | expand |
On 9/4/23 09:40, Roberto Sassu wrote: > From: Roberto Sassu <roberto.sassu@huawei.com> > > Do the registration of IMA-Appraisal functions separately from the rest of > IMA functions, as appraisal is a separate feature not necessarily enabled > in the kernel configuration. > > Reuse the same approach as for other IMA functions, remove hardcoded calls > from the LSM infrastructure or the other places, declare the functions as > static and register them as hook implementations in > init_ima_appraise_lsm(), called by init_ima_lsm() to chain the > initialization. > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > --- > fs/attr.c | 2 - > include/linux/ima.h | 55 --------------------------- > security/integrity/ima/ima.h | 5 +++ > security/integrity/ima/ima_appraise.c | 38 +++++++++++++----- > security/integrity/ima/ima_main.c | 1 + > security/security.c | 13 ------- > 6 files changed, 35 insertions(+), 79 deletions(-) > > diff --git a/fs/attr.c b/fs/attr.c > index 3c309eb456c6..63fb60195409 100644 > --- a/fs/attr.c > +++ b/fs/attr.c > @@ -17,7 +17,6 @@ > #include <linux/filelock.h> > #include <linux/security.h> > #include <linux/evm.h> > -#include <linux/ima.h> > > #include "internal.h" > > @@ -487,7 +486,6 @@ int notify_change(struct mnt_idmap *idmap, struct dentry *dentry, > if (!error) { > fsnotify_change(dentry, ia_valid); > security_inode_post_setattr(idmap, dentry, ia_valid); > - ima_inode_post_setattr(idmap, dentry, ia_valid); > evm_inode_post_setattr(idmap, dentry, ia_valid); > } > > diff --git a/include/linux/ima.h b/include/linux/ima.h > index 58591b5cbdb4..586e2e18e494 100644 > --- a/include/linux/ima.h > +++ b/include/linux/ima.h > @@ -98,66 +98,11 @@ static inline void ima_add_kexec_buffer(struct kimage *image) > > #ifdef CONFIG_IMA_APPRAISE > extern bool is_ima_appraise_enabled(void); > -extern void ima_inode_post_setattr(struct mnt_idmap *idmap, > - struct dentry *dentry, int ia_valid); > -int ima_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry, > - const char *xattr_name, const void *xattr_value, > - size_t xattr_value_len, int flags); > -extern int ima_inode_set_acl(struct mnt_idmap *idmap, > - struct dentry *dentry, const char *acl_name, > - struct posix_acl *kacl); > -static inline int ima_inode_remove_acl(struct mnt_idmap *idmap, > - struct dentry *dentry, > - const char *acl_name) > -{ > - return ima_inode_set_acl(idmap, dentry, acl_name, NULL); > -} > - > -int ima_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry, > - const char *xattr_name); > #else > static inline bool is_ima_appraise_enabled(void) > { > return 0; > } > - > -static inline void ima_inode_post_setattr(struct mnt_idmap *idmap, > - struct dentry *dentry, int ia_valid) > -{ > - return; > -} > - > -static inline int ima_inode_setxattr(struct mnt_idmap *idmap, > - struct dentry *dentry, > - const char *xattr_name, > - const void *xattr_value, > - size_t xattr_value_len, > - int flags) > -{ > - return 0; > -} > - > -static inline int ima_inode_set_acl(struct mnt_idmap *idmap, > - struct dentry *dentry, const char *acl_name, > - struct posix_acl *kacl) > -{ > - > - return 0; > -} > - > -static inline int ima_inode_removexattr(struct mnt_idmap *idmap, > - struct dentry *dentry, > - const char *xattr_name) > -{ > - return 0; > -} > - > -static inline int ima_inode_remove_acl(struct mnt_idmap *idmap, > - struct dentry *dentry, > - const char *acl_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 c0412100023e..d19aa9c068da 100644 > --- a/security/integrity/ima/ima.h > +++ b/security/integrity/ima/ima.h > @@ -334,6 +334,7 @@ enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value, > int xattr_len); > int ima_read_xattr(struct dentry *dentry, > struct evm_ima_xattr_data **xattr_value, int xattr_len); > +void __init init_ima_appraise_lsm(void); > > #else > static inline int ima_check_blacklist(struct integrity_iint_cache *iint, > @@ -385,6 +386,10 @@ static inline int ima_read_xattr(struct dentry *dentry, > return 0; > } > > +static inline void __init init_ima_appraise_lsm(void) > +{ > +} > + > #endif /* CONFIG_IMA_APPRAISE */ > > #ifdef CONFIG_IMA_APPRAISE_MODSIG > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c > index c35e3537eb87..8fe6ea70f02b 100644 > --- a/security/integrity/ima/ima_appraise.c > +++ b/security/integrity/ima/ima_appraise.c > @@ -634,8 +634,8 @@ void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file) > * This function is called from notify_change(), which expects the caller > * to lock the inode's i_mutex. > */ > -void ima_inode_post_setattr(struct mnt_idmap *idmap, > - struct dentry *dentry, int ia_valid) > +static void ima_inode_post_setattr(struct mnt_idmap *idmap, > + struct dentry *dentry, int ia_valid) > { > struct inode *inode = d_backing_inode(dentry); > struct integrity_iint_cache *iint; > @@ -748,9 +748,9 @@ static int validate_hash_algo(struct dentry *dentry, > return -EACCES; > } > > -int ima_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry, > - const char *xattr_name, const void *xattr_value, > - size_t xattr_value_len, int flags) > +static int ima_inode_setxattr(struct mnt_idmap *idmap, 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; > @@ -779,8 +779,8 @@ int ima_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry, > return result; > } > > -int ima_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry, > - const char *acl_name, struct posix_acl *kacl) > +static int ima_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry, > + const char *acl_name, struct posix_acl *kacl) > { > if (evm_revalidate_status(acl_name)) > ima_reset_appraise_flags(d_backing_inode(dentry), 0); > @@ -788,8 +788,8 @@ int ima_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry, > return 0; > } > > -int ima_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry, > - const char *xattr_name) > +static int ima_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry, > + const char *xattr_name) > { > int result; > > @@ -801,3 +801,23 @@ int ima_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry, > } > return result; > } > + > +static int ima_inode_remove_acl(struct mnt_idmap *idmap, struct dentry *dentry, > + const char *acl_name) > +{ > + return ima_inode_set_acl(idmap, dentry, acl_name, NULL); > +} > + > +static struct security_hook_list ima_appraise_hooks[] __ro_after_init = { > + LSM_HOOK_INIT(inode_post_setattr, ima_inode_post_setattr), > + LSM_HOOK_INIT(inode_setxattr, ima_inode_setxattr), > + LSM_HOOK_INIT(inode_set_acl, ima_inode_set_acl), > + LSM_HOOK_INIT(inode_removexattr, ima_inode_removexattr), > + LSM_HOOK_INIT(inode_remove_acl, ima_inode_remove_acl), > +}; > + > +void __init init_ima_appraise_lsm(void) > +{ > + security_add_hooks(ima_appraise_hooks, ARRAY_SIZE(ima_appraise_hooks), > + "integrity"); > +} > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c > index 0e4f882fcdce..c31e11a5bc31 100644 > --- a/security/integrity/ima/ima_main.c > +++ b/security/integrity/ima/ima_main.c > @@ -1141,6 +1141,7 @@ static struct security_hook_list ima_hooks[] __ro_after_init = { > void __init init_ima_lsm(void) > { > security_add_hooks(ima_hooks, ARRAY_SIZE(ima_hooks), "integrity"); > + init_ima_appraise_lsm(); > } > > late_initcall(init_ima); /* Start IMA after the TPM is available */ > diff --git a/security/security.c b/security/security.c > index 0b196d3f01b8..eb686f9471ea 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -20,7 +20,6 @@ > #include <linux/kernel_read_file.h> > #include <linux/lsm_hooks.h> > #include <linux/integrity.h> > -#include <linux/ima.h> > #include <linux/evm.h> > #include <linux/fsnotify.h> > #include <linux/mman.h> > @@ -2217,9 +2216,6 @@ int security_inode_setxattr(struct mnt_idmap *idmap, > > if (ret == 1) > ret = cap_inode_setxattr(dentry, name, value, size, flags); > - if (ret) > - return ret; > - ret = ima_inode_setxattr(idmap, dentry, name, value, size, flags); > if (ret) > return ret; > return evm_inode_setxattr(idmap, dentry, name, value, size, flags); > @@ -2247,9 +2243,6 @@ int security_inode_set_acl(struct mnt_idmap *idmap, > return 0; > ret = call_int_hook(inode_set_acl, 0, idmap, dentry, acl_name, > kacl); > - if (ret) > - return ret; > - ret = ima_inode_set_acl(idmap, dentry, acl_name, kacl); > if (ret) > return ret; > return evm_inode_set_acl(idmap, dentry, acl_name, kacl); > @@ -2310,9 +2303,6 @@ int security_inode_remove_acl(struct mnt_idmap *idmap, > if (unlikely(IS_PRIVATE(d_backing_inode(dentry)))) > return 0; > ret = call_int_hook(inode_remove_acl, 0, idmap, dentry, acl_name); > - if (ret) > - return ret; > - ret = ima_inode_remove_acl(idmap, dentry, acl_name); > if (ret) > return ret; > return evm_inode_remove_acl(idmap, dentry, acl_name); > @@ -2412,9 +2402,6 @@ int security_inode_removexattr(struct mnt_idmap *idmap, > ret = call_int_hook(inode_removexattr, 1, idmap, dentry, name); > if (ret == 1) > ret = cap_inode_removexattr(idmap, dentry, name); > - if (ret) > - return ret; > - ret = ima_inode_removexattr(idmap, dentry, name); > if (ret) > return ret; > return evm_inode_removexattr(idmap, dentry, name); Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
diff --git a/fs/attr.c b/fs/attr.c index 3c309eb456c6..63fb60195409 100644 --- a/fs/attr.c +++ b/fs/attr.c @@ -17,7 +17,6 @@ #include <linux/filelock.h> #include <linux/security.h> #include <linux/evm.h> -#include <linux/ima.h> #include "internal.h" @@ -487,7 +486,6 @@ int notify_change(struct mnt_idmap *idmap, struct dentry *dentry, if (!error) { fsnotify_change(dentry, ia_valid); security_inode_post_setattr(idmap, dentry, ia_valid); - ima_inode_post_setattr(idmap, dentry, ia_valid); evm_inode_post_setattr(idmap, dentry, ia_valid); } diff --git a/include/linux/ima.h b/include/linux/ima.h index 58591b5cbdb4..586e2e18e494 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -98,66 +98,11 @@ static inline void ima_add_kexec_buffer(struct kimage *image) #ifdef CONFIG_IMA_APPRAISE extern bool is_ima_appraise_enabled(void); -extern void ima_inode_post_setattr(struct mnt_idmap *idmap, - struct dentry *dentry, int ia_valid); -int ima_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry, - const char *xattr_name, const void *xattr_value, - size_t xattr_value_len, int flags); -extern int ima_inode_set_acl(struct mnt_idmap *idmap, - struct dentry *dentry, const char *acl_name, - struct posix_acl *kacl); -static inline int ima_inode_remove_acl(struct mnt_idmap *idmap, - struct dentry *dentry, - const char *acl_name) -{ - return ima_inode_set_acl(idmap, dentry, acl_name, NULL); -} - -int ima_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry, - const char *xattr_name); #else static inline bool is_ima_appraise_enabled(void) { return 0; } - -static inline void ima_inode_post_setattr(struct mnt_idmap *idmap, - struct dentry *dentry, int ia_valid) -{ - return; -} - -static inline int ima_inode_setxattr(struct mnt_idmap *idmap, - struct dentry *dentry, - const char *xattr_name, - const void *xattr_value, - size_t xattr_value_len, - int flags) -{ - return 0; -} - -static inline int ima_inode_set_acl(struct mnt_idmap *idmap, - struct dentry *dentry, const char *acl_name, - struct posix_acl *kacl) -{ - - return 0; -} - -static inline int ima_inode_removexattr(struct mnt_idmap *idmap, - struct dentry *dentry, - const char *xattr_name) -{ - return 0; -} - -static inline int ima_inode_remove_acl(struct mnt_idmap *idmap, - struct dentry *dentry, - const char *acl_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 c0412100023e..d19aa9c068da 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -334,6 +334,7 @@ enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value, int xattr_len); int ima_read_xattr(struct dentry *dentry, struct evm_ima_xattr_data **xattr_value, int xattr_len); +void __init init_ima_appraise_lsm(void); #else static inline int ima_check_blacklist(struct integrity_iint_cache *iint, @@ -385,6 +386,10 @@ static inline int ima_read_xattr(struct dentry *dentry, return 0; } +static inline void __init init_ima_appraise_lsm(void) +{ +} + #endif /* CONFIG_IMA_APPRAISE */ #ifdef CONFIG_IMA_APPRAISE_MODSIG diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index c35e3537eb87..8fe6ea70f02b 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -634,8 +634,8 @@ void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file) * This function is called from notify_change(), which expects the caller * to lock the inode's i_mutex. */ -void ima_inode_post_setattr(struct mnt_idmap *idmap, - struct dentry *dentry, int ia_valid) +static void ima_inode_post_setattr(struct mnt_idmap *idmap, + struct dentry *dentry, int ia_valid) { struct inode *inode = d_backing_inode(dentry); struct integrity_iint_cache *iint; @@ -748,9 +748,9 @@ static int validate_hash_algo(struct dentry *dentry, return -EACCES; } -int ima_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry, - const char *xattr_name, const void *xattr_value, - size_t xattr_value_len, int flags) +static int ima_inode_setxattr(struct mnt_idmap *idmap, 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; @@ -779,8 +779,8 @@ int ima_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry, return result; } -int ima_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry, - const char *acl_name, struct posix_acl *kacl) +static int ima_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry, + const char *acl_name, struct posix_acl *kacl) { if (evm_revalidate_status(acl_name)) ima_reset_appraise_flags(d_backing_inode(dentry), 0); @@ -788,8 +788,8 @@ int ima_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry, return 0; } -int ima_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry, - const char *xattr_name) +static int ima_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry, + const char *xattr_name) { int result; @@ -801,3 +801,23 @@ int ima_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry, } return result; } + +static int ima_inode_remove_acl(struct mnt_idmap *idmap, struct dentry *dentry, + const char *acl_name) +{ + return ima_inode_set_acl(idmap, dentry, acl_name, NULL); +} + +static struct security_hook_list ima_appraise_hooks[] __ro_after_init = { + LSM_HOOK_INIT(inode_post_setattr, ima_inode_post_setattr), + LSM_HOOK_INIT(inode_setxattr, ima_inode_setxattr), + LSM_HOOK_INIT(inode_set_acl, ima_inode_set_acl), + LSM_HOOK_INIT(inode_removexattr, ima_inode_removexattr), + LSM_HOOK_INIT(inode_remove_acl, ima_inode_remove_acl), +}; + +void __init init_ima_appraise_lsm(void) +{ + security_add_hooks(ima_appraise_hooks, ARRAY_SIZE(ima_appraise_hooks), + "integrity"); +} diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 0e4f882fcdce..c31e11a5bc31 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -1141,6 +1141,7 @@ static struct security_hook_list ima_hooks[] __ro_after_init = { void __init init_ima_lsm(void) { security_add_hooks(ima_hooks, ARRAY_SIZE(ima_hooks), "integrity"); + init_ima_appraise_lsm(); } late_initcall(init_ima); /* Start IMA after the TPM is available */ diff --git a/security/security.c b/security/security.c index 0b196d3f01b8..eb686f9471ea 100644 --- a/security/security.c +++ b/security/security.c @@ -20,7 +20,6 @@ #include <linux/kernel_read_file.h> #include <linux/lsm_hooks.h> #include <linux/integrity.h> -#include <linux/ima.h> #include <linux/evm.h> #include <linux/fsnotify.h> #include <linux/mman.h> @@ -2217,9 +2216,6 @@ int security_inode_setxattr(struct mnt_idmap *idmap, if (ret == 1) ret = cap_inode_setxattr(dentry, name, value, size, flags); - if (ret) - return ret; - ret = ima_inode_setxattr(idmap, dentry, name, value, size, flags); if (ret) return ret; return evm_inode_setxattr(idmap, dentry, name, value, size, flags); @@ -2247,9 +2243,6 @@ int security_inode_set_acl(struct mnt_idmap *idmap, return 0; ret = call_int_hook(inode_set_acl, 0, idmap, dentry, acl_name, kacl); - if (ret) - return ret; - ret = ima_inode_set_acl(idmap, dentry, acl_name, kacl); if (ret) return ret; return evm_inode_set_acl(idmap, dentry, acl_name, kacl); @@ -2310,9 +2303,6 @@ int security_inode_remove_acl(struct mnt_idmap *idmap, if (unlikely(IS_PRIVATE(d_backing_inode(dentry)))) return 0; ret = call_int_hook(inode_remove_acl, 0, idmap, dentry, acl_name); - if (ret) - return ret; - ret = ima_inode_remove_acl(idmap, dentry, acl_name); if (ret) return ret; return evm_inode_remove_acl(idmap, dentry, acl_name); @@ -2412,9 +2402,6 @@ int security_inode_removexattr(struct mnt_idmap *idmap, ret = call_int_hook(inode_removexattr, 1, idmap, dentry, name); if (ret == 1) ret = cap_inode_removexattr(idmap, dentry, name); - if (ret) - return ret; - ret = ima_inode_removexattr(idmap, dentry, name); if (ret) return ret; return evm_inode_removexattr(idmap, dentry, name);