| Message ID | 20231107134012.682009-21-roberto.sassu@huaweicloud.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | security: Move IMA and EVM to the LSM infrastructure | expand |
On 11/7/2023 5:40 AM, Roberto Sassu wrote: > From: Roberto Sassu <roberto.sassu@huawei.com> > > Do the registration of IMA-Appraisal only 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, move hardcoded calls > from various places in the kernel to the LSM infrastructure. Declare the > functions as static and register them as hook implementations in > init_ima_appraise_lsm(), called by init_ima_lsm(). > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> > Reviewed-by: Mimi Zohar <zohar@linux.ibm.com> Ackeded-by: Casey Schaufler <casey@schaufler-ca.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 221d2bb0a906..38841f3ebbcb 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" > > @@ -503,7 +502,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 23ae24b60ecf..0bae61a15b60 100644 > --- a/include/linux/ima.h > +++ b/include/linux/ima.h > @@ -92,66 +92,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); > -extern 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); > -} > - > -extern 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..a27fc10f84f7 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(const struct lsm_id *lsmid); > > #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(const struct lsm_id *lsmid) > +{ > +} > + > #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 36abc84ba299..076451109637 100644 > --- a/security/integrity/ima/ima_appraise.c > +++ b/security/integrity/ima/ima_appraise.c > @@ -636,8 +636,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; > @@ -750,9 +750,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; > @@ -781,8 +781,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); > @@ -790,8 +790,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; > > @@ -803,3 +803,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(const struct lsm_id *lsmid) > +{ > + security_add_hooks(ima_appraise_hooks, ARRAY_SIZE(ima_appraise_hooks), > + lsmid); > +} > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c > index f923ff5c6524..9aabbc37916c 100644 > --- a/security/integrity/ima/ima_main.c > +++ b/security/integrity/ima/ima_main.c > @@ -1161,6 +1161,7 @@ const struct lsm_id *ima_get_lsm_id(void) > void __init init_ima_lsm(void) > { > security_add_hooks(ima_hooks, ARRAY_SIZE(ima_hooks), &ima_lsmid); > + init_ima_appraise_lsm(&ima_lsmid); > } > > /* Introduce a dummy function as 'ima' init method (it cannot be NULL). */ > diff --git a/security/security.c b/security/security.c > index b2fdcbaa4b30..456f3fe74116 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> > @@ -2308,9 +2307,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); > @@ -2338,9 +2334,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); > @@ -2401,9 +2394,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); > @@ -2503,9 +2493,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);
diff --git a/fs/attr.c b/fs/attr.c index 221d2bb0a906..38841f3ebbcb 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" @@ -503,7 +502,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 23ae24b60ecf..0bae61a15b60 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -92,66 +92,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); -extern 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); -} - -extern 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..a27fc10f84f7 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(const struct lsm_id *lsmid); #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(const struct lsm_id *lsmid) +{ +} + #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 36abc84ba299..076451109637 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -636,8 +636,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; @@ -750,9 +750,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; @@ -781,8 +781,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); @@ -790,8 +790,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; @@ -803,3 +803,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(const struct lsm_id *lsmid) +{ + security_add_hooks(ima_appraise_hooks, ARRAY_SIZE(ima_appraise_hooks), + lsmid); +} diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index f923ff5c6524..9aabbc37916c 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -1161,6 +1161,7 @@ const struct lsm_id *ima_get_lsm_id(void) void __init init_ima_lsm(void) { security_add_hooks(ima_hooks, ARRAY_SIZE(ima_hooks), &ima_lsmid); + init_ima_appraise_lsm(&ima_lsmid); } /* Introduce a dummy function as 'ima' init method (it cannot be NULL). */ diff --git a/security/security.c b/security/security.c index b2fdcbaa4b30..456f3fe74116 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> @@ -2308,9 +2307,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); @@ -2338,9 +2334,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); @@ -2401,9 +2394,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); @@ -2503,9 +2493,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);