Re: [RFC][PATCH 1/3] evm: Move hooks outside LSM infrastructure

[Mimi Zohar]

On Mon, 2020-05-11 at 14:13 +0000, Roberto Sassu wrote:
> > From: Mimi Zohar [mailto:zohar@xxxxxxxxxxxxx]
> > Sent: Friday, May 8, 2020 7:08 PM
> > On Fri, 2020-05-08 at 10:20 +0000, Roberto Sassu wrote:
> > > > From: Mimi Zohar [mailto:zohar@xxxxxxxxxxxxx]
> > > > On Thu, 2020-05-07 at 16:47 +0000, Roberto Sassu wrote:
> > 
> > <snip>
> > 
> > > > > > the file metadata to the file data. ÂThe IMA and EVM policies really
> > > > > > need to be in sync.
> > > > >
> > > > > It would be nice, but at the moment EVM considers also files that are
> > > > > not selected by the IMA policy. An example of why this is a problem is
> > > > > the audit service that fails to start when it tries to adjust the
> > permissions
> > > > > of the log files. Those files don't have security.evm because they are
> > > > > not appraised by IMA, but EVM denies the operation.
> > > >
> > > > No, this is a timing issue as to whether or not the builtin policy or
> > > > a custom policy has been loaded. ÂA custom policy could exclude the
> > > > log files based on LSM labels, but they are included in the builtin
> > > > policy.
> > >
> > > Yes, I was referring to a custom policy. In this case, EVM will not adapt
> > > to the custom policy but still verifies all files. If access control is done
> > > exclusively by IMA at the time evm_verifyxattr() is called, we wouldn't
> > > need to add security.evm to all files.
> > 
> > Roberto, EVM is only triggered by IMA, unless you've modified the
> > kernel to do otherwise.
> 
> EVM would deny xattr/attr operations even if IMA is disabled in the
> kernel configuration. For example, evm_setxattr() returns the value
> from evm_protect_xattr(). IMA is not involved there.

CommitÂae1ba1676b88 ("EVM: Allow userland to permit modification of
EVM-protected metadata") introducedÂEVM_ALLOW_METADATA_WRITES to allow
writing the EVM portable and immutable file signatures.Â

> 
> > I'm not interested in a complicated solution, just one that addresses
> > the new EVM immutable and portable signature. ÂIt might require EVM
> > HMAC, IMA differentiating between a new file and an existing file, or:q

> > it might require writing the new EVM signature last, after all the
> > other xattrs or metadata are updated. ÂPlease nothing that changes
> > existing expectations.
> 
> Ok. Introducing the new status INTEGRITY_FAIL_IMMUTABLE, as I
> mentioned in '[PATCH] ima: Allow imasig requirement to be satisfied by
> EVM portable signatures' seems to have an additional benefit. We
> could introduce an additional exception in evm_protect_xattr(), other
> than INTEGRITY_NOXATTRS, as we know that xattr/attr update won't
> cause HMAC update.

Refer to Documentation/ABI/testing/evm describes on how to permit
writing the security.evm signatures.
> 
> However, it won't work unless the IMA policy says that the file should
> be appraised when the mknod() system call is executed. Otherwise,
> integrity_iint_cache is not created for the file and the IMA_NEW_FILE
> flag is not set.
> 
> Granting an exception for INTEGRITY_FAIL_IMMUTABLE solves the case
> where security.evm is the first xattr set. If a protected xattr is the first to
> be added, then we also have to handle the INTEGRITY_NOLABEL error.
> It should be fine to add an exception for this error if the HMAC key is not
> loaded.
> 
> This still does not solve all problems. INTEGRITY_NOLABEL cannot be
> ignored if the HMAC key is loaded, which means that all files need to be
> protected by EVM to avoid issues like the one I described (auditd).

The application still needs to defer writing the EVM portable and
immutable file signatures until after all the other xattrs are written
otherwise it won't validate.

Mimi