Re: [PATCH v4 13/30] evm: add post set acl hook

[Mimi Zohar <zohar@linux.ibm.com>]

Hi Christian,

On Thu, 2022-09-29 at 17:30 +0200, Christian Brauner wrote:
> The security_inode_post_setxattr() hook is used by security modules to
> update their own security.* xattrs. Consequently none of the security
> modules operate on posix acls. So we don't need an additional security
> hook when post setting posix acls.
> 
> However, the integrity subsystem wants to be informed about posix acl
> changes and specifically evm to update their hashes when the xattrs
> change. 

^... to be informed about posix acl changes in order to reset the EVM
status flag.

> The callchain for evm_inode_post_setxattr() is:
> 
> -> evm_inode_post_setxattr()

Resets the EVM status flag for both EVM signatures and HMAC.

>    -> evm_update_evmxattr()

evm_update_evmxattr() is only called for "security.evm", not acls.  

>       -> evm_calc_hmac()
>          -> evm_calc_hmac_or_hash()
> 
> and evm_cacl_hmac_or_hash() walks the global list of protected xattr
> names evm_config_xattrnames. This global list can be modified via
> /sys/security/integrity/evm/evm_xattrs. The write to "evm_xattrs" is
> restricted to security.* xattrs and the default xattrs in
> evm_config_xattrnames only contains security.* xattrs as well.
> 
> So the actual value for posix acls is currently completely irrelevant
> for evm during evm_inode_post_setxattr() and frankly it should stay that
> way in the future to not cause the vfs any more headaches. But if the
> actual posix acl values matter then evm shouldn't operate on the binary
> void blob and try to hack around in the uapi struct anyway. Instead it
> should then in the future add a dedicated hook which takes a struct
> posix_acl argument passing the posix acls in the proper vfs format.
> 
> For now it is sufficient to make evm_inode_post_set_acl() a wrapper
> around evm_inode_post_setxattr() not passing any actual values down.
> This will still cause the hashes to be updated as before.

^This will cause the EVM status flag to be reset.

> 
> Reviewed-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>

-- 
thanks,

Mimi