On Tue, 2020-08-25 at 08:46 -0700, Lakshmi Ramasubramanian wrote:
> On 8/25/20 8:40 AM, Mimi Zohar wrote:
> > On Fri, 2020-08-21 at 16:12 -0700, Lakshmi Ramasubramanian wrote:
> > > The current implementation of early boot measurement in
> > > the IMA subsystem is very specific to asymmetric keys. It does not
> > > handle early boot measurement of data from other subsystems such as
> > > Linux Security Module (LSM), Device-Mapper, etc. As a result data,
> > > provided by these subsystems during system boot are not measured by IMA.
> > >
> > > Update the early boot key measurement to handle any early boot data.
> > > Refactor the code from ima_queue_keys.c to a new file ima_queue_data.c.
> > > Rename the kernel configuration CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS to
> > > CONFIG_IMA_QUEUE_EARLY_BOOT_DATA so it can be used for enabling any
> > > early boot data measurement. Since measurement of asymmetric keys is
> > > the first consumer of early boot measurement, this kernel configuration
> > > is enabled if IMA_MEASURE_ASYMMETRIC_KEYS and SYSTEM_TRUSTED_KEYRING are
> > > both enabled.
> > >
> > > Update the IMA hook ima_measure_critical_data() to utilize early boot
> > > measurement support.
> >
> > Please limit the changes in this patch to renaming the functions and/or
> > files. For example, adding "measure_payload_hash" should be a separate
> > patch, not hidden here.
> >
>
> Thanks for the feedback Mimi.
>
> I'll split this into 2 patches:
>
> PATCH 1: Rename files + rename CONFIG
> PATCH 2: Update IMA hook to utilize early boot data measurement.
I'm referring to introducing the "measure_payload_hash" flag. I assume
this is to indicate whether the buffer should be hashed or not.
Example 1: ima_alloc_key_entry() and ima_alloc_data_entry(0 comparison
> -static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring,
> - const void *payload,
> - size_t payload_len)
> -{
> +static struct ima_data_entry *ima_alloc_data_entry(const char *event_name,
> + const void *payload,
> + size_t payload_len,
> + const char *event_data,
> + enum ima_hooks func,
> + bool measure_payload_hash) <====
> +{
Example 2:
diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c
index a74095793936..65423754765f 100644
--- a/security/integrity/ima/ima_asymmetric_keys.c
+++ b/security/integrity/ima/ima_asymmetric_keys.c
@@ -37,9 +37,10 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
if (!payload || (payload_len == 0))
return;
- if (ima_should_queue_key())
- queued = ima_queue_key(keyring, payload, payload_len);
-
+ if (ima_should_queue_data())
+ queued = ima_queue_data(keyring->description, payload,
+ payload_len, keyring->description,
+ KEY_CHECK, false); <===
if (queued)
return;
But in general, as much as possible function and file name changes
should be done independently of other changes.
thanks,
Mimi
