On Wed, Apr 20, 2022 at 10:06:21AM -0400, Stefan Berger wrote:
> Implement hierarchical processing of file accesses in IMA namespaces by
> walking the list of user namespaces towards the root. This way file
> accesses can be audited in an IMA namespace and also be evaluated against
> the IMA policies of parent IMA namespaces.
>
> Pass the user_namespace into process_measurement since we will be walking
> the hierarchy of user_namespaces towards the init_user_ns and we can easily
> derive the ima_namespace from the user_namespace.
>
> __process_measurement() returns either 0 or -EACCES. For hierarchical
> processing remember the -EACCES returned by this function but continue
> to the parent user namespace. At the end either return 0 or -EACCES
> if an error occurred in one of the IMA namespaces.
>
> Currently the ima_ns pointer of the user_namespace is always NULL except
> at the init_user_ns, so test ima_ns for NULL pointer and skip the call to
> __process_measurement() if it is NULL. Once IMA namespacing is fully
> enabled, the pointer may still be NULL due to late initialization of the
> IMA namespace.
>
> Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
> Reviewed-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
Reviewed-by: Serge Hallyn <serge@xxxxxxxxxx>
>
> ---
>
> v10:
> - Fixed compilation issue
>
> v9:
> - Switch callers to pass user_namespace rather than ima_namespace with
> potential NULL pointer
> - Add default case to switch statement and warn if this happens
> - Implement ima_ns_from_user_ns() in this patch now
> ---
> security/integrity/ima/ima.h | 8 ++++
> security/integrity/ima/ima_main.c | 76 +++++++++++++++++++++++--------
> 2 files changed, 65 insertions(+), 19 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 626a6ce2453c..2775a6d89e6d 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -497,4 +497,12 @@ struct user_namespace *ima_user_ns_from_file(const struct file *filp)
> return file_inode(filp)->i_sb->s_user_ns;
> }
>
> +static inline struct ima_namespace
> +*ima_ns_from_user_ns(struct user_namespace *user_ns)
> +{
> + if (user_ns == &init_user_ns)
> + return &init_ima_ns;
> + return NULL;
> +}
> +
> #endif /* __LINUX_IMA_H */
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index d44faf1c065d..17eb2971eb36 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -196,10 +196,10 @@ void ima_file_free(struct file *file)
> ima_check_last_writer(iint, inode, file);
> }
>
> -static int process_measurement(struct ima_namespace *ns,
> - struct file *file, const struct cred *cred,
> - u32 secid, char *buf, loff_t size, int mask,
> - enum ima_hooks func)
> +static int __process_measurement(struct ima_namespace *ns,
> + struct file *file, const struct cred *cred,
> + u32 secid, char *buf, loff_t size, int mask,
> + enum ima_hooks func)
> {
> struct inode *inode = file_inode(file);
> struct integrity_iint_cache *iint = NULL;
> @@ -391,6 +391,41 @@ static int process_measurement(struct ima_namespace *ns,
> return 0;
> }
>
> +static int process_measurement(struct user_namespace *user_ns,
> + struct file *file, const struct cred *cred,
> + u32 secid, char *buf, loff_t size, int mask,
> + enum ima_hooks func)
> +{
> + struct ima_namespace *ns;
> + int ret = 0;
> +
> + while (user_ns) {
> + ns = ima_ns_from_user_ns(user_ns);
> + if (ns) {
> + int rc;
> +
> + rc = __process_measurement(ns, file, cred, secid, buf,
> + size, mask, func);
> + switch (rc) {
> + case 0:
> + break;
> + case -EACCES:
> + /* return this error at the end but continue */
> + ret = -EACCES;
> + break;
> + default:
> + /* should not happen */
> + ret = -EACCES;
> + WARN_ON_ONCE(true);
> + }
> + }
> +
> + user_ns = user_ns->parent;
> + }
> +
> + return ret;
> +}
> +
> /**
> * ima_file_mmap - based on policy, collect/store measurement.
> * @file: pointer to the file to be measured (May be NULL)
> @@ -404,13 +439,14 @@ static int process_measurement(struct ima_namespace *ns,
> */
> int ima_file_mmap(struct file *file, unsigned long prot)
> {
> - struct ima_namespace *ns = &init_ima_ns;
> + struct user_namespace *user_ns = current_user_ns();
> u32 secid;
>
> if (file && (prot & PROT_EXEC)) {
> security_current_getsecid_subj(&secid);
> - return process_measurement(ns, file, current_cred(), secid,
> - NULL, 0, MAY_EXEC, MMAP_CHECK);
> + return process_measurement(user_ns, file, current_cred(),
> + secid, NULL, 0, MAY_EXEC,
> + MMAP_CHECK);
> }
>
> return 0;
> @@ -486,19 +522,19 @@ int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot)
> */
> int ima_bprm_check(struct linux_binprm *bprm)
> {
> - struct ima_namespace *ns = &init_ima_ns;
> + struct user_namespace *user_ns = current_user_ns();
> int ret;
> u32 secid;
>
> security_current_getsecid_subj(&secid);
> - ret = process_measurement(ns, bprm->file, current_cred(), secid, NULL,
> - 0, MAY_EXEC, BPRM_CHECK);
> + ret = process_measurement(user_ns, bprm->file, current_cred(), secid,
> + NULL, 0, MAY_EXEC, BPRM_CHECK);
> if (ret)
> return ret;
>
> security_cred_getsecid(bprm->cred, &secid);
> - return process_measurement(ns, bprm->file, bprm->cred, secid, NULL, 0,
> - MAY_EXEC, CREDS_CHECK);
> + return process_measurement(user_ns, bprm->file, bprm->cred, secid,
> + NULL, 0, MAY_EXEC, CREDS_CHECK);
> }
>
> /**
> @@ -513,11 +549,12 @@ int ima_bprm_check(struct linux_binprm *bprm)
> */
> int ima_file_check(struct file *file, int mask)
> {
> - struct ima_namespace *ns = &init_ima_ns;
> + struct user_namespace *user_ns = current_user_ns();
> u32 secid;
>
> security_current_getsecid_subj(&secid);
> - return process_measurement(ns, file, current_cred(), secid, NULL, 0,
> + return process_measurement(user_ns, file, current_cred(), secid,
> + NULL, 0,
> mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
> MAY_APPEND), FILE_CHECK);
> }
> @@ -718,7 +755,7 @@ void ima_post_path_mknod(struct user_namespace *mnt_userns,
> int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
> bool contents)
> {
> - struct ima_namespace *ns = &init_ima_ns;
> + struct user_namespace *user_ns = current_user_ns();
> enum ima_hooks func;
> u32 secid;
>
> @@ -741,7 +778,7 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
> /* Read entire file for all partial reads. */
> func = read_idmap[read_id] ?: FILE_CHECK;
> security_current_getsecid_subj(&secid);
> - return process_measurement(ns, file, current_cred(), secid, NULL,
> + return process_measurement(user_ns, file, current_cred(), secid, NULL,
> 0, MAY_READ, func);
> }
>
> @@ -769,7 +806,8 @@ const int read_idmap[READING_MAX_ID] = {
> int ima_post_read_file(struct file *file, void *buf, loff_t size,
> enum kernel_read_file_id read_id)
> {
> - struct ima_namespace *ns = &init_ima_ns;
> + struct user_namespace *user_ns = current_user_ns();
> + struct ima_namespace *ns = ima_ns_from_user_ns(user_ns);
> enum ima_hooks func;
> u32 secid;
>
> @@ -786,8 +824,8 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
>
> func = read_idmap[read_id] ?: FILE_CHECK;
> security_current_getsecid_subj(&secid);
> - return process_measurement(ns, file, current_cred(), secid, buf, size,
> - MAY_READ, func);
> + return process_measurement(user_ns, file, current_cred(), secid,
> + buf, size, MAY_READ, func);
> }
>
> /**
> --
> 2.34.1
