lkml.org 
[lkml]   [2020]   [Nov]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v5 2/7] IMA: update process_buffer_measurement to measure buffer hash
From
Date


On 2020-11-06 4:11 a.m., Mimi Zohar wrote:
> Hi Tushar,
>
> Below inline are a few additional comments.
>
>> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
>> index ae5da9f3339d..4485d87c0aa5 100644
>> --- a/security/integrity/ima/ima_main.c
>> +++ b/security/integrity/ima/ima_main.c
>> @@ -787,12 +787,15 @@ int ima_post_load_data(char *buf, loff_t size,
>> * @func: IMA hook
>> * @pcr: pcr to extend the measurement
>> * @func_data: private data specific to @func, can be NULL.
>> + * @measure_buf_hash: if set to true - will measure hash of the buf,
>> + * instead of buf
>> *
>> * Based on policy, the buffer is measured into the ima log.
>
> Both the brief and longer function descriptions need to be updated, as
> well as the last argument description. The last argument should be
> limited to "measure buffer hash". How it is used could be included in
> the longer function description. The longer function description would
> include adding the buffer data or the buffer data hash to the IMA
> measurement list and extending the PCR.
>
> For example,
> process_buffer_measurement - measure the buffer data or the buffer data
> hash
>
Thanks Mimi. Will update the brief and longer descriptions accordingly.
>
>> */
>> void process_buffer_measurement(struct inode *inode, const void *buf, int size,
>> const char *eventname, enum ima_hooks func,
>> - int pcr, const char *func_data)
>> + int pcr, const char *func_data,
>> + bool measure_buf_hash)
>> {
>> int ret = 0;
>> const char *audit_cause = "ENOMEM";
>> @@ -807,6 +810,8 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
>> struct ima_digest_data hdr;
>> char digest[IMA_MAX_DIGEST_SIZE];
>> } hash = {};
>> + char digest_hash[IMA_MAX_DIGEST_SIZE];
>> + int hash_len = hash_digest_size[ima_hash_algo];
>> int violation = 0;
>> int action = 0;
>> u32 secid;
>> @@ -855,6 +860,21 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
>> goto out;
>> }
>>
>> + if (measure_buf_hash) {
>> + memcpy(digest_hash, hash.hdr.digest, hash_len);
>
> Instead of digest_hash and hash_len, consider naming the variables
> buf_hash and buf_hashlen.
>
Thanks. Will do.
>> +
>> + ret = ima_calc_buffer_hash(digest_hash,
>> + hash_len,
>> + iint.ima_hash);
>
> There's no need for each variable to be on a separate line.
>
Thanks, will fix.
~Tushar

> thanks,
>
> Mimi
>
>> + if (ret < 0) {
>> + audit_cause = "measure_buf_hash_error";
>> + goto out;
>> + }
>> +
>> + event_data.buf = digest_hash;
>> + event_data.buf_len = hash_len;
>> + }
>> +
>> ret = ima_alloc_init_template(&event_data, &entry, template);
>> if (ret < 0) {
>> audit_cause = "alloc_entry";

\
 
 \ /
  Last update: 2020-11-12 22:49    [W:0.070 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site