Re: [Linux-ima-devel] [PATCH v5] ima: add the ability to query ima for the hash of a given file.

[Dmitry K. <dmi...@gm...>]

On 10 November 2014 18:53, Peter Moody <pm...@go...> wrote:
>
> On Fri, Nov 07 2014 at 05:50, Dmitry Kasatkin wrote:
>> Hi Peter,
>>
>> We had a chat with Mimi and thought/discussed about couple of issues...
>>
>> It would be nice that patch description to discuss about use-case scenario.
>> It is unclear why, when and how this API is going to be used.
>
> Hey Dmitry, Mimi.
>

Hi Peter,

> so the usecase would be for me to call this from something like hone
> (http://marc.info/?l=linux-security-module&m=140685613922280&w=2, I need
> to resend that patchset). The basic idea is that I have a stacked
> security module that monitors forks/execs/exits and packets sent &
> received.
>
> The purpose of the module is to help an admin (or an incident response
> team) to:
>
>  a) correlate a packet seen on the network with the process that sent or
>  received it.
>  b) determine the process ancestry of a potentially malicious process (eg.
>  an ntpd exploit that led to wget being called that led to a rootkit being
>  installed)
>
> Part of what hone does is send events for forks/execs/exits to a
> userland daemon, along with the pertinents like uid, euid, gid, egid,
> ppid, path, exit status .. etc. What I'm trying to do here is, for
> systems with IMA enabled with an appropriate policy, include the
> calculated hash with the exec events. It's true that the existence of
> the hash in the iint_cache means that the hash is either in the audit
> logs or in /sys/kernel/security/ima/ascii_runtime_measurements, but it
> can be difficult to pull that information out for every subsequent
> exec. This is especially true if a give host is up for a long time and
> the binary was first measured a long time ago.
>
> Does that make sense?
>

I think API makes sense as it always fetches latest hash value.

- Dmitry

>
>> Also please see comment bellow...
>>
>> On 06/11/14 00:08, Peter Moody wrote:
>>> It's useful to be able to get the hash of a previously measured file.
>>> Querying IMA directly is more efficient than grepping the ascii
>>> measurements of the audit logs.
>>> ---
>>>  include/linux/ima.h               |  6 ++++++
>>>  security/integrity/ima/ima_main.c | 35 +++++++++++++++++++++++++++++++++++
>>>  2 files changed, 41 insertions(+)
>>>
>>> diff --git a/include/linux/ima.h b/include/linux/ima.h
>>> index 120ccc5..27cc4f6 100644
>>> --- a/include/linux/ima.h
>>> +++ b/include/linux/ima.h
>>> @@ -20,6 +20,7 @@ extern void ima_file_free(struct file *file);
>>>  extern int ima_file_mmap(struct file *file, unsigned long prot);
>>>  extern int ima_module_check(struct file *file);
>>>  extern int ima_fw_from_file(struct file *file, char *buf, size_t size);
>>> +extern char *ima_file_hash(struct file *file, char *buf, size_t size);
>>>
>>>  #else
>>>  static inline int ima_bprm_check(struct linux_binprm *bprm)
>>> @@ -52,6 +53,11 @@ static inline int ima_fw_from_file(struct file *file, char *buf, size_t size)
>>>      return 0;
>>>  }
>>>
>>> +static inline char *ima_file_hash(struct file *file, char *buf, size_t size)
>>> +{
>>> +    return NULL;
>>> +}
>>> +
>>>  #endif /* CONFIG_IMA */
>>>
>>>  #ifdef CONFIG_IMA_APPRAISE
>>> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
>>> index 62f59ec..39a139d 100644
>>> --- a/security/integrity/ima/ima_main.c
>>> +++ b/security/integrity/ima/ima_main.c
>>> @@ -309,6 +309,41 @@ int ima_file_check(struct file *file, int mask, int opened)
>>>  EXPORT_SYMBOL_GPL(ima_file_check);
>>>
>>>  /**
>>> + * ima_file_hash - return the stored measurement if a file's been hashed.
>>> + * @file: pointer to the file.
>>> + * @buf: buffer in which to store the hash.
>>> + * @size: length of the buffer.
>>> + *
>>> + * On success returns the buffer. Otherwise returns NULL. If the hash is larger
>>> + * than buffer, then only size bytes will be copied. It generally just makes
>>> + * sense to pass a buffer capable of holding the largest possible hash string.
>>> + * This is currently 135 (IMA_MAX_DIGEST_SIZE * 2 + 6 + 1).
>>> + */
>>> +char *ima_file_hash(struct file *file, char *buf, size_t size)
>>> +{
>>> +    struct inode *inode;
>>> +    struct integrity_iint_cache *iint;
>>> +
>>> +    if (!file)
>>> +            return NULL;
>>> +
>>> +    inode = file_inode(file);
>>> +
>>
>> We think mutex is taken too early. It makes sense to verify if inode was
>> handled by IMA.
>> Locking would be useless because IMA is disabled by policy or 'iint'
>> will be NULL...
>>
>> Please check ima_file_free(). It makes sense to use the same pattern
>> ------------
>>     if (!ima_policy_flag || !S_ISREG(inode->i_mode))
>>         return NULL;
>>
>>     iint = integrity_iint_find(inode);
>>     if (!iint)
>>         return NULL;
>>
>>     mutex_lock(&inode->i_mutex);
>> ----------------
>>
>> S_ISREG is probably not needed for your hook.
>>
>> in reality integrity_iint_find() is just enough but we optimize for not
>> calling it when IMA is disabled..
>>
>>> +    mutex_lock(&inode->i_mutex);
>>> +    if (iint && (iint->flags & IMA_COLLECTED))
>>
>> Also we thought that using iint->flags may be inappropriate at all.
>> They can be reset and you probably want old hash value anyway.
>> In such case it is better to test for "zero" hash...
>>
>>> +            snprintf(buf, size, "%s:%*phN",
>>> +                    hash_algo_name[iint->ima_hash->algo],
>>> +                    iint->ima_hash->length, iint->ima_hash->digest);
>>> +    else
>>> +            buf = NULL;
>>> +    mutex_unlock(&inode->i_mutex);
>>> +
>>> +    return buf;
>>
>> Also may be if you need error code to indicate if IMA is disabled you
>> could use 'return ERR_PTR()'.
>
> Ack. I'll make these changes.
>
>> Dmitry
>>
>>
>>> +}
>>> +EXPORT_SYMBOL_GPL(ima_file_hash);
>>> +
>>> +/**
>>>   * ima_module_check - based on policy, collect/store/appraise measurement.
>>>   * @file: pointer to the file to be measured/appraised
>>>   *
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Linux-ima-devel mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linux-ima-devel



-- 
Thanks,
Dmitry