Re: [PATCH V3 2/2] EVM: Allow runtime modification of the set of verified xattrs

[Mimi Zohar <zohar@linux.vnet.ibm.com>]

On Tue, 2018-05-08 at 22:51 +0000, Matthew Garrett wrote:
> On Tue, May 8, 2018 at 2:43 PM Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > On Tue, 2018-05-08 at 21:30 +0000, Matthew Garrett wrote:
> > > As far as I can tell the VFS doesn't do that, and I wouldn't put it past
> > > someone to use UTF-8 at some point...
> 
> > The audit subsystem uses audit_log_untrustedstrings() to check for
> > control characters.  I'm not sure what should be included, but not
> > checking userspace strings doesn't sound right.
> 
> As far as I can tell it's legitimate for xattr names to include control
> characters. I can't think of /why/ someone would do that, but...
> 
> > > I think a config option would make sense here. Locking doesn't seem
> > > unreasonable, but I'm not sure what the threat model would really be -
> > > adding new xattrs would only result in additional signatures validating
> if
> > > they were signed with a trusted key in the first place.
> 
> > Remember adding additional EVM xattrs isn't limited to just EVM
> > signatures, but for the original EVM HMAC as well.  Did you want to
> > limit it to the EVM portable & immutable signatures?
> 
> If you add entries and then signatures are created you'll end up with
> signatures that won't validate on next boot until the same attributes are
> added. That feels at worst like root being able to trigger a DoS, which
> they'd be able to do by tampering with the signatures via the raw device
> node anyway.

With a system with EVM HMAC enabled and an "ima_policy=appraise_tcb"
policy, which appraises all executables and files owned by root, any
new xattrs would cause the EVM HMAC to be recalculated immediately.
 On boot, if any of these files are accessed prior to the additional
xattrs are added, it could cause the system not to boot properly.

True in both cases it is a DoS, but there is a difference between
accessing the raw device and normal usage.

Adding support for additional xattr names is fine.  Making it a
Kconfig option is required.  Being able to lock adding additional
 xattr names should also be required.

Mimi