Menu
▾
▴
Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.
|
From: Mimi Z. <zo...@li...> - 2016-12-28 14:24:52
|
On Wed, 2016-12-28 at 15:51 +0300, Mikhail Kurinnoi wrote:
> Removed from kernel boot command line "ima_appraise=fix evm=fix", I
> faced with a little bit strange audit log output. For example:
> ...
> pid=3237 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> cause="fail" comm="cp" name="depconfig" fowner=0 dev="tmpfs" ino=6839
> res=0
> pid=4418 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> cause="fail" comm="chgrp" name="utmp" fowner=0 dev="tmpfs" ino=11295
> res=0
> pid=4419 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> cause="fail" comm="chmod" name="utmp" fowner=0 dev="tmpfs" ino=11295
> res=0
> pid=5040 uid=0 auid=4294967295 ses=4294967295 op="appraise_metadata"
> cause="fail" comm="cupsd" name="0" fowner=0 dev="tmpfs" ino=10778
> res=0
> pid=5611 uid=1000 auid=1000 ses=3 op="appraise_metadata" cause="fail"
> comm="X" name=".tX0-lock" fowner=0 dev="tmpfs" ino=11650 res=0
> ...
>
> This output were logged with default policy (ima_appraise_tcb
> ima_tcb). As you can see (op="appraise_metadata"), this issue
> connected to evm_inode_setattr(), evm_inode_removexattr() and
> evm_inode_setxattr(). After some digging I found, that we don't count
> on getxattr() support in inode. I mean, we don't count on EOPNOTSUPP
> error code from evm_find_protected_xattrs(), as result,
> evm_verify_hmac() will return only INTEGRITY_FAIL error and legitimate
> attr/xattr(acls) changes will be not allowed by EVM.
Before EVM allows a file to write file metadata it validates the
existing security.evm xattr. Only if it is valid, does EVM permit the
file metadata to change. Otherwise the updated security.evm xattr
would be based on bogus file metadata. Is the issue really that
getxattr() isn't supported or rather that these files were created under
a different policy, which didn't label the files properly? (tmpfs has
xattr support.)
Mimi
> Probably, we should not prevent attr/xattr(acls) changes in EVM code,
> if inode don't support getxattr().
>
> This patch add exceptions for inodes without getxattr() support.
>
> Signed-off-by: Mikhail Kurinnoi <vie...@vi...>
>
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -138,6 +138,8 @@ static enum integrity_status evm_verify_hmac
> evm_status = INTEGRITY_NOLABEL;
> else if (rc == 0)
> evm_status = INTEGRITY_NOXATTRS; /* new file */
> + else if (rc == -EOPNOTSUPP)
> + evm_status = INTEGRITY_UNKNOWN;
> } else if (rc == -EOPNOTSUPP) {
> evm_status = INTEGRITY_UNKNOWN;
> }
> @@ -291,12 +293,14 @@ static int evm_protect_xattr(struct dentry *dentry,
> return 0;
> evm_status = evm_verify_current_integrity(dentry);
> if ((evm_status == INTEGRITY_PASS) ||
> - (evm_status == INTEGRITY_NOXATTRS))
> + (evm_status == INTEGRITY_NOXATTRS) ||
> + (evm_status == INTEGRITY_UNKNOWN))
> return 0;
> goto out;
> }
> evm_status = evm_verify_current_integrity(dentry);
> - if (evm_status == INTEGRITY_NOXATTRS) {
> + if ((evm_status == INTEGRITY_NOXATTRS) ||
> + (evm_status == INTEGRITY_UNKNOWN)) {
> struct integrity_iint_cache *iint;
>
> iint = integrity_iint_find(d_backing_inode(dentry));
> @@ -432,7 +436,8 @@ int evm_inode_setattr(struct dentry *dentry, struct iattr *attr)
>
> evm_status = evm_verify_current_integrity(dentry);
> if ((evm_status == INTEGRITY_PASS) ||
> - (evm_status == INTEGRITY_NOXATTRS))
> + (evm_status == INTEGRITY_NOXATTRS) ||
> + (evm_status == INTEGRITY_UNKNOWN))
> return 0;
> integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry),
> dentry->d_name.name, "appraise_metadata",
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-ima-user mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linux-ima-user
>
|
