Re: [PATCH v4 12/30] integrity: implement get and set acl hook

[Paul Moore <paul@paul-moore.com>]

On Thu, Sep 29, 2022 at 11:19 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> Hi Paul,
>
> On Thu, 2022-09-29 at 15:14 -0400, Paul Moore wrote:
> > > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> > > index bde74fcecee3..698a8ae2fe3e 100644
> > > --- a/security/integrity/ima/ima_appraise.c
> > > +++ b/security/integrity/ima/ima_appraise.c
> > > @@ -770,6 +770,15 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
> > >         return result;
> > >  }
> > >
> > > +int ima_inode_set_acl(struct user_namespace *mnt_userns, 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);
> > > +
> > > +       return 0;
> > > +}
> >
> > While the ima_inode_set_acl() implementation above looks okay for the
> > remove case, I do see that the ima_inode_setxattr() function has a
> > call to validate_hash_algo() before calling
> > ima_reset_appraise_flags().  IANAIE (I Am Not An Ima Expert), but it
> > seems like we would still want that check in the ACL case.
>
> Thanks, Paul.  The "ima: fix blocking of security.ima xattrs of
> unsupported algorithms" patch in next-integrity branch, moves the hash
> algorithm checking earlier.

Okay, thanks.  When comparing against the status quo I usually just
stick with what is in Linus' tree, but I'm happy to hear this patch is
correct.

-- 
paul-moore.com