Re: [PATCH v2 1/4] KEYS: Defined an ima hook for measuring keys on key create or update

From: Lakshmi Ramasubramanian
Date: Fri Oct 25 2019 - 15:49:30 EST

On 10/25/2019 12:40 PM, Mimi Zohar wrote:

On Wed, 2019-10-23 at 16:39 -0700, Lakshmi Ramasubramanian wrote:
Defined an ima hook to measure keys created or updated in the system.

"IMA" is an anacronym. ÂUnless it is a part of a function name, it
should be capitalized.
Will fix that.

Before describing "what" you're doing, describe the problem. ÂFor
example, "The asymmetric keys used for verifying file signatures or
certificates are currently not included in the IMA measurement list.
ÂThis patch defines a new IMA hook named ima_key_create_or_update() to
measure keys."
Agree - will update.

+ if (!ima_initialized)
+ return;

There's no reason to define a new variable to determine if IMA is
initialized. ÂUse ima_policy_flag.
Will change it.

ÂLike process_measurements, the test should be in process_buffer_measurement(), not here.

Currently, queuing of requests when IMA is not initialized is done for keys only. Moving that check inside process_buffer_measurement would mean handling queuing for all buffer measurements.

Can that be done as a separate patch set and not in this one?

@@ -936,6 +937,9 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
goto error_link_end;
+ /* let the ima module know about the created key. */
+ ima_post_key_create_or_update(keyring, key, flags, true);
key_ref = make_key_ref(key, is_key_possessed(keyring_ref));

This patch defines the new IMA hook. ÂThis call and the subsequent one
below can be defined in a separate patch. ÂThe subject line of that
patch would be "keys: Add ima_key_create_or_update call to measure

Agree - will change it.