Re: [Linux-ima-user] [PATCH v3] evm: provide portable EVM signature version.

[Mimi Z. <zo...@li...>]

On Tue, 2017-01-10 at 06:33 -0500, Mimi Zohar wrote:
> On Tue, 2017-01-10 at 07:25 +0300, Mikhail Kurinnoi wrote:
> > В Mon, 09 Jan 2017 21:38:41 -0500
> > Mimi Zohar <zo...@li...> пишет:
> >    
> > > > > Currently, after each xattr is written, the security.evm is
> > > > > updated. Do we remove the existing security.evm before verifying
> > > > > the portable security.evm?  
> > > > 
> > > > After each xattr is written, the security.evm is updated, so, we
> > > > have proper HMAC all the time, this is exactly what we need. We
> > > > don't need remove existing security.evm nor before, nor after if
> > > > EVM portable signature verification is ok.
> > > > 
> > > > If we move all work with EVM portable signature into
> > > > evm_inode_setxattr():
> > > > 
> > > > 1) If we have EVM portable signature verification is ok - no
> > > > changes in EVM xattr are needed. We should only prevent EVM xattr
> > > > changes by __vfs_setxattr_noperm() in vfs_setxattr(). EVM code will
> > > > do everything what we need. We don't need update EVM xattr or
> > > > something. Everything is fine by current EVM/IMA code. This mean,
> > > > we should have HMAC on protected xattrs copy and/or file data copy,
> > > > lets EVM/IMA care about HMAC.  
> > > 
> > > Failing the evm_inode_setxattr() will prevent the portable xattr from
> > > being written.
> > 
> > Exactly my point. We could use some error code returned by
> > evm_inode_setxattr() in order to prevent the portable xattr from being
> > written, but return 0 from vfs_setxattr() instead of error code what we
> > are using in the evm_inode_setxattr(), since we should return "all
> > is ok" if EVM portable signature verification is ok and we stay
> > with HMAC.
> 
> Agreed this works.  The LSM hooks expect 0 on success and non zero on
> failure.  Here we're inverting that.   Please introduce this concept of
> a valid, non-zero return code from evm_inode_setxattr() in
> security_inode_setxattr() as a separate patch.
> 
> > > > 2) If we have EVM portable signature verification is failed - we
> > > > should prevent EVM xattr changes by __vfs_setxattr_noperm() in
> > > > vfs_setxattr(), we should replace EVM xattr by some type of EVM
> > > > signature that:  
> > > 
> > > > a) should not be changed/removed if we not in fix mode;
> > > > b) return INTEGRITY_FAIL in evm_verify_hmac().
> > > > This is all about my previous proposition about
> > > > EVM_XATTR_PROTECTED.  
> > > 
> > > Unlike IMA, where file signatures are considered immutable, EVM
> > > converts signatures to HMACs.  We probably don't need a new signature
> > > format, just a new type which isn't converted to an HMAC.
> > > 
> > > For failed portable signatures, we would write security.evm as this
> > > new type, but with an invalid payload. 
> > 
> > Exactly. We don't need any new signature format for EVM. We don't need
> > immutable EVM xattr or something like that for sure.
> 
> "immutable" in this context just means that the security.evm xattr can
> not be replaced with an HMAC.
> 
> > We just need new type that we could use for failed portable signatures
> > that will not be converted to an HMAC. And sure, it must have invalid
> > payload, I even propose stay with zero-filled xattr (what I am using
> > now in patch), since we don't really need anything else for our
> > purposes.
> 
> Others have requested an immutable file signature type.  The only
> difference between this signature type and the original one, is that
> this type is never replaced with an HMAC.
> 
> We've defined the "portable" signature to not include certain fields
> (eg. i_version, i_ino).   Instead of removing them, would it make sense
> to zero out these fields.  Then converting a "portable" signature to an
> "immutable" one is a matter of changing just the file signature type.
> With these fields zeroed out, the normal file signature verification
> will fail.

That should have been the normal EVM file meta-data signature
verification will fail.

> Assuming you agree with an "immutable" file signature type, the first
> patch in this series should introduce the concept.
> 
> Mimi