Re: [PATCH v5 00/21] EVM

[Andrew Morton <akpm@linux-foundation.org>]

On Mon, 16 May 2011 10:44:54 -0400
Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> Extended Verification Module(EVM) detects offline tampering of the security
> extended attributes (e.g. security.selinux, security.SMACK64, security.ima),
> which is the basis for LSM permission decisions and, with the IMA-appraisal
> patchset, integrity appraisal decisions. This patchset provides the framework
> and an initial method to detect offline tampering of the security extended
> attributes.  The initial method maintains an HMAC-sha1 across a set of
> security extended attributes, storing the HMAC as the extended attribute
> 'security.evm'. To verify the integrity of an extended attribute, EVM exports
> evm_verifyxattr(), which re-calculates the HMAC and compares it with the
> version stored in 'security.evm'.  Other methods of validating the integrity
> of a file's metadata will be posted separately (eg. EVM-digital-signatures).
> 
> While this patchset does authenticate the security xattrs, and
> cryptographically binds them to the inode, coming extensions will bind other
> directory and inode metadata for more complete protection.  To help simplify
> the review and upstreaming process, each extension will be posted separately
> (eg. IMA-appraisal, IMA-appraisal-directory).  For a general overview of the
> proposed Linux integrity subsystem, refer to Dave Safford's whitepaper:
> http://downloads.sf.net/project/linux-ima/linux-ima/Integrity_overview.pdf.
> 
> Much appreciation to Dave Hansen, Serge Hallyn, and Matt Helsley for
> reviewing the original patches.

The changelog forgot to define "offline tampering".  I can guess what
it means, but my guess will surely be incomplete.  I can do a web
search for the term but that really only leads me to this patchset.

It would be much better if I didn't have to guess.  A good description
would list all the known offline tampering scenarios and would explain
how the patchset addresses them and whether there are any scenarios
which are not addressed.

Secondly, the changelog didn't attempt to give anyone a reason to merge
the patchset.  Why is offline tampering bad?  Why should we bother
addressing it?  What value does this patchset provide to our users? 
Again, I could guess.  But it would be much much better to have these
things explained to us by the people who understand them.

Thirdly, I'm not seeing a description of the user interface anywhere. 
Please fully describe it in a manner which can be efficiently reviewed.
For example, if there are any user-provided inputs, describe them.  If
tampering has been detected then this is presumably communicated to the
operator in some fashion.  Please fully describe that reporting scheme.
What steps do we expect the operator to take after tampering has been
detected?  How does the patchset aid them in that?  etc.


Once we have a better understanding of what the feature does and why it
does it and how it interfaces with the user, we can start looking at
the implementation.