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

From: Matthew Garrett <mjg59@google.com>
To: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: linux-integrity <linux-integrity@vger.kernel.org>
Subject: Re: [PATCH V3] EVM: Allow userland to permit modification of EVM-protected metadata
Date: Fri, 3 Nov 2017 11:49:51 -0700	[thread overview]
Message-ID: <CACdnJusn8c-8GMX3iY+6xsOGu6uNKQAMr_begLLTp13_p_bdPw@mail.gmail.com> (raw)
In-Reply-To: <1509725025.3416.73.camel@linux.vnet.ibm.com>

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?

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

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

to work without an error, but I guess that's not a big deal.