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

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

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.

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

Mimi