On Tue, 2022-02-01 at 15:37 -0500, 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>
>
> ---
>
> 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 0057b1fd6c18..aea8fb8d2854 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -493,4 +493,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 ae0e9b14554a..917504319e7f 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;
> +}
> +
Very nice and concise!
Reviewed-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
--
thanks,
Mimi
