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

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

В Thu, 05 Jan 2017 22:19:11 -0500
Mimi Zohar <zo...@li...> пишет:

> On Wed, 2017-01-04 at 14:07 -0600, Serge E. Hallyn wrote:
> > Quoting Mimi Zohar (zo...@li...):  
> > > [Cc'ing Serge]
> > > 
> > > On Wed, 2017-01-04 at 18:17 +0300, Mikhail Kurinnoi wrote:  
> > > > В 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.  
> > > 
> > > Really strange that getxattr for capabilities returns
> > > -EOPNOTSUPP, even though the inode->i_opflags supports xattrs.
> > > Serge, any insights?  
> > 
> > cap_inode_getxattr and cap_inode_getsecurity don't exist, so the
> > EOPNOTSUPP is actually returned by securit/security.c itself i
> > think.  
> 
> Looking at security/security.c: security_inode_getxattr() it looks
> like it should return 0, if there aren't any LSMs registered for the
> hook. (The value of the second parameter to
> "call_int_hook(inode_getxattr, 0, dentry, name);" defines the value
> returned.
> 
> Mikhail, are there any LSMs registered?

I don't have LSM registered.
As I could see, inode_getxattr hook used only by SELinux and SMACK.
I have SELinux and SMACK disabled for sure:

# CONFIG_SECURITY_SELINUX is not set
# CONFIG_SECURITY_SMACK is not set


> 
> > I suspect if you check a kernel before the xattr rewrite in Sep
> > 2016 you'll find it behaved differently?  
> 
> Next place to look ...
> 
> Mimi
> 



-- 
Best regards,
Mikhail Kurinnoi