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

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

В Tue, 03 Jan 2017 08:13:42 -0500
Mimi Zohar <zo...@li...> пишет:

> On Fri, 2016-12-30 at 19:01 +0300, Mikhail Kurinnoi wrote:
> > В Fri, 30 Dec 2016 08:15:31 -0500
> > Mimi Zohar <zo...@li...> пишет:
> >   
> > > On Thu, 2016-12-29 at 23:56 +0300, Mikhail Kurinnoi wrote:
> > >   
> > > > > On Fedora, "touch" creates the selinux label, which results
> > > > > in an EVM label as well.  Everything works as expected.    
> > > > 
> > > > 
> > > > I don't use SELinux, with "dont_appraise" policy "touch" don't
> > > > creates any xattrs that should be protected. And, since I see
> > > > -EOPNOTSUPP error from getxattr(), this mean ->list, ->get,
> > > > and ->set xattr handler operations removed in inode and I have
> > > > disabled xattr support in inode. Have no idea why it's so, this
> > > > is probably some kind of xattrs related work issue.  
> 
> From your test below, you are able to write security.foo.  So, the
> setxattr/getxattr ops are valid.  This sounds like commit
> "c68ed80c97d9 ima: limit file hash setting by user to fix and log
> modes" is preventing writing security.ima hashes.  We've just
> reverted this patch during the lastest open window.  Writing a file
> signature as security.ima should work.

I reverted back this patch on my kernel sources. Nothing changed.
If file covered by dont_appraise policy, evmctl still return:
setxattr failed: /var/tmp/test
errno: Operation not permitted (1)



> > > It's kind of irrelevant, but I'd like to know why it isn't
> > > working. Are you running these tests in a VM or container?  (If
> > > in a container, what type of container?)  
> > 
> > I am testing on hardware directly and don't use containers.
> > 
> >   
> > > > I tested "security.foo" xattr, but this is not working. Looks
> > > > like this work only with xattrs that should be protected by
> > > > EVM.  
> > > 
> > > By "not working", what do you mean?  Are you able to write the
> > > security.foo xattr?  
> > 
> > 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.

Probably, we should protect uid/gid/mode even if we don't have xattrs
for protection. In this case all files will have security.evm xattr.


-- 
Best regards,
Mikhail Kurinnoi