Re: [Linux-ima-user] [PATCH] evm: add exceptions for inodes without getxattr() support.

[Mikhail K. <vie...@vi...>]

В Wed, 04 Jan 2017 09:15:47 -0500
Mimi Zohar <zo...@li...> пишет:

> On Wed, 2017-01-04 at 05:49 +0300, Mikhail Kurinnoi wrote:
> > В Tue, 03 Jan 2017 18:42:34 -0500
> > Mimi Zohar <zo...@li...> пишет:
> >   
> > > On Tue, 2017-01-03 at 21:03 +0300, Mikhail Kurinnoi wrote:  
> > > > В Tue, 03 Jan 2017 08:13:42 -0500
> > > > Mimi Zohar <zo...@li...> пишет:    
> > >   
> > > > > > Let me fix this misunderstanding by tests.
> > > > > > Test with default code:
> > > > > > 
> > > > > > # touch /var/tmp/test
> > > > > > # getfattr -m . -d -e hex /var/tmp/test
> > > > > > (no output, file don't have any xattrs)
> > > > > > # setfattr -n security.foo -v "123" /var/tmp/test      
> > > > > 
> > > > > Right, the ops exist.
> > > > >     
> > > > > > # getfattr -m . -d -e hex /var/tmp/test
> > > > > >     getfattr: Removing leading '/' from absolute path names
> > > > > >     # file: var/tmp/test
> > > > > >     security.foo=0x313233
> > > > > > # chmod 666 /var/tmp/test
> > > > > >     chmod: changing permissions of '/var/tmp/test':
> > > > > > Operation not permitted      
> > > > > 
> > > > > All files in the IMA appraise policy require an IMA hash or
> > > > > signature. With the "ima_appraise_policy", all files owned by
> > > > > root must be appraised either based on the file hash or file
> > > > > signature.  Other policies might require file signatures.    
> > > > 
> > > > 
> > > > I think the issue also related to evm_calc_hmac_or_hash().  
> 
> > > > Before cycle, we set error to -ENODATA. In the same time, if we
> > > > don't have any xattrs for protection, error will never changed
> > > > from -ENODATA to 0. So, we will return -ENODATA error into
> > > > evm_update_evmxattr(). In this case, evm_update_evmxattr() will
> > > > not protect uid/gid/mode by security.evm, but remove
> > > > security.evm instead, if it's available.    
> > > 
> > > For files not in policy (there aren't any xattrs), I was able to
> > > do the chmod without problems.
> > > 
> > > # touch /var/tmp/test
> > > # getfattr -m ^security -e hex --dump /var/tmp/test
> > > # chmod 666 /var/tmp/test
> > > # getfattr -m ^security -e hex --dump /var/tmp/test
> > > # ls -lat /var/tmp/test
> > > -rw-rw-rw- 1 root root 0 Jan  3 18:16 /var/tmp/test
> > >
> > > We've already shown that you can write security xattrs
> > > (security.foo). So it isn't an issue that the xattr ops don't
> > > exist.  Why isn't it returning INTEGRITY_NOXATTRS?  
> > 
> > I have exactly same question, and I still don't have answer for it.
> > It must returning INTEGRITY_NOXATTRS or don't be able set any
> > "security." xattrs. 
> > Plus, we should have -EOPNOTSUPP error returned by
> > vfs_getxattr_alloc() if we have xattrs support issue, I don't
> > understand why -EOPNOTSUPP error returned by
> > evm_find_protected_xattrs() instead.  
> 
>  Unlike SELinux, it seems that kernel/commoncaps:
> get_vfs_caps_from_disk() treats -ENODATA and -EOPNOTSUPP the same, as
> though there is no data.  Could you see if modifying
> evm_find_protected_xattrs() like below fixes the problem?
> 
>                if (error < 0) {
>                         if (error == -ENODATA || error == -EOPNOTSUPP)
>                                 continue;
>                         return error;
>                 }
>                 count++;
> 

Mimi, I can confirm that this code modification fix the issue.
I can create now files without xattrs and use chmod on both - tmpfs
and ext4 partitions covered by "dont_appraise" policy.



-- 
Best regards,
Mikhail Kurinnoi