Re: [PATCH V3] EVM: Allow userland to permit modification of EVM-protected metadata

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

On Fri, 2017-11-03 at 11:49 -0700, Matthew Garrett wrote:
> On Fri, Nov 3, 2017 at 9:03 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Thu, 2017-11-02 at 15:10 -0700, Matthew Garrett wrote:
> >> -             1: enable HMAC validation and creation
> >> -             2: enable digital signature validation
> >> -             3: enable HMAC and digital signature validation and HMAC
> >> -                creation
> >> +             Bit       Effect
> >> +             0         Enable HMAC validation and creation
> >
> > The code and documentation do not seem to be in sync.  Dracut is
> > currently using 1 to indicate the HMAC key has been loaded.
> 
> I've changed from describing the raw values to the bits they
> correspond to, so bit 0 corresponds to a value of 1. I can switch back
> to describing the raw values instead?

Ok, bits are fine.

> >>
> >> +     /* Policy permits modification of the protected attrs */
> >
> > Could we indicate that there is no HMAC key loaded.

Just as a reminder in the comment.

> In the comment, or in kernel output?
> >> +     /* Don't allow a request to freshly enable metadata writes if
> >> +      * keys are loaded.
> >> +      */
> >> +     if ((i & EVM_ALLOW_METADATA_WRITES) &&
> >> +         ((evm_initialized & EVM_KEY_MASK) != 0) &&
> >> +         !(evm_initialized & EVM_ALLOW_METADATA_WRITES))
> >
> > Ok, not sure that the "(evm_initialized & EVM_ALLOW_METADATA_WRITES)"
> > is needed, but it doesn't hurt.
> 
> Goal here was to allow:
> 
> echo 6 >evm
> echo 7 >evm
> 
> t work without an error, but I guess that's not a big deal.
> 
That's fine.

Thanks!