lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 25 Aug 2020 12:35:19 -0700 From: Lakshmi Ramasubramanian <nramas@...ux.microsoft.com> To: Mimi Zohar <zohar@...ux.ibm.com>, stephen.smalley.work@...il.com, casey@...aufler-ca.com Cc: tyhicks@...ux.microsoft.com, tusharsu@...ux.microsoft.com, sashal@...nel.org, jmorris@...ei.org, linux-integrity@...r.kernel.org, selinux@...r.kernel.org, linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH] IMA: Handle early boot data measurement On 8/25/20 11:03 AM, Mimi Zohar wrote: > On Tue, 2020-08-25 at 10:55 -0700, Lakshmi Ramasubramanian wrote: >> On 8/25/20 10:42 AM, Mimi Zohar wrote: >> >>>>> 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, >> >> I agree - but in this case, Tushar's patch series on adding support for >> "Critical Data" measurement has already introduced >> "measure_payload_hash" flag. His patch updates >> "process_buffer_measurement()" to take this new flag and measure hash of >> the given data. >> >> My patches extend that to queuing the early boot requests and processing >> them after a custom IMA policy is loaded. >> >> If you still think "measure_payload_hash" flag should be introduced in >> the queuing change as a separate patch I'll split the patches further. >> Please let me know. > > There's a major problem if his changes add new function arguments > without modifying all the callers of the function. I assume the kernel > would fail to compile properly. Tushar's patch series does update all the existing callers of process_buffer_measurement() to handle the new arguments. His patch series is self contained, and builds and works fine. > > Changing the function parameters to include "measure_payload_hash" > needs to be a separate patch, whether it is part of his patch set or > yours. > ok - I'll split the queuing patch to include "measure_payload_hash" in a separate patch. thanks, -lakshmi
Powered by blists - more mailing lists